From 4dc61f8ec4f9c8956320032894dda97cd4592268 Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Tue, 20 Mar 2012 12:09:09 +0400 Subject: [PATCH] correct GrUnusedIncDecInpection --- .../confusing/GrUnusedIncDecInspection.java | 26 ++++++++++++++++++- .../utils/ControlFlowUtils.java | 26 ++++++++++++------- .../groovy/lang/GroovyHighlightingTest.groovy | 24 ++++++++++------- .../highlighting/IfIncrementElseReturn.groovy | 2 +- .../testdata/highlighting/UnusedInc.groovy | 7 +++++ 5 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 plugins/groovy/testdata/highlighting/UnusedInc.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrUnusedIncDecInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrUnusedIncDecInspection.java index 8f8ec88a5cec..fadb994f0ec0 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrUnusedIncDecInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrUnusedIncDecInspection.java @@ -18,6 +18,7 @@ package org.jetbrains.plugins.groovy.codeInspection.confusing; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiElement; import com.intellij.psi.tree.IElementType; @@ -30,21 +31,25 @@ import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor; import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle; import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; +import org.jetbrains.plugins.groovy.lang.psi.GrControlFlowOwner; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrUnaryExpression; +import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.ReadWriteVariableInstruction; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; +import java.util.Iterator; import java.util.List; /** * @author Max Medvedev */ public class GrUnusedIncDecInspection extends BaseInspection { + private static final Logger LOG = Logger.getInstance(GrUnusedIncDecInspection.class); @Override protected BaseInspectionVisitor buildVisitor() { return new GrUnusedIncDecInspectionVisitor(); @@ -87,7 +92,26 @@ public class GrUnusedIncDecInspection extends BaseInspection { PsiElement resolved = ((GrReferenceExpression)operand).resolve(); if (!(resolved instanceof GrVariable) || resolved instanceof GrField) return; - List accesses = ControlFlowUtils.findAccess((GrVariable)resolved, expression.getOperand(), true, false); + final GrControlFlowOwner owner = ControlFlowUtils.findControlFlowOwner(expression); + assert owner != null; + GrControlFlowOwner ownerOfDeclaration = ControlFlowUtils.findControlFlowOwner(resolved); + if (ownerOfDeclaration != owner) return; + + final Instruction cur = ControlFlowUtils.findInstruction(operand, owner.getControlFlow()); + + if (cur == null) { + LOG.error("no instruction found in flow." + "operand: " + operand.getText() + " cfo: " + owner.getText()); + } + + //get write access for inc or dec + Iterable successors = cur.allSuccessors(); + Iterator iterator = successors.iterator(); + LOG.assertTrue(iterator.hasNext()); + Instruction writeAccess = iterator.next(); + LOG.assertTrue(!iterator.hasNext()); + + List accesses = ControlFlowUtils.findAccess((GrVariable)resolved, true, false, writeAccess); + boolean allAreWrite = true; for (ReadWriteVariableInstruction access : accesses) { if (!access.isWrite()) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java index 84ce78bb2550..9acf0f2512e3 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java @@ -679,15 +679,21 @@ public class ControlFlowUtils { final Instruction cur = findInstruction(place, owner.getControlFlow()); - if (cur == null) throw new IllegalArgumentException("place is not in the flow"); + if (cur == null) { + throw new IllegalArgumentException("place is not in the flow"); + } + return findAccess(local, ahead, writeAccessOnly, cur); + } + + public static List findAccess(GrVariable local, boolean ahead, boolean writeAccessOnly, Instruction cur) { String name = local.getName(); - + final ArrayList result = new ArrayList(); final HashSet visited = new HashSet(); - + visited.add(cur); - + Queue queue = new ArrayDeque(); for (Instruction i : ahead ? cur.allSuccessors() : cur.allPredecessors()) { @@ -695,11 +701,11 @@ public class ControlFlowUtils { queue.add(i); } } - + while (true) { Instruction instruction = queue.poll(); if (instruction == null) break; - + if (instruction instanceof ReadWriteVariableInstruction) { ReadWriteVariableInstruction rw = (ReadWriteVariableInstruction)instruction; if (name.equals(rw.getVariableName())) { @@ -707,13 +713,13 @@ public class ControlFlowUtils { result.add(rw); continue; } - + if (!writeAccessOnly) { result.add(rw); } } } - + for (Instruction i : ahead ? instruction.allSuccessors() : instruction.allPredecessors()) { if (visited.add(i)) { queue.add(i); @@ -723,9 +729,9 @@ public class ControlFlowUtils { return result; } - + @Nullable - private static Instruction findInstruction(final PsiElement place, Instruction[] controlFlow) { + public static Instruction findInstruction(final PsiElement place, Instruction[] controlFlow) { return ContainerUtil.find(controlFlow, new Condition() { @Override public boolean value(Instruction instruction) { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy index 6a241766c841..e0ba3cd80dad 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy @@ -189,10 +189,10 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testUnassigned3() throws Exception { doTest(new UnassignedVariableAccessInspection()); } public void testUnassignedTryFinally() throws Exception { doTest(new UnassignedVariableAccessInspection()); } - public void testUnusedVariable() throws Exception { doTest(new UnusedDefInspection()); } - public void testDefinitionUsedInClosure() throws Exception { doTest(new UnusedDefInspection()); } - public void testDefinitionUsedInClosure2() throws Exception { doTest(new UnusedDefInspection()); } - public void testDefinitionUsedInSwitchCase() throws Exception { doTest(new UnusedDefInspection()); } + public void testUnusedVariable() throws Exception { doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } + public void testDefinitionUsedInClosure() throws Exception { doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } + public void testDefinitionUsedInClosure2() throws Exception { doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } + public void testDefinitionUsedInSwitchCase() throws Exception { doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } public void testDuplicateInnerClass() throws Throwable{doTest();} public void testThisInStaticContext() throws Throwable {doTest();} @@ -316,7 +316,7 @@ class A { public void testBuiltInTypeInstantiation() {doTest();} - public void testSwitchControlFlow() {doTest(new UnusedDefInspection(), new GroovyResultOfAssignmentUsedInspection());} + public void testSwitchControlFlow() {doTest(new UnusedDefInspection(), new GroovyResultOfAssignmentUsedInspection(), new GrUnusedIncDecInspection());} public void testRawTypeInAssignment() {doTest(new GroovyAssignabilityCheckInspection());} @@ -326,7 +326,7 @@ class A { IdeaTestUtil.assertTiming("", 10000, 1, new Runnable() { @Override public void run() { - doTest(new GroovyAssignabilityCheckInspection(), new UnusedDefInspection()); + doTest(new GroovyAssignabilityCheckInspection(), new UnusedDefInspection(), new GrUnusedIncDecInspection()); } }); } @@ -398,7 +398,7 @@ class A { doTest(new GroovyUnresolvedAccessInspection(), new GroovyUntypedAccessInspection()); } - public void testUsageInInjection() { doTest(new UnusedDefInspection()); } + public void testUsageInInjection() { doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } public void testDuplicatedNamedArgs() {doTest();} @@ -420,15 +420,19 @@ class A { } public void testUnusedDefsForArgs() { - doTest(new UnusedDefInspection()); + doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } public void testUsedDefBeforeTry1() { - doTest(new UnusedDefInspection()); + doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } public void testUsedDefBeforeTry2() { - doTest(new UnusedDefInspection()); + doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); + } + + public void testUnusedInc() { + doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()) } public void testStringAssignableToChar() { diff --git a/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy b/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy index 3eb7602a44a8..04e1d86bc2e5 100644 --- a/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy +++ b/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy @@ -1,7 +1,7 @@ int numPermutationsPrinted = 1; for ( int pos_right = 2; pos_right <= n; pos_right++ ) { - if (numPermutationsPrinted++ < 30) + if (numPermutationsPrinted++ < 30) { println "a" } diff --git a/plugins/groovy/testdata/highlighting/UnusedInc.groovy b/plugins/groovy/testdata/highlighting/UnusedInc.groovy new file mode 100644 index 000000000000..3483ac2f7b68 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/UnusedInc.groovy @@ -0,0 +1,7 @@ +def a = 4 +print (++a) +print (a++) + +def b = 3 +b++ +b = 3 \ No newline at end of file