Issue checkstyle#13782: Fix false negative for annotation indentation

kalpadiptyaroy · Feb 3, 2024 · c916d7a · c916d7a
1 parent a5d0468
commit c916d7a
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 9 deletions.
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/LineWrappingHandler.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/LineWrappingHandler.java
@@ -22,6 +22,7 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.NavigableMap;
+import java.util.Optional;
 import java.util.TreeMap;
 
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
@@ -291,6 +292,49 @@ private static DetailAST getNextCurNode(DetailAST curNode) {
         return nodeToVisit;
     }
 
+    /**
+     * Checks whether current node is an annotation wrapped within a class definition.
+     *
+     * @param node It is the current node in consideration.
+     * @param parentNode It is the parent node of the current node.
+     * @return true if the current node is annotation
+     *     and present between access specifier and class keyword.
+     */
+    private static boolean isCurrentNodeAnnotationLineWrappedInClassDef(DetailAST node,
+            DetailAST parentNode) {
+        DetailAST tempNode = parentNode;
+        while (TokenUtil.isOfType(tempNode.getPreviousSibling(), TokenTypes.ANNOTATION)) {
+            tempNode = tempNode.getPreviousSibling();
+        }
+        final boolean isPreviousToPreviousSiblingAccessModifier =
+            Optional.ofNullable(tempNode).map(DetailAST::getPreviousSibling)
+                .filter(sibling -> {
+                    return TokenUtil.isOfType(sibling, TokenTypes.LITERAL_PUBLIC,
+                    TokenTypes.FINAL, TokenTypes.ABSTRACT);
+                }).isPresent();
+        final boolean isCurrentNodeAtNode = TokenUtil.isOfType(node, TokenTypes.AT);
+        final boolean isPreviousSiblingAnnotation =
+                TokenUtil.isOfType(parentNode.getPreviousSibling(), TokenTypes.ANNOTATION);
+        return isCurrentNodeAtNode
+            && isPreviousSiblingAnnotation
+            && isPreviousToPreviousSiblingAccessModifier;
+    }
+
+    /**
+     * Checks whether current node is close annotation and alone in line.
+     *
+     * @param node is the current node in consideration.
+     * @param lastAnnotationNode is last child of current atNode.
+     * @return true if current node is a close annotation and alone in line
+     *     or current node is '@' node or not else false.
+     */
+    private static boolean isCurrentNodeCloseAnnotationAloneInLine(
+        DetailAST node, DetailAST lastAnnotationNode) {
+        final int lastAnnotationLine = lastAnnotationNode.getLineNo();
+        return node.getLineNo() == lastAnnotationLine
+            && isEndOfScope(lastAnnotationNode, node);
+    }
+
     /**
      * Checks line wrapping into annotations.
      *
@@ -305,7 +349,6 @@ private void checkAnnotationIndentation(DetailAST atNode,
         final int currentIndent = firstNodeIndent + indentLevel;
         final Collection<DetailAST> values = firstNodesOnLines.values();
         final DetailAST lastAnnotationNode = atNode.getParent().getLastChild();
-        final int lastAnnotationLine = lastAnnotationNode.getLineNo();
 
         final Iterator<DetailAST> itr = values.iterator();
         while (firstNodesOnLines.size() > 1) {
@@ -315,14 +358,20 @@ private void checkAnnotationIndentation(DetailAST atNode,
             final boolean isArrayInitPresentInAncestors =
                 isParentContainsTokenType(node, TokenTypes.ANNOTATION_ARRAY_INIT);
             final boolean isCurrentNodeCloseAnnotationAloneInLine =
-                node.getLineNo() == lastAnnotationLine
-                    && isEndOfScope(lastAnnotationNode, node);
-            if (!isArrayInitPresentInAncestors
-                    && (isCurrentNodeCloseAnnotationAloneInLine
-                    || node.getType() == TokenTypes.AT
-                    && (parentNode.getParent().getType() == TokenTypes.MODIFIERS
-                        || parentNode.getParent().getType() == TokenTypes.ANNOTATIONS)
-                    || TokenUtil.areOnSameLine(node, atNode))) {
+                isCurrentNodeCloseAnnotationAloneInLine(node, lastAnnotationNode);
+
+            final boolean condition = !isArrayInitPresentInAncestors
+                && (isCurrentNodeCloseAnnotationAloneInLine
+                    || TokenUtil.isOfType(node, TokenTypes.AT)
+                    && TokenUtil.isOfType(parentNode.getParent(),
+                        TokenTypes.MODIFIERS, TokenTypes.ANNOTATIONS)
+                    || TokenUtil.areOnSameLine(node, atNode));
+
+            if (condition && firstNodeIndent > indentLevel
+                && isCurrentNodeAnnotationLineWrappedInClassDef(node, parentNode)) {
+                logWarningMessage(node, currentIndent);
+            }
+            else if (condition) {
                 logWarningMessage(node, firstNodeIndent);
             }
             else if (!isArrayInitPresentInAncestors) {

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java
@@ -2997,6 +2997,44 @@ public void testIndentationAnnotationFieldDefinition() throws Exception {
                 expected);
     }
 
+    @Test
+    public void testIndentationAnnotationMultiLineIncorrect() throws Exception {
+        final DefaultConfiguration checkConfig = createModuleConfig(IndentationCheck.class);
+        checkConfig.addProperty("tabWidth", "4");
+        checkConfig.addProperty("basicOffset", "2");
+        checkConfig.addProperty("braceAdjustment", "2");
+        checkConfig.addProperty("caseIndent", "2");
+        checkConfig.addProperty("throwsIndent", "4");
+        checkConfig.addProperty("lineWrappingIndentation", "4");
+        checkConfig.addProperty("arrayInitIndent", "2");
+
+        final String[] expected = {
+            "16:1: " + getCheckMessage(MSG_ERROR, "@", 0, 4),
+            "35:1: " + getCheckMessage(MSG_ERROR, "@", 0, 4),
+            "40:1: " + getCheckMessage(MSG_ERROR, "@", 0, 4),
+        };
+
+        verifyWarns(checkConfig,
+            getPath("InputIndentationAnnotationMultiLineIncorrect.java"), expected);
+    }
+
+    @Test
+    public void testIndentationAnnotationMultiLineCorrect() throws Exception {
+        final DefaultConfiguration checkConfig = createModuleConfig(IndentationCheck.class);
+        checkConfig.addProperty("tabWidth", "4");
+        checkConfig.addProperty("basicOffset", "2");
+        checkConfig.addProperty("braceAdjustment", "2");
+        checkConfig.addProperty("caseIndent", "2");
+        checkConfig.addProperty("throwsIndent", "4");
+        checkConfig.addProperty("lineWrappingIndentation", "4");
+        checkConfig.addProperty("arrayInitIndent", "2");
+
+        final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
+
+        verifyWarns(checkConfig,
+            getPath("InputIndentationAnnotationMultiLineCorrect.java"), expected);
+    }
+
     @Test
     public void testIndentationLongConcatenatedString() throws Exception {
         final DefaultConfiguration checkConfig = createModuleConfig(IndentationCheck.class);

diff --git a/...checkstyle/checks/indentation/indentation/InputIndentationAnnotationMultiLineCorrect.java b/...checkstyle/checks/indentation/indentation/InputIndentationAnnotationMultiLineCorrect.java
@@ -0,0 +1,41 @@
+package com.puppycrawl.tools.checkstyle.checks.indentation.indentation;//indent:0 exp:0
+//indent:0 exp:0
+/**                                                                           //indent:0 exp:0
+ * This test-input is intended to be checked using following configuration:   //indent:1 exp:1
+ *                                                                            //indent:1 exp:1
+ * arrayInitIndent = 2                                                        //indent:1 exp:1
+ * basicOffset = 2                                                            //indent:1 exp:1
+ * braceAdjustment = 2                                                        //indent:1 exp:1
+ * caseIndent = 4                                                             //indent:1 exp:1
+ * lineWrappingIndentation = 4                                                //indent:1 exp:1
+ * throwsIndent = 4                                                           //indent:1 exp:1
+ */                                                                           //indent:1 exp:1
+//indent:0 exp:0
+@X//indent:0 exp:0
+public @Z//indent:0 exp:0
+    @W class InputIndentationAnnotationMultiLineCorrect//indent:4 exp:4
+    implements MyInterfaceTwo {//indent:4 exp:4
+//indent:0 exp:0
+//indent:0 exp:0
+}//indent:0 exp:0
+@M class AnotherClassTwo {}//indent:0 exp:0
+//indent:0 exp:0
+@interface X {}//indent:0 exp:0
+@interface Y {}//indent:0 exp:0
+@interface Z {}//indent:0 exp:0
+@interface W {}//indent:0 exp:0
+@interface M {}//indent:0 exp:0
+@interface K {}//indent:0 exp:0
+@interface P {}//indent:0 exp:0
+@interface Q {}//indent:0 exp:0
+interface MyInterfaceTwo {}//indent:0 exp:0
+//indent:0 exp:0
+@X @Y//indent:0 exp:0
+final @P @K//indent:0 exp:0
+    @M class MyClassTwoTwo//indent:4 exp:4
+    implements MyInterfaceTwo {}//indent:4 exp:4
+//indent:0 exp:0
+@X @Y//indent:0 exp:0
+abstract @P//indent:0 exp:0
+    @M @K class HisClassTwo//indent:4 exp:4
+    implements MyInterfaceTwo {}//indent:4 exp:4
diff --git a/...eckstyle/checks/indentation/indentation/InputIndentationAnnotationMultiLineIncorrect.java b/...eckstyle/checks/indentation/indentation/InputIndentationAnnotationMultiLineIncorrect.java
@@ -0,0 +1,41 @@
+package com.puppycrawl.tools.checkstyle.checks.indentation.indentation;//indent:0 exp:0
+//indent:0 exp:0
+/**                                                                           //indent:0 exp:0
+ * This test-input is intended to be checked using following configuration:   //indent:1 exp:1
+ *                                                                            //indent:1 exp:1
+ * arrayInitIndent = 2                                                        //indent:1 exp:1
+ * basicOffset = 2                                                            //indent:1 exp:1
+ * braceAdjustment = 2                                                        //indent:1 exp:1
+ * caseIndent = 4                                                             //indent:1 exp:1
+ * lineWrappingIndentation = 4                                                //indent:1 exp:1
+ * throwsIndent = 4                                                           //indent:1 exp:1
+ */                                                                           //indent:1 exp:1
+//indent:0 exp:0
+@A//indent:0 exp:0
+public @G//indent:0 exp:0
+@N class InputIndentationAnnotationMultiLineIncorrect//indent:0 exp:4 warn
+    implements MyInterface {//indent:4 exp:4
+//indent:0 exp:0
+//indent:0 exp:0
+}//indent:0 exp:0
+@T class AnotherClass {}//indent:0 exp:0
+//indent:0 exp:0
+@interface A {}//indent:0 exp:0
+@interface B {}//indent:0 exp:0
+@interface G {}//indent:0 exp:0
+@interface N {}//indent:0 exp:0
+@interface T {}//indent:0 exp:0
+@interface C {}//indent:0 exp:0
+@interface D {}//indent:0 exp:0
+@interface E {}//indent:0 exp:0
+interface MyInterface {}//indent:0 exp:0
+//indent:0 exp:0
+@A @B//indent:0 exp:0
+final @C @D//indent:0 exp:0
+@E class MyClass//indent:0 exp:4 warn
+    implements MyInterface {}//indent:4 exp:4
+//indent:0 exp:0
+@A @B//indent:0 exp:0
+abstract @C//indent:0 exp:0
+@E @D class HisClass//indent:0 exp:4 warn
+    implements MyInterface {}//indent:4 exp:4