From c64f6af1578cf759f22648345e0e1a88817e2dae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?=
 <juansotuyo@gmail.com>
Date: Sun, 31 Dec 2017 01:20:04 -0300
Subject: [PATCH] [java] Avoid NPE in ForLoopCanBeForeachRule

 - Take the chance to make this rule use the rulechain
 - This partially fixes #800, but the FN is still unhandled
---
 .../ForLoopCanBeForeachRule.java              | 25 +++++++++++++------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java
index 71c4f68b85..66bd8cbbb5 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java
@@ -35,6 +35,10 @@ import net.sourceforge.pmd.lang.symboltable.Scope;
  */
 public class ForLoopCanBeForeachRule extends AbstractJavaRule {
 
+    public ForLoopCanBeForeachRule() {
+        addRuleChainVisit(ASTForStatement.class);
+    }
+    
     @Override
     public Object visit(ASTForStatement node, Object data) {
 
@@ -43,13 +47,13 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
         final ASTExpression guardCondition = node.getFirstChildOfType(ASTExpression.class);
 
         if (init == null && update == null || guardCondition == null) {
-            return super.visit(node, data);
+            return data;
         }
 
         Entry<VariableNameDeclaration, List<NameOccurrence>> indexDecl = getIndexVarDeclaration(init, update);
 
         if (indexDecl == null) {
-            return super.visit(node, data);
+            return data;
         }
 
 
@@ -67,7 +71,7 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
 
 
         if (occurrences == null || !"int".equals(index.getTypeImage()) || !indexStartsAtZero(index)) {
-            return super.visit(node, data);
+            return data;
         }
 
 
@@ -76,14 +80,14 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
 
 
         if (!isForUpdateSimpleEnough(update, itName) || iterableName == null) {
-            return super.visit(node, data);
+            return data;
         }
 
         Entry<VariableNameDeclaration, List<NameOccurrence>> iterableInfo = findDeclaration(iterableName, node.getScope());
         VariableNameDeclaration iterableDeclaration = iterableInfo == null ? null : iterableInfo.getKey();
 
         if (iterableDeclaration == null) {
-            return super.visit(node, data);
+            return data;
         }
 
         if (iterableDeclaration.isArray() && isReplaceableArrayLoop(node, occurrences, iterableDeclaration)) {
@@ -94,7 +98,7 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
             addViolation(data, node);
         }
 
-        return super.visit(node, data);
+        return data;
     }
 
 
@@ -240,8 +244,13 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
             return null;
         }
 
-        String name = initializer.getFirstDescendantOfType(ASTName.class)
-                                 .getImage();
+        ASTName nameNode = initializer.getFirstDescendantOfType(ASTName.class);
+        if (nameNode == null) {
+            // TODO : This can happen if we are calling a local / statically imported method that returns the iterable - currently unhandled
+            return null;
+        }
+        
+        String name = nameNode.getImage();
         int dotIndex = name.indexOf('.');
 
         if (dotIndex > 0) {