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) { // ... }
結束。