Issue #9260: fix SuppressionWarningsHolder for annotation default val…

…ues and array inits
checkstyle · Feb 11, 2021 · 77721c4 · 77721c4
1 parent db7215d
commit 77721c4
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 48 deletions.
diff --git a/.ci/openjdk14-excluded.files b/.ci/openjdk14-excluded.files
@@ -58,7 +58,7 @@
 
 <!-- until https://github.com/checkstyle/checkstyle/issues/8653 -->
 <module name="BeforeExecutionExclusionFileFilter">
-  <property name="fileNamePattern" value="[\\/]TrailingComma.java$"/>
+  <property name="fileNamePattern" value="enum[\\/]TrailingComma.java$"/>
 </module>
 
 <!-- until https://github.com/checkstyle/checkstyle/issues/8943 -->

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java
@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.regex.Pattern;
 
 import com.puppycrawl.tools.checkstyle.StatelessCheck;
@@ -307,35 +308,42 @@ public void visitToken(DetailAST ast) {
             identifier = identifier.substring(JAVA_LANG_PREFIX.length());
         }
         if ("SuppressWarnings".equals(identifier)) {
-            final List<String> values = getAllAnnotationValues(ast);
-            if (!isAnnotationEmpty(values)) {
-                final DetailAST targetAST = getAnnotationTarget(ast);
-
-                // get text range of target
-                final int firstLine = targetAST.getLineNo();
-                final int firstColumn = targetAST.getColumnNo();
-                final DetailAST nextAST = targetAST.getNextSibling();
-                final int lastLine;
-                final int lastColumn;
-                if (nextAST == null) {
-                    lastLine = Integer.MAX_VALUE;
-                    lastColumn = Integer.MAX_VALUE;
-                }
-                else {
-                    lastLine = nextAST.getLineNo();
-                    lastColumn = nextAST.getColumnNo() - 1;
-                }
-
-                // add suppression entries for listed checks
-                final List<Entry> entries = ENTRIES.get();
-                for (String value : values) {
-                    String checkName = value;
-                    // strip off the checkstyle-only prefix if present
-                    checkName = removeCheckstylePrefixIfExists(checkName);
-                    entries.add(new Entry(checkName, firstLine, firstColumn,
-                            lastLine, lastColumn));
-                }
-            }
+            getAnnotationTarget(ast).ifPresent(targetAST -> {
+                addSuppressions(getAllAnnotationValues(ast), targetAST);
+            });
+        }
+    }
+
+    /**
+     * Method to populate list of suppression entries.
+     *
+     * @param values
+     *            - list of check names
+     * @param targetAST
+     *            - annotation target
+     */
+    private static void addSuppressions(List<String> values, DetailAST targetAST) {
+        // get text range of target
+        final int firstLine = targetAST.getLineNo();
+        final int firstColumn = targetAST.getColumnNo();
+        final DetailAST nextAST = targetAST.getNextSibling();
+        final int lastLine;
+        final int lastColumn;
+        if (nextAST == null) {
+            lastLine = Integer.MAX_VALUE;
+            lastColumn = Integer.MAX_VALUE;
+        }
+        else {
+            lastLine = nextAST.getLineNo();
+            lastColumn = nextAST.getColumnNo() - 1;
+        }
+
+        final List<Entry> entries = ENTRIES.get();
+        for (String value : values) {
+            // strip off the checkstyle-only prefix if present
+            final String checkName = removeCheckstylePrefixIfExists(value);
+            entries.add(new Entry(checkName, firstLine, firstColumn,
+                    lastLine, lastColumn));
         }
     }
 
@@ -362,7 +370,7 @@ private static String removeCheckstylePrefixIfExists(String checkName) {
      */
     private static List<String> getAllAnnotationValues(DetailAST ast) {
         // get values of annotation
-        List<String> values = null;
+        List<String> values = Collections.emptyList();
         final DetailAST lparenAST = ast.findFirstToken(TokenTypes.LPAREN);
         if (lparenAST != null) {
             final DetailAST nextAST = lparenAST.getNextSibling();
@@ -391,37 +399,33 @@ private static List<String> getAllAnnotationValues(DetailAST ast) {
         return values;
     }
 
-    /**
-     * Checks that annotation is empty.
-     *
-     * @param values list of values in the annotation
-     * @return whether annotation is empty or contains some values
-     */
-    private static boolean isAnnotationEmpty(List<String> values) {
-        return values == null;
-    }
-
     /**
      * Get target of annotation.
      *
      * @param ast the AST node to get the child of
      * @return get target of annotation
      */
-    private static DetailAST getAnnotationTarget(DetailAST ast) {
-        final DetailAST targetAST;
+    private static Optional<DetailAST> getAnnotationTarget(DetailAST ast) {
+        final Optional<DetailAST> result;
         final DetailAST parentAST = ast.getParent();
         switch (parentAST.getType()) {
             case TokenTypes.MODIFIERS:
             case TokenTypes.ANNOTATIONS:
             case TokenTypes.ANNOTATION:
             case TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR:
-                targetAST = parentAST.getParent();
+                result = Optional.of(parentAST.getParent());
+                break;
+            case TokenTypes.LITERAL_DEFAULT:
+                result = Optional.empty();
+                break;
+            case TokenTypes.ANNOTATION_ARRAY_INIT:
+                result = getAnnotationTarget(parentAST);
                 break;
             default:
                 // unexpected container type
                 throw new IllegalArgumentException("Unexpected container AST: " + parentAST);
         }
-        return targetAST;
+        return result;
     }
 
     /**

diff --git a/...ppycrawl/tools/checkstyle/checks/suppresswarningsholder/InputSuppressWarningsHolder7.java b/...ppycrawl/tools/checkstyle/checks/suppresswarningsholder/InputSuppressWarningsHolder7.java
@@ -8,21 +8,27 @@ public class InputSuppressWarningsHolder7 {
     @TestSwAnnotation(@SuppressWarnings("unchecked"))
     private List<String> testSwAnnotation;
 
-    @TestSwAnnotationVal(onMethod = @SuppressWarnings("unchecked"))
+    @TestSwAnnotationVal(value = @SuppressWarnings("unchecked"))
     private List<String> testSwAnnotationVal;
+
+    @TestSwAnnotationVal(value = {@SuppressWarnings("unchecked")})
+    private List<String> list1;
+
+    @TestSwAnnotationVal({@SuppressWarnings("unchecked"), @SuppressWarnings("unchecked")})
+    private List<String> list2;
 }
 
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.FIELD)
 @interface TestSwAnnotation {
-    SuppressWarnings value();
+    SuppressWarnings value() default @SuppressWarnings({"",});
 }
 
 @Target({ElementType.FIELD, ElementType.TYPE})
 @Retention(RetentionPolicy.SOURCE)
 @interface TestSwAnnotationVal{
 
-    SuppressWarnings[] onMethod() default {};
+    SuppressWarnings[] value() default {@SuppressWarnings({"",})};
 
     @Deprecated
     @Retention(RetentionPolicy.SOURCE)