#1213 AvoidLiteralsInIfCondition -- switch for integer comparison with 0

pmd · Jul 14, 2014 · ee2645f · ee2645f
1 parent 629eb02
commit ee2645f
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 6 deletions.
diff --git a/pmd/src/main/resources/rulesets/java/controversial.xml b/pmd/src/main/resources/rulesets/java/controversial.xml
@@ -740,31 +740,44 @@ public class Foo {
       externalInfoUrl="${pmd.website.baseurl}/rules/java/controversial.html#AvoidLiteralsInIfCondition">
     <description>
 Avoid using hard-coded literals in conditional statements. By declaring them as static variables
-or private members with descriptive names maintainability is enhanced.
+or private members with descriptive names maintainability is enhanced. By default, the literals "-1" and "0" are ignored.
+More exceptions can be defined with the property "ignoreMagicNumbers".
     </description>
     <priority>3</priority>
     <properties>
+      <property name="ignoreMagicNumbers" description="Comma-separated list of magic numbers, that should be ignored"
+        type="String" value="-1,0"/>
       <property name="xpath">
         <value>
 <![CDATA[
-//IfStatement/Expression/*/PrimaryExpression/PrimaryPrefix/Literal[not(NullLiteral)]
+//IfStatement/Expression/*/PrimaryExpression/PrimaryPrefix/Literal[
+not(NullLiteral)
+and empty(index-of(tokenize($ignoreMagicNumbers, ','), @Image))
+]
 ]]>
         </value>
       </property>
+      <property name="version" value="2.0"/>
     </properties>
     <example>
 <![CDATA[
-private static final int TEN = 10;
+private static final int MAX_NUMBER_OF_REQUESTS = 10;
 
-public void downCastPrimitiveType() {
+public void checkRequests() {
 
-    if (i == 10) {		// magic number, buried in a method
+    if (i == 10) {                        // magic number, buried in a method
       doSomething();
     }
 
-    if (i == TEN) {		// preferred approach
+    if (i == MAX_NUMBER_OF_REQUESTS) {    // preferred approach
       doSomething();
     }
+
+    if (aString.indexOf('.') != -1) {}     // magic number -1, by default ignored
+    if (aString.indexOf('.') >= 0) { }     // alternative approach
+
+    if (aDouble > 0.0) {}                  // magic number 0.0
+    if (aDouble >= Double.MIN_VALUE) {}    // preferred approach
 }
 ]]>
     </example>

diff --git a/pmd/src/site/markdown/changelog.md b/pmd/src/site/markdown/changelog.md
@@ -28,9 +28,11 @@
 
 **Feature Requests and Improvements:**
 
+* [#1213]: AvoidLiteralsInIfCondition -- switch for integer comparison with 0
 * [#1217]: SystemPrintln always says "System.out.print is used"
 * [#1221]: OneDeclarationPerLine really checks for one declaration each statement
 
+[#1213]: https://sourceforge.net/p/pmd/bugs/1213/
 [#1217]: https://sourceforge.net/p/pmd/bugs/1217/
 [#1221]: https://sourceforge.net/p/pmd/bugs/1221/
 

diff --git a/...urces/net/sourceforge/pmd/lang/java/rule/controversial/xml/AvoidLiteralsInIfCondition.xml b/...urces/net/sourceforge/pmd/lang/java/rule/controversial/xml/AvoidLiteralsInIfCondition.xml
@@ -34,6 +34,31 @@ public class MyClass {
     if(foo != null) {
     }
   }
+}
+    ]]></code>
+  </test-code>
+  <test-code>
+    <description>#1213 AvoidLiteralsInIfCondition -- switch for integer comparison with 0</description>
+    <expected-problems>0</expected-problems>
+    <code><![CDATA[
+public class Foo {
+    public void bar() {
+        if( ( flags & Flag.IMPORTANT ) != 0 ) {}
+        if (aString.indexOf('.') != -1) {}     // magic number -1, by default ignored
+    }
+}
+    ]]></code>
+  </test-code>
+  <test-code>
+    <description>#1213 AvoidLiteralsInIfCondition -- switch for integer comparison with 0.0</description>
+    <expected-problems>0</expected-problems>
+    <rule-property name="ignoreMagicNumbers">-1,0,0.0</rule-property>
+    <code><![CDATA[
+public class Foo {
+    public void bar() {
+        if (aDouble > 0.0) {}                  // magic number 0.0
+        if (aDouble >= Double.MIN_VALUE) {}    // preferred approach
+    }
 }
     ]]></code>
   </test-code>