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