Merge PR #312 (https://github.com/JetBrains/intellij-community/pull/312)

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyResultOfAssignmentUsedInspection.java

@@ -15,15 +15,33 @@
  */
 package org.jetbrains.plugins.groovy.codeInspection.assignment;
 
+import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
+import com.intellij.psi.PsiElement;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
+import org.jetbrains.plugins.groovy.lang.psi.GroovyFile;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrAssignmentExpression;
 import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
 
+import javax.swing.*;
+
 public class GroovyResultOfAssignmentUsedInspection extends BaseInspection {
+  /**
+   * @noinspection PublicField, WeakerAccess
+   */
+  public boolean inspectClosures = false;
+
+  @Override
+  @Nullable
+  public JComponent createOptionsPanel() {
+    final MultipleCheckboxOptionsPanel optionsPanel = new MultipleCheckboxOptionsPanel(this);
+    optionsPanel.addCheckbox("Inspect anonymous closures", "inspectClosures");
+    return optionsPanel;
+  }
 
   @Override
   @Nls
@@ -51,14 +69,21 @@ public class GroovyResultOfAssignmentUsedInspection extends BaseInspection {
     return new Visitor();
   }
 
-  private static class Visitor extends BaseInspectionVisitor {
+  private class Visitor extends BaseInspectionVisitor {
 
     @Override
     public void visitAssignmentExpression(GrAssignmentExpression grAssignmentExpression) {
       super.visitAssignmentExpression(grAssignmentExpression);
-      if (PsiUtil.isExpressionUsed(grAssignmentExpression)) {
+      if (isConfusingAssignmentUsage(grAssignmentExpression)) {
         registerError(grAssignmentExpression);
       }
     }
+
+    private boolean isConfusingAssignmentUsage(PsiElement expr) {
+      PsiElement parent = expr.getParent();
+      return !(parent instanceof GroovyFile) &&
+             (inspectClosures || !(parent instanceof GrClosableBlock)) &&
+             PsiUtil.isExpressionUsed(expr);
+    }
   }
 }

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java

@@ -66,6 +66,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnState
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseSection;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrForInClause;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrTraditionalForClause;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrLiteral;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrIndexProperty;
@@ -1171,7 +1172,8 @@ public class PsiUtil {
         parent instanceof GrAssertStatement ||
         parent instanceof GrThrowStatement ||
         parent instanceof GrSwitchStatement ||
-        parent instanceof GrVariable) {
+        parent instanceof GrVariable ||
+        parent instanceof GrWhileStatement) {
       return true;
     }
 
@@ -1188,6 +1190,12 @@ public class PsiUtil {
       }
       return true;
     }
+
+    if (parent instanceof GrTraditionalForClause) {
+      GrTraditionalForClause forClause = (GrTraditionalForClause)parent;
+      return expr == forClause.getCondition();
+    }
+
     return isReturnStatement(expr);
   }
 

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy

@@ -50,11 +50,11 @@ class GrInspectionTest extends GrHighlightingTestBase {
 
   public void testResolveMetaClass() { doTest(new GroovyAccessibilityInspection()) }
 
-  public void testResultOfAssignmentUsed() { doTest(new GroovyResultOfAssignmentUsedInspection()) }
+  public void testResultOfAssignmentUsed() { doTest(new GroovyResultOfAssignmentUsedInspection(inspectClosures: true)) }
 
   public void testSuppressions() { doTest(new GrUnresolvedAccessInspection(), new GroovyUntypedAccessInspection()) }
 
-  public void testInnerClassConstructorThis() { doTest(true, true, true, new GroovyResultOfAssignmentUsedInspection()) }
+  public void testInnerClassConstructorThis() { doTest(true, true, true, new GroovyResultOfAssignmentUsedInspection(inspectClosures: true)) }
 
   public void testUnnecessaryReturnInSwitch() { doTest(new GroovyUnnecessaryReturnInspection()) }
 

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrResultOfAssignmentUsedTest.groovy

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2000-2015 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jetbrains.plugins.groovy.lang.highlighting
+
+import com.intellij.codeInspection.InspectionProfileEntry
+import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyResultOfAssignmentUsedInspection
+
+class GrResultOfAssignmentUsedTest extends GrHighlightingTestBase {
+  final inspection = new GroovyResultOfAssignmentUsedInspection()
+
+  @Override
+  InspectionProfileEntry[] getCustomInspections() { [inspection] }
+
+  final warnStart = '<warning descr="Result of assignment expression used">'
+  final warnEnd = '</warning>'
+
+  void testUsedVar() {
+    testHighlighting """
+      def foo(a) {
+        if ((${warnStart}a = 5${warnEnd}) || a) {
+          ${warnStart}a = 4${warnEnd}
+        }
+      }
+
+      def foo2(a) {
+        def b = 'b'
+        if (!a) {
+          println b
+          b = 5
+        }                            
+        return 0 // make b = 5 not a return statement
+      }
+
+      def bar(a) {
+        print ((${warnStart}a = 5${warnEnd})?:a)
+      }
+
+      def a(b) {
+        if (2 && (${warnStart}b = 5${warnEnd})) {
+          b
+        }
+      }
+    """
+  }
+
+  void testResultOfAssignmentUsedInspection() {
+    testHighlighting """
+      if ((${warnStart}a = b${warnEnd}) == null) {
+      }
+    """
+
+    testHighlighting """
+      while (${warnStart}a = b${warnEnd}) {
+      }
+    """
+
+    testHighlighting """
+      for (i = 0; ${warnStart}a = b${warnEnd}; i++) {
+      }
+    """
+
+    testHighlighting """
+      System.out.println(${warnStart}a = b${warnEnd})
+    """
+
+    testHighlighting """
+      (${warnStart}a = b${warnEnd}).each {}
+    """
+  }
+
+  void testInspectClosuresOption_isTrue() {
+    inspection.inspectClosures = true
+    testHighlighting 'a = 1 '
+    testHighlighting "a(0, { ${warnStart}b = 1${warnEnd} }, 2)"
+    testHighlighting "def a = { ${warnStart}b = 1${warnEnd} }"
+    testHighlighting "a(0, ${warnStart}b = 1${warnEnd}, 2)"
+    testHighlighting "def a() { ${warnStart}b = 1${warnEnd} }"
+    inspection.inspectClosures = false
+  }
+
+  void testInspectClosuresOption_isFalse() {
+    inspection.inspectClosures = false
+    testHighlighting 'a = 1 '
+    testHighlighting 'a(0, { b = 1 }, 2)'
+    testHighlighting "def a = { b = 1 }"
+    testHighlighting "a(0, ${warnStart}b = 1${warnEnd}, 2)"
+    testHighlighting "def a() { ${warnStart}b = 1${warnEnd} }"
+  }
+}

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrUnusedDefTest.groovy

@@ -18,7 +18,6 @@ package org.jetbrains.plugins.groovy.lang.highlighting
 import com.intellij.codeInspection.InspectionProfileEntry
 import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase
 import org.jetbrains.plugins.groovy.codeInspection.GroovyUnusedDeclarationInspection
-import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyResultOfAssignmentUsedInspection
 import org.jetbrains.plugins.groovy.codeInspection.confusing.GrUnusedIncDecInspection
 import org.jetbrains.plugins.groovy.codeInspection.unusedDef.UnusedDefInspection
 
@@ -28,7 +27,10 @@ import org.jetbrains.plugins.groovy.codeInspection.unusedDef.UnusedDefInspection
 class GrUnusedDefTest extends GrHighlightingTestBase {
   @Override
   InspectionProfileEntry[] getCustomInspections() {
-    return [new UnusedDefInspection(), new GrUnusedIncDecInspection(), new GroovyUnusedDeclarationInspection(), new UnusedDeclarationInspectionBase(true), new GroovyResultOfAssignmentUsedInspection()] as InspectionProfileEntry[]
+    [new UnusedDefInspection(),
+     new GrUnusedIncDecInspection(),
+     new GroovyUnusedDeclarationInspection(),
+     new UnusedDeclarationInspectionBase(true)]
   }
 
   public void testUnusedVariable() { doTest() }
@@ -39,13 +41,13 @@ class GrUnusedDefTest extends GrHighlightingTestBase {
 
   public void testDefinitionUsedInSwitchCase() { doTest() }
 
-  public void testUnusedDefinitionForMethodMissing() { doTest()}
+  public void testUnusedDefinitionForMethodMissing() { doTest() }
 
   public void testPrefixIncrementCfa() { doTest() }
 
   public void testIfIncrementElseReturn() { doTest() }
 
-  public void testSwitchControlFlow() { doTest()}
+  public void testSwitchControlFlow() { doTest() }
 
   public void testUsageInInjection() { doTest() }
 
@@ -62,7 +64,7 @@ class GrUnusedDefTest extends GrHighlightingTestBase {
   public void testGloballyUnusedSymbols() { doTest() }
 
   public void testGloballyUnusedInnerMethods() {
-    myFixture.addClass 'package junit.framework public class TestCase {}'
+    myFixture.addClass 'package junit.framework; public class TestCase {}'
     doTest()
   }
 
@@ -82,33 +84,21 @@ class <warning descr="Class Foo is unused">Foo</warning> {
   }
 
   void testUsedVar() {
-    testHighlighting('''\
-def <warning descr="Method foo is unused">foo</warning>(xxx) {
-  if ((<warning descr="Result of assignment expression used">xxx = 5</warning>) || xxx) {
-    <warning descr="Result of assignment expression used"><warning descr="Assignment is not used">xxx</warning>=4</warning>
-  }
-}
+    testHighlighting '''
+      def <warning descr="Method foo is unused">foo</warning>(xxx) {
+        if ((xxx = 5) || xxx) {
+          <warning descr="Assignment is not used">xxx</warning>=4
+        }
+      }
 
-def <warning descr="Method foxo is unused">foxo</warning>(doo) {
-  def xxx = 'asdf'
-  if (!doo) {
-    println xxx
-    <warning descr="Result of assignment expression used"><warning descr="Assignment is not used">xxx</warning>=5</warning>
-  }
-}
-
-def <warning descr="Method bar is unused">bar</warning>(xxx) {
-  print ((<warning descr="Result of assignment expression used">xxx=5</warning>)?:xxx)
-}
-
-def <warning descr="Method a is unused">a</warning>(xxx) {
-  if (2 && (<warning descr="Result of assignment expression used">xxx=5</warning>)) {
-    xxx
-  }
-  else {
-  }
-}
-''')
+      def <warning descr="Method foxo is unused">foxo</warning>(doo) {
+        def xxx = 'asdf'
+        if (!doo) {
+          println xxx
+          <warning descr="Assignment is not used">xxx</warning>=5
+        }
+      }
+    '''
   }
 
   void testFallthroughInSwitch() {
@@ -134,5 +124,4 @@ def <warning descr="Method f2 is unused">f2</warning>(String foo, int mode) {
 def <warning descr="Variable is not used">abc</warning>
 ''')
   }
-
 }

plugins/groovy/testdata/highlighting/ResultOfAssignmentUsed.groovy

@@ -1,7 +1,7 @@
 public class AssResult {
   public void context(boolean b, String ps) {
     String vs = 'prefix'
-    if (b) vs += ps  // warning
+    if (b) vs += ps  // no warning
     if (b) { vs += ps } // no warning
     print <warning descr="Result of assignment expression used">vs = 4</warning>
     println ps

plugins/groovy/testdata/highlighting/UnusedDefsForArgs.groovy

@@ -1 +1 @@
-<warning descr="Result of assignment expression used"><warning descr="Assignment is not used">args</warning> = []</warning>
+<warning descr="Assignment is not used">args</warning> = []

plugins/groovy/testdata/highlighting/UnusedInc.groovy

@@ -4,4 +4,4 @@ print (a<warning descr="Unused ++">++</warning>)
 
 def b = 3
 b<warning descr="Unused ++">++</warning>
-       <warning descr="Result of assignment expression used"><warning descr="Assignment is not used">b</warning> = 3</warning>
+       <warning descr="Assignment is not used">b</warning> = 3