```go Bad theme={null} for i := 1; i <= 10; j++ { // Noncompliant // ... } ``` ```go Fix theme={null} for i := 1; i <= 10; i++ { // ... } ```

Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.

Unresolved directive in \ - include::\{noncompliant}\[]

Write one statement per line to improve readability.

Unresolved directive in \ - include::\{compliant}\[]

```go Bad theme={null} if condition { doSomething() } // Compliant by exception ``` ```go Fix theme={null} ```

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.

```go Bad theme={null} if (((condition1 && condition2) || (condition3 && condition4)) && condition5) { ... } ``` ```go Fix theme={null} if ( (myFirstCondition() || mySecondCondition()) && myLastCondition()) { ... } ``` ```go Bad theme={null} var letters = make(map[string]string) letters["a"] = "Apple" letters["a"] = "Boy" // Noncompliant var towns = make(map[int]string) towns[i] = "London" towns[i] = "Chicago" // Noncompliant ``` ```go Fix theme={null} ```

A loop with at most one iteration is equivalent to the use of an \`if statement to conditionally execute one piece of code. No developer expects to find such a use of a loop statement. If the initial intention of the author was really to conditionally execute one piece of code, an if statement should be used instead.

At worst that was not the initial intention of the author and so the body of the loop should be fixed to use the nested return, break or throw\` statements in a more appropriate way.

```go Bad theme={null} for i := 0; i < 10; i++ { // noncompliant, loop only executes once fmt.Println(i) break } ``` ```go Fix theme={null} for i := 0; i < 10; i++ { fmt.Println(i) } ```

Development tools and frameworks usually have options to make debugging easier for developers. Although these features are useful during development, they should never be enabled for applications deployed in production. Debug instructions or error messages can leak detailed information about the system, like the application’s path or file names.

```go Bad theme={null} import "runtime/debug" _, err := funcThatFails() if err != nil { fmt.Printf("Error calling funcThatFails: %v\n", err) debug.PrintStack() // Sensitive return } ``` ```go Fix theme={null} func main() { pprof.Lookup("goroutine").WriteTo(os.Stdout, 1) // Sensitive } ```

A boolean literal can be represented in two different ways: \{true} or \{false}. They can be combined with logical operators (\{ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.

```go Bad theme={null} if boolFunc() || false { // ... } flag := x && true ``` ```go Fix theme={null} if boolFunc() { // ... } flag := x ```

Integer literals starting with a zero are octal rather than decimal values. While using octal values is fully supported, most developers do not have experience with them. They may not recognize octal values as such, mistaking them instead for decimal values.

```go Bad theme={null} func printTen() { myNumber := 010 // Noncompliant. myNumber will hold 8, not 10 - was this really expected? fmt.Println(myNumber) } ``` ```go Fix theme={null} func printTen() { myNumber := 10 fmt.Println(myNumber) } ```

Using pseudorandom number generators (PRNGs) is security-sensitive. For example, it has led in the past to the following vulnerabilities:

When software generates predictable values in a context requiring unpredictability, it may be possible for an attacker to guess the next value that will be generated, and use this guess to impersonate another user or access sensitive information.

```go Bad theme={null} import "math/rand" a := make([]byte, 10) rand.Read(a) // Sensitive ``` ```go Fix theme={null} import "math/rand" num := rand.Intn(100) // Sensitive ```

Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.

Merging if statements when possible will decrease the nesting of the code and improve its readability.

```go Bad theme={null} if condition1 { if condition2 { // Noncompliant fmt.Println("Hello World") } } ``` ```go Fix theme={null} if condition1 && condition2 { // Compliant fmt.Println("Hello World") } ```

An empty \{operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty \{operationName}s bring no functionality and are misleading to others as they might think the \{operationName} implementation fulfills a specific and identified requirement.

There are several reasons for a \{operationName} not to have a body:

It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

```go Bad theme={null} func shouldNotBeEmpty() { // Noncompliant - method is empty } func notImplemented() { // Noncompliant - method is empty } func emptyOnPurpose() { // Noncompliant - method is empty } ``` ```go Fix theme={null} func shouldNotBeEmpty() { doSomething(); } func notImplemented() { return "", errors.New("notImplemented() cannot be performed because ...") } func emptyOnPurpose() { // comment explaining why the method is empty } ```

Clear-text protocols such as \`ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:

sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information

Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.

For example, attackers could successfully compromise prior security layers by:

bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)

In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.

Note that using the http\` protocol is being deprecated by major web browsers.

In the past, it has led to the following vulnerabilities:

```go Bad theme={null} import "net/http" response, err := http.Get("http://www.example.com/") // Sensitive ``` ```go Fix theme={null} import "net/smtp" connection, err := smtp.Dial("mail.example.com:25") // Sensitive connection.Hello("my-sending-server.example.com") // authenticate and send email connection.Quit() ```

The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.

```go Bad theme={null} func foo(a bool, y int) int { x := ((y / 2 + 1)) // Noncompliant if a && ((x+y > 0)) { // Noncompliant return ((x + 1)) // Noncompliant } } ``` ```go Fix theme={null} func foo(a bool, y int) int { x := (y / 2 + 1) if a && (x+y > 0) { return (x + 1) } } ```

Using the same value on both sides of a binary operator is a code defect. In the case of logical operators, it is either a copy/paste error and, therefore, a bug, or it is simply duplicated code and should be simplified. In the case of bitwise operators and most binary mathematical operators, having the same value on both sides of an operator yields predictable results and should be simplified as well.

```go Bad theme={null} func main() { v1 := (true && false) && (true && false) // Noncompliant } ``` ```go Fix theme={null} func main() { v1 := (true && false) // Compliant } ```

A chain of if/else if statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.

Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.

```go Bad theme={null} func example(condition1, condition2 bool) { if condition1 { } else if condition1 { // Noncompliant } } ``` ```go Fix theme={null} func SwitchWithMultipleConditions(param int) { switch param { case 1, 2, 3: fmt.Println(">1") case 3, 4, 5: // Noncompliant; 3 is duplicated fmt.Println("<1") } } ```

Formatted SQL queries can be difficult to maintain, debug and can increase the risk of SQL injection when concatenating untrusted values into the query. However, this rule doesn’t detect SQL injections (unlike rule S3649), the goal is only to highlight complex/formatted queries.

```go Bad theme={null} func getName(db *sql.DB, id string) (string, error) { var name string row := db.QueryRow("SELECT name FROM users WHERE id = " + id) // Sensitive if err := row.Scan(&name); err != nil { if err == sql.ErrNoRows { return name, fmt.Errorf("No name found for id %s", id) } } return name, nil } ``` ```go Fix theme={null} func getName(db *sql.DB, id string) (string, error) { var name string row := db.QueryRow("SELECT name FROM users WHERE id = ?", id) if err := row.Scan(&name); err != nil { if err == sql.ErrNoRows { return name, fmt.Errorf("No name found for id %s", id) } } return name, nil } ```

A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.

```go Bad theme={null} func compute(start int) { // Noncompliant; start is not used sum := 0 for i := 0; i < 10; i++ { sum += i } fmt.Println("Result:", sum) } ``` ```go Fix theme={null} func compute() { sum := 0 for i := 0; i < 10; i++ { sum += i } fmt.Println("Result:", sum) } ```

Some statements (return, break, continue, goto, switch) and throw expressions move control flow out of the current code block. So any unlabeled statements that come after such a jump are unreachable, and either this dead code should be removed, or the logic should be corrected.

```go Bad theme={null} func add(x, y int) int { return x + y // Noncompliant z := x + y // dead code } ``` ```go Fix theme={null} func add(x, y int) int { return x + y // Compliant } ```

Hard-coding a URI makes it difficult to test a program for a variety of reasons:

path literals are not always portable across operating systems
a given absolute path may not exist in a specific test environment
a specified Internet URL may not be available when executing the tests
production environment filesystems usually differ from the development environment

In addition, hard-coded URIs can contain sensitive information, like IP addresses, and they should not be stored in the code.

For all those reasons, a URI should never be hard coded. Instead, it should be replaced by a customizable parameter.

Further, even if the elements of a URI are obtained dynamically, portability can still be limited if the path delimiters are hard-coded.

This rule raises an issue when URIs or path delimiters are hard-coded.

```go Bad theme={null} file, err := os.Open("accounts.txt") // Noncompliant if err != nil { log.Fatal(err) } bs, err := ioutil.ReadFile("accounts.txt") // Noncompliant if err != nil { return } ``` ```go Fix theme={null} var location string = prop.Read("myApplication.mySpecificFile") file, err := os.Open(location) if err != nil { log.Fatal(err) } ```

There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.

```go Bad theme={null} func (user *User) rename(name string) { name = name // Noncompliant } ``` ```go Fix theme={null} func (user *User) rename(name string) { user.name = name } ```

This rule applies whenever an \`if statement is followed by one or more else if statements; the final else if should be followed by an else statement.

The requirement for a final else statement is defensive programming.

The else statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final default clause in a switch\` statement.

```go Bad theme={null} if x == 0 { doSomething() } else if x == 1 { doSomethingElse() } ``` ```go Fix theme={null} if x == 0 { doSomething() } else if x == 1 { doSomethingElse() } else { return errors.New("unsupported int") } ```

Nested control flow statements such as if, for, while, switch, and try are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to understand and maintain.

When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.

```go Bad theme={null} if condition1 { // Compliant - depth = 1 /* ... */ if condition2 { // Compliant - depth = 2 /* ... */ for i := 1; i <= 10; i++ { // Compliant - depth = 3, not exceeding the limit /* ... */ if condition4 { // Noncompliant - depth = 4 if condition5 { // Depth = 5, exceeding the limit, but issues are only reported on depth = 4 /* ... */ } return } } } } ``` ```go Fix theme={null} ```

Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.

```go Bad theme={null} func run() { prepare("This should be a constant") // Noncompliant; 'This should ...' is duplicated 3 times execute("This should be a constant") release("This should be a constant") } ``` ```go Fix theme={null} const ACTION = "This should be a constant" func run() { prepare(ACTION) execute(ACTION) release(ACTION) } ```

The switch statement should be used only to clearly define some new branches in the control flow. As soon as a case clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of the case clause should be extracted into a dedicated method.

```go Bad theme={null} func foo(tag int) { switch tag { case 0: methodCall1() methodCall2() methodCall3() methodCall4() methodCall5() methodCall6() case 1: bar() } } ``` ```go Fix theme={null} func foo(tag int) { switch tag { case 0: executeAll() case 1: bar() } } func executeAll() { methodCall1() methodCall2() methodCall3() methodCall4() methodCall5() methodCall6() } ```

Two \{func\_name}s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.

```go Bad theme={null} func fun1() (x, y int) { a, b := 1, 2 b, a = a, b return a, b } func fun2() (x, y int) { // Noncompliant; duplicates fun1 a, b := 1, 2 b, a = a, b return a, b } ``` ```go Fix theme={null} func fun1() (x, y int) { a, b := 1, 2 b, a = a, b return a, b } func fun2() (x, y int) { // Intent is clear return fun1() } ```

Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:

Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:

The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).

Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.

```go Bad theme={null} config, err := ReadConfig("properties.ini") ip := config["ip"] port := config["ip"] SocketClient(ip, port) ``` ```go Fix theme={null} ```

Developers often use TODO tags to mark areas in the code where additional work or improvements are needed but are not implemented immediately. However, these TODO tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This rule aims to identify and address unattended TODO tags to ensure a clean and maintainable codebase. This description explores why this is a problem and how it can be fixed to improve the overall code quality.

```go Bad theme={null} func foo() { // TODO } ``` ```go Fix theme={null} ```

Having all branches of a switch or if chain with the same implementation indicates a problem.

In the following code:

Unresolved directive in \ - include::\{example}\[]

Either there is a copy-paste error that needs fixing or an unnecessary switch or if chain that should be removed.

```go Bad theme={null} if b == 0 { //no issue, this could have been done on purpose to make the code more readable doSomething() } else if b == 1 { doSomething() } ``` ```go Fix theme={null} ```

In Unix file system permissions, the "`others`" category refers to all users except the owner of the file system resource and the members of the group assigned to this resource.

Granting permissions to this category can lead to unintended access to files or directories that could allow attackers to obtain sensitive information, disrupt services or elevate privileges.

```go Bad theme={null} import ( "os" ) func main() { err := os.Chmod("/tmp/fs", 0777) // Sensitive if err != nil { panic(err) } } ``` ```go Fix theme={null} import ( "golang.org/x/sys/unix" ) func main() { oldMask := unix.Umask(0) // Sensitive } ```

\`if statements with conditions that are always false have the effect of making blocks of code non-functional. if statements with conditions that are always true are completely redundant, and make the code less readable.

There are three possible causes for the presence of such code:

An if statement was changed during debugging and that debug code has been committed.
Some value was left unset.
Some logic is not doing what the programmer thought it did.

In any of these cases, unconditional if\` statements should be removed.

```go Bad theme={null} if true { doSomething() } if false { doSomething() } ``` ```go Fix theme={null} ```

Empty statements represented by a semicolon ; are statements that do not perform any operation. They are often the result of a typo or a misunderstanding of the language syntax. It is a good practice to remove empty statements since they don’t add value and lead to confusion and errors.

```go Bad theme={null} func doSomething() { ; // Noncompliant } func doSomethingElse() { fmt.Println("doSomethingElse");; // Noncompliant - double useless ; ... } ``` ```go Fix theme={null} func doSomething() { } func doSomethingElse() { fmt.Println("doSomethingElse") ... } ```

Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions (little computational effort is enough to find two or more different inputs that produce the same hash).

```go Bad theme={null} import ( "crypto" "fmt" ) func calculateHash(data []byte) string { hashInstance := crypto.Hash.New(crypto.MD5) // Sensitive hashInstance.Write(data) hash := hashInstance.Sum(nil) return fmt.Sprintf("%x", hash) } ``` ```go Fix theme={null} import ( "crypto/sha1" "fmt" ) func calculateHash(data []byte) string { hash := sha1.Sum(data) // Sensitive return fmt.Sprintf("%x", hash) } ```

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.

```go Bad theme={null} switch tag { // Noncompliant - default case is missing case 0, 1, 2, 3: foo() case 4, 5, 6, 7: bar() } ``` ```go Fix theme={null} switch tag { case 0, 1, 2, 3: foo() case 4, 5, 6, 7: bar() default: qix() } ```

Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.

In the past, it has led to the following vulnerabilities:

Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.

This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.

It’s recommended to customize the configuration of this rule with additional credential words such as "oauthToken", "secret", …

```go Bad theme={null} func connect() { user := getEncryptedUser() password:= getEncryptedPass() // Compliant url := "login=" + user + "&passwd=" + password } ``` ```go Fix theme={null} ```

Nested \`switch structures are difficult to understand because you can easily confuse the cases of an inner switch as belonging to an outer statement. Therefore nested switch statements should be avoided.

Specifically, you should structure your code to avoid the need for nested switch statements, but if you cannot, then consider moving the inner switch\` to another function.

```go Bad theme={null} func foo(x,y int) { switch x { case 0: switch y { // Noncompliant; nested switch // ... } case 1: // ... default: // ... } } ``` ```go Fix theme={null} func foo(x,y int) { switch x { case 0: bar(y) case 1: // ... default: // ... } } func bar(y int) { switch y { // ... } } ```

A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. \{identifier\_capital\_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.

This rule checks that \{identifier} names match a provided regular expression.

```go Bad theme={null} func doSomething(my_param int) { // Noncompliant var local_ int; // Noncompliant ... } ``` ```go Fix theme={null} func doSomething(myParam int) { var local int; ... } ```