From cd465918b5fbb77a350c0476ae875ca7c431c7db Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:05:20 +0200 Subject: [PATCH 1/5] Changed location for AssignmentToNonFinalStatic violations from field declaration at to assignment in constructor. This makes it easier to identify the code that needs to be adjusted in order to resolve the violation. --- .../errorprone/AssignmentToNonFinalStaticRule.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java index 4870e31778..0fdb1a95e1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java @@ -33,16 +33,15 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { continue; } - if (initializedInConstructor(entry.getValue())) { - addViolation(data, decl.getNode(), decl.getImage()); + final Node location = initializedInConstructor(entry.getValue()); + if (location != null) { + addViolation(data, location, decl.getImage()); } } return super.visit(node, data); } - private boolean initializedInConstructor(List usages) { - boolean initInConstructor = false; - + private Node initializedInConstructor(List usages) { for (NameOccurrence occ : usages) { // specifically omitting prefix and postfix operators as there are // legitimate usages of these with static fields, e.g. typesafe enum pattern. @@ -50,12 +49,12 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { Node node = occ.getLocation(); Node constructor = node.getFirstParentOfType(ASTConstructorDeclaration.class); if (constructor != null) { - initInConstructor = true; + return node; } } } - return initInConstructor; + return null; } } From 4a8c23472c0e3eb9df0e161890abccc89a0e8524 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:17:03 +0200 Subject: [PATCH 2/5] Added test case for multiple violations of AssignmentToNonFinalStatic on the same variable. Rationale: whenever this rule produces a violation, all of the unsafe assignments need to be corrected. It's annoying to fix one of these, rerun PMD and then realize there's another unsafe assignment left. Therefore, all of these violations should be reported at once. --- .../errorprone/xml/AssignmentToNonFinalStatic.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml index 7f4c7844d1..249aeb6839 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml @@ -28,6 +28,21 @@ public class Foo { Foo(int y) { x = y; } +} + ]]> + + + + 2 + From d4ca21bfd3e8d8a284a0bbfaa00cd847caae9b93 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:20:15 +0200 Subject: [PATCH 3/5] AssignmentToNonFinalStatic now reports multiple violations in case there are multiple unsafe assignments on the same variable. --- .../errorprone/AssignmentToNonFinalStaticRule.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java index 0fdb1a95e1..9c3f01631e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -33,15 +34,16 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { continue; } - final Node location = initializedInConstructor(entry.getValue()); - if (location != null) { + final List locations = initializedInConstructor(entry.getValue()); + for (final Node location : locations) { addViolation(data, location, decl.getImage()); } } return super.visit(node, data); } - private Node initializedInConstructor(List usages) { + private List initializedInConstructor(List usages) { + final List unsafeAssignments = new ArrayList<>(); for (NameOccurrence occ : usages) { // specifically omitting prefix and postfix operators as there are // legitimate usages of these with static fields, e.g. typesafe enum pattern. @@ -49,12 +51,12 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { Node node = occ.getLocation(); Node constructor = node.getFirstParentOfType(ASTConstructorDeclaration.class); if (constructor != null) { - return node; + unsafeAssignments.add(node); } } } - return null; + return unsafeAssignments; } } From fd13a4bf21a94e8cc7ec471400a77b5d2347bd48 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 16:00:38 +0200 Subject: [PATCH 4/5] Improved test cases for AssignmentToNonFinalStatic by including the expected line numbers for violations in the test cases. --- .../java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml index 249aeb6839..a0ccf0047b 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml @@ -8,6 +8,7 @@ clear rule violation ]]> 1 + 4 2 + 4,5 Date: Wed, 17 Apr 2019 11:32:09 -0300 Subject: [PATCH 5/5] Update changelog, refs #1781 --- docs/pages/release_notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index ec8d27e00e..e1b3170486 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -101,6 +101,12 @@ The designer will still be shipped with PMD's binaries. * The new PLSQL rule {% rule "plsql/codestyle/LineLength" %} (`plsql-codestyle`) helps to enforce a maximum line length. +### Modified Rules + +* The Java rule {% rule "java/errorprone/AssignmentToNonFinalStatic" %} (`java-errorprone`) will now report on each + assignment made within a constructor rather than on the field declaration. This makes it easier for developers to + find the offending statements. + ### Fixed Issues * doc @@ -152,6 +158,7 @@ The previously available variables such as `OPTS` or `HEAPSIZE` are deprecated a * [#1717](https://github.com/pmd/pmd/pull/1717): \[java] Fix false positive in useTryWithResources when using a custom close method with multiple arguments - [Rishabh Jain](https://github.com/jainrish) * [#1724](https://github.com/pmd/pmd/pull/1724): \[doc] Correct property override example - [Felix W. Dekker](https://github.com/FWDekker) * [#1737](https://github.com/pmd/pmd/pull/1737): \[java] fix escaping of CommentDefaultAccessModifier documentation - [itaigilo](https://github.com/itaigilo) +* [#1781](https://github.com/pmd/pmd/pull/1781): \[java] Location change in AssignmentToNonFinalStatic - [Maikel Steneker](https://github.com/maikelsteneker) {% endtocmaker %}