New Patterns

[Xiaoven]

New Patterns

[Xiaoven]

Co: Covariant compareTo() method defined (CO_SELF_NO_OBJECT)

Description

This class defines a covariant version of compareTo(). To correctly override the compareTo() method in the Comparable interface, the parameter of compareTo() must have type java.lang.Object.

SP Implementation

link

• Condition 1: !hasCompareToObject && !hasCompareToBridgeMethod && hasCompareToSelf
  • hasCompareToObject = true if "compareTo".equals(name) && sig.endsWith(")I") && !obj.isStatic() and "(Ljava/lang/Object;)I".equals(sig)
  • hasCompareToSelf = true if "compareTo".equals(name) && sig.endsWith(")I") && !obj.isStatic() and sig.equals("(L" + getClassName() + ";)I")
  • hasCompareToBridgeMethod = true if "compareTo".equals(name) && sig.endsWith(")I") && !obj.isStatic() and BCELUtil.isSynthetic(obj)
• Condition 2 : !extendsObject

Java synthetic methods

Any constructs introduced by the compiler that do not have a corresponding construct in the source code must be marked as synthetic, except for default constructors and the class initialization method.

Member.isSynthetic() returns true if and only if this member was introduced by the compiler.

The Java compiler must create synthetic methods on nested classes when their attributes specified with the private modifier are accessed by the enclosing class.

当存在内嵌class定义，而且需要在外包class和内嵌class之间访问对方的private修饰的属性的时候，java编译器必须创建synthetic method.
(请记住:对于java编译器而言，内部类也会被单独编译成一个class文件。那么原有代码中的相关属性可见性就难以维持，synthetic method也就是为了这个目的而生成的。生成的synthetic方法是包访问性的static方法.)

加上 online/local search 可进一步判断是不是 abstract class，进一步区分出 Co: Abstract class defines covariant compareTo() method

[Xiaoven]

RV: Negating the result of compareTo()/compare() (RV_NEGATING_RESULT_OF_COMPARETO)

This code negatives the return value of a compareTo or compare method. This is a questionable or bad programming practice, since if the return value is Integer.MIN_VALUE, negating the return value won't negate the sign of the result. You can achieve the same intended result by reversing the order of the operands rather than by negating the results.

检查没有中间变量那种

[Xiaoven]

ME: Enum field is public and mutable (ME_MUTABLE_ENUM_FIELD)

A mutable public field is defined inside a public enum, thus can be changed by malicious code or by accident from another package. Though mutable enum fields may be used for lazy initialization, it's a bad practice to expose them to the outer world. Consider declaring this field final and/or package-private.

SP Implementation

link

• classContext.getJavaClass().isEnum() && classContext.getJavaClass().isPublic()
• !field.isStatic() && !field.isFinal() && !field.isSynthetic() and field.isPublic()

Online/local search 判断 class 是否为 public enum，根据 public 关键词可以判断 是否为 field

例子

public enum Element {
    H("Hydrogen"),
    HE("Helium"),
    // ...
    NE("Neon");

    public String label;

    private Element(String label) {
        this.label = label;
    }
}

[Xiaoven]

Dm: Useless/vacuous call to EasyMock method (DMI_VACUOUS_CALL_TO_EASYMOCK_METHOD)

SP Implementation

https://github.com/spotbugs/spotbugs/blob/d9557689c2a752a833eedf4eb5f37ee712a9c84f/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DumbMethods.java#L850

Easymock API

检查的是特定的 static method invocation

EasyMock.verify();
EasyMock.replay();
EasyMock.reset();

[Xiaoven]

Dm: Futile attempt to change max pool size of ScheduledThreadPoolExecutor

https://github.com/spotbugs/spotbugs/blob/d9557689c2a752a833eedf4eb5f37ee712a9c84f/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DumbMethods.java#L209

检查是否调用 java/util/concurrent/ScheduledThreadPoolExecutor 对象的 setMaximumPoolSize 方法。方法名比较具体，是否可以实现一下？

[Xiaoven]

DMI: BigDecimal constructed from double that isn’t represented precisely (DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE)

https://github.com/spotbugs/spotbugs/blob/d9557689c2a752a833eedf4eb5f37ee712a9c84f/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DumbMethods.java#L1399

检查像 new BigDecimal(0.1) 这样的

[Xiaoven]

RV: Bad attempt to compute absolute value of signed random integer (RV_ABSOLUTE_VALUE_OF_RANDOM_INT)

This code generates a random signed integer and then computes the absolute value of that random integer. If the number returned by the random number generator is Integer.MIN_VALUE, then the result will be negative as well (since Math.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE). (Same problem arises for long values as well).

Spotbugs Implementation

https://github.com/spotbugs/spotbugs/blob/d9557689c2a752a833eedf4eb5f37ee712a9c84f/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DumbMethods.java#L904

Examples

int x = Math.abs(r.nextInt());

FP, spotbugs 有小小的data flow analysis. 我们可以用 local search 和 global search 看看有没有出现 "Integer.MIN_VALUE", 出现就不报了

    @DesireNoWarning("RV_ABSOLUTE_VALUE_OF_RANDOM_INT")
    public int testRandomInt() {
        int x = Math.abs(r.nextInt());
        if (x == Integer.MIN_VALUE)
            return 0;
        return x;
    }

RV: Bad attempt to compute absolute value of signed 32-bit hashcode (RV_ABSOLUTE_VALUE_OF_HASHCODE)

类似上一个

return Math.abs(key.hashCode());

[Xiaoven]

Nm: Apparent method/constructor confusion (NM_METHOD_CONSTRUCTOR_CONFUSION)

Spotbugs Implementation

https://github.com/spotbugs/spotbugs/blob/a6f9acb2932b54f5b70ea8bc206afb552321a222/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/Naming.java#L181

检查 method name 是否和 file name 相同

[Xiaoven]

RV: Method ignores return value (RV_RETURN_VALUE_IGNORED)

The return value of this method should be checked. One common cause of this warning is to invoke a method on an immutable object, thinking that it updates the object. For example, in the following code fragment

Spotbugs Implementation

• CheckReturnValueAnnotation 定义的 CHECK_RETURN_VALUE_UNKNOWN, CHECK_RETURN_VALUE_HIGH, CHECK_RETURN_VALUE_MEDIUM, CHECK_RETURN_VALUE_LOW, CHECK_RETURN_VALUE_IGNORE 都跟这个 pattern 有关

• CheckReturnAnnotationDatabase 中，根据前面的 annotation 可以找到一堆要检查的 methods， 可以选取一些通过名字不容易认错的方法实现

• MethodReturnCheck 报warning的条件, 不太明白为什么前面为什么要出现 Const.NEW。建议多试试前面的 method lists，看看什么情况下会报。

[Xiaoven]

RV: Code checks for specific values returned by compareTo (RV_CHECK_COMPARETO_FOR_SPECIFIC_RETURN_VALUE)

This code invoked a compareTo or compare method, and checks to see if the return value is a specific value, such as 1 or -1. When invoking these methods, you should only check the sign of the result, not for any specific non-zero value. While many or most compareTo and compare methods only return -1, 0 or 1, some of them will return other values.

[Xiaoven]

DMI: Reversed method arguments (DMI_ARGUMENTS_WRONG_ORDER)

The arguments to this method call seem to be in the wrong order. For example, a call Preconditions.checkNotNull("message", message) has reserved arguments: the value to be checked is the first argument.

Spotbugs Implementation

link 只检查特定的 methods，看第二个参数是否是 string constant ？

[Xiaoven]

IJU: TestCase declares a bad suite method (IJU_BAD_SUITE_METHOD)

检查方法定义是否是以下之一

public static junit.framework.Test suite()

public static junit.framework.TestSuite suite()

如果用 online search 是否可以看 import junit.framework.Test和 import junit.framework.TestCase 信息？

IJU: TestCase implements a non-static suite method (IJU_SUITE_NOT_STATIC)

spotbugs
spotbugs 只看了是否是 TestCase 的子类，方法名和static

[Xiaoven]

BOA: Class overrides a method implemented in super class Adapter wrongly (BOA_BADLY_OVERRIDDEN_ADAPTER)

SQL: Method attempts to access a result set field with index 0 (SQL_BAD_RESULTSET_ACCESS)

检查特定 methods call 的第一个参数是不是 0，难点在于如何确认 object 是 java/sql/ResultSet 和 java/sql/PreparedStatement 类型的

spotbugs link

[Xiaoven]

STI: Unneeded use of currentThread() call, to call interrupted() (STI_INTERRUPTED_ON_CURRENTTHREAD)

SpotBugs 有记录在看到 Thread.interrupted() call 之前是否看到了 thread object

可以检查 Thread.currentThread().interrupted(); 的用法，interrupted() 是一个 static 方法，不需要通过 Thread object 调用

STI: Static Thread.interrupted() method invoked on thread instance (STI_INTERRUPTED_ON_UNKNOWNTHREAD)

除了 currentThread 外的 thread，如果能够确定 object 类型就好了

[Xiaoven]

IM: Integer multiply of result of integer remainder (IM_MULTIPLYING_RESULT_OF_IREM)

The code multiplies the result of an integer remaining by an integer constant. Be sure you don't have your operator precedence confused. For example i % 60 * 1000 is (i % 60) * 1000, not i % (60 * 1000).

spotbugs 连它的官方例子都不报了，感觉不是很重要的样子，也有可能是默认关掉了

[Xiaoven]

USELESS_STRING: Invocation of toString on an unnamed array (DMI_INVOKING_TOSTRING_ON_ANONYMOUS_ARRAY)

可以检测到到类型

System.out.println(new String[] { "one", "two" }.toString());
System.out.println((new String[] { "one", "two" }).toString());
System.out.println(new String[10].toString());

检测不了到类型

    String[] getArray(){
        return new String[] { "one", "two" };
    }

    void m(){
        System.out.println(getArray().toString());
    }

[Xiaoven]

DMI: D’oh! A nonsensical method invocation (DMI_DOH)

This partical method invocation doesn't make sense, for reasons that should be apparent from inspection.

Strings.isNullOrEmpty("uid")
Strings.emptyToNull("uid")
Strings.nullToEmpty("uid")

Assert.assertNotNull(x, "x must be nonnull");
Assert.assertNotNull("x must be nonnull");
Assert.assertNotNull("x must be nonnull", x);

Preconditions.checkNotNull("x must be nonnull", x);
Preconditions.checkNotNull("x must be nonnull");
Preconditions.checkNotNull(x, "x must be nonnull");

SP 实现

https://github.com/spotbugs/spotbugs/search?l=Java&q=DMI_DOH

// equals  方法
if (calledMethodAnnotation != null && calledMethodAnnotation.isStatic()) {

if (seen == Const.INVOKESTATIC && ("com/google/common/base/Preconditions".equals(getClassConstantOperand())
                    && "checkNotNull".equals(getNameConstantOperand())
                    || "com/google/common/base/Strings".equals(getClassConstantOperand())
                            && ("nullToEmpty".equals(getNameConstantOperand()) ||
                                    "emptyToNull".equals(getNameConstantOperand()) ||
                                    "isNullOrEmpty".equals(getNameConstantOperand())))) {

if (seen == Const.INVOKESTATIC && ("junit/framework/Assert".equals(getClassConstantOperand()) || "org/junit/Assert".equals(
                    getClassConstantOperand()))
                    && "assertNotNull".equals(getNameConstantOperand())) {

[Xiaoven]

Dm: Consider using Locale parameterized version of invoked method (DM_CONVERT_CASE)

建议使用带 locale 参数的 String.toUpperCase( Locale l ) 和 String.toLowerCase( Locale l ) 方法

Spotbugs Implementations

if ((seen == Const.INVOKEVIRTUAL) && "java/lang/String".equals(getClassConstantOperand())
                    && ("toUpperCase".equals(getNameConstantOperand()) || "toLowerCase".equals(getNameConstantOperand()))
                    && "()Ljava/lang/String;".equals(getSigConstantOperand())) {

[Xiaoven]

VO: A volatile reference to an array doesn’t treat the array elements as volatile (VO_VOLATILE_REFERENCE_TO_ARRAY)

建议试一下例子，看看它还会不会报

• example 1
• example 2

找到一个会报的

    volatile String[] array = new String[10];  // private volatile String[] array = { "a", "b", "c", "d", "e" }; 也会

    public void m() {

        for (int i = 0; i < array.length; i++) {

//            synchronized (array[i]) {
//
//                array[i] = array[i] + "*";
//
//                System.out.println(array[i]);
//
//            }

            array[i] = array[i] + "*";

            System.out.println(array[i]);

        }

    }

[Xiaoven]

Dm: A thread was created using the default empty run method (DM_USELESS_THREAD)

Spotbugs 识别 java/lang/Thread 的 3 种 constructor：Thread(), Thread(Runnable target, String name), Thread(String name).

我们至少可以识别 new Thread() and new Thread("name")

Thread 的所有 constructors

[Xiaoven]

不建议实现，因为一般是 synchronized 一个 static field，直接 synchronized 一个带双引号的string或者boolean值的情况几乎不会出现，只是 spotbugs 也会报而已

DL: Synchronization on interned String (DL_SYNCHRONIZATION_ON_SHARED_CONSTANT)

    private static String LOCK = "LOCK";

    void m1() {
        synchronized (LOCK) {
            System.out.println(LOCK);
        }
    }

    void m3(){
        synchronized ("Lock") {
            System.out.println("Lock");
        }
    }

DL: Synchronization on Boolean (DL_SYNCHRONIZATION_ON_BOOLEAN)

void m2(){
        synchronized(inited) {
            System.out.println(inited);
        }
    }

DL: Synchronization on boxed primitive values (DL_SYNCHRONIZATION_ON_UNSHARED_BOXED_PRIMITIVE)

    void m4(){
        synchronized(Boolean.FALSE) {
            System.out.println("Lock");
        }
    }

[Xiaoven]

WL: Synchronization on getClass rather than class literal (WL_USING_GETCLASS_RATHER_THAN_CLASS_LITERAL)

据观察，只检查 synchronized (getClass()) 是不够的，只有当 synchronized block 里有用到 static field 的时候，spotbugs 才会报，这一点 spotbgus 的代码没有很明显的表示出来。

但是我们可以假设 synchronized block 里总是会需要 read/write 一些 shared 的 fields，否则不需要 synchronized 也能直接 read/write 。

    private static final String base = "label";
    private static int nameCounter = 0;

    String constructComponentName() {
        synchronized (getClass()) {
            return base + nameCounter++;
        }
    }

    // 只有它没有报
    int m(){
        synchronized (getClass()){
            return 0;
        }
    }

    void m1(){
        synchronized (getClass()){
            nameCounter++;
        }
    }

ESync: Empty synchronized block (ESync_EMPTY_SYNC)

重点在于 {}

synchronized( anyExpression ) {
}

[Xiaoven]

No: Using notify() rather than notifyAll() (NO_NOTIFY_NOT_NOTIFYALL)

void notify() 是定义在 Object class 里的，所以不需要检查 type 信息， 只要检查method名字就可以

spotbugs implementation

[Xiaoven]

• Dm: Explicit garbage collection; extremely dubious except in benchmarking code (DM_GC)
  需要检查是不是在 main 函数里，或者 method name 是否包含 test。如果实现的话，这部分 FP 估计难以消除

• Dm: Method invokes inefficient Boolean constructor; use Boolean.valueOf(…) instead (DM_BOOLEAN_CTOR)
  需要检查 java version

• Bx: Method invokes inefficient Number constructor; use static valueOf instead (DM_NUMBER_CTOR)

• Bx: Method invokes inefficient floating-point Number constructor; use static valueOf instead (DM_FP_NUMBER_CTOR)

• Bx: Method allocates a boxed primitive just to call toString (DM_BOXED_PRIMITIVE_TOSTRING)

• Bx: Boxing/unboxing to parse a primitive (DM_BOXED_PRIMITIVE_FOR_PARSING)
  检查类似于 Integer.valueOf("1").intValue() 的调用，不如使用类似于 Integer.parseInt("1") 高效

• Bx: Boxing a primitive to compare (DM_BOXED_PRIMITIVE_FOR_COMPARE)
  需要 java version 信息，检查类似 ((Integer)a).compareTo(b) 的调用

• Bx: Primitive value is boxed then unboxed to perform primitive coercion (BX_BOXING_IMMEDIATELY_UNBOXED_TO_PERFORM_COERCION)
  new Double(3.14).intValue(), 直接使用 (int) 3.14 更高效

• Dm: Method allocates an object, only to get the class object (DM_NEW_FOR_GETCLASS)
  检查 (new XXX(...)).getClass() 的用法？

• ITA: Method uses toArray() with zero-length array argument (ITA_INEFFICIENT_TO_ARRAY)
  检查 toArray() 方法，建议用更高效的 myCollection.toArray(new Foo[myCollection.size()]) 代替

• UM: Method calls static Math class method on a constant value (UM_UNNECESSARY_MATH)

• Dm: Hardcoded constant database password (DMI_CONSTANT_DB_PASSWORD)
  检查 java.sql.DriverManager 的 static Connection getConnection(String url, String user, String password) 中，password 是否为 String constant

• Dm: Empty database password (DMI_EMPTY_DB_PASSWORD)

• UCF: Useless control flow to next line (UCF_USELESS_CONTROL_FLOW_NEXT_LINE)
  比如 if (argv.length == 1);

• DMI: Code contains a hard coded reference to an absolute pathname (DMI_HARDCODED_ABSOLUTE_FILENAME)
  比如 new File("/home/dannyc/workspace/j2ee/src/share/com/sun/enterprise/deployment");. Spotbugs 定义了一系列方法，检查 string constant 的格式

• DMI: Invocation of substring(0), which returns the original value (DMI_USELESS_SUBSTRING)

• XFB: Method directly allocates a specific implementation of xml interfaces (XFB_XML_FACTORY_BYPASS)
  检查是否直接 new 了提前定义好的 classes

• RpC: Repeated conditional tests (RpC_REPEATED_CONDITIONAL_TEST)

[Xiaoven]

Local variable 和 field 版本可以合为一个 pattern 的

• CAA: Covariant array assignment to a field (CAA_COVARIANT_ARRAY_FIELD) 和 local 版本合为一个
  检查形如 Number[] arr = new Integer[10]; 的语句，看前后的 array type 是否相同
• SA: Double assignment of local variable (SA_LOCAL_DOUBLE_ASSIGNMENT) 和 SA_FIELD_DOUBLE_ASSIGNMENT
• Nm: Use of identifier that is a keyword in later versions of Java (NM_FUTURE_KEYWORD_USED_AS_IDENTIFIER) 和 member identify 版本

[Xiaoven]

Check signature/definition

• Se: Private readResolve method not inherited by subclasses (SE_PRIVATE_READ_RESOLVE_NOT_INHERITED)
  还需要检查是否是 Serializable 的 subtype
• Se: Transient field of class that isn’t Serializable. (SE_TRANSIENT_FIELD_OF_NONSERIALIZABLE_CLASS)
  需 inheritance 信息

[Xiaoven]

Chain calls

• UC: Useless object created on stack (UC_USELESS_OBJECT_STACK)
  没有中间变量，如new Point().x = 2;， 详见 spotbugs 官方例子

[Xiaoven]

根据Constant 和运算符识别

需要 type 信息过滤 FP

• INT: Integer remainder modulo 1 (INT_BAD_REM_BY_1)
  形如 exp % 1, 但若 exp 的类型是 long, float, double 则为 FP
• Vacuous comparison of integer value (INT_VACUOUS_COMPARISON)
  形如 x <= Integer.MAX_VALUE, 但若 x 类型是 long, float, double 则为 FP

不需要额外的 type 信息

• INT: Vacuous bit mask operation on integer value (INT_VACUOUS_BIT_OPERATION)
  检查 v & 0xffffffff, v|0. 而运算符&, |, ^ 在 Const 里只有 int 和 long 型，spotbugs 全都检查
• FE: Test for floating point equality (FE_FLOATING_POINT_EQUALITY)
  num > 3.14

需要non-null/nonnegative分析

• IM: Check for oddness that won’t work for negative numbers (IM_BAD_CHECK_FOR_ODD)
  检查 x % 2 == 1, 但如果 x = collection.size(), x = array.length, x = Math.abs(...) 就不改报

[Xiaoven]

• Dm: Can’t use reflection to check for presence of annotation without runtime retention (DMI_ANNOTATION_IS_NOT_VISIBLE_TO_REFLECTION)
  检查 classObject.isAnnotationPresentation(annotationClass) 的调用