diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 53daf9db02..5077d432a2 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -37,6 +37,8 @@ Both are bugfixing releases. * [#907](https://github.com/pmd/pmd/issues/907): \[java] UnusedPrivateField false-positive with @FXML * java-bestpracrtices * [#963](https://github.com/pmd/pmd/issues/965): \[java] ArrayIsStoredDirectly not triggered from variadic functions +* java-design + * [#968](https://github.com/pmd/pmd/issues/968): \[java] UseUtilityClassRule reports false positive with lombok NoArgsConstructor ### API Changes @@ -46,5 +48,6 @@ Both are bugfixing releases. * [#943](https://github.com/pmd/pmd/pull/943): \[java] UnusedPrivateField false-positive with @FXML - [BBG](https://github.com/djydewang) * [#965](https://github.com/pmd/pmd/pull/965): \[java] Make Varargs trigger ArrayIsStoredDirectly - [Stephen](https://github.com/pmd/pmd/issues/907) * [#967](https://github.com/pmd/pmd/pull/967): \[doc] Issue 959: fixed broken link to XPath Rule Tutorial - [Andrey Mochalov](https://github.com/epidemia) +* [#969](https://github.com/pmd/pmd/pull/969): \[java] Issue 968 Add logic to handle lombok private constructors with utility classes - [Kirk Clemens](https://github.com/clem0110) * [#970](https://github.com/pmd/pmd/pull/970): \[java] Fixed inefficient use of keySet iterator instead of entrySet iterator - [Andrey Mochalov](https://github.com/epidemia) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java index 178838664a..65671ff7bc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java @@ -36,6 +36,7 @@ public class AbstractLombokAwareRule extends AbstractJavaRule { LOMBOK_ANNOTATIONS.add("Value"); LOMBOK_ANNOTATIONS.add("RequiredArgsConstructor"); LOMBOK_ANNOTATIONS.add("AllArgsConstructor"); + LOMBOK_ANNOTATIONS.add("NoArgsConstructor"); LOMBOK_ANNOTATIONS.add("Builder"); } @@ -110,4 +111,28 @@ public class AbstractLombokAwareRule extends AbstractJavaRule { } return result; } + + protected ASTAnnotation getLombokAnnotation(Node node, String lombokAnnotation) { + 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 (lombokAnnotation.equals(annotationName)) { + return annotation; + } + } else { + if (annotationName.startsWith(LOMBOK_PACKAGE + ".")) { + String shortName = annotationName.substring(LOMBOK_PACKAGE.length() + 1); + if (lombokAnnotation.equals(shortName)) { + return annotation; + } + } + } + } + } + return null; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java index 00012eb06c..5fff1becc4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang.java.rule.design; +import java.util.List; + import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; @@ -12,11 +14,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTResultType; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule; -public class UseUtilityClassRule extends AbstractJavaRule { +public class UseUtilityClassRule extends AbstractLombokAwareRule { @Override public Object visit(ASTClassOrInterfaceBody decl, Object data) { @@ -25,6 +29,11 @@ public class UseUtilityClassRule extends AbstractJavaRule { if (parent.isAbstract() || parent.isInterface() || isExceptionType(parent)) { return super.visit(decl, data); } + + if (isOkUsingLombok(parent)) { + return super.visit(decl, data); + } + int i = decl.jjtGetNumChildren(); int methodCount = 0; boolean isOK = false; @@ -63,7 +72,6 @@ public class UseUtilityClassRule extends AbstractJavaRule { break; } } - } } if (!isOK && methodCount > 0) { @@ -73,6 +81,35 @@ public class UseUtilityClassRule extends AbstractJavaRule { return super.visit(decl, data); } + private boolean isOkUsingLombok(ASTClassOrInterfaceDeclaration parent) { + // check if there's a lombok no arg private constructor, if so skip the rest of the rules + if (hasClassLombokAnnotation()) { + ASTAnnotation annotation = getLombokAnnotation(parent, "NoArgsConstructor"); + + if (annotation != null) { + + List memberValuePairs = annotation.findDescendantsOfType(ASTMemberValuePair.class); + + for (ASTMemberValuePair memberValuePair : memberValuePairs) { + // to set the access level of a constructor in lombok, you set the access property on the annotation + if ("access".equals(memberValuePair.getImage())) { + List names = memberValuePair.findDescendantsOfType(ASTName.class); + + for (ASTName name : names) { + // check to see if the value of the member value pair ends PRIVATE. This is from the AccessLevel enum in Lombok + if (name.getImage().endsWith("PRIVATE")) { + // if the constructor is found and the accesslevel is private no need to check anything else + return true; + } + } + } + } + } + } + + return false; + } + private Node skipAnnotations(Node p) { int index = 0; Node n = p.jjtGetChild(index++); @@ -95,5 +132,4 @@ public class UseUtilityClassRule extends AbstractJavaRule { } return false; } - } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml index 364d32784f..12edbf26cf 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml @@ -238,4 +238,111 @@ public class AccountFragment extends Fragment { } ]]> + + + + 0 + + + + + + 0 + + + + + + 0 + + + + + + 1 + + + + + + 1 + + + + + + 1 + + +