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 611d800f4e..95022f6d09 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 @@ -4,16 +4,22 @@ package net.sourceforge.pmd.lang.java.rule.design; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; +import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInitializer; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTSynchronizedStatement; @@ -42,14 +48,49 @@ public class SingularFieldRule extends AbstractJavaRule { definePropertyDescriptor(CHECK_INNER_CLASSES); definePropertyDescriptor(DISALLOW_NOT_ASSIGNMENT); } - + + private boolean lombokImported = false; + private boolean classHasLombokAnnotation = false; + private static final String LOMBOK_PACKAGE = "lombok"; + private static final Set LOMBOK_ANNOTATIONS = new HashSet(); + static { + LOMBOK_ANNOTATIONS.add("Data"); + LOMBOK_ANNOTATIONS.add("Getter"); + LOMBOK_ANNOTATIONS.add("Setter"); + LOMBOK_ANNOTATIONS.add("Value"); + LOMBOK_ANNOTATIONS.add("RequiredArgsConstructor"); + LOMBOK_ANNOTATIONS.add("AllArgsConstructor"); + LOMBOK_ANNOTATIONS.add("Builder"); + } + + @Override + public Object visit(ASTCompilationUnit node, Object data) { + lombokImported = false; + return super.visit(node, data); + } + + @Override + public Object visit(ASTImportDeclaration node, Object data) { + ASTName name = node.getFirstChildOfType(ASTName.class); + if (!lombokImported && name != null && name.getImage() != null & name.getImage().startsWith(LOMBOK_PACKAGE)) { + lombokImported = true; + } + return super.visit(node, data); + } + + @Override + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + classHasLombokAnnotation = hasLombokAnnotation(node); + return super.visit(node, data); + } + @SuppressWarnings("PMD.CompareObjectsWithEquals") @Override public Object visit(ASTFieldDeclaration node, Object data) { boolean checkInnerClasses = getProperty(CHECK_INNER_CLASSES); boolean disallowNotAssignment = getProperty(DISALLOW_NOT_ASSIGNMENT); - if (node.isPrivate() && !node.isStatic()) { + if (node.isPrivate() && !node.isStatic() && !classHasLombokAnnotation && !hasLombokAnnotation(node)) { for (ASTVariableDeclarator declarator: node.findChildrenOfType(ASTVariableDeclarator.class)) { ASTVariableDeclaratorId declaration = (ASTVariableDeclaratorId) declarator.jjtGetChild(0); List usages = declaration.getUsages(); @@ -150,4 +191,30 @@ public class SingularFieldRule extends AbstractJavaRule { return false; } } + + private boolean hasLombokAnnotation(Node node) { + boolean result = false; + Node parent = node.jjtGetParent(); + List annotations = parent.findChildrenOfType(ASTAnnotation.class); + for (ASTAnnotation annotation : annotations) { + ASTName name = annotation.getFirstDescendantOfType(ASTName.class); + if (name != null) { + String annotationName = name.getImage(); + if (lombokImported) { + if (LOMBOK_ANNOTATIONS.contains(annotationName)) { + result = true; + } + } else { + if (annotationName.startsWith(LOMBOK_PACKAGE + ".")) { + String shortName = annotationName.substring(LOMBOK_PACKAGE.length() + 1); + if (LOMBOK_ANNOTATIONS.contains(shortName)) { + result = true; + } + } + } + } + } + return result; + } + } 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 afba85c668..626a48c15c 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 @@ -530,6 +530,42 @@ public enum Supplier { public static String getSupplierNameIfPresent(String supplier) { return Optional.ofNullable(supplier).map(foo -> valueOf(supplier).supplierName).orElse(""); } +} + ]]> + + + + #1494 [java] SingularField: lombok.Data false positive - part1 + 0 + + + + + #1494 [java] SingularField: lombok.Data false positive - part2 + 0 + diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 7534ce0ac3..fdc3fb6cc7 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -27,6 +27,8 @@ * [#1533](https://sourceforge.net/p/pmd/bugs/1533/): \[java] BooleanInstantiation: ClassCastException with Annotation * java-comments * [#1522](https://sourceforge.net/p/pmd/bugs/1522/): \[java] CommentRequired: false positive +* java-design/SingularField + * [#1494](https://sourceforge.net/p/pmd/bugs/1494/): \[java] SingularField: lombok.Data false positive * java-imports/UnusedImports * [#1529](https://sourceforge.net/p/pmd/bugs/1529/): \[java] UnusedImports: The created rule violation has no class name * java-typeresolution/CloneMethodMustImplementCloneable