Merge branch 'issue-2181'

docs/pages/release_notes.md

@@ -36,6 +36,7 @@ This is a {{ site.pmd.release_type }} release.
     *   [#2569](https://github.com/pmd/pmd/issues/2569): \[java] LiteralsFirstInComparisons: False negative for methods returning Strings
 *   java-design
     *   [#2174](https://github.com/pmd/pmd/issues/2174): \[java] LawOfDemeter: False positive with 'this' pointer
+    *   [#2181](https://github.com/pmd/pmd/issues/2181): \[java] LawOfDemeter: False positive with indexed array access
     *   [#2189](https://github.com/pmd/pmd/issues/2189): \[java] LawOfDemeter: False positive when casting to derived class
     *   [#2580](https://github.com/pmd/pmd/issues/2580): \[java] AvoidThrowingNullPointerException marks all NullPointerException objects as wrong, whether or not thrown
 *   java-errorprone

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java

@@ -25,6 +25,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
 import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
 import net.sourceforge.pmd.lang.java.symboltable.LocalScope;
@@ -56,6 +57,10 @@ public class LawOfDemeterRule extends AbstractJavaRule {
     private static final String REASON_OBJECT_NOT_CREATED_LOCALLY = "object not created locally";
     private static final String REASON_STATIC_ACCESS = "static property access";
 
+    public LawOfDemeterRule() {
+        addRuleChainVisit(ASTMethodDeclaration.class);
+    }
+
     /**
      * That's a new method. We are going to check each method call inside the
      * method.
@@ -140,11 +145,17 @@ public class LawOfDemeterRule extends AbstractJavaRule {
             List<MethodCall> result = new ArrayList<>();
 
             if (isNotAConstructorCall(expression) && isNotLiteral(expression) && hasSuffixesWithArguments(expression)) {
-                ASTPrimaryPrefix prefixNode = expression.getFirstDescendantOfType(ASTPrimaryPrefix.class);
-                MethodCall firstMethodCallInChain = new MethodCall(expression, prefixNode);
-                result.add(firstMethodCallInChain);
+                ASTPrimaryPrefix prefixNode = expression.getFirstChildOfType(ASTPrimaryPrefix.class);
+                ASTPrimarySuffix followingSuffix = getFollowingSuffix(prefixNode);
+                boolean firstExpressionIsMethodCall = followingSuffix != null && followingSuffix.isArguments();
+                MethodCall firstMethodCallInChain = null;
 
-                if (firstMethodCallInChain.isNotBuilder()) {
+                if (firstExpressionIsMethodCall) {
+                    firstMethodCallInChain = new MethodCall(expression, prefixNode);
+                    result.add(firstMethodCallInChain);
+                }
+
+                if (firstMethodCallInChain == null || firstMethodCallInChain.isNotBuilder()) {
                     List<ASTPrimarySuffix> suffixes = findSuffixesWithoutArguments(expression);
                     for (ASTPrimarySuffix suffix : suffixes) {
                         if (!expression.hasDescendantOfType(ASTCastExpression.class)) {
@@ -154,9 +165,28 @@ public class LawOfDemeterRule extends AbstractJavaRule {
                 }
             }
 
+            // when method chaining is detected, it must consist of multiple method calls
+            if (result.size() == 1) {
+                MethodCall m = result.get(0);
+                if (m.isViolation() && SCOPE_METHOD_CHAINING.equals(m.baseScope)) {
+                    result.clear();
+                }
+            }
+
             return result;
         }
 
+        private static ASTPrimarySuffix getFollowingSuffix(JavaNode node) {
+            int current = node.getIndexInParent();
+            if (node.getParent().getNumChildren() > current + 1) {
+                JavaNode following = node.getParent().getChild(current + 1);
+                if (following instanceof ASTPrimarySuffix) {
+                    return (ASTPrimarySuffix) following;
+                }
+            }
+            return null;
+        }
+
         private static boolean isNotAConstructorCall(ASTPrimaryExpression expression) {
             return !expression.hasDescendantOfType(ASTAllocationExpression.class);
         }
@@ -180,7 +210,7 @@ public class LawOfDemeterRule extends AbstractJavaRule {
             if (hasRealPrefix(expr)) {
                 List<ASTPrimarySuffix> suffixes = expr.findChildrenOfType(ASTPrimarySuffix.class);
                 for (ASTPrimarySuffix suffix : suffixes) {
-                    if (!suffix.isArguments()) {
+                    if (!suffix.isArguments() && !suffix.isArrayDereference()) {
                         result.add(suffix);
                     }
                 }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/LawOfDemeter.xml

@@ -342,6 +342,24 @@ class TaskManager {
             }
         }
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>[java] LawOfDemeter: False positive with indexed array access #2181</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public final class Util {
+    public static boolean check(String passwd, String hashed) {
+        try {
+            final String[] parts = hashed.split("\\$");
+
+            if (parts.length != 5 || !parts[1].equals("s0")) {      // wrong violation - method chain calls
+                throw new IllegalArgumentException("Invalid hashed value");
+            }
+        } catch (Exception e) { }
+    }
 }
         ]]></code>
     </test-code>