diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspection.java index f71c7b4aea41..1c403f20386a 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspection.java @@ -16,19 +16,23 @@ package com.siyeh.ig.bugs; import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.ComparisonUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.VariableAccessUtils; import org.jetbrains.annotations.NotNull; -public class ForLoopThatDoesntUseLoopVariableInspection - extends BaseInspection { +import static com.intellij.util.ObjectUtils.tryCast; + +public class ForLoopThatDoesntUseLoopVariableInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "for.loop.not.use.loop.variable.display.name"); + return InspectionGadgetsBundle.message("for.loop.not.use.loop.variable.display.name"); } @Override @@ -53,135 +57,57 @@ public class ForLoopThatDoesntUseLoopVariableInspection return new ForLoopThatDoesntUseLoopVariableVisitor(); } - private static class ForLoopThatDoesntUseLoopVariableVisitor - extends BaseInspectionVisitor { + private static class ForLoopThatDoesntUseLoopVariableVisitor extends BaseInspectionVisitor { @Override public void visitForStatement(@NotNull PsiForStatement statement) { super.visitForStatement(statement); - if (conditionUsesInitializer(statement)) { - if (!updateUsesInitializer(statement)) { - registerStatementError(statement, - Boolean.FALSE, Boolean.TRUE); - } - } - else { - if (updateUsesInitializer(statement)) { - registerStatementError(statement, - Boolean.TRUE, Boolean.FALSE); - } - else { - registerStatementError(statement, - Boolean.TRUE, Boolean.TRUE); - } + PsiLocalVariable variable = extractInitializerVariable(statement); + if (variable == null) return; + boolean notUsedInCondition = !conditionUsesVariable(statement, variable); + boolean notUsedInUpdate = !updateUsesVariable(statement, variable); + if (notUsedInCondition || notUsedInUpdate) { + if (!notUsedInCondition && isDeclarationUsedAsBound(statement, variable)) return; + registerStatementError(statement, notUsedInCondition, notUsedInUpdate); } } - private static boolean conditionUsesInitializer( - PsiForStatement statement) { - final PsiStatement initialization = statement.getInitialization(); + private static boolean isDeclarationUsedAsBound(PsiForStatement statement, PsiLocalVariable boundVar) { + PsiBinaryExpression condition = tryCast(PsiUtil.skipParenthesizedExprDown(statement.getCondition()), PsiBinaryExpression.class); + if (condition == null || !ComparisonUtils.isComparisonOperation(condition.getOperationTokenType())) return false; + PsiExpression otherOperand = null; + if (ExpressionUtils.isReferenceTo(condition.getLOperand(), boundVar)) { + otherOperand = condition.getROperand(); + } else if (ExpressionUtils.isReferenceTo(condition.getROperand(), boundVar)) { + otherOperand = condition.getLOperand(); + } + if (otherOperand == null) return false; + PsiReferenceExpression ref = tryCast(PsiUtil.skipParenthesizedExprDown(otherOperand), PsiReferenceExpression.class); + if (ref == null) return false; + PsiLocalVariable indexVar = tryCast(ref.resolve(), PsiLocalVariable.class); + if (indexVar == null) return false; + PsiStatement update = statement.getUpdate(); + return VariableAccessUtils.variableIsIncremented(indexVar, update) || VariableAccessUtils.variableIsDecremented(indexVar, update); + } + + private static PsiLocalVariable extractInitializerVariable(PsiForStatement statement) { + final PsiDeclarationStatement declaration = tryCast(statement.getInitialization(), PsiDeclarationStatement.class); + if (declaration == null) return null; + + final PsiElement[] declaredElements = declaration.getDeclaredElements(); + if (declaredElements.length != 1) return null; + + return tryCast(declaredElements[0], PsiLocalVariable.class); + } + + private static boolean conditionUsesVariable(PsiForStatement statement, PsiLocalVariable variable) { final PsiExpression condition = statement.getCondition(); - - if (initialization == null) { - return true; - } - if (condition == null) { - return true; - } - if (!(initialization instanceof PsiDeclarationStatement)) { - return true; - } - final PsiDeclarationStatement declaration = - (PsiDeclarationStatement)initialization; - final PsiElement[] declaredElements = - declaration.getDeclaredElements(); - if (declaredElements.length != 1) { - return true; - } - if (declaredElements[0] == null || - !(declaredElements[0] instanceof PsiLocalVariable)) { - return true; - } - final PsiLocalVariable localVar = - (PsiLocalVariable)declaredElements[0]; - return expressionUsesVariable(condition, localVar); + return condition == null || VariableAccessUtils.variableIsUsed(variable, condition); } - private static boolean updateUsesInitializer(PsiForStatement statement) { - final PsiStatement initialization = statement.getInitialization(); + private static boolean updateUsesVariable(PsiForStatement statement, PsiLocalVariable variable) { final PsiStatement update = statement.getUpdate(); - - if (initialization == null) { - return true; - } - if (update == null) { - return true; - } - if (!(initialization instanceof PsiDeclarationStatement)) { - return true; - } - final PsiDeclarationStatement declaration = - (PsiDeclarationStatement)initialization; - final PsiElement[] declaredElements = - declaration.getDeclaredElements(); - if (declaredElements.length != 1) { - return true; - } - if (declaredElements[0] == null || - !(declaredElements[0] instanceof PsiLocalVariable)) { - return true; - } - final PsiLocalVariable localVar = - (PsiLocalVariable)declaredElements[0]; - return statementUsesVariable(update, localVar); - } - - private static boolean statementUsesVariable(PsiStatement statement, - PsiLocalVariable localVar) { - final UseVisitor useVisitor = new UseVisitor(localVar); - statement.accept(useVisitor); - return useVisitor.isUsed(); - } - - private static boolean expressionUsesVariable(PsiExpression expression, - PsiLocalVariable localVar) { - final UseVisitor useVisitor = new UseVisitor(localVar); - expression.accept(useVisitor); - return useVisitor.isUsed(); - } - } - - private static class UseVisitor extends JavaRecursiveElementWalkingVisitor { - - private final PsiLocalVariable variable; - private boolean used; - - private UseVisitor(PsiLocalVariable var) { - variable = var; - } - - @Override - public void visitElement(@NotNull PsiElement element) { - if (!used) { - super.visitElement(element); - } - } - - @Override - public void visitReferenceExpression( - @NotNull PsiReferenceExpression ref) { - if (used) { - return; - } - super.visitReferenceExpression(ref); - final PsiElement resolvedElement = ref.resolve(); - if (variable.equals(resolvedElement)) { - used = true; - } - } - - public boolean isUsed() { - return used; + return update == null || VariableAccessUtils.variableIsUsed(variable, update); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/VariableUsedVisitor.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/VariableUsedVisitor.java index bd852930419b..ee0dc59f451a 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/VariableUsedVisitor.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/VariableUsedVisitor.java @@ -44,13 +44,8 @@ class VariableUsedVisitor extends JavaRecursiveElementWalkingVisitor { return; } super.visitReferenceExpression(referenceExpression); - final PsiElement target = referenceExpression.resolve(); - if (target == null) { - return; - } - if (target.equals(variable)) { + if (referenceExpression.isReferenceTo(variable)) { used = true; - //stopWalking(); } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/for_does_not_use_var/ForLoopThatDoesntUseLoopVariable.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/for_does_not_use_var/ForLoopThatDoesntUseLoopVariable.java new file mode 100644 index 000000000000..1a5e9d4dd2cd --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/for_does_not_use_var/ForLoopThatDoesntUseLoopVariable.java @@ -0,0 +1,55 @@ +/* + * 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. + * 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. + */ +import java.util.List; + +public class ForLoopThatDoesntUseLoopVariable { + boolean test(int i) { + return i < 10; + } + + void bug() { + for (int i = 0; test(i); i++) { + for (int j = 0; test(j); i++) { + System.out.println(i + ":" + j); + } + } + } + + void bug2() { + for (int i = 0; test(i); i++) { + for (int j = 0; test(i); j++) { + System.out.println(i + ":" + j); + } + } + } + + // IDEA-166869 + void test(List lines) { + int i = 0; + for (int size = lines.size(); i < size; i++) { + if (lines.get(i).isEmpty()) break; + } + System.out.println(i); + } + + void test2(List lines) { + int i = 0, j = 0; + for (int size = lines.size(); i < size; j++) { + if (lines.get(i).isEmpty()) break; + } + System.out.println(j); + } +} diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspectionTest.java new file mode 100644 index 000000000000..603df5de585f --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ForLoopThatDoesntUseLoopVariableInspectionTest.java @@ -0,0 +1,44 @@ +/* + * 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. + * 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 com.siyeh.ig.bugs; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.testFramework.LightProjectDescriptor; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; + +public class ForLoopThatDoesntUseLoopVariableInspectionTest extends LightInspectionTestCase { + + public void testForLoopThatDoesntUseLoopVariable() { + doTest(); + } + + @Override + protected InspectionProfileEntry getInspection() { + return new ForLoopThatDoesntUseLoopVariableInspection(); + } + + @Override + protected String getBasePath() { + return "/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/for_does_not_use_var"; + } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } +} \ No newline at end of file