From 6edd0b8035fca4f8b60b2c1d016717bda796891d Mon Sep 17 00:00:00 2001
From: eller86 <skypencil@gmail.com>
Date: Wed, 21 Aug 2013 20:50:12 +0800
Subject: [PATCH 1/9] implement detector to check implicit elements.

---
 .../findbugs/jpa/AbstractColumnDetector.java  | 68 +++++++++++++++++++
 .../findbugs/jpa/ImplicitLengthDetector.java  | 38 +++++++++++
 .../jpa/ImplicitNullnessDetector.java         | 28 ++++++++
 .../findbugs/jpa/LongColumnNameDetector.java  | 15 +---
 .../oss/findbugs/jpa/VisitedFieldFinder.java  | 20 +++++-
 .../oss/findbugs/jpa/ColumnWithLength.java    | 12 ++++
 .../oss/findbugs/jpa/ColumnWithNullable.java  | 12 ++++
 .../findbugs/jpa/ColumnWithoutElement.java    | 12 ++++
 .../oss/findbugs/jpa/ImplicitLengthTest.java  | 33 +++++++++
 .../findbugs/jpa/ImplicitNullnessTest.java    | 33 +++++++++
 10 files changed, 256 insertions(+), 15 deletions(-)
 create mode 100644 src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
 create mode 100644 src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
 create mode 100644 src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessDetector.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLength.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNullable.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithoutElement.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
new file mode 100644
index 0000000..2e799d1
--- /dev/null
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
@@ -0,0 +1,68 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import java.util.Map;
+
+import javax.annotation.CheckReturnValue;
+import javax.annotation.Nonnull;
+
+import org.apache.bcel.classfile.ElementValue;
+import org.apache.bcel.classfile.Field;
+import org.apache.bcel.generic.Type;
+
+import com.google.common.base.Objects;
+
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.bcel.AnnotationDetector;
+import edu.umd.cs.findbugs.internalAnnotations.DottedClassName;
+
+abstract class AbstractColumnDetector extends AnnotationDetector {
+    private final BugReporter bugReporter;
+
+    AbstractColumnDetector(BugReporter bugReporter) {
+        this.bugReporter = bugReporter;
+    }
+
+    @Nonnull
+    @CheckReturnValue
+    protected final BugReporter getBugReporter() {
+    	return bugReporter;
+    }
+
+    @Override
+    public final void visitAnnotation(@DottedClassName String annotationClass,
+            Map<String, ElementValue> map, boolean runtimeVisible) {
+        if (!Objects.equal(annotationClass, "javax.persistence.Column")) {
+            return;
+        }
+
+        Type columnType;
+        if (visitingField()) {
+            columnType = getField().getType();
+        } else if (visitingMethod()) {
+            Field visitingField = findFieldInVisitingMethod();
+            columnType = visitingField.getType();
+        } else {
+            throw new IllegalStateException("@Column should annotate field or method.");
+        }
+        verifyColumn(columnType, map);
+    }
+
+    @Nonnull
+    private Field findFieldInVisitingMethod() {
+        String fieldName = VisitedFieldFinder.findFieldWhichisVisitedInVisitingMethod(this);
+        Field visitingField = null;
+        for (Field field : getThisClass().getFields()) {
+            if (Objects.equal(field.getName(), fieldName)) {
+                visitingField = field;
+                break;
+            }
+        }
+        if (visitingField == null) {
+            throw new IllegalStateException("Cannot find field which named as " + fieldName + ".");
+        }
+        return visitingField;
+    }
+
+    protected abstract void verifyColumn(Type columnType, Map<String, ElementValue> elements);
+
+}
diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
new file mode 100644
index 0000000..f04fc36
--- /dev/null
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
@@ -0,0 +1,38 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import java.util.Map;
+
+import org.apache.bcel.classfile.ElementValue;
+import org.apache.bcel.generic.Type;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+
+public class ImplicitLengthDetector extends AbstractColumnDetector {
+
+    public ImplicitLengthDetector(BugReporter bugReporter) {
+        super(bugReporter);
+    }
+
+    @Override
+    protected void verifyColumn(Type columnType,
+            Map<String, ElementValue> elements) {
+        if (verifyColumnType(columnType) && !elements.containsKey("length")) {
+            BugInstance bug = new BugInstance(this, "IMPLICIT_LENGTH", HIGH_PRIORITY);
+            if (visitingMethod()) {
+                bug.addMethod(this);
+            }
+            getBugReporter().reportBug(bug);
+        }
+    }
+
+    /**
+     * @param columnType
+     * @return true if column type requires length property.
+     */
+    private boolean verifyColumnType(Type columnType) {
+        // TODO true if target is annotated with @Lob
+        return Type.STRING.equals(columnType);
+    }
+
+}
diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessDetector.java
new file mode 100644
index 0000000..c702ff7
--- /dev/null
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessDetector.java
@@ -0,0 +1,28 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import java.util.Map;
+
+import org.apache.bcel.classfile.ElementValue;
+import org.apache.bcel.generic.Type;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+
+public class ImplicitNullnessDetector extends AbstractColumnDetector {
+
+    public ImplicitNullnessDetector(BugReporter bugReporter) {
+        super(bugReporter);
+    }
+
+    @Override
+    protected void verifyColumn(Type columnType,
+            Map<String, ElementValue> elements) {
+        if (! elements.containsKey("nullable")) {
+            BugInstance bug = new BugInstance(this, "IMPLICIT_NULLNESS", HIGH_PRIORITY);
+            if (visitingMethod()) {
+                bug.addMethod(this);
+            }
+            getBugReporter().reportBug(bug);
+        }
+    }
+}
diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/LongColumnNameDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/LongColumnNameDetector.java
index 98c631d..b2672f1 100755
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/LongColumnNameDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/LongColumnNameDetector.java
@@ -5,7 +5,6 @@
 import org.apache.bcel.classfile.ElementValue;
 import org.apache.bcel.classfile.Method;
 import org.apache.commons.lang.IllegalClassException;
-import org.objectweb.asm.ClassReader;
 
 import com.google.common.base.Objects;
 
@@ -49,7 +48,7 @@ public void visitAnnotation(@DottedClassName String annotationClass,
             columnName = getFieldName();
         } else if (visitingMethod()) {
             Method targetMethod = getMethod();
-            columnName = findFieldWhichisVisitedIn(targetMethod);
+            columnName = VisitedFieldFinder.findFieldWhichisVisitedInVisitingMethod(this);
             if (columnName == null) {
                 throw new IllegalClassException(String.format(
                         "Method which is annotated with @Column should access to field, but %s#%s does not access.",
@@ -62,18 +61,6 @@ public void visitAnnotation(@DottedClassName String annotationClass,
         detectLongName(columnName);
     }
 
-    private String findFieldWhichisVisitedIn(Method targetMethod) {
-        byte[] classByteCode = getClassContext().getJavaClass().getBytes();
-        ClassReader reader = new ClassReader(classByteCode);
-
-        // note: bcel's #getSignature() method returns String like "(J)V", this is named as "descriptor" in the context of ASM.
-        // This is the reason why we call `targetMethod.getSignature()` to get value for `targetMethodDescriptor` argument.
-        VisitedFieldFinder visitedFieldFinder = new VisitedFieldFinder(targetMethod.getName(), targetMethod.getSignature());
-
-        reader.accept(visitedFieldFinder, 0);
-        return visitedFieldFinder.getVisitedFieldName();
-    }
-
     private void detectLongName(String tableName) {
         if (tableName.length() > MAX_TABLE_LENGTH) {
             bugReporter.reportBug(new BugInstance(this, "LONG_COLUMN_NAME",
diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/VisitedFieldFinder.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/VisitedFieldFinder.java
index 607af8d..fbff640 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/VisitedFieldFinder.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/VisitedFieldFinder.java
@@ -6,9 +6,13 @@
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.bcel.classfile.FieldOrMethod;
+import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.commons.EmptyVisitor;
 
+import edu.umd.cs.findbugs.bcel.AnnotationDetector;
+
 /**
  * <p>Simple ClassVisitor implementation to find visited field in the specified method.</p>
  * <p>To create instance, you need to provide name and descriptor to specify the target method.</p>
@@ -28,7 +32,7 @@ public VisitedFieldFinder(@Nonnull String targetMethodName, @Nonnull String targ
 
     @CheckReturnValue
     @Nullable
-    String getVisitedFieldName() {
+    private String getVisitedFieldName() {
         return visitedFieldName;
     }
 
@@ -48,4 +52,18 @@ public void visitFieldInsn(int code, String owner, String name, String descripti
         this.visitedFieldName = name;
     }
 
+    @Nullable
+    @CheckReturnValue
+    static String findFieldWhichisVisitedInVisitingMethod(AnnotationDetector detector) {
+        byte[] classByteCode = detector.getClassContext().getJavaClass().getBytes();
+        ClassReader reader = new ClassReader(classByteCode);
+
+        FieldOrMethod targetMethod = detector.getMethod();
+        // note: bcel's #getSignature() method returns String like "(J)V", this is named as "descriptor" in the context of ASM.
+        // This is the reason why we call `targetMethod.getSignature()` to get value for `targetMethodDescriptor` argument.
+        VisitedFieldFinder visitedFieldFinder = new VisitedFieldFinder(targetMethod .getName(), targetMethod.getSignature());
+
+        reader.accept(visitedFieldFinder, 0);
+        return visitedFieldFinder.getVisitedFieldName();
+    }
 }
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLength.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLength.java
new file mode 100644
index 0000000..938a83f
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLength.java
@@ -0,0 +1,12 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+
+public class ColumnWithLength {
+    @Column(length = 100)
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNullable.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNullable.java
new file mode 100644
index 0000000..9c7d88f
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNullable.java
@@ -0,0 +1,12 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+
+public class ColumnWithNullable {
+    @Column(nullable = false)
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithoutElement.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithoutElement.java
new file mode 100644
index 0000000..075d361
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithoutElement.java
@@ -0,0 +1,12 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+
+public class ColumnWithoutElement {
+    @Column
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
new file mode 100644
index 0000000..228b61c
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
@@ -0,0 +1,33 @@
+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 org.junit.Before;
+import org.junit.Test;
+
+import edu.umd.cs.findbugs.BugReporter;
+
+public class ImplicitLengthTest {
+    private BugReporter bugReporter;
+    private ImplicitLengthDetector detector;
+
+    @Before
+    public void before() {
+        this.bugReporter = bugReporterForTesting();
+        this.detector = new ImplicitLengthDetector(bugReporter);
+    }
+
+    @Test
+    public void testImplicitLength() throws Exception {
+        assertNoBugsReported(ColumnWithLength.class, detector,
+                bugReporter);
+    }
+
+    @Test
+    public void testExplicitLength() throws Exception {
+        assertBugReported(ColumnWithoutElement.class, detector,
+                bugReporter);
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java
new file mode 100644
index 0000000..0888847
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java
@@ -0,0 +1,33 @@
+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 org.junit.Before;
+import org.junit.Test;
+
+import edu.umd.cs.findbugs.BugReporter;
+
+public class ImplicitNullnessTest {
+    private BugReporter bugReporter;
+    private ImplicitNullnessDetector detector;
+
+    @Before
+    public void before() {
+        this.bugReporter = bugReporterForTesting();
+        this.detector = new ImplicitNullnessDetector(bugReporter);
+    }
+
+    @Test
+    public void testImplicitNullness() throws Exception {
+        assertNoBugsReported(ColumnWithNullable.class, detector,
+                bugReporter);
+    }
+
+    @Test
+    public void testExplicitNullness() throws Exception {
+        assertBugReported(ColumnWithoutElement.class, detector,
+                bugReporter);
+    }
+}

From a09bab417e52db5e163f8e84e54a398603062481 Mon Sep 17 00:00:00 2001
From: eller86 <skypencil@gmail.com>
Date: Wed, 21 Aug 2013 21:13:07 +0800
Subject: [PATCH 2/9] update metadata and README.

---
 README.md                  |  2 ++
 src/main/meta/bugrank.txt  |  4 +++-
 src/main/meta/findbugs.xml | 10 ++++++++++
 src/main/meta/messages.xml | 39 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index c767d3b..0614cec 100755
--- a/README.md
+++ b/README.md
@@ -34,6 +34,8 @@ To use this product, please configure your findbugs-maven-plugin like below.
 - added LongColumnNameDetector
 - added UnknownNullnessDetector
 - added UndocumentedIgnoreDetector
+- added ImplicitLengthDetector
+- added ImplicitNullnessDetector
 
 ## 0.0.1
 
diff --git a/src/main/meta/bugrank.txt b/src/main/meta/bugrank.txt
index c47fcfb..102c40e 100755
--- a/src/main/meta/bugrank.txt
+++ b/src/main/meta/bugrank.txt
@@ -6,4 +6,6 @@
 0 BugPattern LONG_COLUMN_NAME
 0 BugPattern UNKNOWN_NULLNESS_OF_PARAMETER
 0 BugPattern UNKNOWN_NULLNESS_OF_RETURNED_VALUE
-0 BugPattern UNDOCUMENTED_IGNORE
\ No newline at end of file
+0 BugPattern UNDOCUMENTED_IGNORE
+0 BugPattern IMPLICIT_NULLNESS
+0 BugPattern IMPLICIT_LENGTH
diff --git a/src/main/meta/findbugs.xml b/src/main/meta/findbugs.xml
index 8a352e6..4deb29f 100755
--- a/src/main/meta/findbugs.xml
+++ b/src/main/meta/findbugs.xml
@@ -37,6 +37,16 @@
   <BugPattern type="LONG_COLUMN_NAME" abbrev="JPA"
     category="CORRECTNESS" />
 
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitLengthDetector"
+    speed="fast" hidden="false" />
+  <BugPattern type="IMPLICIT_LENGTH" abbrev="JPA"
+    category="BAD_PRACTICE" />
+
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitNullnessDetector"
+    speed="fast" hidden="false" />
+  <BugPattern type="IMPLICIT_NULLNESS" abbrev="JPA"
+    category="BAD_PRACTICE" />
+
   <Detector class="jp.co.worksap.oss.findbugs.junit.UndocumentedIgnoreDetector"
     speed="fast" hidden="false" />
   <BugPattern type="UNDOCUMENTED_IGNORE" abbrev="JUNIT"
diff --git a/src/main/meta/messages.xml b/src/main/meta/messages.xml
index 0ad4d98..17f040f 100755
--- a/src/main/meta/messages.xml
+++ b/src/main/meta/messages.xml
@@ -150,6 +150,45 @@
     </Details>
   </BugPattern>
 
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitLengthDetector">
+    <Details>
+      The default value of length element (255) might be not enough in some case.
+      It is good to specify the value of length element clear, it tells that you have considered about it.
+    </Details>
+  </Detector>
+
+  <BugPattern type="IMPLICIT_LENGTH">
+    <ShortDescription>Specify length of column, its default (255) might be not enough.
+    </ShortDescription>
+    <LongDescription>
+      Specify length of column, its default (255) might be not enough.
+    </LongDescription>
+    <Details>  
+  <![CDATA[
+    <p>Specify length of column, its default (255) might be not enough.</p>
+  ]]>
+    </Details>
+  </BugPattern>
+
+  <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitNullnessDetector">
+    <Details>
+      It is good to specify the value of nullable element clear, it tells that you have considered about it.
+    </Details>
+  </Detector>
+
+  <BugPattern type="IMPLICIT_NULLNESS">
+    <ShortDescription>It is good to specify the value of nullable element clear, it tells that you have considered about it.
+    </ShortDescription>
+    <LongDescription>
+      It is good to specify the value of nullable element clear, it tells that you have considered about it.
+    </LongDescription>
+    <Details>  
+  <![CDATA[
+    <p>It is good to specify the value of nullable element clear, it tells that you have considered about it.</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.

From 7dbc6e20002247b1695fa6cbc87e8c7886bd6bea Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 13 Mar 2014 18:26:03 +0800
Subject: [PATCH 3/9] refs #14: follow Mr.LI's suggestion.

https://github.com/WorksApplications/findbugs-plugin/pull/14#discussion_r6960545
---
 .../findbugs/jpa/ImplicitLengthDetector.java  | 36 +++++++++++++++++--
 src/main/meta/bugrank.txt                     |  1 +
 src/main/meta/findbugs.xml                    |  2 ++
 src/main/meta/messages.xml                    | 13 +++++++
 4 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
index f04fc36..af9b8c6 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
@@ -9,6 +9,16 @@
 import edu.umd.cs.findbugs.BugReporter;
 
 public class ImplicitLengthDetector extends AbstractColumnDetector {
+    /**
+     * @see http://docs.oracle.com/cd/B28359_01/server.111/b28320/limits001.htm
+     */
+    private static final int MAX_LENGTH_OF_ORACLE_VARCHAR = 4000;
+    /**
+     * @see http://www-01.ibm.com/support/knowledgecenter/SSEPEK_10.0.0/com.ibm.db2z10.doc.intro/src/tpc/db2z_stringdatatypes.htm
+     */
+    private static final int MAX_LENGTH_OF_DB2_VARCHAR = 32704;
+
+    private static final int MAX_LENGTH_OF_VARCHAR = Math.min(MAX_LENGTH_OF_ORACLE_VARCHAR, MAX_LENGTH_OF_DB2_VARCHAR);
 
     public ImplicitLengthDetector(BugReporter bugReporter) {
         super(bugReporter);
@@ -17,20 +27,42 @@ public ImplicitLengthDetector(BugReporter bugReporter) {
     @Override
     protected void verifyColumn(Type columnType,
             Map<String, ElementValue> elements) {
-        if (verifyColumnType(columnType) && !elements.containsKey("length")) {
+        if (! isTarget(columnType)) {
+            return;
+        }
+
+        if (! elements.containsKey("length")) {
             BugInstance bug = new BugInstance(this, "IMPLICIT_LENGTH", HIGH_PRIORITY);
             if (visitingMethod()) {
                 bug.addMethod(this);
             }
             getBugReporter().reportBug(bug);
+        } else {
+            ElementValue value = elements.get("length");
+            int lengthValue = Integer.parseInt(value.stringifyValue());
+
+            if (lengthValue <= 0) {
+                reportIllegalLength(lengthValue);
+            } else if (MAX_LENGTH_OF_VARCHAR < lengthValue) {
+                // TODO: consider that column type can be CLOB, then it might be greater than MAX_LENGTH_OF_VARCHAR
+                reportIllegalLength(lengthValue);
+            }
+        }
+    }
+
+    private void reportIllegalLength(int lengthValue) {
+        BugInstance bug = new BugInstance(this, "ILLEGAL_LENGTH", HIGH_PRIORITY);
+        if (visitingMethod()) {
+            bug.addMethod(this);
         }
+        getBugReporter().reportBug(bug);
     }
 
     /**
      * @param columnType
      * @return true if column type requires length property.
      */
-    private boolean verifyColumnType(Type columnType) {
+    private boolean isTarget(Type columnType) {
         // TODO true if target is annotated with @Lob
         return Type.STRING.equals(columnType);
     }
diff --git a/src/main/meta/bugrank.txt b/src/main/meta/bugrank.txt
index 44e3ada..832ee33 100755
--- a/src/main/meta/bugrank.txt
+++ b/src/main/meta/bugrank.txt
@@ -9,4 +9,5 @@
 0 BugPattern UNDOCUMENTED_IGNORE
 0 BugPattern IMPLICIT_NULLNESS
 0 BugPattern IMPLICIT_LENGTH
+0 BugPattern ILLEGAL_LENGTH
 0 BugPattern USE_COLUMN_DEFINITION
diff --git a/src/main/meta/findbugs.xml b/src/main/meta/findbugs.xml
index 5c89e4b..15afa6d 100755
--- a/src/main/meta/findbugs.xml
+++ b/src/main/meta/findbugs.xml
@@ -41,6 +41,8 @@
     speed="fast" hidden="false" />
   <BugPattern type="IMPLICIT_LENGTH" abbrev="JPA"
     category="BAD_PRACTICE" />
+  <BugPattern type="ILLEGAL_LENGTH" abbrev="JPA"
+    category="CORRECTNESS" />
 
   <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitNullnessDetector"
     speed="fast" hidden="false" />
diff --git a/src/main/meta/messages.xml b/src/main/meta/messages.xml
index cd21110..f7d4bf5 100755
--- a/src/main/meta/messages.xml
+++ b/src/main/meta/messages.xml
@@ -170,6 +170,19 @@
     </Details>
   </BugPattern>
 
+  <BugPattern type="ILLEGAL_LENGTH">
+    <ShortDescription>Specified length of column should be greater than 0, and smaller than 4,000 bytes.
+    </ShortDescription>
+    <LongDescription>
+      Specified length of column ({1}) should be greater than 0, and smaller than 4,000 bytes.
+    </LongDescription>
+    <Details>
+  <![CDATA[
+    <p>Specified length of column should be greater than 0, and smaller than 4,000 bytes.</p>
+  ]]>
+    </Details>
+  </BugPattern>
+
   <Detector class="jp.co.worksap.oss.findbugs.jpa.ImplicitNullnessDetector">
     <Details>
       It is good to specify the value of nullable element clear, it tells that you have considered about it.

From 9cb8f6129aabb3071026934e533a3f1965a9beaa Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 13 Mar 2014 18:38:20 +0800
Subject: [PATCH 4/9] refs #14: consider LOB type to support.

---
 .../findbugs/jpa/AbstractColumnDetector.java  | 36 +++++++++++++++++--
 .../findbugs/jpa/ImplicitLengthDetector.java  |  6 ++--
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
index 2e799d1..13d10a0 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
@@ -1,15 +1,19 @@
 package jp.co.worksap.oss.findbugs.jpa;
 
+import java.util.List;
 import java.util.Map;
 
 import javax.annotation.CheckReturnValue;
 import javax.annotation.Nonnull;
 
+import org.apache.bcel.classfile.AnnotationEntry;
 import org.apache.bcel.classfile.ElementValue;
 import org.apache.bcel.classfile.Field;
+import org.apache.bcel.classfile.FieldOrMethod;
 import org.apache.bcel.generic.Type;
 
 import com.google.common.base.Objects;
+import com.google.common.collect.Lists;
 
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.bcel.AnnotationDetector;
@@ -35,7 +39,35 @@ public final void visitAnnotation(@DottedClassName String annotationClass,
             return;
         }
 
-        Type columnType;
+        Type columnType = findVisitingColumnType();
+        verifyColumn(columnType, map);
+    }
+
+    protected boolean isVisitingLob() {
+        List<FieldOrMethod> targetListToSearch = Lists.newArrayList();
+        if (visitingField()) {
+            targetListToSearch.add(getField());
+        } else if (visitingMethod()) {
+            targetListToSearch.add(getMethod());
+            targetListToSearch.add(findFieldInVisitingMethod());
+        } else {
+            throw new IllegalStateException("@Column should annotate field or method.");
+        }
+
+        for (FieldOrMethod targetToSearch : targetListToSearch) {
+            for (AnnotationEntry annotation : targetToSearch.getAnnotationEntries()) {
+                if (!Objects.equal(annotation.getAnnotationType(), "javax.persistence.Lob")) {
+                    continue;
+                }
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private Type findVisitingColumnType() {
+        final Type columnType;
         if (visitingField()) {
             columnType = getField().getType();
         } else if (visitingMethod()) {
@@ -44,7 +76,7 @@ public final void visitAnnotation(@DottedClassName String annotationClass,
         } else {
             throw new IllegalStateException("@Column should annotate field or method.");
         }
-        verifyColumn(columnType, map);
+        return columnType;
     }
 
     @Nonnull
diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
index af9b8c6..73e2a67 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
@@ -43,8 +43,7 @@ protected void verifyColumn(Type columnType,
 
             if (lengthValue <= 0) {
                 reportIllegalLength(lengthValue);
-            } else if (MAX_LENGTH_OF_VARCHAR < lengthValue) {
-                // TODO: consider that column type can be CLOB, then it might be greater than MAX_LENGTH_OF_VARCHAR
+            } else if (MAX_LENGTH_OF_VARCHAR < lengthValue && !isVisitingLob()) {
                 reportIllegalLength(lengthValue);
             }
         }
@@ -63,8 +62,7 @@ private void reportIllegalLength(int lengthValue) {
      * @return true if column type requires length property.
      */
     private boolean isTarget(Type columnType) {
-        // TODO true if target is annotated with @Lob
-        return Type.STRING.equals(columnType);
+        return Type.STRING.equals(columnType) || isVisitingLob();
     }
 
 }

From cdd7d623caf9d64885d63561a8fdc2b5cb2473db Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 13 Mar 2014 18:39:05 +0800
Subject: [PATCH 5/9] refs #14: consider StringBuffer as target type.

---
 .../jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
index 73e2a67..1fdce49 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
@@ -58,11 +58,10 @@ private void reportIllegalLength(int lengthValue) {
     }
 
     /**
-     * @param columnType
      * @return true if column type requires length property.
      */
     private boolean isTarget(Type columnType) {
-        return Type.STRING.equals(columnType) || isVisitingLob();
+        return Type.STRING.equals(columnType) || Type.STRINGBUFFER.equals(columnType) || isVisitingLob();
     }
 
 }

From d7b2ec60e84231517433e00950851741e5eae7c6 Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 13 Mar 2014 18:49:09 +0800
Subject: [PATCH 6/9] refs #14: fix and update test cases.

---
 .../findbugs/jpa/AbstractColumnDetector.java  |  2 +-
 .../jpa/ColumnWithLongLengthAndLob.java       | 14 +++++++++++
 .../jpa/ColumnWithNegativeLength.java         | 12 +++++++++
 .../findbugs/jpa/ColumnWithTooLongLength.java | 12 +++++++++
 .../oss/findbugs/jpa/ImplicitLengthTest.java  | 25 ++++++++++++++++---
 5 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLongLengthAndLob.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNegativeLength.java
 create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithTooLongLength.java

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
index 13d10a0..01026cd 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/AbstractColumnDetector.java
@@ -56,7 +56,7 @@ protected boolean isVisitingLob() {
 
         for (FieldOrMethod targetToSearch : targetListToSearch) {
             for (AnnotationEntry annotation : targetToSearch.getAnnotationEntries()) {
-                if (!Objects.equal(annotation.getAnnotationType(), "javax.persistence.Lob")) {
+                if (!Objects.equal(annotation.getAnnotationType(), "Ljavax/persistence/Lob;")) {
                     continue;
                 }
                 return true;
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLongLengthAndLob.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLongLengthAndLob.java
new file mode 100644
index 0000000..eba2c91
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithLongLengthAndLob.java
@@ -0,0 +1,14 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+import javax.persistence.Lob;
+
+public class ColumnWithLongLengthAndLob {
+    @Lob
+    @Column(length = 10000)
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNegativeLength.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNegativeLength.java
new file mode 100644
index 0000000..8724691
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithNegativeLength.java
@@ -0,0 +1,12 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+
+public class ColumnWithNegativeLength {
+    @Column(length = -1)
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithTooLongLength.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithTooLongLength.java
new file mode 100644
index 0000000..b598a5e
--- /dev/null
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ColumnWithTooLongLength.java
@@ -0,0 +1,12 @@
+package jp.co.worksap.oss.findbugs.jpa;
+
+import javax.persistence.Column;
+
+public class ColumnWithTooLongLength {
+    @Column(length = 10000)
+    private String name;
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
index 228b61c..8a3b9ef 100644
--- a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthTest.java
@@ -3,6 +3,7 @@
 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;
@@ -20,14 +21,32 @@ public void before() {
     }
 
     @Test
-    public void testImplicitLength() throws Exception {
-        assertNoBugsReported(ColumnWithLength.class, detector,
+    public void testNegativeLength() throws Exception {
+        assertBugReported(ColumnWithNegativeLength.class, detector,
+                bugReporter, ofType("ILLEGAL_LENGTH"));
+    }
+
+    @Test
+    public void testTooLongLength() throws Exception {
+        assertBugReported(ColumnWithTooLongLength.class, detector,
+                bugReporter, ofType("ILLEGAL_LENGTH"));
+    }
+
+    @Test
+    public void testLongLengthWithLob() throws Exception {
+        assertNoBugsReported(ColumnWithLongLengthAndLob.class, detector,
                 bugReporter);
     }
 
     @Test
     public void testExplicitLength() throws Exception {
-        assertBugReported(ColumnWithoutElement.class, detector,
+        assertNoBugsReported(ColumnWithLength.class, detector,
                 bugReporter);
     }
+
+    @Test
+    public void testImplicitLength() throws Exception {
+        assertBugReported(ColumnWithoutElement.class, detector,
+                bugReporter, ofType("IMPLICIT_LENGTH"));
+    }
 }

From 336179fa2404eee8584198e416a07ea084eb19c9 Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 13 Mar 2014 19:07:30 +0800
Subject: [PATCH 7/9] refs #14: @Lob might be annotated for BLOB.

---
 .../jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
index 1fdce49..bf75817 100644
--- a/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
+++ b/src/main/java/jp/co/worksap/oss/findbugs/jpa/ImplicitLengthDetector.java
@@ -61,7 +61,7 @@ private void reportIllegalLength(int lengthValue) {
      * @return true if column type requires length property.
      */
     private boolean isTarget(Type columnType) {
-        return Type.STRING.equals(columnType) || Type.STRINGBUFFER.equals(columnType) || isVisitingLob();
+        return Type.STRING.equals(columnType) || Type.STRINGBUFFER.equals(columnType);
     }
 
 }

From 5e7711896d21118f49fc919894dee12b508498d5 Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 20 Mar 2014 10:40:24 +0800
Subject: [PATCH 8/9] refs #14: correct wrong naming, thanks @wliyongfeng!

---
 .../jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java
index 0888847..1038896 100644
--- a/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java
+++ b/src/test/java/jp/co/worksap/oss/findbugs/jpa/ImplicitNullnessTest.java
@@ -20,13 +20,13 @@ public void before() {
     }
 
     @Test
-    public void testImplicitNullness() throws Exception {
+    public void testExplicitNullness() throws Exception {
         assertNoBugsReported(ColumnWithNullable.class, detector,
                 bugReporter);
     }
 
     @Test
-    public void testExplicitNullness() throws Exception {
+    public void testImplicitNullness() throws Exception {
         assertBugReported(ColumnWithoutElement.class, detector,
                 bugReporter);
     }

From a193db5fd8b2e4141d3ec4d5f741baea2d7ef092 Mon Sep 17 00:00:00 2001
From: Kengo TODA <toda_k@worksap.co.jp>
Date: Thu, 20 Mar 2014 13:05:46 +0800
Subject: [PATCH 9/9] refs #14: explain why user should care about too long
 varchar column.

---
 src/main/meta/messages.xml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/main/meta/messages.xml b/src/main/meta/messages.xml
index f7d4bf5..ac8fcaf 100755
--- a/src/main/meta/messages.xml
+++ b/src/main/meta/messages.xml
@@ -179,6 +179,8 @@
     <Details>
   <![CDATA[
     <p>Specified length of column should be greater than 0, and smaller than 4,000 bytes.</p>
+    <p>Oracle&#39;s maximum varchar size is 4000 bytes, and DB2&#39;s is 32704 bytes.
+    To keep portability of your product, it is better to shorten length to 4000 bytes, or to use @Lob annotation to use CLOB.</p>
   ]]>
     </Details>
   </BugPattern>