Merge branch 'issue-1448' into pmd/5.4.x

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

@@ -17,6 +17,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTDoStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
@@ -32,9 +33,14 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
  */
 public class ImmutableFieldRule extends AbstractJavaRule {
 
-    private static final int MUTABLE = 0;
-    private static final int IMMUTABLE = 1;
-    private static final int CHECKDECL = 2;
+    private enum FieldImmutabilityType {
+        /** Variable is changed in methods and/or in lambdas */
+        MUTABLE,
+        /** Variable is not changed outside the constructor. */
+        IMMUTABLE,
+        /** Variable is only written during declaration, if at all. */
+        CHECKDECL;
+    }
 
     @Override
     public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
@@ -47,11 +53,11 @@ public class ImmutableFieldRule extends AbstractJavaRule {
                 continue;
             }
 
-            int result = initializedInConstructor(entry.getValue(), new HashSet<>(constructors));
-            if (result == MUTABLE) {
+            FieldImmutabilityType result = initializedInConstructor(entry.getValue(), new HashSet<>(constructors));
+            if (result == FieldImmutabilityType.MUTABLE) {
                 continue;
             }
-            if (result == IMMUTABLE || result == CHECKDECL && initializedWhenDeclared(field)) {
+            if (result == FieldImmutabilityType.IMMUTABLE || result == FieldImmutabilityType.CHECKDECL && initializedWhenDeclared(field)) {
                 addViolation(data, field.getNode(), field.getImage());
             }
         }
@@ -62,9 +68,10 @@ public class ImmutableFieldRule extends AbstractJavaRule {
         return ((Node)field.getAccessNodeParent()).hasDescendantOfType(ASTVariableInitializer.class);
     }
 
-    private int initializedInConstructor(List<NameOccurrence> usages, Set<ASTConstructorDeclaration> allConstructors) {
-        int result = MUTABLE;
+    private FieldImmutabilityType initializedInConstructor(List<NameOccurrence> usages, Set<ASTConstructorDeclaration> allConstructors) {
+        FieldImmutabilityType result = FieldImmutabilityType.MUTABLE;
         int methodInitCount = 0;
+        int lambdaUsage = 0;
         Set<Node> consSet = new HashSet<>();
         for (NameOccurrence occ: usages) {
             JavaNameOccurrence jocc = (JavaNameOccurrence)occ;
@@ -89,16 +96,18 @@ public class ImmutableFieldRule extends AbstractJavaRule {
                 } else {
                     if (node.getFirstParentOfType(ASTMethodDeclaration.class) != null) {
                         methodInitCount++;
+                    } else if (node.getFirstParentOfType(ASTLambdaExpression.class) != null) {
+                        lambdaUsage++;
                     }
                 }
             }
         }
-        if (usages.isEmpty() || methodInitCount == 0 && consSet.isEmpty()) {
-            result = CHECKDECL;
+        if (usages.isEmpty() || methodInitCount == 0 && lambdaUsage == 0 && consSet.isEmpty()) {
+            result = FieldImmutabilityType.CHECKDECL;
         } else {
             allConstructors.removeAll(consSet);
-            if (allConstructors.isEmpty() && methodInitCount == 0) {
-                result = IMMUTABLE;
+            if (allConstructors.isEmpty() && methodInitCount == 0 && lambdaUsage == 0) {
+                result = FieldImmutabilityType.IMMUTABLE;
             }
         }
         return result;

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

@@ -362,6 +362,29 @@ public class Foo {
         b.i=0;
         return b.i;
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>#1448 [java] ImmutableField: Private field in inner class gives false positive with lambdas</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class CombinersTest {
+
+private static final BinaryOperator<Purse> ADDITION = (p1, p2) -> {
+    p1.amount += p2.amount;
+    return p1;
+};
+
+private static class Purse {
+    private int amount;
+
+    public Purse(final int amount) {
+        this.amount = amount;
+    }
+  }
+
 }
         ]]></code>
     </test-code>

src/site/markdown/overview/changelog.md

@@ -47,6 +47,7 @@ See also [bugfix #1556](https://sourceforge.net/p/pmd/bugs/1556/).
     *   [#208](https://github.com/pmd/pmd/issues/208): \[java] Parse error with local class with 2 or more annotations
     *   [#213](https://github.com/pmd/pmd/issues/213): \[java] CPD: OutOfMemory when analyzing Lucene
 *   java-design
+    *   [#1448](https://sourceforge.net/p/pmd/bugs/1448/): \[java] ImmutableField: Private field in inner class gives false positive with lambdas
     *   [#1552](https://sourceforge.net/p/pmd/bugs/1552/): \[java] MissingBreakInSwitch - False positive for continue
     *   [#1556](https://sourceforge.net/p/pmd/bugs/1556/): \[java] UseLocaleWithCaseConversions does not works with `ResultSet` (false negative)
     *   [#177](https://github.com/pmd/pmd/issues/177): \[java] SingularField with lambdas as final fields