[java] CommentDefaultAccessModifier: add new property "checkTopLevelTypes"

Fixes #1880

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java

@@ -21,8 +21,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.ast.AbstractAnyTypeDeclaration;
-import net.sourceforge.pmd.lang.java.ast.AbstractJavaAccessNode;
+import net.sourceforge.pmd.lang.java.ast.AccessNode;
+import net.sourceforge.pmd.lang.java.ast.Annotatable;
 import net.sourceforge.pmd.lang.java.ast.Comment;
 import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
@@ -39,13 +39,17 @@ import net.sourceforge.pmd.properties.PropertyFactory;
 public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationRule {
 
     private static final PropertyDescriptor<Pattern> REGEX_DESCRIPTOR = PropertyFactory.regexProperty("regex")
-                                                                                       .desc("Regular expression").defaultValue("\\/\\*\\s+(default|package)\\s+\\*\\/").build();
+            .desc("Regular expression").defaultValue("\\/\\*\\s+(default|package)\\s+\\*\\/").build();
+    private static final PropertyDescriptor<Boolean> TOP_LEVEL_TYPES = PropertyFactory.booleanProperty("checkTopLevelTypes")
+            .desc("Check for default access modifier in top-level classes, annotations, and enums")
+            .defaultValue(false).build();
     private static final String MESSAGE = "To avoid mistakes add a comment "
             + "at the beginning of the %s %s if you want a default access modifier";
     private final Set<Integer> interestingLineNumberComments = new HashSet<>();
 
     public CommentDefaultAccessModifierRule() {
         definePropertyDescriptor(REGEX_DESCRIPTOR);
+        definePropertyDescriptor(TOP_LEVEL_TYPES);
     }
 
     @Override
@@ -104,7 +108,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
 
     @Override
     public Object visit(final ASTClassOrInterfaceDeclaration decl, final Object data) {
-        if (decl.isNested() && shouldReport(decl)) { // check for nested classes
+        if (decl.isNested() && shouldReportTypeDeclaration(decl)) { // check for nested classes
             addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "nested class"));
         } else if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // and for top-level ones
             addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level class"));
@@ -120,17 +124,24 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
         return super.visit(decl, data);
     }
 
-    private boolean shouldReport(final AbstractJavaAccessNode decl) {
-        final AbstractAnyTypeDeclaration parentClassOrInterface = decl
-                .getFirstParentOfType(AbstractAnyTypeDeclaration.class);
+    private boolean shouldReport(final AccessNode decl) {
+        final ASTAnyTypeDeclaration parentClassOrInterface = decl
+                .getFirstParentOfType(ASTAnyTypeDeclaration.class);
 
         boolean isConcreteClass = parentClassOrInterface.getTypeKind() == ASTAnyTypeDeclaration.TypeKind.CLASS;
 
-        // ignore if it's an Interface / Annotation
-        return isConcreteClass && shouldReportTypeDeclaration(decl);
+        // ignore if it's inside an interface / Annotation
+        return isConcreteClass && isMissingComment(decl);
     }
 
-    private boolean shouldReportTypeDeclaration(final AbstractJavaAccessNode decl) {
+    protected boolean hasIgnoredAnnotation(AccessNode node) {
+        if (node instanceof Annotatable) {
+            return hasIgnoredAnnotation((Annotatable) node);
+        }
+        return false;
+    }
+
+    private boolean isMissingComment(AccessNode decl) {
         // check if the class/method/field has a default access
         // modifier
         return decl.isPackagePrivate()
@@ -140,4 +151,12 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
                 // that it is not annotated with e.g. @VisibleForTesting
                 && !hasIgnoredAnnotation(decl);
     }
+
+    private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) {
+        // don't report on interfaces
+        return decl.getTypeKind() != ASTAnyTypeDeclaration.TypeKind.INTERFACE
+                && isMissingComment(decl)
+                // either nested or top level and we should check it
+                && (decl.isNested() || getProperty(TOP_LEVEL_TYPES));
+    }
 }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml

@@ -129,7 +129,14 @@ public @interface Foo {}
 /* default */ @interface FoobarWithComment {}
     ]]></code-fragment>
     <test-code>
-        <description>Top-level annotations with default access modifier</description>
+        <description>Top-level annotations with default access modifier are ignored by default (#1880)</description>
+        <expected-problems>0</expected-problems>
+        <code-ref id="top-level-annotations-with-default-access-modifier"/>
+    </test-code>
+
+    <test-code>
+        <description>Top-level annotations with default access modifier checks enabled with property</description>
+        <rule-property name="checkTopLevelTypes">true</rule-property>
         <expected-problems>2</expected-problems>
         <expected-linenumbers>1,6</expected-linenumbers>
         <code-ref id="top-level-annotations-with-default-access-modifier"/>
@@ -149,7 +156,13 @@ enum Foobar {}
 /* default */ enum FoobarWithComment {}
     ]]></code-fragment>
     <test-code>
-        <description>Top-level enums with default access modifier</description>
+        <description>Top-level enums with default access modifier are ignored by default (#1880)</description>
+        <expected-problems>0</expected-problems>
+        <code-ref id="top-level-enums-with-default-access-modifier"/>
+    </test-code>
+    <test-code>
+        <description>Top-level enums with default access modifier checks enabled with property</description>
+        <rule-property name="checkTopLevelTypes">true</rule-property>
         <expected-problems>2</expected-problems>
         <expected-linenumbers>1,6</expected-linenumbers>
         <code-ref id="top-level-enums-with-default-access-modifier"/>
@@ -169,7 +182,13 @@ class Foobar {}
 /* default */ class FoobarWithComment {}
     ]]></code-fragment>
     <test-code>
-        <description>Top-level classes with default access modifier</description>
+        <description>Top-level classes with default access modifier are ignored by default (#1880)</description>
+        <expected-problems>0</expected-problems>
+        <code-ref id="top-level-classes-with-default-access-modifier"/>
+    </test-code>
+    <test-code>
+        <description>Top-level classes with default access modifier checks enabled with property</description>
+        <rule-property name="checkTopLevelTypes">true</rule-property>
         <expected-problems>2</expected-problems>
         <expected-linenumbers>1,6</expected-linenumbers>
         <code-ref id="top-level-classes-with-default-access-modifier"/>