Sonarqube代码质量检测笔记

1：Jump statements should not occur in "finally" blocks：

不允许在finally里面做return, break, throw等操作，当正常流程抛出异常的时候，紧跟着的finally里面的colse也可能抛出异常，

最终抛出的异常时finally抛出的异常，不符合预期。

错误代码：
try {
     if (scaledBitmap != null && !scaledBitmap.isRecycled()) {
         scaledBitmap.recycle();
         scaledBitmap = null;
         System.gc();
     }

     compressedBytes = JBigCompress.compress(os.toByteArray());
 } finally {
     try {
         os.close();
     } catch (IOException e) {
         // TODO Auto-generated catch block
         //Timber.e(e);
         throw new RuntimeException(e);
     }
 }


解析

Using return, break, throw, and so on from a finally block suppresses the propagation of any unhandled Throwable which was thrown in the try or catch block.

This rule raises an issue when a jump statement (break, continue, return, throw, and goto) would force control flow to leave a finally block.

Noncompliant Code Example

public static void main(String[] args) {
  try {
    doSomethingWhichThrowsException();
    System.out.println("OK");   // incorrect "OK" message is printed
  } catch (RuntimeException e) {
    System.out.println("ERROR");  // this message is not shown
  }
}

public static void doSomethingWhichThrowsException() {
  try {
    throw new RuntimeException();
  } finally {
    for (int i = 0; i < 10; i ++) {
      //...
      if (q == i) {
        break; // ignored
      }
    }

    /* ... */
    return;      // Noncompliant - prevents the RuntimeException from being propagated
  }
}

Compliant Solution

public static void main(String[] args) {
  try {
    doSomethingWhichThrowsException();
    System.out.println("OK");
  } catch (RuntimeException e) {
    System.out.println("ERROR");  // "ERROR" is printed as expected
  }
}

public static void doSomethingWhichThrowsException() {
  try {
    throw new RuntimeException();
  } finally {
    for (int i = 0; i < 10; i ++) {
      //...
      if (q == i) {
        break; // ignored
      }
    }

    /* ... */
  }
}

2：Resources should be closed

关闭资源需要在finally里面操作，因为如果没有使用try finally，前面的操作如果抛出异常，会直接结束流程，不会走到close。
把close放在finally里面不管是正常走完流程还是异常，都会走到finally里面去关闭资源。
Java7之后有更简单的方法，使用try-with_resource,无需繁琐的在finally里面操作
参考：http://ifeve.com/java-7%E4%B8%AD%E7%9A%84try-with-resources/

Connections, streams, files, and other classes that implement the Closeable interface or its super-interface, AutoCloseable, needs to be closed after use. Further, that close call must be made in a finally block otherwise an exception could keep the call from being made. Preferably, when class implements AutoCloseable, resource should be created using "try-with-resources" pattern and will be closed automatically.

Failure to properly close resources will result in a resource leak which could bring first the application and then perhaps the box it's on to their knees.
Noncompliant Code Example

private void readTheFile() throws IOException {
  Path path = Paths.get(this.fileName);
  BufferedReader reader = Files.newBufferedReader(path, this.charset);
  // ...
  reader.close();  // Noncompliant
  // ...
  Files.lines("input.txt").forEach(System.out::println); // Noncompliant: The stream needs to be closed
}

private void doSomething() {
  OutputStream stream = null;
  try {
    for (String property : propertyList) {
      stream = new FileOutputStream("myfile.txt");  // Noncompliant
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();  // Multiple streams were opened. Only the last is closed.
  }
}

Compliant Solution

private void readTheFile(String fileName) throws IOException {
    Path path = Paths.get(fileName);
    try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
      reader.readLine();
      // ...
    }
    // ..
    try (Stream<String> input = Files.lines("input.txt"))  {
      input.forEach(System.out::println);
    }
}

private void doSomething() {
  OutputStream stream = null;
  try {
    stream = new FileOutputStream("myfile.txt");
    for (String property : propertyList) {
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();
  }
}

Exceptions

Instances of the following classes are ignored by this rule because close has no effect:

    java.io.ByteArrayOutputStream
    java.io.ByteArrayInputStream
    java.io.CharArrayReader
    java.io.CharArrayWriter
    java.io.StringReader
    java.io.StringWriter

Java 7 introduced the try-with-resources statement, which implicitly closes Closeables. All resources opened in a try-with-resources statement are ignored by this rule.

try (BufferedReader br = new BufferedReader(new FileReader(fileName))) {
  //...
}
catch ( ... ) {
  //...
}

3：null pointers should not be dereferenced

错误代码：

 String cardholderName = cardRecordConfirmInfo.getCardholderName();

解析：
A reference to null should never be dereferenced/accessed. Doing so will cause a NullPointerException to be thrown. At best, such an exception will cause abrupt program termination. At worst, it could expose debugging information that would be useful to an attacker, or it could allow an attacker to bypass security measures.

Note that when they are present, this rule takes advantage of @CheckForNull and @Nonnull annotations defined in JSR-305 to understand which values are and are not nullable except when @Nonnull is used on the parameter to equals, which by contract should always work with null.
Noncompliant Code Example

@CheckForNull
String getName(){...}

public boolean isNameEmpty() {
  return getName().length() == 0; // Noncompliant; the result of getName() could be null, but isn't null-checked
}

Connection conn = null;
Statement stmt = null;
try{
  conn = DriverManager.getConnection(DB_URL,USER,PASS);
  stmt = conn.createStatement();
  // ...

}catch(Exception e){
  e.printStackTrace();
}finally{
  stmt.close();   // Noncompliant; stmt could be null if an exception was thrown in the try{} block
  conn.close();  // Noncompliant; conn could be null if an exception was thrown
}

private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor){...}

public  void append(@CheckForNull Color color) {
    merge(currentColor, color);  // Noncompliant; color should be null-checked because merge(...) doesn't accept nullable parameters
}

void paint(Color color) {
  if(color == null) {
    System.out.println("Unable to apply color " + color.toString());  // Noncompliant; NullPointerException will be thrown
    return;
  }
  ...
}

4："toString()" and "clone()" methods should not return null

错误代码：
 @Override
    public String toString() {
        if (TextUtils.isEmpty(tag) || (nullable && TextUtils.isEmpty(value))) {
            return null;---》改为“”
        }

        StringBuilder tlvData = new StringBuilder();
        tlvData.append(FieldTypeUtil.extend(tag + len));

        if (isValueNeedExtend() && !TextUtils.isEmpty(value)) {
            tlvData.append(FieldTypeUtil.extend(value));
        } else {
            tlvData.append(value);
        }

        return tlvData.toString();
    }

解析：

Calling toString() or clone() on an object should always return a string or an object. Returning null instead contravenes the method's implicit contract.

Noncompliant Code Example

public String toString () {
  if (this.collection.isEmpty()) {
    return null; // Noncompliant
  } else {
    // ...

Compliant Solution

public String toString () {
  if (this.collection.isEmpty()) {
    return "";
  } else {
    // ...

 5：Math operands should be cast before assignment

错误：
canvas.scale(scaleX, scaleY, qrCodeWidth / 2, qrCodeHeight / 2);

public final void scale(float sx, float sy, float px, float py) {
    if (sx == 1.0f && sy == 1.0f) return;
    translate(px, py);
    scale(sx, sy);
    translate(-px, -py);
}

改为：

canvas.scale(scaleX, scaleY, qrCodeWidth / 2f, qrCodeHeight / 2f);

解析：
For small numbers, float math has enough precision to yield the expected value, but for larger numbers, it does not. BigDecimal is the best alternative, but if a primitive is required, use a double.

Noncompliant Code Example

float a = 16777216.0f;
float b = 1.0f;
float c = a + b; // Noncompliant; yields 1.6777216E7 not 1.6777217E7

double d = a + b; // Noncompliant; addition is still between 2 floats

Compliant Solution

float a = 16777216.0f;
float b = 1.0f;
BigDecimal c = BigDecimal.valueOf(a).add(BigDecimal.valueOf(b));

double d = (double)a + (double)b;

Exceptions

This rule doesn't raise an issue when the mathematical expression is only used to build a string.

System.out.println("["+getName()+"] " +
           "
	Max time to retrieve connection:"+(max/1000f/1000f)+" ms.");

6：Double Brace Initialization should not be used

Because Double Brace Initialization (DBI) creates an anonymous class with a reference to the instance of the owning object, its use can lead to memory leaks if the anonymous inner class is returned and held by other objects. Even when there's no leak, DBI is so obscure that it's bound to confuse most maintainers.

For collections, use Arrays.asList instead, or explicitly add each item directly to the collection.
Noncompliant Code Example

Map source = new HashMap(){{ // Noncompliant
    put("firstName", "John");
    put("lastName", "Smith");
}};

Compliant Solution

Map source = new HashMap();
// ...
source.put("firstName", "John");
source.put("lastName", "Smith");
// ...

7："equals(Object obj)" and "hashCode()" should be overridden in pairs

问题：一个类复写了equals但没有复写hashCode

可以添加一个最简单的hashCode方法　　

public int hashCode() {return 0;}　

According to the Java Language Specification, there is a contract between equals(Object) and hashCode():

    If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

    It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results.

    However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.

In order to comply with this contract, those methods should be either both inherited, or both overridden.
Noncompliant Code Example

class MyClass {    // Noncompliant - should also override "hashCode()"

  @Override
  public boolean equals(Object obj) {
    /* ... */
  }

}

Compliant Solution

class MyClass {    // Compliant

  @Override
  public boolean equals(Object obj) {
    /* ... */
  }

  @Override
  public int hashCode() {
    /* ... */
  }

}

8：Child class methods named for parent class methods should be overrides

When a method in a child class has the same signature as a method in a parent class, it is assumed to be an override. However, that's not the case when:

    the parent class method is static and the child class method is not.
    the arguments or return types of the child method are in different packages than those of the parent method.
    the parent class method is private.

Typically, these things are done unintentionally; the private parent class method is overlooked, the static keyword in the parent declaration is overlooked, or the wrong class is imported in the child. But if the intent is truly for the child class method to be different, then the method should be renamed to prevent confusion.
Noncompliant Code Example

// Parent.java
import computer.Pear;
public class Parent {

  public void doSomething(Pear p) {
    //,,,
  }

  public static void doSomethingElse() {
    //...
  }
}

// Child.java
import fruit.Pear;
public class Child extends Parent {

  public void doSomething(Pear p) {  // Noncompliant; this is not an override
    // ...
  }


  public void doSomethingElse() {  // Noncompliant; parent method is static
    //...
  }
}

Compliant Solution

// Parent.java
import computer.Pear;
public class Parent {

  public void doSomething(Pear p) {
    //,,,
  }

  public static void doSomethingElse() {
    //...
  }
}

// Child.java
import computer.Pear;  // import corrected
public class Child extends Parent {

  public void doSomething(Pear p) {  // true override (see import)
    //,,,
  }

  public static void doSomethingElse() {
    //...
  }
}

9：Throwable.printStackTrace(...) should not be called

不要使用printStackTrace来打印日志，参考：

https://baijiahao.baidu.com/s?id=1639733404613101605&wfr=spider&for=pc

Throwable.printStackTrace(...) prints a Throwable and its stack trace to some stream. By default that stream System.Err, which could inadvertently expose sensitive information.

Loggers should be used instead to print Throwables, as they have many advantages:

    Users are able to easily retrieve the logs.
    The format of log messages is uniform and allow users to browse the logs easily.

This rule raises an issue when printStackTrace is used without arguments, i.e. when the stack trace is printed to the default stream.
Noncompliant Code Example

try {
  /* ... */
} catch(Exception e) {
  e.printStackTrace();        // Noncompliant
}

Compliant Solution

try {
  /* ... */
} catch(Exception e) {
  LOGGER.log("context", e);
}