ForLoopThatDoesntUseLoopVariableInspection: simplified and tested; fix for IDEA-166869 Unexpected 'for loop where update or condition doesn't use loop variable

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);
     }
   }
 }

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();
     }
   }
 

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++) {
+      <warning descr="'for' statement has update which does not use the for loop variable">for</warning> (int j = 0; test(j); i++) {
+        System.out.println(i + ":" + j);
+      }
+    }
+  }
+
+  void bug2() {
+    for (int i = 0; test(i); i++) {
+      <warning descr="'for' statement has condition which does not use the for loop variable">for</warning> (int j = 0; test(i); j++) {
+        System.out.println(i + ":" + j);
+      }
+    }
+  }
+
+  // IDEA-166869
+  void test(List<String> 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<String> lines) {
+    int i = 0, j = 0;
+    <warning descr="'for' statement has update which does not use the for loop variable">for</warning> (int size = lines.size(); i < size; j++) {
+      if (lines.get(i).isEmpty()) break;
+    }
+    System.out.println(j);
+  }
+}

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;
+  }
+}