SideEffectChecker: ignore side effects in asserts; MismatchedStringBuilderQueryUpdate: support any kind of side-effect free statements (not only ifStatement) (IDEA-CR-17743)

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2017 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1345,7 +1345,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     return !contracts.isEmpty() ? contracts : getMethodContracts(method);
   }
 
-  static List<MethodContract> getMethodContracts(@NotNull final PsiMethod method) {
+  public static List<MethodContract> getMethodContracts(@NotNull final PsiMethod method) {
     return CachedValuesManager.getCachedValue(method, () -> {
       final PsiAnnotation contractAnno = findContractAnnotation(method);
       if (contractAnno != null) {

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedStringBuilderQueryUpdateInspection.java

@@ -268,9 +268,11 @@ public class MismatchedStringBuilderQueryUpdateInspection extends BaseInspection
         return;
       }
       if (hasReferenceToVariable(variable, qualifierExpression)) {
-        PsiIfStatement ifStatement =
-          PsiTreeUtil.getParentOfType(expression, PsiIfStatement.class, true, PsiStatement.class, PsiLambdaExpression.class);
-        if (ifStatement != null && !SideEffectChecker.mayHaveSideEffects(ifStatement, this::isSideEffectFreeBuilderMethodCall)) return;
+        PsiElement parent = PsiTreeUtil.getParentOfType(expression, PsiStatement.class, PsiLambdaExpression.class);
+        if (parent instanceof PsiStatement &&
+            !SideEffectChecker.mayHaveSideEffects((PsiStatement)parent, this::isSideEffectFreeBuilderMethodCall)) {
+          return;
+        }
         queried = true;
       }
     }

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java

@@ -16,6 +16,7 @@
 package com.siyeh.ig.psiutils;
 
 import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer;
+import com.intellij.codeInspection.dataFlow.MethodContract;
 import com.intellij.psi.*;
 import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PropertyUtil;
@@ -118,13 +119,23 @@ public class SideEffectChecker {
         return;
       }
       final PsiMethod method = expression.resolveMethod();
-      if (method != null && (PropertyUtil.isSimpleGetter(method) || ControlFlowAnalyzer.isPure(method))) {
+      if (isPure(method)) {
         return;
       }
       
       sideEffect = expression;
     }
 
+    protected boolean isPure(PsiMethod method) {
+      if (method == null) return false;
+      if (PropertyUtil.isSimpleGetter(method)) return true;
+      if (ControlFlowAnalyzer.isPure(method)) {
+        return ControlFlowAnalyzer.getMethodContracts(method).stream()
+          .noneMatch(mc -> mc.returnValue == MethodContract.ValueConstraint.THROW_EXCEPTION);
+      }
+      return false;
+    }
+
     @Override
     public void visitNewExpression(@NotNull PsiNewExpression expression) {
       if (sideEffect != null) {
@@ -163,9 +174,9 @@ public class SideEffectChecker {
     }
 
     @Override
-    public void visitAssertStatement(PsiAssertStatement statement) {
+    public void visitDeclarationStatement(PsiDeclarationStatement statement) {
       sideEffect = statement;
-      super.visitAssertStatement(statement);
+      super.visitDeclarationStatement(statement);
     }
 
     @Override
@@ -176,8 +187,7 @@ public class SideEffectChecker {
 
     @Override
     public void visitClass(PsiClass aClass) {
-      sideEffect = aClass;
-      super.visitClass(aClass);
+      // local or anonymous class declaration is not side effect per se (unless it's instantiated)
     }
 
     @Override

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_string_builder_query_update/MismatchedStringBuilderQueryUpdate.java

@@ -68,6 +68,20 @@ public class MismatchedStringBuilderQueryUpdate {
     return null;
   }
 
+  static String testUselessQueryTernary(java.util.List<String> list) {
+    StringBuilder sb = new StringBuilder();
+    for (String s : list) {
+      if (s != null) {
+        if(sb.length() > 0) {
+          assert sb.length() > 5;
+        }
+        sb.append(sb.length() > 0 ? ',' : "start");
+        sb.append(s.length());
+      }
+    }
+    return null;
+  }
+
   static String testQueryWithSideEffect(java.util.List<String> list) {
     StringBuilder sb = new StringBuilder();
     for (String s : list) {