单元测试系列之九：Sonar 常用代码规则整理（一）

2023-08-12 11:31:46

更多原创测试技术文章同步更新到微信公众号：三国测，敬请扫码关注个人的微信号，感谢！

摘要：公司部署了一套sonar，经过一段时间运行，发现有一些问题出现频率很高，因此有必要将这些问题进行整理总结和分析，避免再次出现类似问题。

作者原创技术文章，转载请注明出处
id: 83 name: A method/constructor shouldnt explicitly throw java.lang.Exception type: CODE SMELL severity： MAJOR Comment: It is unclear which exceptions that can be thrown from the methods. It might be difficult to document and understand the vague interfaces. Use either a class derived from RuntimeException or a checked exception. definition: 目前还不清楚可以从方法中抛出哪些异常。可能难以记录和了解模糊界面。 advice: 建议使用从RuntimeException派生的类或checked的异常。
id: 144 name: Avoid really long methods. type: CODE SMELL severity： MAJOR Comment: Violations of this rule usually indicate that the method is doing too much. Try to reduce the method size by creating helper methods and removing any copy/pasted code. definition: 违反此规则通常表明该方法做得太多。尝试通过创建帮助方法并删除任何复制/粘贴代码来减少方法大小。 advice: 建议方法代码行数不超过75行
id: 1032 name: Redundant nullcheck of tbOfflinePay, which is known to be non-null in com.ctrip.market.web.controller.OrderController.getOrderPrintDetail(String, Integer, Long, String, String, String) type: CODE SMELL severity： MAJOR Comment: This method contains a redundant check of a known non-null value against the constant null. definition: 此方法包含对常量null的已知非空值的冗余检查。 advice: 建议：先检查是否为空再进行相关操作
id: 1097 name: Write to static field com.ctrip.market.materialfile.utils.SpringContextHolder.applicationContext from instance method com.ctrip.market.materialfile.utils.SpringContextHolder.destroy() type: CODE SMELL severity： CRITICAL Comment: This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice. definition: 此实例方法写入静态字段。如果多个实例被操纵，这是很难获得正确的。这通常是坏的做法。 advice: 不要通过实例方法给静态变量赋值
id: 1217 name: input must be non-null but is marked as nullable type: CODE SMELL severity： MAJOR Comment: This parameter is always used in a way that requires it to be non-null, but the parameter is explicitly annotated as being Nullable. Either the use of the parameter or the annotation is wrong. definition: 该参数始终以使其不为空的方式使用，但是将该参数显式注释为Nullable。所以，参数或注释是错误的。 advice: 参数值在任何情况下都不能为空，但是有明确的注释它可以为空。
id: 1624 name: Assign this magic number 3 to a well-named constant, and use the constant instead. type: CODE SMELL severity： MAJOR Comment: "A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.

Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.

That is why magic numbers must be demystified by first being assigned to clearly named variables before being used.

-1, 0 and 1 are not considered magic numbers." definition: "一个魔术数字是一个从无处出现的数字，直接用在一个语句中。经常使用魔数，例如限制循环的迭代次数，以测试属性的值等。
当您编写一段代码时，使用魔术数字可能看起来显而易见，但在调试时间上它们不那么明显和直截了当。
这就是为什么魔术数字必须被神秘化，首先被分配给明确命名的变量才能使用。
-1，0和1不被视为魔术数字。" advice: "不合规：
public static void doSomething() {
for(int i = 0; i < 4; i++){ // Noncompliant, 4 is a magic number
...
}
}
合规：
public static final int NUMBER_OF_CYCLES = 4;
public static void doSomething() {
for(int i = 0; i < NUMBER_OF_CYCLES ; i++){
...
}
}
例外：
这条规则忽略 hashCode 方法。"
id: 1645 name: This method has 158 lines, which is greater than the 100 lines authorized. Split it into smaller methods. type: CODE SMELL severity： MAJOR Comment: A method that grows too large tends to aggregate too many responsibilities. Such method inevitably become harder to understand and therefore harder to maintain. definition: 增长太大的方法往往会累积太多的责任。这种方法不可避免地变得难以理解，因此更难维护。 advice: 较小的方法不但会更容易理解，也可能更容易测试。
id: 1701 name: Remove this empty statement. type: BUG severity： MINOR Comment: "Empty statements, i.e. ;, are usually introduced by mistake, for example because:

It was meant to be replaced by an actual statement, but this was forgotten.
There was a typo which lead the semicolon to be doubled, i.e. ;;." definition: "空的声明，即.;，通常是错误的引入，例如：
这意味着被一个实际的陈述所取代，但这被遗忘了。
有一个拼写错误，导致分号增加一倍，即. ;;。" advice: "不合规：
void doSomething() {
; // Noncompliant - was used as a kind of TODO marker
}

void doSomethingElse() {
System.out.println(""Hello, world!"");; // Noncompliant - double ;
...
for (int i = 0; i < 3; System.out.println(i), i++); // Noncompliant - Rarely, they are used on purpose as the body of a loop. It is a bad practice to have side-effects outside of the loop body
...
}
合规：
void doSomething() {}

void doSomethingElse() {
System.out.println(""Hello, world!"");
...
for (int i = 0; i < 3; i++){
System.out.println(i);
}
...
}"
id: 1722 name: Reduce the number of conditional operators (4) used in the expression (maximum allowed 3). type: CODE SMELL severity： CRITICAL Comment: The complexity of an expression is defined by the number of &&, || and condition ? ifTrue : ifFalse operators it contains. A single expression's complexity should not become too high to keep the code readable. definition: 表达式的复杂性由&&，||的数量定义和它所包含的条件判断？ ifTrue：ifFalse等操作符所决定。单个表达式的复杂度不应该变得太高，以致不能保持代码的可读性。 advice: "不合规代码范例：

默认阈值为3的情况:

if (((condition1 && condition2) || (condition3 && condition4)) && condition5) { ... }
合规范例：

if ( (myFirstCondition() || mySecondCondition()) && myLastCondition()) { ... }"
id: 1728 name: This method has 122 lines, which is greater than the 100 lines authorized. Split it into smaller methods. type: CODE SMELL severity： MAJOR Comment: A method that grows too large tends to aggregate too many responsibilities. Such method inevitably become harder to understand and therefore harder to maintain. Above a specific threshold, it is strongly advised to refactor into smaller methods which focus on well-defined tasks. Those smaller methods will not only be easier to understand, but also probably easier to test. definition: 方法增长太大往往会累积太多的责任/干系。这种方法不可避免地变得难以理解，更难维护。 advice: 建议方法中最大行数：75
id: 1738 name: The Cyclomatic Complexity of this method "remarketingDataHandler" is 15 which is greater than 10 authorized. type: CODE SMELL severity： CRITICAL Comment: "The cyclomatic complexity of methods should not exceed a defined threshold.

Complex code can perform poorly and will in any case be difficult to understand and therefore to maintain." definition: 方法的循环复杂度不应超过定义的阈值。复杂的代码表现较差，在任何情况下都难以理解，需要维护。 advice: 复杂度建议为10
id: 1759 name: Add a default case to this switch. type: CODE SMELL severity： MAJOR Comment: The requirement for a final default clause is defensive programming. The clause should either take appropriate action, or contain a suitable comment as to why no action is taken. Even when the switch covers all current values of an enum, a default case should still be used because there is no guarantee that the enum won't be extended. definition: switch语句应该以default结束，这是一种defensive programming思想 advice: "不合规:
switch (param) { //missing default clause
case 0:
doSomething();
break;
case 1:
doSomethingElse();
break;
}

switch (param) {
default: // default clause should be the last one
error();
break;
case 0:
doSomething();
break;
case 1:
doSomethingElse();
break;
}
合规:
switch (param) {
case 0:
doSomething();
break;
case 1:
doSomethingElse();
break;
default:
error();
break;
}"
id: 1809 name: Remove this call from a constructor to the overridable "setFormat" method. type: CODE SMELL severity： CRITICAL Comment: Calling an overridable method from a constructor could result in failures or strange behaviors when instantiating a subclass which overrides the method. definition: 从构造函数调用一个可覆盖的方法,可能导致在实例化覆盖该方法的子类时出现故障或奇怪的行为。 advice: "不合规：

public class Parent {

public Parent () {
doSomething(); // Noncompliant
}

public void doSomething () { // not final; can be overridden
...
}
}

public class Child extends Parent {

private String foo;

public Child(String foo) {
super(); // leads to call doSomething() in Parent constructor which triggers a NullPointerException as foo has not yet been initialized
this.foo = foo;
}

public void doSomething () {
System.out.println(this.foo.length());
}

}"
id: 1819 name: Throw some other exception here, such as "IllegalArgumentException". type: CODE SMELL severity： MAJOR Comment: A NullPointerException should indicate that a null value was unexpectedly encountered. Good programming practice dictates that code is structured to avoid NPE's. definition: 良好的编程实践规定代码的结构是避免空值异常的。 advice: "不合规：
public void doSomething (String aString) throws NullPointerException {
throw new NullPointerException();
}
合规：
public void doSomething (@NotNull String aString) {
}"
id: 1826 name: Change this comparison to use the equals method. type: BUG severity： MAJOR Comment: Using the equality (==) and inequality (!=) operators to compare two objects does not check to see if they have the same values. Rather it checks to see if both object references point to exactly the same object in memory. The vast majority of the time, this is not what you want to do. Use the .equals() method to compare the values of two objects or to compare a string object to a string literal. definition: 使用等于（==）和不等式（！=）运算符比较两个对象不会检查它们是否具有相同的值。相反，它会检查两个对象引用是否指向内存中完全相同的对象。绝大多数时间，这不是你想做的事情。使用.equals（）方法来比较两个对象的值，或比较字符串对象与字符串文字。 advice: "不合规：
String str1 = ""blue"";
String str2 = ""blue"";
String str3 = str1;

if (str1 == str2)
{
System.out.println(""they're both 'blue'""); // this doesn't print because the objects are different
}

if (str1 == ""blue"")
{
System.out.println(""they're both 'blue'""); // this doesn't print because the objects are different
}

if (str1 == str3)
{
System.out.println(""they're the same object""); // this prints
}
合规：
String str1 = ""blue"";
String str2 = ""blue"";
String str3 = str1;

if (str1.equals(str2))
{
System.out.println(""they're both 'blue'""); // this prints
}

if (str1.equals(""blue""))
{
System.out.println(""they're both 'blue'""); // this prints
}

if (str1 == str3)
{
System.out.println(""they're the same object""); // this still prints, but it's probably not what you meant to do
}"
id: 1837 name: Make this class "final" or add a public constructor. type: CODE SMELL severity： MAJOR Comment: Classes with only private constructors should be marked final to prevent any mistaken extension attempts. definition: 只有私有构造函数的类应该被标记为final advice: "不合规：
public class PrivateConstructorClass { // Noncompliant
private PrivateConstructorClass() {
// ...
}

public static int magic(){
return 42;
}
}
合规：
public final class PrivateConstructorClass { // Compliant
private PrivateConstructorClass() {
// ...
}

public static int magic(){
return 42;
}
}"
id: 1846 name: Rename this local variable name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MINOR Comment: Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all local variable and function parameter names match a provided regular expression. definition: 共享一些命名约定是使团队有可能高效协作的关键。此规则允许检查所有本地变量和函数参数名称是否与提供的正则表达式匹配。 advice: "不合规：
With the default regular expression ^[a-z][a-zA-Z0-9]*$:

public void doSomething(int my_param) {
int LOCAL;
...
}
合规：

public void doSomething(int myParam) {
int local;
...
}
例外：

循环计数可以忽略这条规则

for (int i = 0; i < limit; i++) { // Compliant
// ...
}"
id: 1856 name: Rename this field "TopLevel" to match the regular expression '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MINOR Comment: Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all local variable and function parameter names match a provided regular expression. definition: 共享一些命名约定是使团队有可能高效协作的关键。此规则允许检查所有本地变量和函数参数名称是否与提供的正则表达式匹配。 advice: "不合规:
With the default regular expression ^[a-z][a-zA-Z0-9]*$:

public void doSomething(int my_param) {
int LOCAL;
...
}
合规:
public void doSomething(int myParam) {
int local;
...
}
例外：

循环计数被该规则忽略

for (int i = 0; i < limit; i++) { // Compliant
// ...
}
"
id: 1861 name: Cyclomatic Complexity is 11 (max allowed is 10). type: CODE SMELL severity： MAJOR Comment: Checks cyclomatic complexity of methods against a specified limit. The complexity is measured by the number of if, while, do, for, ?:, catch, switch, case statements, and operators && and || (plus one) in the body of a constructor, method, static initializer, or instance initializer. It is a measure of the minimum number of possible paths through the source and therefore the number of required tests. Generally 1-4 is considered good, 5-7 ok, 8-10 consider re-factoring, and 11+ re-factor now definition: 检查针对特定限制的方法的循环复杂性。复杂度由if，while，do，for，...，catch，switch，case语句和运算符&&和||的数量来衡量。（加一个）在构造函数，方法，静态初始化程序或实例初始化程序的正文中。它是通过源代码的可能路径的最小数量的度量，因此是所需测试的数量。一般来说，1-4被认为是好的，5-7可以，8-10考虑重新分解，11+的建议马上重构 advice: 大于10建议重构
id: 1884 name: Rename this class name to match the regular expression '^[A-Z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MINOR Comment: Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all class names match a provided regular expression. definition: 共享一些命名约定是使团队有可能高效协作的关键。此规则允许检查所有类名称是否与提供的正则表达式相匹配。 advice: "不合规：
默认提供正则表达式 ^[A-Z][a-zA-Z0-9]*$:
class my_class {...}

合规：
class MyClass {...}"
id: 1885 name: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MAJOR Comment: Shared naming conventions allow teams to collaborate efficiently. This rule checks that all method names match a provided regular expression. definition: 此规则检查所有方法名称是否与提供的正则表达式匹配。 advice: "不合规：
public int DoSomething(){...}
合规：
public int doSomething(){...}
例外：（重写的方法）
@Override
public int Do_Something(){...}"
id: 1921 name: Remove this unused "MKT_Clean_GroupID" private field. type: CODE SMELL severity： MAJOR Comment: If a private field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will improve maintainability because developers will not wonder what the variable is used for. definition: 如果一个私有变量被声明但未被使用，那么它是死代码应该被删除。这样做会提高可维护性，因为开发人员并不想知道这个变量用来作甚。 advice: "不合规：
public class MyClass {
private int foo = 42;
public int compute(int a) {
return a * 42;
}
}
合规：
public class MyClass {
public int compute(int a) {
return a * 42;
}
}"
id: 1926 name: Name XXXXXX must match pattern '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MAJOR Comment: Checks that parameter names conform to the specified format definition: 检查参数名称是否符合指定格式 advice: 建议按照命名规范
id: 1929 name: Remove this unused method parameter "payinfo". type: CODE SMELL severity： MAJOR Comment: Unused parameters are misleading. Whatever the value passed to such parameters is, the behavior will be the same. definition: 未使用的参数会造成误导。因为无论传递给这些参数的值是什么，函数行为并不会发生变化。 advice: "不合规：
void doSomething(int a, int b) { // ""b"" is unused
compute(a);
}
合规：
void doSomething(int a) {
compute(a);
}"
id: 1944 name: Remove this unused "empsEntity" local variable. type: CODE SMELL severity： MINOR Comment: "If a local variable is declared but not used, it is dead code and should be removed. Doing so will improve maintainability because developers will not wonder what the variable is used for.
" definition: 如果一个局部变量被声明但未被使用，那么它是死代码应该被删除。这样做会提高可维护性，因为开发人员并不想知道这个变量用来作甚。 advice: "不合规：
public int numberOfMinutes(int hours) {
int seconds = 0; // seconds is never used
return hours * 60;
}
合规：
public int numberOfMinutes(int hours) {
return hours * 60;
}"
id: 1964 name: Remove this unused private "setUpVidList" method. type: CODE SMELL severity： MINOR Comment: private methods that are never executed are dead code: unnecessary, inoperative code that should be removed. Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced. definition: 从未执行的私有方法是死码：不必要的，应该被删除的代码。清理死码会减少维护的代码库的大小，从而更容易理解程序并防止引入错误。 advice: "不合规：
public class Foo implements Serializable
{
private Foo(){} //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
public static void doSomething(){
Foo foo = new Foo();
...
}
private void unusedPrivateMethod(){...}
private void writeObject(ObjectOutputStream s){...} //Compliant, relates to the java serialization mechanism
private void readObject(ObjectInputStream in){...} //Compliant, relates to the java serialization mechanism
}
合规：
public class Foo implements Serializable
{
private Foo(){} //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
public static void doSomething(){
Foo foo = new Foo();
...
}

private void writeObject(ObjectOutputStream s){...} //Compliant, relates to the java serialization mechanism

private void readObject(ObjectInputStream in){...} //Compliant, relates to the java serialization mechanism
}"
id: 1965 name: Remove this unused import 'com.ctrip.market.dmp.remarketing.business.Models.DashboardMetrics'. type: CODE SMELL severity： INFO Comment: The imports part of a file should be handled by the Integrated Development Environment (IDE), not manually by the developer. definition: 无用的imports应该删除 advice: 文件的导入部分应由集成开发环境（IDE）处理，而不是由开发人员手动处理。
id: 1986 name: Unused import - XXXXXX type: CODE SMELL severity： INFO Comment: The imports part of a file should be handled by the Integrated Development Environment (IDE), not manually by the developer. definition: 无用的imports应该删除 advice: 文件的导入部分应由集成开发环境（IDE）处理，而不是由开发人员手动处理。
id: 1995 name: Name XXX must match pattern '^[a-z]+(\.[a-zA-Z_][a-zA-Z0-9_]*)*$'. type: CODE SMELL severity： MAJOR Comment: Checks that package names conform to the specified format. The default value of format has been chosen to match the requirements in the Java Language specification and the Sun coding conventions. However both underscores and uppercase letters are rather uncommon, so most configurations should probably assign value ^[a-z]+(\.[a-z][a-z0-9]*)*$ to format definition: "
检查包名是否符合指定的格式。已经选择了格式的默认值，以匹配Java语言规范和Sun编码约定中的要求。" advice: 建议大多数配置应该可以分配值^ [a-z] +（\。[a-z] [a-z0-9] *）* $来格式化
id: 2001 name: Name XXX must match pattern '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： MAJOR Comment: Checks that parameter names conform to the specified format definition: 检查参数名称是否符合指定格式 advice: 建议按照命名规范
id: 2003 name: Either remove or fill this block of code. type: CODE SMELL severity： MAJOR Comment: Most of the time a block of code is empty when a piece of code is really missing. So such empty block must be either filled or removed. definition: 当一段代码真的丢失时，大多数时候代码块是空的。所以这样的空块必须被填充或删除。 advice: "不合规：
for (int i = 0; i < 42; i++){} // Empty on purpose or missing piece of code ?"
id: 2014 name: Use a logger to log this exception. type: ISSUE severity： MINOR Comment: "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." definition: "Throwable.printStackTrace（...）将Throwable及其堆栈跟踪打印到某些流。默认情况下，流System.Err可能会无意中暴露敏感信息。
应该使用日志消息来代替打印Throwables，因为它们有很多优点：
用户能够轻松地检索日志。日志消息的格式是统一的，用户可以轻松浏览日志。" advice: "不合规：
try {
/* ... */
} catch(Exception e) {
e.printStackTrace();
}
合规：
try {
/* ... */
} catch(Exception e) {
LOGGER.log(""context"", e);
}"
id: 2018 name: Rename this package name to match the regular expression '^[a-z]+(\\.[a-z][a-z0-9]*)*$'. type: CODE SMELL severity： MAJOR Comment: Shared coding conventions allow teams to collaborate efficiently. This rule checks that all package names match a provided regular expression. definition: 此规则检查所有包名称是否与提供的正则表达式匹配。 advice: "不合规:
package org.exAmple; //不符合
合规:
package org.example;"
id: 2027 name: Rename this local variable name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. type: CODE SMELL severity： INFO Comment: Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all local variable and function parameter names match a provided regular expression. definition: 局部变量和方法参数名称应符合命名约定 advice: "不合规：
public void doSomething(int my_param) {
int LOCAL;
...
}
合规：
public void doSomething(int myParam) {
int local;
...
}
例外：
循环计数器被该规则忽略。
for (int i = 0; i < limit; i++) { // Compliant
// ...
}"