From 85f67d7e2438740c241631842dcb52cc5f5a27b3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 13 Nov 2021 11:14:20 +0100 Subject: [PATCH 1/6] [java] SingularField - fix false negative with anonymous classes --- .../java/rule/design/SingularFieldRule.java | 8 +-- .../java/rule/design/xml/SingularField.xml | 59 +++++++++++++++++-- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java index e82d04763f..9c57f01a80 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java @@ -74,16 +74,16 @@ public class SingularFieldRule extends AbstractLombokAwareRule { boolean disallowNotAssignment = getProperty(DISALLOW_NOT_ASSIGNMENT); if (!node.isPrivate() || node.isStatic()) { - return data; + return super.visit(node, data); } if (hasClassLombokAnnotation() || hasIgnoredAnnotation(node)) { - return data; + return super.visit(node, data); } // lombok.EqualsAndHashCode is a class-level annotation if (hasIgnoredAnnotation((Annotatable) node.getFirstParentOfType(ASTAnyTypeDeclaration.class))) { - return data; + return super.visit(node, data); } for (ASTVariableDeclarator declarator : node.findChildrenOfType(ASTVariableDeclarator.class)) { @@ -194,7 +194,7 @@ public class SingularFieldRule extends AbstractLombokAwareRule { addViolation(data, node, new Object[] { declaration.getImage() }); } } - return data; + return super.visit(node, data); } private boolean isInAssignment(Node potentialStatement) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingularField.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingularField.xml index 4702f1dd2e..09496df92f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingularField.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingularField.xml @@ -7,16 +7,17 @@ failure case 1 + 3 @@ -190,6 +191,7 @@ public class Foo { Reuse variable name as params in method calls 1 + 2 failure, static 1 + 2 failure, second method re-uses class level name 1 + 2 1409944, fields not used to synchronize should trigger 1 + 3 Not ok, since inner classes are checked true 1 + 3 Not ok, violation with first usage = non-assignment true 1 + 2 multiple fields on same line 1 + 2 [java] SingularField: Lombok false positive with annotated inner class 1 + 9 + + + failure case with anonymous class + 4 + 4,13,20,23 + + From 008141cd9201b49147cf57c02f10b0856be249ce Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 13 Nov 2021 11:21:33 +0100 Subject: [PATCH 2/6] [java] SwitchDensity - use super.visit There are no fixed false negatives, but it looks more correct. --- .../pmd/lang/java/rule/design/SwitchDensityRule.java | 10 +++------- .../pmd/lang/java/rule/design/xml/SwitchDensity.xml | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java index 3299c585d1..8567fef925 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java @@ -51,7 +51,6 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule { } public SwitchDensityRule() { - super(); setProperty(MINIMUM_DESCRIPTOR, 10d); } @@ -65,7 +64,7 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule { SwitchDensity density = new SwitchDensity(); - node.childrenAccept(this, density); + super.visit(node, density); DataPoint point = new DataPoint(); point.setNode(node); @@ -86,9 +85,7 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule { ((SwitchDensity) data).addStatement(); } - statement.childrenAccept(this, data); - - return data; + return super.visit(statement, data); } @Override @@ -97,7 +94,6 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule { ((SwitchDensity) data).addSwitchLabel(); } - switchLabel.childrenAccept(this, data); - return data; + return super.visit(switchLabel, data); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SwitchDensity.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SwitchDensity.xml index 8d3c5fc1e2..c1f09d441f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SwitchDensity.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SwitchDensity.xml @@ -8,6 +8,7 @@ Five stmts in one switch case, should be flagged 4 1 + 4 Date: Sat, 13 Nov 2021 11:24:56 +0100 Subject: [PATCH 3/6] [java] AvoidMultipleUnaryOperators - remove unnecessary super --- .../java/rule/errorprone/AvoidMultipleUnaryOperatorsRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidMultipleUnaryOperatorsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidMultipleUnaryOperatorsRule.java index be73e9f279..5ca3e3f5fb 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidMultipleUnaryOperatorsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidMultipleUnaryOperatorsRule.java @@ -15,8 +15,8 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; public class AvoidMultipleUnaryOperatorsRule extends AbstractJavaRule { public AvoidMultipleUnaryOperatorsRule() { - super.addRuleChainVisit(ASTUnaryExpression.class); - super.addRuleChainVisit(ASTUnaryExpressionNotPlusMinus.class); + addRuleChainVisit(ASTUnaryExpression.class); + addRuleChainVisit(ASTUnaryExpressionNotPlusMinus.class); } @Override From 62dd4c89fdaa17caaf654cf1fe3b58713a3569c9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 13 Nov 2021 11:26:40 +0100 Subject: [PATCH 4/6] [java] AvoidUsingOctalValues - use rulechain While there is no fixed false negative, this is more correct and in PMD 7, rule chain is already used. --- .../pmd/lang/java/rule/errorprone/AvoidUsingOctalValuesRule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidUsingOctalValuesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidUsingOctalValuesRule.java index c91705bfe2..bf21173584 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidUsingOctalValuesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidUsingOctalValuesRule.java @@ -27,6 +27,7 @@ public class AvoidUsingOctalValuesRule extends AbstractJavaRule { public AvoidUsingOctalValuesRule() { definePropertyDescriptor(STRICT_METHODS_DESCRIPTOR); + addRuleChainVisit(ASTLiteral.class); } @Override From 9747f76f5c5ee82e1e22d6374077eecd6c55b921 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 13 Nov 2021 11:29:21 +0100 Subject: [PATCH 5/6] [java] CheckSkipResult - use rulechain While there is no fixed false negative, this is more correct and in PMD 7, rule chain is already used. --- .../pmd/lang/java/rule/errorprone/CheckSkipResultRule.java | 4 ++++ .../pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml | 2 ++ 2 files changed, 6 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java index 3b5710f780..bf12f843ee 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java @@ -19,6 +19,10 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence; public class CheckSkipResultRule extends AbstractJavaRule { + public CheckSkipResultRule() { + addRuleChainVisit(ASTVariableDeclaratorId.class); + } + @Override public Object visit(ASTVariableDeclaratorId node, Object data) { if (!TypeTestUtil.isA(InputStream.class, node.getTypeNode())) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml index 2c2bc31c5e..b398fe2a51 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml @@ -7,6 +7,7 @@ failure case 1 + 8 failure case but obfuscated 1 + 8 Date: Sat, 13 Nov 2021 11:40:59 +0100 Subject: [PATCH 6/6] [doc] Update release notes (#3620) --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..41818d3e5b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* java-design + * [#3620](https://github.com/pmd/pmd/issues/3620): \[java] SingularField doesn't consider anonymous classes defined in non-private fields + ### API Changes ### External Contributions