IDEA-172876 Purity inference algorithm does not match hardcoded contracts for assertion methods

GitOrigin-RevId: 98da482dde66ecc9d0137aedfbc33a3a4c2cc4e0

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/ContractInferenceIndex.kt

@@ -23,7 +23,7 @@ import kotlin.collections.HashMap
  * @author peter
  */
 
-private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 11, MethodDataExternalizer) { file ->
+private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 12, MethodDataExternalizer) { file ->
   indexFile(file.node.lighterAST)
 }
 
@@ -110,8 +110,8 @@ private class InferenceVisitor(val tree : LighterAST) : RecursiveLighterASTNodeW
     val clsData = classData[tree.getParent(method)]
     val fieldMap = clsData?.fieldModifiers ?: emptyMap()
     // Constructor which has super classes may implicitly call impure super constructor, so don't infer purity for subclasses
-    val ctor = clsData != null && !clsData.hasSuper && clsData.hasPureInitializer && 
-               LightTreeUtil.firstChildOfType(tree, method, TYPE) == null
+    val ctor = LightTreeUtil.firstChildOfType(tree, method, TYPE) == null
+    val maybeImpureCtor = ctor && (clsData == null || clsData.hasSuper || !clsData.hasPureInitializer)
     val statements = ContractInferenceInterpreter.getStatements(body, tree)
 
     val contractInference = ContractInferenceInterpreter(tree, method, body)
@@ -120,11 +120,16 @@ private class InferenceVisitor(val tree : LighterAST) : RecursiveLighterASTNodeW
     val nullityVisitor = MethodReturnInferenceVisitor(tree, contractInference.getParameters(), body)
     val purityVisitor = PurityInferenceVisitor(tree, body, fieldMap, ctor)
     for (statement in statements) {
-      walkMethodBody(statement) { nullityVisitor.visitNode(it); purityVisitor.visitNode(it) }
+      walkMethodBody(statement) {
+        nullityVisitor.visitNode(it)
+        if (!maybeImpureCtor) {
+          purityVisitor.visitNode(it)
+        }
+      }
     }
     val notNullParams = inferNotNullParameters(tree, method, statements)
 
-    return createData(body, contracts, nullityVisitor.result, purityVisitor.result, notNullParams)
+    return createData(body, contracts, nullityVisitor.result, if (maybeImpureCtor) null else purityVisitor.result, notNullParams)
   }
 
   private fun walkMethodBody(root: LighterASTNode, processor: (LighterASTNode) -> Unit) {

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/JavaSourceInference.java

@@ -124,7 +124,6 @@ public class JavaSourceInference {
   }
   
   private static boolean findPurity(@NotNull PsiMethodImpl method, @NotNull MethodData data) {
-    if (PsiType.VOID.equals(method.getReturnType())) return false;
     PurityInferenceResult result = data.getPurity();
     if (result == null) return false;
     return Boolean.TRUE.equals(RecursionManager.doPreventingRecursion(method, true, () -> result.isPure(method, data.methodBody(method))));

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/PurityInferenceVisitor.java

@@ -24,7 +24,6 @@ class PurityInferenceVisitor {
   private final Map<String, LighterASTNode> myFieldModifiers;
   private final List<LighterASTNode> mutatedRefs = new ArrayList<>();
   private final boolean constructor;
-  private boolean hasReturns;
   private boolean hasVolatileReads;
   private final List<LighterASTNode> calls = new ArrayList<>();
 
@@ -40,9 +39,6 @@ class PurityInferenceVisitor {
     if (type == ASSIGNMENT_EXPRESSION) {
       addMutation(tree.getChildren(element).get(0));
     }
-    else if (type == RETURN_STATEMENT && JavaLightTreeUtil.findExpressionChild(tree, element) != null) {
-      hasReturns = true;
-    }
     else if ((type == PREFIX_EXPRESSION || type == POSTFIX_EXPRESSION) && isMutatingOperation(element)) {
       addMutation(JavaLightTreeUtil.findExpressionChild(tree, element));
     }
@@ -100,7 +96,7 @@ class PurityInferenceVisitor {
 
   @Nullable
   PurityInferenceResult getResult() {
-    if (calls.size() > 1 || (!constructor && (!hasReturns || hasVolatileReads))) return null;
+    if (calls.size() > 1 || (!constructor && hasVolatileReads)) return null;
 
     int bodyStart = body.getStartOffset();
     return new PurityInferenceResult(ContainerUtil.map(mutatedRefs, node -> ExpressionRange.create(node, bodyStart)),

java/java-tests/testData/codeInsight/gotosuper/SiblingInheritanceFinal.java

@@ -4,7 +4,7 @@ interface I {
   void run();
 }
 abstract class A {
-  public final void <caret>run() {}
+  public final void <caret>run() { someCode(); }
 }
 
 class Foo extends A implements I {

java/java-tests/testData/codeInsight/javadocIG/escapingStringValue.java

@@ -2,5 +2,5 @@ class A {
   public static String B = "a<b";
 
   /** The value of B is {@value #B}. */
-  public static void JAVADOC_ME() { }
+  public static void JAVADOC_ME() { someCode(); }
 }

java/java-tests/testData/codeInsight/javadocIG/valueInMethod.java

@@ -2,5 +2,5 @@ class A {
   public static int A = 23;
 
   /** The value of A is {@value #A}. */
-  public static void JAVADOC_ME() { }
+  public static void JAVADOC_ME() { someCode(); }
 }

java/java-tests/testData/codeInsight/javadocIG/valueInMethodNoHash.java

@@ -2,5 +2,5 @@ class A {
   public static int A = 23;
 
   /** The value of A is {@value A}. */
-  public static void JAVADOC_ME() { }
+  public static void JAVADOC_ME() { someCode(); }
 }

java/java-tests/testData/inspection/commonIfParts/afterCommonTailVariableExtractMultiple.java

@@ -5,7 +5,7 @@ import java.util.Map;
 
 public class Main {
 
-  private void work(int i){};
+  void work(int i){};
 
   public int test(int a, int b) {
       int c;

java/java-tests/testData/inspection/commonIfParts/beforeCommonTailVariableExtractMultiple.java

@@ -5,7 +5,7 @@ import java.util.Map;
 
 public class Main {
 
-  private void work(int i){};
+  void work(int i){};
 
   public int test(int a, int b) {
     if<caret>(true) {

java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/DaemonRespondToChangesTest.java

@@ -766,7 +766,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase {
     assertEmpty(errors);
 
     List<LineMarkerInfo<?>> lineMarkers = DaemonCodeAnalyzerImpl.getLineMarkers(myEditor.getDocument(), getProject());
-    assertSize(2, lineMarkers);
+    assertSize(3, lineMarkers);
 
     backspace();
 
@@ -799,7 +799,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase {
 
     assertEmpty(highlightErrors());
 
-    assertSize(2, DaemonCodeAnalyzerImpl.getLineMarkers(myEditor.getDocument(), getProject()));
+    assertSize(3, DaemonCodeAnalyzerImpl.getLineMarkers(myEditor.getDocument(), getProject()));
 
     assertEmpty(changed);
   }

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/GutterIntentionsTest.java

@@ -33,7 +33,7 @@ public class GutterIntentionsTest extends LightJavaCodeInsightFixtureTestCase {
 
   public void testOptions() {
     myFixture.configureByText(JavaFileType.INSTANCE, "public class Foo {\n" +
-                                                     "  public static void <caret>main(String[] args) {}" +
+                                                     "  public static void <caret>main(String[] args) { someCode(); }" +
                                                      "}");
     assertSize(1, myFixture.findGuttersAtCaret());
 
@@ -54,7 +54,7 @@ public class GutterIntentionsTest extends LightJavaCodeInsightFixtureTestCase {
 
   public void testDoNotIncludeActionGroup() {
     myFixture.configureByText(JavaFileType.INSTANCE, "public class Foo {\n" +
-                                                     "  public static void <caret>main(String[] args) {}" +
+                                                     "  public static void <caret>main(String[] args) { someCode(); }" +
                                                      "}");
     assertSize(1, myFixture.findGuttersAtCaret());
 

java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/RunLineMarkerTest.java

@@ -42,8 +42,10 @@ public class RunLineMarkerTest extends LightJavaCodeInsightFixtureTestCase {
   public void testRunLineMarker() {
     myFixture.configureByText("MainTest.java", "public class MainTest {\n" +
                                                "    public static void <caret>foo(String[] args) {\n" +
+                                               "      someCode();\n" +
                                                "    }\n " +
                                                "    public static void main(String[] args) {\n" +
+                                               "      someCode();\n" +
                                                "    }\n" +
                                                "}");
     assertEquals(0, myFixture.findGuttersAtCaret().size());
@@ -143,6 +145,7 @@ public class RunLineMarkerTest extends LightJavaCodeInsightFixtureTestCase {
   public void testTooltip() {
     myFixture.configureByText("Main.java", "public class Main {\n" +
                                            "    public static void m<caret>ain(String[] args) {\n" +
+                                           "      someCode();\n" +
                                            "    }\n" +
                                            "}");
     List<GutterMark> marks = myFixture.findGuttersAtCaret();
@@ -157,6 +160,7 @@ public class RunLineMarkerTest extends LightJavaCodeInsightFixtureTestCase {
   public void testTooltipWithUnderscores() {
     myFixture.configureByText("Main_class_test.java", "public class Main_class_test {\n" +
                                                       "    public static void m<caret>ain(String[] args) {\n" +
+                                                      "      someCode();\n" +
                                                       "    }\n" +
                                                       "}");
     List<GutterMark> marks = myFixture.findGuttersAtCaret();
@@ -171,6 +175,7 @@ public class RunLineMarkerTest extends LightJavaCodeInsightFixtureTestCase {
   public void testEditConfigurationAction() {
     myFixture.configureByText("MainTest.java", "public class MainTest {\n" +
                                                "    public static void ma<caret>in(String[] args) {\n" +
+                                               "      someCode();\n" +
                                                "    }\n" +
                                                "}");
     List<GutterMark> marks = myFixture.findGuttersAtCaret();

java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.groovy

@@ -128,24 +128,6 @@ int smthPure2() { return 42; }
 """
   }
 
-  void "test don't analyze void methods"() {
-    assertPure false, """
-void method() {
-  smthPure();
-}
-int smthPure() { return 3; }
-"""
-  }
-
-  void "test don't analyze methods without returns"() {
-    assertPure false, """
-Object method() {
-    smthPure();
-}
-int smthPure() { return 3; }
-"""
-  }
-
   void "test empty constructor"() {
     assertPure true, """
 public Foo() {
@@ -394,7 +376,14 @@ int get() {
 }
 """
   }
-  
+
+  void "test assertNotNull is pure"() {
+    assertPure true, """
+static void assertNotNull(Object val) {
+  if(val == null) throw new AssertionError();
+}"""
+  }
+
   void "test recursive factorial"() {
     assertPure true, """int factorial(int n) { return n == 1 ? 1 : factorial(n - 1) * n;}"""
   }