[java] ImmutableField false positive with inner class

Fixes 
This commit is contained in:
Andreas Dangel
2019-11-15 10:17:59 +01:00
parent 9923dd28de
commit 9dbbfba420
3 changed files with 42 additions and 3 deletions
docs/pages
pmd-java/src
main/java/net/sourceforge/pmd/lang/java/rule/design
test/resources/net/sourceforge/pmd/lang/java/rule/design/xml

@ -23,6 +23,8 @@ This is a {{ site.pmd.release_type }} release.
* java
* [#1861](https://github.com/pmd/pmd/issues/1861): \[java] Be more lenient with version numbers
* [#2105](https://github.com/pmd/pmd/issues/2105): \[java] Wrong name for inner classes in violations
* java-design
* [#2075](https://github.com/pmd/pmd/issues/2075): \[java] ImmutableField false positive with inner class
### API Changes

@ -60,7 +60,7 @@ public class ImmutableFieldRule extends AbstractLombokAwareRule {
continue;
}
FieldImmutabilityType type = initializedInConstructor(entry.getValue(), new HashSet<>(constructors));
FieldImmutabilityType type = initializedInConstructor(field, entry.getValue(), new HashSet<>(constructors));
if (type == FieldImmutabilityType.MUTABLE) {
continue;
}
@ -75,7 +75,7 @@ public class ImmutableFieldRule extends AbstractLombokAwareRule {
return field.getAccessNodeParent().hasDescendantOfType(ASTVariableInitializer.class);
}
private FieldImmutabilityType initializedInConstructor(List<NameOccurrence> usages, Set<ASTConstructorDeclaration> allConstructors) {
private FieldImmutabilityType initializedInConstructor(VariableNameDeclaration field, List<NameOccurrence> usages, Set<ASTConstructorDeclaration> allConstructors) {
FieldImmutabilityType result = FieldImmutabilityType.MUTABLE;
int methodInitCount = 0;
int lambdaUsage = 0;
@ -85,7 +85,7 @@ public class ImmutableFieldRule extends AbstractLombokAwareRule {
if (jocc.isOnLeftHandSide() || jocc.isSelfAssignment()) {
Node node = jocc.getLocation();
ASTConstructorDeclaration constructor = node.getFirstParentOfType(ASTConstructorDeclaration.class);
if (constructor != null) {
if (constructor != null && isSameClass(field, constructor)) {
if (inLoopOrTry(node)) {
continue;
}
@ -124,6 +124,14 @@ public class ImmutableFieldRule extends AbstractLombokAwareRule {
return result;
}
/**
* Checks whether the given constructor belongs to the class, in which the field is declared.
* This might not be the case for inner classes, which accesses the fields of the outer class.
*/
private boolean isSameClass(VariableNameDeclaration field, ASTConstructorDeclaration constructor) {
return constructor.getFirstParentOfType(ASTClassOrInterfaceBody.class) == field.getNode().getFirstParentOfType(ASTClassOrInterfaceBody.class);
}
private boolean inLoopOrTry(Node node) {
return node.getFirstParentOfType(ASTTryStatement.class) != null
|| node.getFirstParentOfType(ASTForStatement.class) != null

@ -496,6 +496,35 @@ public class Foo {
public Foo() {
x = "bar";
}
}
]]></code>
</test-code>
<test-code>
<description>#2075 [java] ImmutableField false positive with inner class</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Stack {
private Node first;
private int n; // can't be made final
public void insert(int x) {
first = new Node(first, x);
}
public int size() {
return n;
}
private class Node {
Node next;
int x;
private Node(Node next, int x) {
this.next = next;
this.x = x;
n++; // inner class updates instance variable in outer class
}
}
}
]]></code>
</test-code>