Merge pull request WorksApplications#18 from eller86/nullable-primitive

Detector for nullable primitive

README.md

@@ -38,6 +38,7 @@ To use this product, please configure your findbugs-maven-plugin like below.
 - added ImplicitLengthDetector
 - added ImplicitNullnessDetector
 - added ColumnDefinitionDetector
+- added NullablePrimitiveDetector
 
 ## 0.0.1
 

src/main/java/jp/co/worksap/oss/findbugs/jpa/NullablePrimitiveDetector.java

@@ -0,0 +1,58 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import java.util.Map;
+
+import org.apache.bcel.classfile.ElementValue;
+import org.apache.bcel.generic.ObjectType;
+import org.apache.bcel.generic.Type;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+
+public class NullablePrimitiveDetector extends AbstractColumnDetector {
+
+    public NullablePrimitiveDetector(BugReporter bugReporter) {
+        super(bugReporter);
+    }
+
+    @Override
+    protected void verifyColumn(Type columnType,
+            Map<String, ElementValue> elements) {
+        if (! isPrimitive(columnType)) {
+            return;
+        }
+
+        boolean isNullableColumn = detectNullability(elements);
+        if (isNullableColumn) {
+            reportNullablePrimitive(columnType);
+        }
+    }
+
+    private void reportNullablePrimitive(Type columnType) {
+        BugInstance bug = new BugInstance(this, "NULLABLE_PRIMITIVE", NORMAL_PRIORITY);
+        if (visitingMethod()) {
+            bug.addMethod(this);
+        }
+        getBugReporter().reportBug(bug);
+
+    }
+
+    private boolean detectNullability(Map<String, ElementValue> elements) {
+        if (! elements.containsKey("nullable")) {
+            // in JPA 1.0 specification, default value of 'nullable' parameter is true
+            // note that this case will be reported by ImplicitNullnessDetector.
+            return true;
+        }
+
+        String nullability = elements.get("nullable").stringifyValue();
+        return "true".equals(nullability);
+    }
+
+    /**
+     * @return true if column type is primitive value (not reference type).
+     */
+    private boolean isPrimitive(Type columnType) {
+        return ! (columnType instanceof ObjectType); // looks bad, but simple way to check primitive or not.
+    }
+
+}

src/main/meta/bugrank.txt

@@ -11,3 +11,4 @@
 0 BugPattern IMPLICIT_LENGTH
 0 BugPattern ILLEGAL_LENGTH
 0 BugPattern USE_COLUMN_DEFINITION
+0 BugPattern NULLABLE_PRIMITIVE

src/main/meta/findbugs.xml

@@ -54,6 +54,11 @@
   <BugPattern type="USE_COLUMN_DEFINITION" abbrev="JPA"
     category="BAD_PRACTICE" />
 
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.NullablePrimitiveDetector"
+    speed="fast" hidden="false" />
+  <BugPattern type="NULLABLE_PRIMITIVE" abbrev="JPA"
+    category="CORRECTNESS" />
+
   <Detector class="jp.co.worksap.oss.findbugs.junit.UndocumentedIgnoreDetector"
     speed="fast" hidden="false" />
   <BugPattern type="UNDOCUMENTED_IGNORE" abbrev="JUNIT"

src/main/meta/messages.xml

@@ -20,7 +20,7 @@
     <LongDescription>we can not use System.out and System.err, please use log to
       output information
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>System.out and System.err are forbidden, please use log</p>
   ]]>
@@ -39,7 +39,7 @@
     </ShortDescription>
     <LongDescription>To prevent from bad extending, immutable class should be final.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>To prevent from bad extending, immutable class should be final.</p>
   ]]>
@@ -52,7 +52,7 @@
     <LongDescription>
       Field ({1}) in class ({2}) should be final to make a class ({3}) immutable.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Field of immutable class should be final.</p>
   ]]>
@@ -70,7 +70,7 @@
     </ShortDescription>
     <LongDescription>Nullness of this parameter is unknown.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Nullness of this parameter is unknown.</p>
   ]]>
@@ -83,7 +83,7 @@
     <LongDescription>
       Nullness of this returned value is unknown.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Nullness of this returned value is unknown.</p>
   ]]>
@@ -103,7 +103,7 @@
     <LongDescription>
       Index name should be shorter than or equal to 30 bytes.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Index name should be shorter than or equal to 30 bytes.</p>
   ]]>
@@ -123,7 +123,7 @@
     <LongDescription>
       Table name should be shorter than or equal to 30 bytes.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Table name should be shorter than or equal to 30 bytes.</p>
   ]]>
@@ -143,7 +143,7 @@
     <LongDescription>
       Column name should be shorter than or equal to 30 bytes.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Column name should be shorter than or equal to 30 bytes.</p>
   ]]>
@@ -163,7 +163,7 @@
     <LongDescription>
       Specify length of column, its default (255) might be not enough.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>Specify length of column, its default (255) might be not enough.</p>
   ]]>
@@ -197,7 +197,7 @@
     <LongDescription>
       It is good to specify the value of nullable element clear, it tells that you have considered about it.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>It is good to specify the value of nullable element clear, it tells that you have considered about it.</p>
   ]]>
@@ -217,13 +217,34 @@
     <LongDescription>
       @Column annotation has columnDefinition property, it might break RDBMS portability.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>@Column annotation has columnDefinition property, it might break RDBMS portability.</p>
   ]]>
     </Details>
   </BugPattern>
 
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.NullablePrimitiveDetector">
+    <Details>
+      This detector will find illegal nullable property of primitive type.
+      When developer changes type of field from primitive type to reference type (ex. from int to Integer),
+      he/she might forget to care about this property. This detector helps developer to find this human-error.
+    </Details>
+  </Detector>
+
+  <BugPattern type="NULLABLE_PRIMITIVE">
+    <ShortDescription>Nullable property of primitive type should be false.
+    </ShortDescription>
+    <LongDescription>
+      Nullable property of primitive type should be false.
+    </LongDescription>
+    <Details>
+  <![CDATA[
+    <p>Nullable property of primitive type should be false.</p>
+  ]]>
+    </Details>
+  </BugPattern>
+
   <Detector class="jp.co.worksap.oss.findbugs.junit.UndocumentedIgnoreDetector">
     <Details>
       This detector will find ignored test case which has no explanation about why it is ignored.
@@ -236,7 +257,7 @@
     <LongDescription>
       To tell why test case is ignored, put explanation as a parameter.
     </LongDescription>
-    <Details>  
+    <Details>
   <![CDATA[
     <p>To tell why test case is ignored, put explanation as a parameter.</p>
   ]]>

src/test/java/jp/co/worksap/oss/findbugs/jpa/NonNullablePrimitiveColumn.java

@@ -0,0 +1,28 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NonNullablePrimitiveColumn {
+    @Column(nullable = false)
+    private boolean booleanValue;
+
+    @Column(nullable = false)
+    private byte byteValue;
+
+    @Column(nullable = false)
+    private short shortValue;
+
+    @Column(nullable = false)
+    private int intValue;
+
+    @Column(nullable = false)
+    private long longValue;
+
+    @Column(nullable = false)
+    private float floatValue;
+
+    @Column(nullable = false)
+    private double doubleValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableBooleanColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableBooleanColumn {
+    @Column(nullable = true)
+    private boolean booleanValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableByteColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableByteColumn {
+    @Column(nullable = true)
+    private byte byteValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableDoubleColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableDoubleColumn {
+    @Column(nullable = true)
+    private double doubleValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableFloatColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableFloatColumn {
+    @Column(nullable = true)
+    private float floatValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableIntColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableIntColumn {
+    @Column(nullable = true)
+    private int intValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableLongColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableLongColumn {
+    @Column(nullable = true)
+    private long longValue;
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullablePrimitiveDetectorTest.java

@@ -0,0 +1,56 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import static com.youdevise.fbplugins.tdd4fb.DetectorAssert.assertBugReported;
+import static com.youdevise.fbplugins.tdd4fb.DetectorAssert.assertNoBugsReported;
+import static com.youdevise.fbplugins.tdd4fb.DetectorAssert.bugReporterForTesting;
+import static com.youdevise.fbplugins.tdd4fb.DetectorAssert.ofType;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import edu.umd.cs.findbugs.BugReporter;
+
+public class NullablePrimitiveDetectorTest {
+    private BugReporter bugReporter;
+    private NullablePrimitiveDetector detector;
+
+    @Before
+    public void before() {
+        this.bugReporter = bugReporterForTesting();
+        this.detector = new NullablePrimitiveDetector(bugReporter);
+    }
+
+    @Test
+    public void testNullableObject() throws Exception {
+        assertNoBugsReported(UseColumnDefinition.class, detector, bugReporter);
+    }
+
+    @Test
+    public void testNullableInt() throws Exception {
+        assertBugReported(NullableBooleanColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableByteColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableShortColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableIntColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableLongColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableFloatColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+        assertBugReported(NullableDoubleColumn.class, detector,
+                bugReporter, ofType("NULLABLE_PRIMITIVE"));
+    }
+
+    @Test
+    public void testNonNullableObject() throws Exception {
+        assertNoBugsReported(ColumnWithNullable.class, detector, bugReporter);
+    }
+
+    @Test
+    public void testNonNullableInt() throws Exception {
+        assertNoBugsReported(NonNullablePrimitiveColumn.class, detector, bugReporter);
+    }
+
+}

src/test/java/jp/co/worksap/oss/findbugs/jpa/NullableShortColumn.java

@@ -0,0 +1,10 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+@Entity
+public class NullableShortColumn {
+    @Column(nullable = true)
+    private short shortValue;
+}