Issue #3426: remove warning on PACKAGE_DEF predicted by javadoc not s…

…eparated by line
checkstyle · Nov 26, 2016 · fff7302 · fff7302
1 parent 548ecd7
commit fff7302
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 31 deletions.
diff --git a/.../google/checkstyle/test/chapter3filestructure/rule3sourcefile/EmptyLineSeparatorTest.java b/.../google/checkstyle/test/chapter3filestructure/rule3sourcefile/EmptyLineSeparatorTest.java
@@ -57,7 +57,10 @@ public void emptyLineSeparatorTest() throws Exception {
         final Configuration checkConfig = getCheckConfig("EmptyLineSeparator");
         final String filePath = getPath("InputEmptyLineSeparator.java");
 
-        final Integer[] warnList = getLinesWithWarn(filePath);
+        //Hardcoded because warn and ok comments was removed in test resource file.
+        //Comments after accessible tokens causes warning that it should be separated by empty line
+        final Integer[] warnList = {19, 20, 33, 37, 66, 75, 82, 113, 119 };
+
         verify(checkConfig, filePath, expected, warnList);
     }
 }
diff --git a/.../checkstyle/test/chapter4formatting/rule461verticalwhitespace/EmptyLineSeparatorTest.java b/.../checkstyle/test/chapter4formatting/rule461verticalwhitespace/EmptyLineSeparatorTest.java
@@ -57,7 +57,10 @@ public void emptyLineSeparatorTest() throws Exception {
         final Configuration checkConfig = getCheckConfig("EmptyLineSeparator");
         final String filePath = getPath("InputEmptyLineSeparator.java");
 
-        final Integer[] warnList = getLinesWithWarn(filePath);
+        //Hardcoded because warn and ok comments was removed in test resource file.
+        //Comments after accessible tokens causes warning that it should be separated by empty line
+        final Integer[] warnList = {19, 20, 33, 37, 66, 75, 82, 113, 119 };
+
         verify(checkConfig, filePath, expected, warnList);
     }
 }
diff --git a/...google/checkstyle/test/chapter3filestructure/rule3sourcefile/InputEmptyLineSeparator.java b/...google/checkstyle/test/chapter3filestructure/rule3sourcefile/InputEmptyLineSeparator.java
@@ -16,8 +16,8 @@
 // License along with this library; if not, write to the Free Software
 // Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 ////////////////////////////////////////////////////////////////////////////////
-package com.google.checkstyle.test.chapter3filestructure.rule3sourcefile; //warn
-import java.io.Serializable; //warn
+package com.google.checkstyle.test.chapter3filestructure.rule3sourcefile;
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -30,11 +30,11 @@
 import javax.swing.AbstractAction;
 
 import org.apache.commons.beanutils.locale.converters.ByteLocaleConverter;
-class InputEmptyLineSeparator //warn
+class InputEmptyLineSeparator
 {
 	public static final double FOO_PI = 3.1415;
 	private boolean flag = true; 
-	static {  //warn
+	static {
         //empty static initializer
 	}
 
@@ -108,7 +108,7 @@ public int compareTo(InputEmptyLineSeparator aObject) //ok
 }
 
 class Class1 { //ok
-    private Class1() {} //ok
+    private Class1() {}
 } 
 class Class2{ //warn
     public int compareTo(InputEmptyLineSeparator aObject) //ok

diff --git a/...checkstyle/test/chapter4formatting/rule461verticalwhitespace/InputEmptyLineSeparator.java b/...checkstyle/test/chapter4formatting/rule461verticalwhitespace/InputEmptyLineSeparator.java
@@ -16,8 +16,8 @@
 // License along with this library; if not, write to the Free Software
 // Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 ////////////////////////////////////////////////////////////////////////////////
-package com.google.checkstyle.test.chapter4formatting.rule461verticalwhitespace; //warn
-import java.io.Serializable; //warn
+package com.google.checkstyle.test.chapter4formatting.rule461verticalwhitespace;
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -34,7 +34,7 @@ class InputEmptyLineSeparator //warn
 {
 	public static final double FOO_PI = 3.1415;
 	private boolean flag = true; 
-	static {  //warn
+	static {
         //empty static initializer
 	}
 
@@ -108,7 +108,7 @@ public int compareTo(InputEmptyLineSeparator aObject) //ok
 }
 
 class Clazz { //ok
-    private Clazz() {} //ok
+    private Clazz() {}
 } 
 class Class2{ //warn
     public int compareTo(InputEmptyLineSeparator aObject) //ok

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java
@@ -27,6 +27,7 @@
 import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
+import com.puppycrawl.tools.checkstyle.utils.JavadocUtils;
 
 /**
  * Checks for empty line separators after header, package, all import declarations,
@@ -250,6 +251,11 @@ public void setAllowMultipleEmptyLinesInsideClassMembers(boolean allow) {
         allowMultipleEmptyLinesInsideClassMembers = allow;
     }
 
+    @Override
+    public boolean isCommentNodesRequired() {
+        return true;
+    }
+
     @Override
     public int[] getDefaultTokens() {
         return getAcceptableTokens();
@@ -404,7 +410,7 @@ && hasNotAllowedTwoEmptyLinesBefore(ast)) {
      * @param nextToken next token
      */
     private void processPackage(DetailAST ast, DetailAST nextToken) {
-        if (ast.getLineNo() > 1 && !hasEmptyLineBefore(ast)) {
+        if (ast.getLineNo() > 1 && !(hasEmptyLineBefore(ast) || isPrecededByJavadoc(ast))) {
             log(ast.getLineNo(), MSG_SHOULD_BE_SEPARATED, ast.getText());
         }
         if (!hasEmptyLineAfter(ast)) {
@@ -533,6 +539,23 @@ private boolean hasEmptyLineBefore(DetailAST token) {
         return lineBefore.trim().isEmpty();
     }
 
+    /**
+     * Check if token is preceded by javadoc comment
+     * @param token token for check.
+     * @return true, if token is preceded by javadoc comment.
+     */
+    private boolean isPrecededByJavadoc(DetailAST token) {
+        boolean result = false;
+        //If this method calls, PACKAGE_DEF isn't on first line
+        //and hasn't empty line before, so previous sibling is not null
+        final DetailAST previous = token.getPreviousSibling();
+        if (previous.getType() == TokenTypes.BLOCK_COMMENT_BEGIN
+                && JavadocUtils.isJavadocComment(previous)) {
+            result = true;
+        }
+        return result;
+    }
+
     /**
      * If variable definition is a type field.
      * @param variableDef variable definition.

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlTest.java
@@ -60,8 +60,8 @@ public void testEnsureTrailingDot() {
     @Test
     public void testCheckAccess() {
         assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
-                "org.springframework.something",
-                "com.kazgroup.courtlink.common"));
+                "com.kazgroup.courtlink.common",
+                "org.springframework.something"));
         assertEquals(AccessResult.ALLOWED, icCommon
                 .checkAccess("org.apache.commons.something",
                         "com.kazgroup.courtlink.common"));

diff --git a/...t/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java b/...t/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java
@@ -54,15 +54,15 @@ public void testDefault() throws Exception {
         final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class);
 
         final String[] expected = {
-            "21: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"),
-            "35: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
-            "38: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "VARIABLE_DEF"),
-            "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
-            "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
-            "57: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
-            "62: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
-            "79: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
-            "110: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
+            "25: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"),
+            "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
+            "42: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "VARIABLE_DEF"),
+            "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
+            "47: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
+            "61: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
+            "66: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
+            "83: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
+            "114: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
         };
         verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected);
     }
@@ -74,14 +74,14 @@ public void testAllowNoEmptyLineBetweenFields() throws Exception {
         checkConfig.addAttribute("allowNoEmptyLineBetweenFields", "true");
 
         final String[] expected = {
-            "21: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"),
-            "35: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
-            "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
-            "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
-            "57: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
-            "62: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
-            "79: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
-            "110: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
+            "25: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"),
+            "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
+            "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
+            "47: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
+            "61: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
+            "66: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
+            "83: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
+            "114: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
         };
         verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected);
     }
@@ -191,4 +191,15 @@ public void testAllowMultipleEmptyLinesInsideClassMembers() throws Exception {
                 getPath("InputEmptyLineSeparatorMultipleEmptyLinesInside.java"),
                 expected);
     }
+
+    @Test
+    public void testDisAllowMultilineCommentBeforePackageDefinition() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class);
+        final String[] expected = {
+            "5: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"),
+        };
+        verify(checkConfig,
+                getPath("InputEmptyLineSeparatorMultilineCommentHeader.java"),
+                expected);
+    }
 }
diff --git a/.../resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java b/.../resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java
@@ -17,6 +17,10 @@
 // Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 ////////////////////////////////////////////////////////////////////////////////
 
+/**
+  * Some javadoc before PACKAGE_DEF to test that there isn't warning
+  * when javadoc and package definition aren't separate with line.
+  */
 package com.puppycrawl.tools.checkstyle.checks.whitespace;
 import java.io.Serializable;
 import java.util.ArrayList;

diff --git a/...awl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java b/...awl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java
@@ -0,0 +1,9 @@
+/*To check warning
+that causes by miltiline comment
+before package definition.
+*/
+package com.puppycrawl.tools.checkstyle.checks.whitespace;
+
+public class InputEmptyLineSeparatorMultilineCommentHeader
+{
+}
diff --git a/...rawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultipleFieldsInClass.java b/...rawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultipleFieldsInClass.java
@@ -1,3 +1,4 @@
+
 package com.puppycrawl.tools.checkstyle.checks.whitespace;
 
 public class InputEmptyLineSeparatorMultipleFieldsInClass