From 89e7d0433a4acedd1f0fd539a7b670b111a2d2fb Mon Sep 17 00:00:00 2001
From: Damian Techeira <dtecheira@monits.com>
Date: Wed, 22 Jul 2015 14:45:20 -0300
Subject: [PATCH] Add default access modifier as comment rule

Summary: Add rule to look for methods and fields that have a default access modifier

Test Plan: mvn test

Reviewers: jmsotuyo

Reviewed By: jmsotuyo

Differential Revision: http://ph.monits.com/D11936
---
 .../CommentDefaultAccessModifierRule.java     |  92 ++++++++++++
 .../main/resources/rulesets/java/comments.xml |  47 ++++++
 .../java/rule/comments/CommentRulesTest.java  |   1 +
 .../xml/CommentDefaultAccessModifier.xml      | 135 ++++++++++++++++++
 4 files changed, 275 insertions(+)
 create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/comments/CommentDefaultAccessModifierRule.java
 create mode 100755 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/comments/xml/CommentDefaultAccessModifier.xml

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/comments/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/comments/CommentDefaultAccessModifierRule.java
new file mode 100644
index 0000000000..d34ee9454b
--- /dev/null
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/comments/CommentDefaultAccessModifierRule.java
@@ -0,0 +1,92 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+package net.sourceforge.pmd.lang.java.rule.comments;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
+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.AbstractJavaAccessNode;
+import net.sourceforge.pmd.lang.java.ast.Comment;
+import net.sourceforge.pmd.lang.rule.properties.StringProperty;
+
+/**
+ * Check for Methods, Fields and Nested Classes that have a default access modifier
+ *
+ * @author Damián Techeira
+ */
+public class CommentDefaultAccessModifierRule extends AbstractCommentRule {
+
+	private static final StringProperty REGEX_DESCRIPTOR = new StringProperty("regex", "Regular expression", "", 1.0f);
+	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<Integer>();
+
+	public CommentDefaultAccessModifierRule() {
+		definePropertyDescriptor(REGEX_DESCRIPTOR);
+	}
+
+	public CommentDefaultAccessModifierRule(final String regex) {
+		this();
+		setRegex(regex);
+	}
+
+	public void setRegex(final String regex) {
+		setProperty(CommentDefaultAccessModifierRule.REGEX_DESCRIPTOR, regex);
+	}
+
+	@Override
+	public Object visit(final ASTCompilationUnit node, final Object data) {
+		interestingLineNumberComments.clear();
+		final List<Comment> comments = node.getComments();
+		for (final Comment comment : comments) {
+			if (comment.getImage().matches(getProperty(REGEX_DESCRIPTOR).trim())) {
+				interestingLineNumberComments.add(comment.getBeginLine());
+			}
+		}
+		return super.visit(node, data);
+	}
+
+	@Override
+	public Object visit(final ASTMethodDeclaration decl, final Object data) {
+		if (shouldReport(decl)) {
+			addViolationWithMessage(data, decl, String.format(MESSAGE,
+					decl.getFirstChildOfType(ASTMethodDeclarator.class).getImage(), "method"));
+		}
+		return super.visit(decl, data);
+	}
+
+	@Override
+	public Object visit(final ASTFieldDeclaration decl, final Object data) {
+		if (shouldReport(decl)) {
+			addViolationWithMessage(data, decl, String.format(MESSAGE,
+					decl.getFirstDescendantOfType(ASTVariableDeclaratorId.class).getImage(), "field"));
+		}
+		return super.visit(decl, data);
+	}
+
+	@Override
+	public Object visit(final ASTClassOrInterfaceDeclaration decl, final Object data) {
+		// check for nested classes
+		if (decl.isNested() && shouldReport(decl)) {
+			addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "nested class"));
+		}
+		return super.visit(decl, data);
+	}
+
+	private boolean shouldReport(final AbstractJavaAccessNode decl) {
+		// ignore if is a Interface
+		return !decl.getParentsOfType(ASTClassOrInterfaceDeclaration.class).get(0).isInterface()
+		// check if the field/method/nested class has a default access modifier
+		&& decl.isPackagePrivate()
+		// if is a default access modifier check if there is a comment in this line
+		&& !interestingLineNumberComments.contains(decl.getBeginLine());
+	}
+}
\ No newline at end of file
diff --git a/pmd-java/src/main/resources/rulesets/java/comments.xml b/pmd-java/src/main/resources/rulesets/java/comments.xml
index 7965200836..9b9a2c03fa 100755
--- a/pmd-java/src/main/resources/rulesets/java/comments.xml
+++ b/pmd-java/src/main/resources/rulesets/java/comments.xml
@@ -76,5 +76,52 @@ A rule for the politically correct... we don't want to offend anyone.
   </example>
    </rule>
 
+  <rule name="CommentDefaultAccessModifier"
+      since="5.3.2"
+      class="net.sourceforge.pmd.lang.java.rule.comments.CommentDefaultAccessModifierRule"
+      message="Missing commented default access modifier"
+      externalInfoUrl="${pmd.website.baseurl}/rules/java/comments.html#CommentDefaultAccessModifier">
+    <description>
+        <![CDATA[
+        To avoid mistakes if we want that a Method, Field or Nested class have a default access modifier
+        we must add a comment at the beginning of the Method, Field or Nested class.
+        By default the comment must be /* default */, if you want another, you have to provide a regex.
+        ]]>
+    </description>
+    <priority>3</priority>
+    <properties>
+        <property name="regex">
+            <value>
+                <![CDATA[
+\/\*\s+default\s+\*\/
+                ]]>
+            </value>
+        </property>
+    </properties>
+    <example>
+        <![CDATA[
+        public class Foo {
+            final String stringValue = "some string";
+            String getString() {
+               return stringValue;
+            }
+
+            class NestedFoo {
+            }
+        }
+
+        // should be
+        public class Foo {
+            /* default */ final String stringValue = "some string";
+            /* default */ String getString() {
+               return stringValue;
+            }
+
+            /* default */ class NestedFoo {
+            }
+        }
+        ]]>
+    </example>
+  </rule>
 
  </ruleset>
\ No newline at end of file
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/comments/CommentRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/comments/CommentRulesTest.java
index 7e6a22a9b7..87822c7099 100755
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/comments/CommentRulesTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/comments/CommentRulesTest.java
@@ -14,5 +14,6 @@ public class CommentRulesTest extends SimpleAggregatorTst {
         addRule(RULESET, "CommentRequired");
         addRule(RULESET, "CommentSize");
         addRule(RULESET, "CommentContent");
+        addRule(RULESET, "CommentDefaultAccessModifier");
     }
 }
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/comments/xml/CommentDefaultAccessModifier.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/comments/xml/CommentDefaultAccessModifier.xml
new file mode 100755
index 0000000000..094a335de2
--- /dev/null
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/comments/xml/CommentDefaultAccessModifier.xml
@@ -0,0 +1,135 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data>
+    <code-fragment id="some-methods-fields-with-default-access-modifier"><![CDATA[
+public class Foo {
+	public final String stringValue = "stringValue";      // should not be reported
+	final String otherStringValue = "otherStringValue";   // should be reported
+	/* default */ final String otherValue = "otherValue"; // should not be reported
+	/* 		default		 */ final String anotherValue = "anotherValue"; // should not be reported
+
+	public void test() { // should not be reported
+	}
+
+	void test2() { // should be reported
+	}
+
+	/* default */ void test3() { // should not be reported
+	}
+
+	/*	 default 	*/ void test4() { // should not be reported
+	}
+}
+    ]]>
+    </code-fragment>
+    <test-code>
+        <description>Some methods and Fields with default access modifier in a class</description>
+        <expected-problems>2</expected-problems>
+        <code-ref id="some-methods-fields-with-default-access-modifier"/>
+    </test-code>
+
+    <code-fragment id="all-methods-fields-with-default-access-modifier"><![CDATA[
+public class Foo {
+	final String stringValue = "stringValue";
+	final String otherStringValue = "otherStringValue";
+	final String otherValue = "otherValue";
+
+	void test() {
+	}
+
+	void test2() {
+	}
+
+	void test3() {
+	}
+}
+    ]]>
+    </code-fragment>
+    <test-code>
+        <description>All methods and Field with default access modifier in a class</description>
+        <expected-problems>6</expected-problems>
+        <code-ref id="all-methods-fields-with-default-access-modifier"/>
+    </test-code>
+
+    <code-fragment id="all-methods-fields-without-default-access-modifier"><![CDATA[
+public class Foo {
+	private final String stringValue = "stringValue";
+	private final String otherStringValue = "otherStringValue";
+	private final String otherValue = "otherValue";
+
+	public void test() {
+	}
+
+	public void test2() {
+	}
+
+	public void test3() {
+	}
+}
+    ]]>
+    </code-fragment>
+    <test-code>
+        <description>All methods and Field without default access modifier in a class</description>
+        <expected-problems>0</expected-problems>
+        <code-ref id="all-methods-fields-without-default-access-modifier"/>
+    </test-code>
+
+    <code-fragment id="interface-with-methods-with-default-access-modifier"><![CDATA[
+public interface Foo {
+	void test();
+	public void test2();
+	void test3();
+}
+    ]]></code-fragment>
+    <test-code>
+        <description>Methods with default access modifier in an Interface</description>
+        <expected-problems>0</expected-problems>
+        <code-ref id="interface-with-methods-with-default-access-modifier"/>
+    </test-code>
+
+    <code-fragment id="nested-class-with-default-access-modifier"><![CDATA[
+public class Foo {
+	private final String stringValue = "stringValue";
+
+	public void test() {
+	}
+
+	class NestedClass {
+	}
+
+	static class OtherNestedClass {
+	}
+
+	public class AnotherNestedClass {
+	}
+}
+    ]]></code-fragment>
+    <test-code>
+        <description>Nested classes with default access modifier</description>
+        <expected-problems>2</expected-problems>
+        <code-ref id="nested-class-with-default-access-modifier"/>
+    </test-code>
+
+    <code-fragment id="own-regex-to-default-access-modifier-rule"><![CDATA[
+public class Foo {
+	/* default */ final String stringValue = "stringValue";
+
+	void test() {
+	}
+
+	/* package-private */ class NestedClass {
+	}
+
+	/*	 package-private	 */ class OtherNestedClass {
+	}
+
+	static class AnotherNestedClass {
+	}
+}
+    ]]></code-fragment>
+    <test-code>
+        <description>Test own regex to default access modifier rule</description>
+        <rule-property name="regex">\/\*\s+package-private\s+\*\/</rule-property>
+        <expected-problems>3</expected-problems>
+        <code-ref id="own-regex-to-default-access-modifier-rule"/>
+    </test-code>
+</test-data>
\ No newline at end of file