diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyResultOfAssignmentUsedInspection.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyResultOfAssignmentUsedInspection.java index efdd32fb904b..e90c7d99a020 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyResultOfAssignmentUsedInspection.java +++ b/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); + } } } diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java index 4d212ee49869..15e7087baa5d 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java +++ b/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); } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy index d294dbf5c047..e232b54e8720 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy +++ b/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()) } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrResultOfAssignmentUsedTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrResultOfAssignmentUsedTest.groovy new file mode 100644 index 000000000000..cd094170d668 --- /dev/null +++ b/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 = '' + final warnEnd = '' + + 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} }" + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrUnusedDefTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrUnusedDefTest.groovy index 462045b9a785..ee3d798e6c68 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrUnusedDefTest.groovy +++ b/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 Foo { } void testUsedVar() { - testHighlighting('''\ -def foo(xxx) { - if ((xxx = 5) || xxx) { - xxx=4 - } -} + testHighlighting ''' + def foo(xxx) { + if ((xxx = 5) || xxx) { + xxx=4 + } + } -def foxo(doo) { - def xxx = 'asdf' - if (!doo) { - println xxx - xxx=5 - } -} - -def bar(xxx) { - print ((xxx=5)?:xxx) -} - -def a(xxx) { - if (2 && (xxx=5)) { - xxx - } - else { - } -} -''') + def foxo(doo) { + def xxx = 'asdf' + if (!doo) { + println xxx + xxx=5 + } + } + ''' } void testFallthroughInSwitch() { @@ -134,5 +124,4 @@ def f2(String foo, int mode) { def abc ''') } - } diff --git a/plugins/groovy/testdata/highlighting/ResultOfAssignmentUsed.groovy b/plugins/groovy/testdata/highlighting/ResultOfAssignmentUsed.groovy index d47a197deec4..cb5fb5b198a9 100644 --- a/plugins/groovy/testdata/highlighting/ResultOfAssignmentUsed.groovy +++ b/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 vs = 4 println ps diff --git a/plugins/groovy/testdata/highlighting/UnusedDefsForArgs.groovy b/plugins/groovy/testdata/highlighting/UnusedDefsForArgs.groovy index c77f21708083..6c4a1ae35984 100644 --- a/plugins/groovy/testdata/highlighting/UnusedDefsForArgs.groovy +++ b/plugins/groovy/testdata/highlighting/UnusedDefsForArgs.groovy @@ -1 +1 @@ -args = [] \ No newline at end of file +args = [] \ No newline at end of file diff --git a/plugins/groovy/testdata/highlighting/UnusedInc.groovy b/plugins/groovy/testdata/highlighting/UnusedInc.groovy index f5bee8453032..190915f47a33 100644 --- a/plugins/groovy/testdata/highlighting/UnusedInc.groovy +++ b/plugins/groovy/testdata/highlighting/UnusedInc.groovy @@ -4,4 +4,4 @@ print (a++) def b = 3 b++ - b = 3 \ No newline at end of file + b = 3 \ No newline at end of file