From 148c3651fcc0d4944bb76a964d5fa8db98a4a521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 30 May 2018 01:20:13 -0300 Subject: [PATCH 1/2] [java] AtLeastOneConstructor supports Lombok - Lombok annotations that create a constructor whitelist the class - Take the chance to cover a bunch of other FNs - Improve documentation on the rule - Some other minor improvements - Fixes #667 --- .../UnusedPrivateMethodRule.java | 6 +- .../codestyle/AtLeastOneConstructorRule.java | 74 +++++++++++++++++++ .../codestyle/UnnecessaryConstructorRule.java | 4 +- .../resources/category/java/codestyle.xml | 23 ++---- .../codestyle/xml/AtLeastOneConstructor.xml | 35 ++++++++- 5 files changed, 116 insertions(+), 26 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AtLeastOneConstructorRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java index 67159cd1a5..03cbbf9014 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java @@ -4,8 +4,8 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; -import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -33,9 +33,7 @@ public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule { @Override protected Collection defaultSuppressionAnnotations() { - Collection defaultValues = new ArrayList<>(); - defaultValues.add("java.lang.Deprecated"); - return defaultValues; + return Collections.singletonList("java.lang.Deprecated"); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AtLeastOneConstructorRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AtLeastOneConstructorRule.java new file mode 100644 index 0000000000..2983c4eaad --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AtLeastOneConstructorRule.java @@ -0,0 +1,74 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.codestyle; + +import java.util.Arrays; +import java.util.Collection; + +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule; +import net.sourceforge.pmd.lang.java.rule.design.UseUtilityClassRule; + +/** + * This rule detects non-static classes with no constructors; + * requiring even the default constructor to be explicit. + * It ignores classes with solely static methods, + * use {@link UseUtilityClassRule} to flag those. + */ +public class AtLeastOneConstructorRule extends AbstractIgnoredAnnotationRule { + + public AtLeastOneConstructorRule() { + addRuleChainVisit(ASTClassOrInterfaceDeclaration.class); + } + + @Override + protected Collection defaultSuppressionAnnotations() { + return Arrays.asList("lombok.Data", + "lombok.Value", + "lombok.Builder", + "lombok.NoArgsConstructor", + "lombok.RequiredArgsConstructor", + "lombok.AllArgsConstructorAtLeastOneConstructor"); + } + + @Override + public Object visit(final ASTClassOrInterfaceDeclaration node, final Object data) { + // Ignore interfaces / static classes / classes that have a constructor / classes ignored through annotations + if (node.isInterface() || node.isStatic() || node.hasDescendantOfType(ASTConstructorDeclaration.class) + || hasIgnoredAnnotation(node)) { + return data; + } + + boolean atLeastOneMember = false; + + // Do we have any non-static methods? + for (final ASTMethodDeclaration m : node.findDescendantsOfType(ASTMethodDeclaration.class)) { + if (!m.isStatic()) { + addViolation(data, node); + return data; + } + atLeastOneMember = true; + } + + // .. or fields? + for (final ASTFieldDeclaration f : node.findDescendantsOfType(ASTFieldDeclaration.class)) { + if (!f.isStatic()) { + addViolation(data, node); + return data; + } + atLeastOneMember = true; + } + + // Class has no declared members + if (!atLeastOneMember) { + addViolation(data, node); + } + + return data; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryConstructorRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryConstructorRule.java index 3415351032..c9c8ad1e40 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryConstructorRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryConstructorRule.java @@ -4,8 +4,8 @@ package net.sourceforge.pmd.lang.java.rule.codestyle; -import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import net.sourceforge.pmd.lang.ast.Node; @@ -26,7 +26,7 @@ public class UnnecessaryConstructorRule extends AbstractIgnoredAnnotationRule { @Override protected Collection defaultSuppressionAnnotations() { - return Arrays.asList("javax.inject.Inject"); + return Collections.singletonList("javax.inject.Inject"); } @Override diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 2432c9aebd..01143361e8 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -49,28 +49,15 @@ public abstract class Foo { // should be AbstractFoo language="java" since="1.04" message="Each class should declare at least one constructor" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.codestyle.AtLeastOneConstructorRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#atleastoneconstructor"> -Each class should declare at least one constructor. + 3 - - - - - - - 0 @@ -113,6 +113,37 @@ public class TestAtLeastOneConstructor { public void NotStatic() { System.out.println("This class should have a constructor"); } +} + ]]> + + + Ignore classes with lombok-generated constructors + 0 + + + + Report classes with at least one non-static method + 1 + + + + Report classes with at least one non-static field + 1 + From c20ed115120cbb2947564848b4a09713117f9429 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 16 Jun 2018 19:37:52 +0200 Subject: [PATCH 2/2] Update release notes, refs #1155, fixes #667 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 7719b5d2a3..3c1bcb58f1 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,8 @@ This is a minor release. ### New and noteworthy ### Fixed Issues +* java-codestyle + * [#667](https://github.com/pmd/pmd/issues/667): [java] Make AtLeastOneConstructor Lombok-aware ### API Changes