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()+"] " + "\n\tMax 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); }
