From 54624d1864364d120e0d89153096b9e2b1afe78c Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 23 Jan 2017 12:36:50 +0100 Subject: [PATCH] infer no contracts for methods always returning null/true/false they're likely to be used as constant-like flags for functionality being enabled or not (IDEA-166399) --- .../dataFlow/ContractInferenceIndex.kt | 2 +- .../ContractInferenceInterpreter.java | 51 ++++++++++--------- .../dataFlow/fixture/CompileTimeConstant.java | 7 +++ .../ContractInferenceFromSourceTest.groovy | 6 +-- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceIndex.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceIndex.kt index 6da68ecf86e1..8e7bcedcda50 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceIndex.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceIndex.kt @@ -32,7 +32,7 @@ import java.util.* * @author peter */ -private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 0, MethodDataExternalizer) { file -> +private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 1, MethodDataExternalizer) { file -> indexFile(file.node.lighterAST) } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceInterpreter.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceInterpreter.java index 353f053d2c80..111fb4442ea1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceInterpreter.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInferenceInterpreter.java @@ -28,7 +28,6 @@ import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import static com.intellij.codeInspection.dataFlow.MethodContract.ValueConstraint.*; @@ -37,6 +36,7 @@ import static com.intellij.psi.impl.source.JavaLightTreeUtil.getExpressionChildr import static com.intellij.psi.impl.source.tree.JavaElementType.*; import static com.intellij.psi.impl.source.tree.LightTreeUtil.firstChildOfType; import static com.intellij.psi.impl.source.tree.LightTreeUtil.getChildrenOfType; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; class ContractInferenceInterpreter { @@ -53,31 +53,34 @@ class ContractInferenceInterpreter { @NotNull private List getParameters() { LighterASTNode paramList = firstChildOfType(myTree, myMethod, PARAMETER_LIST); - return paramList != null ? getChildrenOfType(myTree, paramList, PARAMETER) : Collections.emptyList(); + return paramList != null ? getChildrenOfType(myTree, paramList, PARAMETER) : emptyList(); } @NotNull List inferContracts(List statements) { - if (statements.isEmpty()) return Collections.emptyList(); + if (statements.isEmpty()) return emptyList(); if (statements.size() == 1) { - LighterASTNode statement = statements.get(0); - if (statement.getTokenType() == RETURN_STATEMENT) { - List result = handleDelegation(findExpressionChild(myTree, statement), false); - if (result != null) { - return result; - } - } - else if (statement.getTokenType() == EXPRESSION_STATEMENT) { - LighterASTNode expr = findExpressionChild(myTree, statement); - List result = expr != null && expr.getTokenType() == METHOD_CALL_EXPRESSION ? handleDelegation(expr, false) : null; - if (result != null) return result; - } + List result = handleSingleStatement(statements.get(0)); + if (result != null) return result; } return visitStatements(singletonList(MethodContract.createConstraintArray(getParameters().size())), statements); } + @Nullable + private List handleSingleStatement(LighterASTNode statement) { + if (statement.getTokenType() == RETURN_STATEMENT) { + LighterASTNode returned = findExpressionChild(myTree, statement); + return getLiteralConstraint(returned) != null ? emptyList() : handleDelegation(returned, false); + } + if (statement.getTokenType() == EXPRESSION_STATEMENT) { + LighterASTNode expr = findExpressionChild(myTree, statement); + return expr != null && expr.getTokenType() == METHOD_CALL_EXPRESSION ? handleDelegation(expr, false) : null; + } + return null; + } + @Nullable private LighterASTNode getCodeBlock(@Nullable LighterASTNode parent) { return firstChildOfType(myTree, parent, CODE_BLOCK); @@ -85,7 +88,7 @@ class ContractInferenceInterpreter { @NotNull static List getStatements(@Nullable LighterASTNode codeBlock, LighterAST tree) { - return codeBlock == null ? Collections.emptyList() : getChildrenOfType(tree, codeBlock, ElementType.JAVA_STATEMENT_BIT_SET); + return codeBlock == null ? emptyList() : getChildrenOfType(tree, codeBlock, ElementType.JAVA_STATEMENT_BIT_SET); } @Nullable @@ -112,9 +115,9 @@ class ContractInferenceInterpreter { @NotNull private List visitExpression(final List states, @Nullable LighterASTNode expr) { - if (expr == null) return Collections.emptyList(); - if (states.isEmpty()) return Collections.emptyList(); - if (states.size() > 300) return Collections.emptyList(); // too complex + if (expr == null) return emptyList(); + if (states.isEmpty()) return emptyList(); + if (states.size() > 300) return emptyList(); // too complex IElementType type = expr.getTokenType(); if (type == POLYADIC_EXPRESSION || type == BINARY_EXPRESSION) { @@ -123,7 +126,7 @@ class ContractInferenceInterpreter { if (type == CONDITIONAL_EXPRESSION) { List children = getExpressionChildren(myTree, expr); - if (children.size() != 3) return Collections.emptyList(); + if (children.size() != 3) return emptyList(); List conditionResults = visitExpression(states, children.get(0)); return ContainerUtil.concat( @@ -150,7 +153,7 @@ class ContractInferenceInterpreter { } } - if (type == NEW_EXPRESSION) { + if (type == NEW_EXPRESSION || type == THIS_EXPRESSION) { return asPreContracts(toContracts(states, NOT_NULL_VALUE)); } if (type == METHOD_CALL_EXPRESSION) { @@ -179,7 +182,7 @@ class ContractInferenceInterpreter { return asPreContracts(result); } - return Collections.emptyList(); + return emptyList(); } @NotNull @@ -199,7 +202,7 @@ class ContractInferenceInterpreter { if (logicalAnd || firstChildOfType(myTree, expr, JavaTokenType.OROR) != null) { return asPreContracts(visitLogicalOperation(operands, logicalAnd, states)); } - return Collections.emptyList(); + return emptyList(); } @NotNull @@ -240,7 +243,7 @@ class ContractInferenceInterpreter { } return result; } - return Collections.emptyList(); + return emptyList(); } @Nullable diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CompileTimeConstant.java b/java/java-tests/testData/inspection/dataFlow/fixture/CompileTimeConstant.java index 05ed50fea2ce..0db952f30043 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/CompileTimeConstant.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/CompileTimeConstant.java @@ -3,10 +3,17 @@ class Fun { public static final boolean isProduction = false; public static final boolean isDebugInProduction = isDebug && isProduction; + public static boolean isFeatureEnabled() { + return true; + } + void foo() { if (isDebug) { System.out.println(); } + if (isFeatureEnabled()) { + System.out.println(); + } } void fooNegated() { if (!isDebug) { diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy index b1ceb92e4502..93a9e3ec077c 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy @@ -532,11 +532,11 @@ class Foo {{ def method = PsiTreeUtil.findChildOfType(myFixture.addClass(""" class Foo {{ new Object() { - Object foo() { return null;} - Object bar() { return foo();} + Object foo(boolean b) { return b ? null : this;} + Object bar(boolean b) { return foo(b);} }; }}"""), PsiAnonymousClass).methods[0] - assert ContractInference.inferContracts(method).collect { it as String } == [' -> null'] + assert ContractInference.inferContracts(method).collect { it as String } == ['true -> null', 'false -> !null'] } void "test anonymous class methods potentially used from outside"() {