SonarLint实践总结

1、集合要用isEmpty()判空。

Minor code smell

Use isEmpty() to check whether the collection is empty or not.

问题代码：

Rule：

Using Collection.size() to test for emptiness works, but using Collection.isEmpty() makes the code more readable and can be more performant.

The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n).
Noncompliant Code Example
  if (myCollection.size() == 0) { // Noncompliant    /* ... */  }  
Compliant Solution
  if (myCollection.isEmpty()) {    /* ... */  }

2、重复的字符串要定义常量。

Critical code smell

Define a constant instead of duplicating this literal "total" 3 times.

rule:

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences. On the other hand, constants can be referenced from many places, but only need to be updated in a single place. Noncompliant Code Example With the default threshold of 3:   public void run() {        prepare("action1"); // Noncompliant - "action1" is duplicated 3 times    execute("action1");        release("action1");   }      @SuppressWarning("all") // Compliant - annotations are excluded  private void method1() { /* ... */ }   @SuppressWarning("all")   private void method2() { /* ... */ }     public String method3(String a) {       System.out.println("'" + a + "'"); // Compliant - literal "'" has less than 5 characters and is excluded       return ""; // Compliant - literal "" has less than 5 characters and is excluded  }   Compliant Solution   private static final String ACTION_1 = "action1"; // Compliant    public void run() {       prepare(ACTION_1); // Compliant    execute(ACTION_1);   release(ACTION_1);     }   Exceptions To prevent generating some false-positives, literals having less than 5 characters are excluded.

3、方法不要返回null

Major code smell

Return an empty collection instead of null.

问题代码：

rule:

Returning null instead of an actual array or collection forces callers of the method to explicitly test for nullity, making them more complex and less readable. Moreover, in many cases, null is used as a synonym for empty. Noncompliant Code Example   public static List<Result> getResults() {     return null; // Noncompliant   }     public static Result[] getResults() {     return null; // Noncompliant   }     public static void main(String[] args) {     Result[] results = getResults();       if (results != null) { // Nullity test required to prevent NPE       for (Result result: results) {        /* ... */      }     }   }     Compliant Solution   public static List<Result> getResults() {     return Collections.emptyList(); // Compliant   }     public static Result[] getResults() {     return new Result[0];   }     public static void main(String[] args) {     for (Result result: getResults()) {      /* ... */    }   }   See CERT, MSC19-C. - For functions that return an array, prefer returning an empty array over a null value CERT, MET55-J. - Return an empty array or collection instead of a null value for methods that return an array or collection

4、父类的静态成员不应该使用子类访问。

Critial code smell

Use static access with "com.alibaba.fastjson.JSON" for "parseObject".

问题代码：

修改方案：

rule:

"static" base class members should not be accessed via derived types   Code smell   Critical squid:S3252   In the interest of code clarity, static members of a base class should never be accessed using a derived type's name. Doing so is confusing and could create the illusion that two different static members exist. Noncompliant Code Example   class Parent {     public static int counter;   }     class Child extends Parent {     public Child() {       Child.counter++; // Noncompliant     }   }   Compliant Solution   class Parent {     public static int counter;   }     class Child extends Parent {     public Child() {       Parent.counter++;     }   }

5、Boolean包装类应该避免在表达式中使用。

Minor code smell

Use the primitive boolean expression here.

问题代码：

修改方案：

rule:

Boxed "Boolean" should be avoided in boolean expressions   Code smell   Minor squid:S5411   When boxed type java.lang.Boolean is used as an expression it will throw NullPointerException if the value is null as defined in Java Language Specification §5.1.8 Unboxing Conversion. It is safer to avoid such conversion altogether and handle the null value explicitly. Noncompliant Code Example   Boolean b = getBoolean();   if (b) { // Noncompliant, it will throw NPE when b == null     foo();   } else {     bar();   }   Compliant Solution   Boolean b = getBoolean();   if (Boolean.TRUE.equals(b)) {     foo();   } else {     bar(); // will be invoked for both b == false and b == null   }   See * Java Language Specification §5.1.8 Unboxing Conversion

6、

Minor code smell

Remove this use of "Integer";it is deprecated.

问题代码：

修改方案：

rule:

"@Deprecated" code should not be used   Code smell   Minor squid:CallToDeprecatedMethod   Once deprecated, classes, and interfaces, and their members should be avoided, rather than used, inherited or extended. Deprecation is a warning that the class or interface has been superseded, and will eventually be removed. The deprecation period allows you to make a smooth transition away from the aging, soon-to-be-retired technology. Noncompliant Code Example   /**   * @deprecated As of release 1.3, replaced by {@link #Fee}   */  @Deprecated  public class Fum { ... }     public class Foo {     /**     * @deprecated As of release 1.7, replaced by {@link #doTheThingBetter()}     */     @Deprecated     public void doTheThing() { ... }       public void doTheThingBetter() { ... }   }     public class Bar extends Foo {     public void doTheThing() { ... } // Noncompliant; don't override a deprecated method or explicitly mark it as @Deprecated   }     public class Bar extends Fum { // Noncompliant; Fum is deprecated      public void myMethod() {       Foo foo = new Foo(); // okay; the class isn't deprecated      foo.doTheThing(); // Noncompliant; doTheThing method is deprecated     }   }   See MITRE, CWE-477 - Use of Obsolete Functions CERT, MET02-J. - Do not use deprecated or obsolete classes or methods

7、不要通过创建随机对象的方式获取随机值。

Critical code smell

Save and re-use this "Random".

问题代码：

修改方案：

rule:

"Random" objects should be reused

Bug

Critical
squid:S2119

Creating a new Random object each time a random value is needed is inefficient and may produce numbers which are not random depending on the JDK.

For better efficiency and randomness, create a single Random, then store, and reuse it.
The Random() constructor tries to set the seed with a distinct value every time. However there is no guarantee that the seed will be random or even

uniformly distributed. Some JDK will use the current time as seed, which makes the generated numbers not random at all.
This rule finds cases where a new Random is created each time a method is invoked and assigned to a local random variable.
Noncompliant Code Example
  public void doSomethingCommon() {
    Random rand = new Random(); // Noncompliant; new instance created with each invocation
    int rValue = rand.nextInt();
    //...  
Compliant Solution
  private Random rand = SecureRandom.getInstanceStrong(); // SecureRandom is preferred to Random
    public void doSomethingCommon() {
    int rValue = this.rand.nextInt();
    //...  
Exceptions
A class which uses a Random in its constructor or in a static main function and nowhere else will be ignored by this rule.
See
OWASP Top 10 2017 Category A6 - Security Misconfiguration

8、方法的复杂度不能太高。

Critical code smell

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed.

修改方案：

（重构这种方法，将其认知复杂性从22降低到15。）

表示一个方法行数很多比较复杂，同样的解决办法就是抽取方法，讲一个方法拆分几个方法。

rule:

Cognitive Complexity of methods should not be too high   Code smell   Critical squid:S3776   Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain. See Cognitive Complexity

9、在循环中不要多次使用break和continue语句。

Minor code smell

Reduce the total number of break and continue statements in this loop to use at most one. 在这个循环中减少break和continue语句的总数，最多使用一个。

rule:

Loops should not contain more than a single "break" or "continue" statement   Code smell   Minor squid:S135   Restricting the number of break and continue statements in a loop is done in the interest of good structured programming. One break and continue statement is acceptable in a loop, since it facilitates optimal coding. If there is more than one, the code should be refactored to increase readability. Noncompliant Code Example   for (int i = 1; i <= 10; i++) { // Noncompliant - 2 continue - one might be tempted to add some logic in between     if (i % 2 == 0) {       continue;     }       if (i % 3 == 0) {       continue;     }       System.out.println("i = " + i);   }

10、局部变量应该遵守命名规则。

Minor code smell

Rename this local variable to match the regular expression '^[a-z][a-zA-Z0-9]*$'

问题代码：

String access_token = weChatUtil.getAccessToken();

修改方案：

String accessToken = weChatUtil.getAccessToken();

rule:

Local variable and method parameter names should comply with a naming convention   Code smell   Minor squid:S00117   Shared naming conventions allow teams to collaborate effectively. This rule raises an issue when a local variable or function parameter name does not match the provided regular expression. Noncompliant Code Example With the default regular expression ^[a-z][a-zA-Z0-9]*$:   public void doSomething(int my_param) {     int LOCAL;     ...   }   Compliant Solution   public void doSomething(int myParam) {     int local;     ...   }   Exceptions Loop counters are ignored by this rule.   for (int i_1 = 0; i_1 < limit; i_1++) { // Compliant     // ...   }   as well as one-character catch variables:   try {   //...   } catch (Exception e) {  // Compliant   }

11、"ThreadLocal" variables should be cleaned up when no longer used

Major code smell

Call "remove()" on "SyncHttpClients".

问题代码：

1     //同步 2     private static ThreadLocal<CloseableHttpClient> SyncHttpClients = new ThreadLocal<CloseableHttpClient>() { 3         @Override 4         protected CloseableHttpClient initialValue() { 5             return HttpClients.createDefault(); 6         } 7  8     };

修改方案：

rule:

"ThreadLocal" variables should be cleaned up when no longer used   Bug   Major squid:S5164   ThreadLocal variables are supposed to be garbage collected once the holding thread is no longer alive. Memory leaks can occur when holding threads are re-used which is the case on application servers using pool of threads. To avoid such problems, it is recommended to always clean up ThreadLocal variables using the remove() method to remove the current thread’s value for the ThreadLocal variable. In addition, calling set(null) to remove the value might keep the reference to this pointer in the map, which can cause memory leak in some scenarios. Using remove is safer to avoid this issue. Noncompliant Code Example   public class ThreadLocalUserSession implements UserSession {      private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();     public UserSession get() {       UserSession session = DELEGATE.get();       if (session != null) {         return session;       }       throw new UnauthorizedException("User is not authenticated");     }     public void set(UserSession session) {       DELEGATE.set(session);     }      public void incorrectCleanup() {        DELEGATE.set(null); // Noncompliant      }       // some other methods without a call to DELEGATE.remove()   }   Compliant Solution   public class ThreadLocalUserSession implements UserSession {      private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();       public UserSession get() {       UserSession session = DELEGATE.get();       if (session != null) {         return session;       }       throw new UnauthorizedException("User is not authenticated");     }      public void set(UserSession session) {       DELEGATE.set(session);     }     public void unload() {       DELEGATE.remove(); // Compliant     }       // ...   }   Exceptions Rule will not detect non-private ThreadLocal variables, because remove() can be called from another class. See Understanding Memory Leaks in Java

12、用ThreadLocal.withInitial创建匿名内部类

Minor code smell

Replace this anonymous class with a call to "ThreadLocal.withInitial".

rule:

"ThreadLocal.withInitial" should be preferred

Code smell

Minor
squid:S4065

Java 8 introduced ThreadLocal.withInitial which is a simpler alternative to creating an anonymous inner class to initialise a ThreadLocal instance.
This rule raises an issue when a ThreadLocal anonymous inner class can be replaced by a call to ThreadLocal.withInitial.
Noncompliant Code Example
  ThreadLocal<List<String>> myThreadLocal =

new ThreadLocal<List<String>>() { // Noncompliant 
         @Override
          protected List<String> initialValue() {
              return new ArrayList<String>(); 
         }
 };  
Compliant Solution
  ThreadLocal<List<String>> myThreadLocal = ThreadLocal.withInitial(ArrayList::new);

13、Replace this lambda with a method reference.

Minor code smell

Replace this lambda with a method reference.

问题代码：

//同步     private static ThreadLocal<CloseableHttpClient> syncHttpClients = ThreadLocal.withInitial(() -> HttpClients.createDefault());

修改方案：

//同步     private static ThreadLocal<CloseableHttpClient> syncHttpClients = ThreadLocal.withInitial(HttpClients::createDefault);

rule:

Lambdas should be replaced with method references   Code smell   Minor squid:S1612   Method/constructor references are more compact and readable than using lambdas, and are therefore preferred. Similarly, null checks can be replaced with references to the Objects::isNull and Objects::nonNull methods. Note that this rule is automatically disabled when the project's sonar.java.source is lower than 8. Noncompliant Code Example   class A {     void process(List<A> list) {       list.stream()         .map(a -> a.<String>getObject())         .forEach(a -> { System.out.println(a); });     }       <T> T getObject() {       return null;     }   }   Compliant Solution   class A {     void process(List<A> list) {       list.stream()         .map(A::<String>getObject)         .forEach(System.out::println);     }       <T> T getObject() {       return null;     }   }

14、Remove this array creation and simply pass the elements.

Minor code smell

Remove this array creation and simply pass the elements.

问题代码：

ALL(0, "全部", Arrays.asList(new Integer[]{15,16,20}))

修改方案：

ALL(0, "全部", Arrays.asList(15,16,20))

rule:

Arrays should not be created for varargs parameters

Code smell

Minor
squid:S3878

There's no point in creating an array solely for the purpose of passing it as a varargs (...) argument; varargs is an array. Simply pass the elements directly. They will be consolidated into an array automatically.

Incidentally passing an array where Object ... is expected makes the intent ambiguous: Is the array supposed to be one object or a collection of objects?
Noncompliant Code Example
  public void callTheThing() {
    //...
    doTheThing(new String[] { "s1", "s2"}); // Noncompliant: unnecessary
    doTheThing(new String[12]); // Compliant
    doTheOtherThing(new String[8]); // Noncompliant: ambiguous
    // ...
  }
    public void doTheThing (String ... args) {
    // ...
  }
    public void doTheOtherThing(Object ... args) {
    // ...
  }  
Compliant Solution
  public void callTheThing() {
    //...
    doTheThing("s1", "s2");
    doTheThing(new String[12]);
    doTheOtherThing((Object[]) new String[8]);
     // ... 
 }
    public void doTheThing (String ... args) {
    // ...
  }
    public void doTheOtherThing(Object ... args) {
    // ...
  }

结束。