TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Solution to Magic Number is not detected properly #1266

This commit is contained in:
Bhavik Patel 2015-07-06 22:13:06 +05:30 committed by Roman Ivanov
parent 71c16c55c3
commit b2a80ceca3
4 changed files with 167 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheck.java
View File

@ -22,6 +22,7 @@ package com.puppycrawl.tools.checkstyle.checks.coding;
import java.util.Arrays;
import com.puppycrawl.tools.checkstyle.ScopeUtils;
import com.puppycrawl.tools.checkstyle.Utils;
import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
@ -35,6 +36,7 @@ import com.puppycrawl.tools.checkstyle.checks.CheckUtils;
* By default, -1, 0, 1, and 2 are not considered to be magic numbers.
* </p>
* <p>
* Constant definition is any variable/field that has 'final' modifier.
* It is fine to have one constant defining multiple numeric literals within one expression:
* <pre>
* <code>static final int SECONDS_PER_DAY = 24 * 60 * 60;
@ -95,12 +97,42 @@ import com.puppycrawl.tools.checkstyle.checks.CheckUtils;
*
* void foo() {
* int i = i + 1; // no violation
* int j = j + 8; // violation
* int j = j + (int)0.5; // no violation
* }
* }
* </code>
* </pre>
*
* <p>
* Config example of constantWaiverParentToken option:
* </p>
* <pre>
* &lt;module name=&quot;MagicNumber&quot;&gt;
* &lt;property name=&quot;constantWaiverParentToken&quot; value=&quot;ASSIGN,ARRAY_INIT,EXPR,
* UNARY_PLUS, UNARY_MINUS, TYPECAST, ELIST, DIV, PLUS &quot;/&gt;
* &lt;/module&gt;
* </pre>
* <p>
* result is following violation:
* </p>
* <pre>
* <code>
* class TestMethodCall {
* public void method2() {
* final TestMethodCall dummyObject = new TestMethodCall(62); //violation
* final int a = 3; // ok as waiver is ASSIGN
* final int [] b = {4, 5} // ok as waiver is ARRAY_INIT
* final int c = -3; // ok as waiver is UNARY_MINUS
* final int d = +4; // ok as waiver is UNARY_PLUS
* final int e = method(1, 2) // ELIST is there but violation due to METHOD_CALL
* final int x = 3 * 4; // violation
* final int y = 3 / 4; // ok as waiver is DIV
* final int z = 3 + 4; // ok as waiver is PLUS
* final int w = 3 - 4; // violation
* final int x = (int)(3.4); //ok as waiver is TYPECAST
* }
* }
* </code>
* </pre>
* @author Rick Giles
* @author Lars Kühne
* @author Daniel Solano Gómez
@ -117,7 +149,7 @@ public class MagicNumberCheck extends Check {
* The token types that are allowed in the AST path from the
* number literal to the enclosing constant definition.
*/
private static final int[] ALLOWED_PATH_TOKENTYPES = {
private int[] constantWaiverParentToken = {
TokenTypes.ASSIGN,
TokenTypes.ARRAY_INIT,
TokenTypes.EXPR,
@ -133,10 +165,6 @@ public class MagicNumberCheck extends Check {
TokenTypes.MINUS,
};
static {
Arrays.sort(ALLOWED_PATH_TOKENTYPES);
}
/** the numbers to ignore in the check, sorted */
private double[] ignoreNumbers = {-1, 0, 1, 2};
@ -149,14 +177,18 @@ public class MagicNumberCheck extends Check {
/** Whether to ignore magic numbers in field declaration. */
private boolean ignoreFieldDeclaration;
/**
* Constructor for MagicNumber Check.
* Sort the allowedTokensBetweenMagicNumberAndConstDef array for binary search.
*/
public MagicNumberCheck() {
super();
Arrays.sort(constantWaiverParentToken);
}
@Override
public int[] getDefaultTokens() {
return new int[] {
TokenTypes.NUM_DOUBLE,
TokenTypes.NUM_FLOAT,
TokenTypes.NUM_INT,
TokenTypes.NUM_LONG,
};
return getAcceptableTokens();
}
@Override
@ -191,7 +223,6 @@ public class MagicNumberCheck extends Check {
final boolean found = isMagicNumberExists(ast, constantDefAST);
if (found) {
reportMagicNumber(ast);
}
}
}
@ -202,12 +233,12 @@ public class MagicNumberCheck extends Check {
* @param constantDefAST constant ast
* @return true if magic number is present
*/
private static boolean isMagicNumberExists(DetailAST ast, DetailAST constantDefAST) {
private boolean isMagicNumberExists(DetailAST ast, DetailAST constantDefAST) {
boolean found = false;
DetailAST astNode = ast.getParent();
while (astNode != constantDefAST) {
final int type = astNode.getType();
if (Arrays.binarySearch(ALLOWED_PATH_TOKENTYPES, type) < 0) {
if (Arrays.binarySearch(constantWaiverParentToken, type) < 0) {
found = true;
break;
}
@ -353,6 +384,17 @@ public class MagicNumberCheck extends Check {
== TokenTypes.CLASS_DEF;
}
/**
* sets the tokens which are allowed between Magic Number and defined Object.
* @param tokens The string representation of the tokens interested in
*/
public void setConstantWaiverParentToken(String... tokens) {
constantWaiverParentToken = new int[tokens.length];
for (int i = 0; i < tokens.length; i++) {
constantWaiverParentToken[i] = Utils.getTokenId(tokens[i]);
}
Arrays.sort(constantWaiverParentToken);
}
/**
* Sets the numbers to ignore in the check.

59
src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/MagicNumberCheckTest.java
View File

@ -373,4 +373,63 @@ public class MagicNumberCheckTest
};
verify(checkConfig, getPath("InputMagicNumber.java"), expected);
}
@Test
public void testwaiverParentToken()
throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(MagicNumberCheck.class);
checkConfig.addAttribute("constantWaiverParentToken", "ASSIGN, ARRAY_INIT,"
+ " EXPR, UNARY_PLUS, UNARY_MINUS, TYPECAST, ELIST, STAR, DIV, PLUS, MINUS");
final String[] expected = {
"41:26: " + getCheckMessage(MSG_KEY, "3_000"),
"42:32: " + getCheckMessage(MSG_KEY, "1.5_0"),
"43:27: " + getCheckMessage(MSG_KEY, "3"),
"43:31: " + getCheckMessage(MSG_KEY, "4"),
"45:29: " + getCheckMessage(MSG_KEY, "3"),
"47:23: " + getCheckMessage(MSG_KEY, "3"),
"48:26: " + getCheckMessage(MSG_KEY, "1.5"),
"50:22: " + getCheckMessage(MSG_KEY, "3"),
"50:29: " + getCheckMessage(MSG_KEY, "5"),
"50:37: " + getCheckMessage(MSG_KEY, "3"),
"54:26: " + getCheckMessage(MSG_KEY, "3"),
"55:39: " + getCheckMessage(MSG_KEY, "3"),
"60:25: " + getCheckMessage(MSG_KEY, "010"),
"61:25: " + getCheckMessage(MSG_KEY, "011"),
"63:30: " + getCheckMessage(MSG_KEY, "0_10L"),
"64:30: " + getCheckMessage(MSG_KEY, "011l"),
"68:24: " + getCheckMessage(MSG_KEY, "0x10"),
"69:24: " + getCheckMessage(MSG_KEY, "0X011"),
"71:29: " + getCheckMessage(MSG_KEY, "0x10L"),
"72:29: " + getCheckMessage(MSG_KEY, "0X11l"),
"85:28: " + getCheckMessage(MSG_KEY, "3"),
"92:14: " + getCheckMessage(MSG_KEY, "0xffffffffL"),
"100:30: " + getCheckMessage(MSG_KEY, "+3"),
"101:29: " + getCheckMessage(MSG_KEY, "-2"),
"102:35: " + getCheckMessage(MSG_KEY, "+3.5"),
"103:36: " + getCheckMessage(MSG_KEY, "-2.5"),
"111:35: " + getCheckMessage(MSG_KEY, "0x80000000"),
"112:36: " + getCheckMessage(MSG_KEY, "0x8000000000000000L"),
"115:37: " + getCheckMessage(MSG_KEY, "020000000000"),
"116:38: " + getCheckMessage(MSG_KEY, "01000000000000000000000L"),
"131:20: " + getCheckMessage(MSG_KEY, "378"),
"140:52: " + getCheckMessage(MSG_KEY, "27"),
"143:53: " + getCheckMessage(MSG_KEY, "3"),
"143:56: " + getCheckMessage(MSG_KEY, "3"),
"143:59: " + getCheckMessage(MSG_KEY, "3"),
"143:62: " + getCheckMessage(MSG_KEY, "3"),
"160:16: " + getCheckMessage(MSG_KEY, "31"),
"165:16: " + getCheckMessage(MSG_KEY, "42"),
"170:16: " + getCheckMessage(MSG_KEY, "13"),
"174:15: " + getCheckMessage(MSG_KEY, "21"),
"178:15: " + getCheckMessage(MSG_KEY, "37"),
"182:15: " + getCheckMessage(MSG_KEY, "101"),
"185:26: " + getCheckMessage(MSG_KEY, "42"),
"189:32: " + getCheckMessage(MSG_KEY, "43"),
"193:26: " + getCheckMessage(MSG_KEY, "-44"),
"197:32: " + getCheckMessage(MSG_KEY, "-45"),
"209:63: " + getCheckMessage(MSG_KEY, "62"),
};
verify(checkConfig, getPath("InputMagicNumber.java"), expected);
}
}

19
src/test/resources/com/puppycrawl/tools/checkstyle/InputMagicNumber.java
View File

@ -136,10 +136,10 @@ class ComplexAndFlagged
class ComplexButNotFlagged
{
// according to user feedback this is typical code that should not be flagged
public static final double SPECIAL_SUM = 2 + 1e10, SPECIAL_DIFFERENCE = 4 - java.lang.Math.PI;
public static final Integer DEFAULT_INT = new Integer(27);
public static final int SECS_PER_DAY = 24 * 60 * 60, SPECIAL_RATIO = 4 / 3;
public static final javax.swing.border.Border STD_BORDER =
public final double SpecialSum = 2 + 1e10, SpecialDifference = 4 - java.lang.Math.PI;
public final Integer DefaultInit = new Integer(27);
public final int SpecsPerDay = 24 * 60 * 60, SpecialRatio = 4 / 3;
public final javax.swing.border.Border StdBorder =
javax.swing.BorderFactory.createEmptyBorder(3, 3, 3, 3);
}
@ -198,3 +198,14 @@ class TestHashCodeMethod {
public void anotherNegative2() {
}
}
class TestMethodCall {
public TestMethodCall(int x){
}
public void method2() {
final TestMethodCall dummyObject = new TestMethodCall(62);
}
}

36
src/xdocs/config_coding.xml
View File

@ -991,6 +991,13 @@ static final Integer ANSWER_TO_THE_ULTIMATE_QUESTION_OF_LIFE = new Integer(42);
<td><a href="property_types.html#boolean">boolean</a></td>
<td><span class="default">false</span></td>
</tr>
<tr>
<td>constantWaiverParentToken</td>
<td>Token that are allowed in the AST path from the number literal to the enclosing constant definition.</td>
<td>subset of tokens</td>
<td><span class="default">ASSIGN, ARRAY_INIT, EXPR, UNARY_PLUS, UNARY_MINUS, TYPECAST, ELIST, STAR, DIV, PLUS,
MINUS, LITERAL_NEW, METHOD_CALL</span></td>
</tr>
</table>
</subsection>
@ -1042,8 +1049,35 @@ class MyClass {
}
}
</source>
<p>
Config Example for constantWaiverParentToken Option:
</p>
<source>
&lt;module name=&quot;MagicNumber&quot;&gt;
&lt;property name=&quot;constantWaiverParentToken&quot; value=&quot;ASSIGN,ARRAY_INIT,EXPR,
UNARY_PLUS, UNARY_MINUS, TYPECAST, ELIST, DIV, PLUS &quot;/&gt;
&lt;/module&gt;
</source>
<p>result is following violation:</p>
<source>
class TestMethodCall {
public void method2() {
final TestMethodCall dummyObject = new TestMethodCall(62); //violation
final int a = 3; // ok as waiver is ASSIGN
final int [] b = {4, 5} // ok as waiver is ARRAY_INIT
final int c = -3; // ok as waiver is UNARY_MINUS
final int d = +4; // ok as waiver is UNARY_PLUS
final int e = method(1, 2) // ELIST is there but violation due to METHOD_CALL
final int x = 3 * 4; // violation
final int y = 3 / 4; // ok as waiver is DIV
final int z = 3 + 4; // ok as waiver is PLUS
final int w = 3 - 4; // violation
final int x = (int)(3.4); //ok as waiver is TYPECAST
}
}
</source>
</subsection>
</subsection>
<subsection name="Package">
<p>