Issue #4383: add ignoreAnnotationElementDefaults property in MagicNumber

checkstyle · Jul 2, 2019 · 1f30d57 · 1f30d57
1 parent 0bdbf15
commit 1f30d57
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 2 deletions.
diff --git a/.ci/jsoref-spellchecker/whitelist.words b/.ci/jsoref-spellchecker/whitelist.words
@@ -31,7 +31,6 @@ alot
 ambig
 Amessages
 androidx
-ANNOS
 annotationlocation
 annotationonsameline
 annotationusestyle

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheck.java
@@ -68,6 +68,11 @@
  * Default value is {@code false}.
  * </li>
  * <li>
+ * Property {@code ignoreAnnotationElementDefaults} -
+ * Ignore magic numbers in annotation elements defaults.
+ * Default value is {@code true}.
+ * </li>
+ * <li>
  * Property {@code constantWaiverParentToken} - Specify tokens that are allowed in the AST path
  * from the number literal to the enclosing constant definition.
  * Default value is
@@ -130,6 +135,9 @@
  *     int j = j + 8; // violation
  *   }
  * }
+ * &#64;interface anno {
+ *   int value() default 10; // no violation
+ * }
  * </pre>
  * <p>
  * To configure the check so that it checks floating-point numbers
@@ -158,6 +166,23 @@
  * }
  * </pre>
  * <p>
+ * To configure the check to check annotation element defaults:
+ * </p>
+ * <pre>
+ * &lt;module name=&quot;MagicNumber&quot;&gt;
+ *   &lt;property name=&quot;ignoreAnnotationElementDefaults&quot; value=&quot;false&quot;/&gt;
+ * &lt;/module&gt;
+ * </pre>
+ * <p>
+ * results in following violations:
+ * </p>
+ * <pre>
+ * &#64;interface anno {
+ *   int value() default 10; // violation
+ *   int[] value2() default {10}; // violation
+ * }
+ * </pre>
+ * <p>
  * Config example of constantWaiverParentToken option:
  * </p>
  * <pre>
@@ -230,6 +255,9 @@ public class MagicNumberCheck extends AbstractCheck {
     /** Ignore magic numbers in field declarations. */
     private boolean ignoreFieldDeclaration;
 
+    /** Ignore magic numbers in annotation elements defaults. */
+    private boolean ignoreAnnotationElementDefaults = true;
+
     /**
      * Constructor for MagicNumber Check.
      * Sort the allowedTokensBetweenMagicNumberAndConstDef array for binary search.
@@ -260,7 +288,8 @@ public int[] getRequiredTokens() {
 
     @Override
     public void visitToken(DetailAST ast) {
-        if ((!ignoreAnnotation || !isChildOf(ast, TokenTypes.ANNOTATION))
+        if (shouldTestAnnotationArgs(ast)
+                && shouldTestAnnotationDefaults(ast)
                 && !isInIgnoreList(ast)
                 && (!ignoreHashCodeMethod || !isInHashCodeMethod(ast))) {
             final DetailAST constantDefAST = findContainingConstantDef(ast);
@@ -279,6 +308,24 @@ public void visitToken(DetailAST ast) {
         }
     }
 
+    /**
+     * Checks if ast is annotation argument and should be checked.
+     * @param ast token to check
+     * @return true if element is skipped, false otherwise
+     */
+    private boolean shouldTestAnnotationArgs(DetailAST ast) {
+        return !ignoreAnnotation || !isChildOf(ast, TokenTypes.ANNOTATION);
+    }
+
+    /**
+     * Checks if ast is annotation element default value and should be checked.
+     * @param ast token to check
+     * @return true if element is skipped, false otherwise
+     */
+    private boolean shouldTestAnnotationDefaults(DetailAST ast) {
+        return !ignoreAnnotationElementDefaults || !isChildOf(ast, TokenTypes.LITERAL_DEFAULT);
+    }
+
     /**
      * Is magic number some where at ast tree.
      * @param ast ast token
@@ -480,6 +527,14 @@ public void setIgnoreFieldDeclaration(boolean ignoreFieldDeclaration) {
         this.ignoreFieldDeclaration = ignoreFieldDeclaration;
     }
 
+    /**
+     * Setter to ignore magic numbers in annotation elements defaults.
+     * @param ignoreAnnotationElementDefaults decide whether to ignore annotation elements defaults
+     */
+    public void setIgnoreAnnotationElementDefaults(boolean ignoreAnnotationElementDefaults) {
+        this.ignoreAnnotationElementDefaults = ignoreAnnotationElementDefaults;
+    }
+
     /**
      * Determines if the given AST node has a parent node with given token type code.
      *

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheckTest.java
@@ -84,6 +84,7 @@ public void testDefault()
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -133,6 +134,7 @@ public void testIgnoreSome()
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -210,6 +212,7 @@ public void testIgnoreNone()
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -258,6 +261,7 @@ public void testIntegersOnly()
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -302,6 +306,7 @@ public void testIgnoreNegativeOctalHex() throws Exception {
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -352,6 +357,7 @@ public void testIgnoreHashCodeMethod() throws Exception {
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -394,6 +400,7 @@ public void testIgnoreFieldDeclaration()
             "189:48: " + getCheckMessage(MSG_KEY, "43"),
             "193:42: " + getCheckMessage(MSG_KEY, "-44"),
             "197:48: " + getCheckMessage(MSG_KEY, "-45"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
@@ -456,8 +463,20 @@ public void testWaiverParentToken()
             "214:20: " + getCheckMessage(MSG_KEY, "0b101"),
             "215:22: " + getCheckMessage(MSG_KEY,
                 "0b1010000101000101101000010100010110100001010001011010000101000101L"),
+            "225:21: " + getCheckMessage(MSG_KEY, "122"),
         };
         verify(checkConfig, getPath("InputMagicNumber.java"), expected);
     }
 
+    @Test
+    public void testIgnoreInAnnotationElementDefault() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createModuleConfig(MagicNumberCheck.class);
+        checkConfig.addAttribute("ignoreAnnotationElementDefaults", "false");
+        final String[] expected = {
+            "7:29: " + getCheckMessage(MSG_KEY, "10"),
+            "8:33: " + getCheckMessage(MSG_KEY, "11"),
+        };
+        verify(checkConfig, getPath("InputMagicNumberAnnotationElement.java"), expected);
+    }
 }
diff --git a/...resources/com/puppycrawl/tools/checkstyle/checks/coding/magicnumber/InputMagicNumber.java b/...resources/com/puppycrawl/tools/checkstyle/checks/coding/magicnumber/InputMagicNumber.java
@@ -214,3 +214,15 @@ class Binary {
     int intValue = 0b101;
     long longValue = 0b1010000101000101101000010100010110100001010001011010000101000101L;
 }
+@interface AnnotationWithDefaultValue {
+    int value() default 101;
+    int[] ar() default {102};
+}
+class A {
+    {
+        switch (Blah2.LOW) {
+        default:
+            int b = 122; // violation
+        }
+    }
+}
diff --git a/...pycrawl/tools/checkstyle/checks/coding/magicnumber/InputMagicNumberAnnotationElement.java b/...pycrawl/tools/checkstyle/checks/coding/magicnumber/InputMagicNumberAnnotationElement.java
@@ -0,0 +1,9 @@
+package com.puppycrawl.tools.checkstyle.checks.coding.magicnumber;
+
+/**
+ * ignoreAnnotationElementDefaults = false
+ */
+@interface InputMagicNumberAnnotationElement {
+	int value() default 10; // violation
+	int[] value2() default {11}; // violation
+}
diff --git a/src/xdocs/config_coding.xml b/src/xdocs/config_coding.xml
@@ -2487,6 +2487,13 @@ static final Integer ANSWER_TO_THE_ULTIMATE_QUESTION_OF_LIFE = new Integer(42);
               <td><code>false</code></td>
               <td>6.6</td>
           </tr>
+          <tr>
+            <td>ignoreAnnotationElementDefaults</td>
+            <td>Ignore magic numbers in annotation elements defaults.</td>
+            <td><a href="property_types.html#boolean">Boolean</a></td>
+            <td><code>true</code></td>
+            <td>8.23</td>
+          </tr>
           <tr>
               <td>constantWaiverParentToken</td>
               <td>Specify tokens that are allowed in the AST path from the number literal to the
@@ -2571,6 +2578,9 @@ class MyClass {
     int i = i + 1; // no violation
     int j = j + 8; // violation
   }
+}
+@interface anno {
+  int value() default 10; // no violation
 }
         </source>
 
@@ -2598,6 +2608,23 @@ class MyClass {
     int i = i + 1; // no violation
     int j = j + 8; // violation
   }
+}
+        </source>
+        <p>
+        To configure the check to check annotation element defaults:
+        </p>
+        <source>
+&lt;module name=&quot;MagicNumber&quot;&gt;
+  &lt;property name=&quot;ignoreAnnotationElementDefaults&quot; value=&quot;false&quot;/&gt;
+&lt;/module&gt;
+        </source>
+        <p>
+        results in following violations:
+        </p>
+        <source>
+@interface anno {
+  int value() default 10; // violation
+  int[] value2() default {10}; // violation
 }
         </source>
         <p>