diff --git a/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedAssignment.html b/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedAssignment.html index f10ac59f3c47..d093014e3eab 100644 --- a/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedAssignment.html +++ b/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedAssignment.html @@ -2,6 +2,6 @@ This inspection reports on unnecessary Groovy assignment statement -
Powered by InspectorGroovy +
diff --git a/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedIncOrDec.html b/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedIncOrDec.html new file mode 100644 index 000000000000..13ba5a2d65e5 --- /dev/null +++ b/plugins/groovy/resources/inspectionDescriptions/GroovyUnusedIncOrDec.html @@ -0,0 +1,6 @@ + + +This inspection reports on unnecessary Groovy incrementing and decrementing expressions +
+ + diff --git a/plugins/groovy/src/META-INF/plugin.xml b/plugins/groovy/src/META-INF/plugin.xml index f16a34ca5f0c..956a30ddf366 100644 --- a/plugins/groovy/src/META-INF/plugin.xml +++ b/plugins/groovy/src/META-INF/plugin.xml @@ -408,6 +408,9 @@ + diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties index b650ca7468de..28f439a39a95 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties @@ -35,6 +35,8 @@ groovy.dfa.issues=Data Flow Issues unused.assignment=Unused Assignment unused.assignment.tooltip=Assignment is not used +unused.inc.dec=Unused Incrementing or Decrementing + unassigned.access=Variable Not Assigned unassigned.access.short.name=VariableNotAssigned unassigned.access.tooltip=Variable ''{0}'' might not be assigned @@ -77,3 +79,7 @@ rtype.cannot.contain.ltype=''{1}'' cannot contain ''{0}'' new.instance.of.singleton=New instance of class annotated with @groovy.lang.Singleton replace.new.expression.with.0.instance=Replace with ''{0}.instance'' getter.0.clashes.with.getter.1={0} clashes with {1} +unused.0=Unused {0} +remove.0=Remove {0} +replace.postfix.0.with.prefix.0=Replace postfix {0} with prefix {0} +replace.0.with.1=Replace {0} with binary {1} 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 new file mode 100644 index 000000000000..8f8ec88a5cec --- /dev/null +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrUnusedIncDecInspection.java @@ -0,0 +1,217 @@ +/* + * Copyright 2000-2012 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.codeInspection.confusing; + +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.openapi.project.Project; +import com.intellij.psi.PsiElement; +import com.intellij.psi.tree.IElementType; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NonNls; +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.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.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.ReadWriteVariableInstruction; +import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; + +import java.util.List; + +/** + * @author Max Medvedev + */ +public class GrUnusedIncDecInspection extends BaseInspection { + @Override + protected BaseInspectionVisitor buildVisitor() { + return new GrUnusedIncDecInspectionVisitor(); + } + + @Override + public boolean isEnabledByDefault() { + return true; + } + + @Nls + @NotNull + public String getGroupDisplayName() { + return GroovyInspectionBundle.message("groovy.dfa.issues"); + } + + @Nls + @NotNull + public String getDisplayName() { + return GroovyInspectionBundle.message("unused.inc.dec"); + } + + @NonNls + @NotNull + public String getShortName() { + return "GroovyUnusedIncOrDec"; + } + + private static class GrUnusedIncDecInspectionVisitor extends BaseInspectionVisitor { + @Override + public void visitUnaryExpression(GrUnaryExpression expression) { + super.visitUnaryExpression(expression); + + IElementType opType = expression.getOperationTokenType(); + if (opType != GroovyTokenTypes.mINC && opType != GroovyTokenTypes.mDEC) return; + + GrExpression operand = expression.getOperand(); + if (!(operand instanceof GrReferenceExpression)) return; + + PsiElement resolved = ((GrReferenceExpression)operand).resolve(); + if (!(resolved instanceof GrVariable) || resolved instanceof GrField) return; + + List accesses = ControlFlowUtils.findAccess((GrVariable)resolved, expression.getOperand(), true, false); + boolean allAreWrite = true; + for (ReadWriteVariableInstruction access : accesses) { + if (!access.isWrite()) { + allAreWrite = false; + break; + } + } + + + if (allAreWrite) { + if (expression.isPostfix() && PsiUtil.isExpressionUsed(expression)) { + registerError(expression.getOperationToken(), + GroovyInspectionBundle.message("unused.0", expression.getOperationToken().getText()), + new LocalQuickFix[]{new ReplacePostfixIncWithPrefixFix(expression), new RemoveIncOrDecFix(expression)}, + ProblemHighlightType.LIKE_UNUSED_SYMBOL); + } + else if (!PsiUtil.isExpressionUsed(expression)) { + registerError(expression.getOperationToken(), + GroovyInspectionBundle.message("unused.0", expression.getOperationToken().getText()), LocalQuickFix.EMPTY_ARRAY, + ProblemHighlightType.LIKE_UNUSED_SYMBOL); + } + } + } + + private static class RemoveIncOrDecFix implements LocalQuickFix { + private final String myMessage; + + public RemoveIncOrDecFix(GrUnaryExpression expression) { + myMessage = GroovyInspectionBundle.message("remove.0", expression.getOperationToken().getText()); + } + + @NotNull + @Override + public String getName() { + return myMessage; + } + + @NotNull + @Override + public String getFamilyName() { + return myMessage; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + GrUnaryExpression expr = findUnaryExpression(descriptor); + if (expr == null) return; + + expr.replaceWithExpression(expr.getOperand(), true); + } + } + + private static class ReplacePostfixIncWithPrefixFix implements LocalQuickFix { + private final String myMessage; + + public ReplacePostfixIncWithPrefixFix(GrUnaryExpression expression) { + myMessage = GroovyInspectionBundle.message("replace.postfix.0.with.prefix.0", expression.getOperationToken().getText()); + } + + @NotNull + @Override + public String getName() { + return myMessage; + } + + @NotNull + @Override + public String getFamilyName() { + return myMessage; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + GrUnaryExpression expr = findUnaryExpression(descriptor); + if (expr == null) return; + + GrExpression prefix = GroovyPsiElementFactory.getInstance(project) + .createExpressionFromText(expr.getOperationToken().getText() + expr.getOperand().getText()); + + expr.replaceWithExpression(prefix, true); + } + } + + private static class ReplaceIncDecWithBinary implements LocalQuickFix { + private final String myMessage; + + public ReplaceIncDecWithBinary(GrUnaryExpression expression) { + String opToken = expression.getOperationToken().getText(); + myMessage = GroovyInspectionBundle.message("replace.0.with.1", opToken, opToken.substring(0, 1)); + } + + @NotNull + @Override + public String getName() { + return myMessage; + } + + @NotNull + @Override + public String getFamilyName() { + return myMessage; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + GrUnaryExpression expr = findUnaryExpression(descriptor); + GrExpression newExpr = GroovyPsiElementFactory.getInstance(project) + .createExpressionFromText(expr.getOperand().getText() + expr.getOperationToken().getText().substring(0, 1) + "1"); + expr.replaceWithExpression(newExpr, true); + } + } + } + + @Nullable + private static GrUnaryExpression findUnaryExpression(ProblemDescriptor descriptor) { + GrUnaryExpression expr; + PsiElement element = descriptor.getPsiElement(); + if (element == null) return null; + PsiElement parent = element.getParent(); + IElementType opType = element.getNode().getElementType(); + if (opType != GroovyTokenTypes.mINC && opType != GroovyTokenTypes.mDEC) return null; + if (!(parent instanceof GrUnaryExpression)) return null; + expr = (GrUnaryExpression)parent; + return expr; + } +} + 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 4103c4f5ddc8..84ce78bb2550 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 @@ -53,7 +53,6 @@ import org.jetbrains.plugins.groovy.lang.psi.dataFlow.DFAEngine; import org.jetbrains.plugins.groovy.lang.psi.dataFlow.DfaInstance; import org.jetbrains.plugins.groovy.lang.psi.dataFlow.Semilattice; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; -import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil; import java.util.*; @@ -673,7 +672,7 @@ public class ControlFlowUtils { } public static List findAccess(GrVariable local, final PsiElement place, boolean ahead, boolean writeAccessOnly) { - LOG.assertTrue(GroovyRefactoringUtil.isLocalVariable(local), local.getClass()); + LOG.assertTrue(!(local instanceof GrField), local.getClass()); final GrControlFlowOwner owner = findControlFlowOwner(local); assert owner != null; 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 d4f6fe10a871..6a241766c841 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy @@ -37,6 +37,7 @@ import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyAssignabilit import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyResultOfAssignmentUsedInspection import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyUncheckedAssignmentOfMemberOfRawTypeInspection import org.jetbrains.plugins.groovy.codeInspection.confusing.ClashingGettersInspection +import org.jetbrains.plugins.groovy.codeInspection.confusing.GrUnusedIncDecInspection import org.jetbrains.plugins.groovy.codeInspection.confusing.GroovyOctalIntegerInspection import org.jetbrains.plugins.groovy.codeInspection.confusing.GroovyResultOfIncrementOrDecrementUsedInspection import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialConditionalInspection @@ -248,8 +249,8 @@ class A { public void testByteArrayArgument() throws Exception {doTest(new GroovyAssignabilityCheckInspection());} public void testForLoopWithNestedEndlessLoop() throws Exception {doTest(new UnassignedVariableAccessInspection());} - public void testPrefixIncrementCfa() throws Exception {doTest(new UnusedDefInspection());} - public void testIfIncrementElseReturn() throws Exception {doTest(new UnusedDefInspection()); } + public void testPrefixIncrementCfa() throws Exception {doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection());} + public void testIfIncrementElseReturn() throws Exception {doTest(new UnusedDefInspection(), new GrUnusedIncDecInspection()); } public void testArrayLikeAccess() throws Exception {doTest();} diff --git a/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy b/plugins/groovy/testdata/highlighting/IfIncrementElseReturn.groovy index 04e1d86bc2e5..3eb7602a44a8 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/PrefixIncrementCfa.groovy b/plugins/groovy/testdata/highlighting/PrefixIncrementCfa.groovy index 711bb7b8c062..e886f8664bdf 100644 --- a/plugins/groovy/testdata/highlighting/PrefixIncrementCfa.groovy +++ b/plugins/groovy/testdata/highlighting/PrefixIncrementCfa.groovy @@ -1,7 +1,7 @@ int idx idx = 2 idx = 3 -if (++idx == 8) { //Assignment is used here +if (++idx == 8) { idx = 33 } print idx \ No newline at end of file