Merge branch 'pr-266'

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

@@ -45,7 +45,7 @@ import net.sourceforge.pmd.lang.symboltable.Scope;
  * <li>K.J. Lieberherr and I.M. Holland. Assuring good style for object-oriented
  * programs. Software, IEEE, 6(5):38–48, 1989.</li>
  * </ul>
- * 
+ *
  * @since 5.0
  *
  */
@@ -57,7 +57,7 @@ public class LawOfDemeterRule extends AbstractJavaRule {
     /**
      * That's a new method. We are going to check each method call inside the
      * method.
-     * 
+     *
      * @return <code>null</code>.
      */
     @Override
@@ -129,7 +129,7 @@ public class LawOfDemeterRule extends AbstractJavaRule {
          * Factory method to convert a given primary expression into
          * MethodCalls. In case the primary expression represents a method chain
          * call, then multiple MethodCalls are returned.
-         * 
+         *
          * @return a list of MethodCalls, might be empty.
          */
         public static List<MethodCall> createMethodCalls(ASTPrimaryExpression expression) {
@@ -317,11 +317,12 @@ public class LawOfDemeterRule extends AbstractJavaRule {
 
             ASTBlock block = expression.getFirstParentOfType(ASTMethodDeclaration.class)
                     .getFirstChildOfType(ASTBlock.class);
-
+            //get all variableDeclarators within this block
             List<ASTVariableDeclarator> variableDeclarators = block.findDescendantsOfType(ASTVariableDeclarator.class);
             for (ASTVariableDeclarator declarator : variableDeclarators) {
                 ASTVariableDeclaratorId variableDeclaratorId = declarator
                         .getFirstChildOfType(ASTVariableDeclaratorId.class);
+                //we only care about it if the image name matches the current baseName
                 if (variableDeclaratorId.hasImageEqualTo(baseName)) {
                     boolean allocationFound = declarator
                             .getFirstDescendantOfType(ASTAllocationExpression.class) != null;
@@ -331,18 +332,33 @@ public class LawOfDemeterRule extends AbstractJavaRule {
                 }
             }
 
+            //get all AssignmentOperators within this block
             List<ASTAssignmentOperator> assignmentStmts = block.findDescendantsOfType(ASTAssignmentOperator.class);
             for (ASTAssignmentOperator stmt : assignmentStmts) {
-                if (stmt.hasImageEqualTo(SIMPLE_ASSIGNMENT_OPERATOR)) {
-                    boolean allocationFound = stmt.jjtGetParent()
-                            .getFirstDescendantOfType(ASTAllocationExpression.class) != null;
-                    boolean iterator = isIterator();
-                    assignments.add(new Assignment(stmt.getBeginLine(), allocationFound, iterator, false));
+                //we only care about it if it occurs prior to (or on) the beginLine of the current expression
+                //and if it is a simple_assignement_operator
+                if (stmt.getBeginLine() <= expression.getBeginLine()
+                        && stmt.hasImageEqualTo(SIMPLE_ASSIGNMENT_OPERATOR)) {
+                    //now we need to make sure it has the right image name
+                    ASTPrimaryPrefix primaryPrefix = stmt.jjtGetParent()
+                            .getFirstDescendantOfType(ASTPrimaryPrefix.class);
+                    if (primaryPrefix != null) {
+                        ASTName prefixName = primaryPrefix.getFirstChildOfType(ASTName.class);
+                        if (prefixName != null && prefixName.hasImageEqualTo(baseName)) {
+                            //this is an assignment related to the baseName we are working with
+                            boolean allocationFound = stmt.jjtGetParent()
+                                    .getFirstDescendantOfType(ASTAllocationExpression.class) != null;
+                            boolean iterator = isIterator();
+                            assignments
+                                    .add(new Assignment(stmt.getBeginLine(), allocationFound, iterator, false));
+                        }
+                    }
                 }
             }
 
             Assignment result = null;
             if (!assignments.isEmpty()) {
+                //sort them in reverse order and return the first one
                 Collections.sort(assignments);
                 result = assignments.get(0);
             }

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

@@ -43,7 +43,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>Simple Method calls with chaining</description>
         <expected-problems>1</expected-problems>
@@ -55,7 +55,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>Simple Method calls with local created object</description>
         <expected-problems>0</expected-problems>
@@ -68,7 +68,22 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
+    <test-code>
+        <description>Simple Method calls with local created object and other variable assignment</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    public void example() {
+        String something;
+        C c = new C();
+        c.doIt();
+        something = "no worries";
+    }
+}
+        ]]></code>
+    </test-code>
+
     <test-code>
         <description>Simple Method calls with local created object and variables</description>
         <expected-problems>0</expected-problems>
@@ -82,7 +97,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>Simple Method call on local created object within nesting local scopes</description>
         <expected-problems>0</expected-problems>
@@ -99,7 +114,7 @@ public class Foo {
 }
         ]]></code>
     </test-code>
-    
+
     <test-code>
         <description>Example documentation</description>
         <expected-problems>2</expected-problems>
@@ -111,18 +126,18 @@ public class Foo {
     public void example(Bar b) {
         // this method call is ok, as b is a parameter of "example"
         C c = b.getC();
-        
+
         // this method call is a violation, as we are using c, which we got from B.
         // We should ask b directly instead, e.g. "b.doItOnC();"
         c.doIt();
-        
+
         // this is also a violation, just differently expressed as a method chain without temporary variables.
         b.getC().doIt();
-        
+
         // that's a constructor call, not a method call.
         D d = new D();
         // this method call is ok, because we have create the new instance of D locally.
-        d.doSomethingElse(); 
+        d.doSomethingElse();
     }
 }
         ]]></code>
@@ -170,7 +185,7 @@ public class Foo {
                 }
             }
         }
-        
+
         List&lt;String&gt; anotherList = calcList();
         for (String s : anotherList) {
             if (!s.isEmpty()) {

src/site/markdown/overview/changelog.md

@@ -319,6 +319,8 @@ For example:
     *   [#1545](https://sourceforge.net/p/pmd/bugs/1545/): \[java] Symbol Table fails to resolve inner classes
 *   java-basic
     *   [#232](https://github.com/pmd/pmd/issues/232): \[java] SimplifiedTernary: Incorrect ternary operation can be simplified.
+*   java-coupling
+    *   [#270](https://github.com/pmd/pmd/issues/270): \[java] LoD false positive
 *   java-design
     *   [#933](https://sourceforge.net/p/pmd/bugs/933/): \[java] UnnecessaryLocalBeforeReturn false positive for SuppressWarnings annotation
     *   [#1448](https://sourceforge.net/p/pmd/bugs/1448/): \[java] ImmutableField: Private field in inner class gives false positive with lambdas
@@ -405,5 +407,6 @@ For example:
 *   [#228](https://github.com/pmd/pmd/pull/228): \[apex] Excluding count from CRUD/FLS checks
 *   [#229](https://github.com/pmd/pmd/pull/229): \[apex] Dynamic SOQL is safe against Integer, Boolean, Double
 *   [#231](https://github.com/pmd/pmd/pull/231): \[apex] CRUD/FLS rule - add support for fields
+*   [#266](https://github.com/pmd/pmd/pull/266): \[java] corrected invalid reporting of LoD violation
 *   [#268](https://github.com/pmd/pmd/pull/268): \[apex] Support safe escaping via String method