CodeAnt AI home pagelight logodark logo
  • Dashboard
  • Dashboard

Go

Type names should comply with a naming convention

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a struct name does not match a provided regular expression.

The convention in Go is to use mixedCaps rather than underscores. See Go documentation for the complete naming conventions.

Note that the casing of the first character determines if the type is exported or not.

For example, with the default provided regular expression ^[a-zA-Z][a-zA-Z0-9]*$, the struct:

Copy
type my_struct struct {...}

Collection sizes and array length comparisons should make sense

The size of a collection and the length of an array are always greater than or equal to zero. Testing it doesn’t make sense, since the result is always true.

Copy
if len(myArr) >= 0 { ... } // Noncompliant: always true

var result = len(myArr) >= 0 // Noncompliant: always true

Function names should comply with a naming convention

For example, with the default provided regular expression ^(_|[a-zA-Z0-9]+)$, the function:

Copy
func execute_all() {
...
}

Unary prefix operators should not be repeated

The repetition of a unary operator is usually a typo. The second operator invalidates the first one in most cases:

Copy
var a int = 1
var flag bool = true

var a1 int = ^^^a // Noncompliant: equivalent to "^a"
var flag2 bool  = !!!flag  // Noncompliant: equivalent to "!flag"

for loop increment clauses should modify variables from loop conditions

It can be extremely confusing when a for condition tests a variable which is not updated inside the for post statement.

Copy
for i := 1; i <= 10; j++ { // Noncompliant
// ...
}

Statements should be on separate lines

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

Unresolved directive in <stdin> - include::{noncompliant}[]

Write one statement per line to improve readability.

Unresolved directive in <stdin> - include::{compliant}[]

Copy
if condition { doSomething() } // Compliant by exception

Expressions should not be too complex

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.

Copy
if (((condition1 && condition2) || (condition3 && condition4)) && condition5) { ... }

Map values should not be replaced unconditionally

Copy
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

Loops with at most one iteration should be refactored

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.

Copy
for i := 0; i < 10; i++ { // noncompliant, loop only executes once
fmt.Println(i)
break
}

Delivering code in production with debug features activated is security-sensitive

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.

Copy
import "runtime/debug"

_, err := funcThatFails()
if err != nil {
fmt.Printf("Error calling funcThatFails: %v\n", err)
debug.PrintStack() // Sensitive
return
}

Boolean literals should not be redundant

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.

Copy
if boolFunc() || false { 
// ...
}

flag := x && true

Octal values should not be used

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.

Copy
func printTen() {                                                                                                                                
myNumber := 010 // Noncompliant. myNumber will hold 8, not 10 - was this really expected?
fmt.Println(myNumber)
}

Using pseudorandom number generators (PRNGs) is security-sensitive

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

  • CVE-2013-6386

  • CVE-2006-3419

  • CVE-2008-4102

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.

Copy
import "math/rand"

a := make([]byte, 10)
rand.Read(a) // Sensitive

Mergeable if statements should be combined

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.

Copy
if condition1 {
if condition2 {            // Noncompliant
	fmt.Println("Hello World")
}
}

Functions should not be empty

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.

Copy
func shouldNotBeEmpty() {  // Noncompliant - method is empty
}

func notImplemented() {  // Noncompliant - method is empty
}

func emptyOnPurpose() {  // Noncompliant - method is empty
}

Using clear-text protocols is security-sensitive

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:

  • CVE-2019-6169

  • CVE-2019-12327

  • CVE-2019-11065

Copy
import "net/http"

response, err := http.Get("http://www.example.com/") // Sensitive

Redundant pairs of parentheses should be removed

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.

Copy
func foo(a bool, y int) int {
x := ((y / 2 + 1)) // Noncompliant

if a && ((x+y > 0)) {  // Noncompliant
return ((x + 1))  // Noncompliant
}
}

Identical expressions should not be used on both sides of a binary operator

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.

Copy
func main() {
v1 := (true && false) && (true && false) // Noncompliant
}

Related if/else if statements should not have the same condition

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.

Copy
func example(condition1, condition2 bool) {
if condition1 {
} else if condition1 { // Noncompliant
}
}

Formatting SQL queries is security-sensitive

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.

Copy
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
}

Unused function parameters should be removed

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.

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

All code should be reachable

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.

Copy
func add(x, y int) int {                   
return x + y // Noncompliant
z := x + y // dead code
}

URIs should not be hardcoded

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.

Copy
file, err := os.Open("accounts.txt") // Noncompliant
if err != nil {
log.Fatal(err)
}

bs, err := ioutil.ReadFile("accounts.txt") // Noncompliant
if err != nil {
return
}

Variables should not be self-assigned

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.

Copy
func (user *User) rename(name string) {
name = name  // Noncompliant
}

if ... else if constructs should end with else clauses

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.

Copy
if x == 0 {                                        
doSomething()
} else if x == 1 {
doSomethingElse()
}

Control flow statements if, for and switch should not be nested too deeply

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.

Copy
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
		}
	}
}
}

String literals should not be duplicated

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

Copy
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")
}

switch case clauses should not have too many lines

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.

Copy
func foo(tag int) {
switch tag {
case 0:
	methodCall1()
	methodCall2()
	methodCall3()
	methodCall4()
            methodCall5()
            methodCall6()
case 1:
	bar()
}
}

Functions should not have identical implementations

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.

Copy
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
}

Using hardcoded IP addresses is security-sensitive

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

  • CVE-2006-5901

  • CVE-2005-3725

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.

Copy
config, err := ReadConfig("properties.ini")

ip := config["ip"]
port := config["ip"]

SocketClient(ip, port)

Track uses of TODO tags

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.

Copy
func foo() {
// TODO
}

All branches in a conditional structure should not have exactly the same implementation

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

In the following code:

Unresolved directive in <stdin> - include::{example}[]

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

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

Setting loose POSIX file permissions is security-sensitive

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.

Copy
import (
"os"
)

func main() {
err := os.Chmod("/tmp/fs", 0777) // Sensitive
if err != nil {
    panic(err)
}
}

Useless if(true) {...} and if(false){...} blocks should be removed

`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.

Copy
if true {
doSomething()
}

if false {
doSomething()
}

Empty statements should be removed

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.

Copy
func doSomething() {
;                                                       // Noncompliant
}

func doSomethingElse() {
fmt.Println("doSomethingElse");;     // Noncompliant - double useless ;
...
}

Using weak hashing algorithms is security-sensitive

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).

Copy
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)
}

switch statements should have default clauses

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.

Copy
switch tag { // Noncompliant - default case is missing
case 0, 1, 2, 3:
foo()
case 4, 5, 6, 7:
bar()
}

Hard-coded credentials are security-sensitive

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:

  • CVE-2019-13466

  • CVE-2018-15389

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”, …​

Copy
func connect()  {
user := getEncryptedUser()
password:= getEncryptedPass() // Compliant

url := "login=" + user + "&passwd=" + password
}

switch statements should not be nested

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.

Copy
func foo(x,y int) {
switch x {
case 0:
	switch y { // Noncompliant; nested switch
	// ...
	}
case 1:
	// ...
default:
	// ...
}
}

Local variable and function parameter names should comply with a naming convention

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.

Copy
func doSomething(my_param int) { // Noncompliant
var local_ int; // Noncompliant
...
}
Assistant
Responses are generated using AI and may contain mistakes.
twitterlinkedin
Powered by Mintlify
  • Documentation
  • Demo Call with CEO
  • Blog
  • Slack
    • Get Started
    • CodeAnt AI
    • Setup
    • Control Center
    • Pull Request Review
    • IDE
    • Compliance
    • Anti-Patterns
    • Code Governance
    • Infrastructure Security Database
    • Application Security Database

    Go

    Type names should comply with a naming convention

    Shared naming conventions allow teams to collaborate efficiently.

    This rule raises an issue when a struct name does not match a provided regular expression.

    The convention in Go is to use mixedCaps rather than underscores. See Go documentation for the complete naming conventions.

    Note that the casing of the first character determines if the type is exported or not.

    For example, with the default provided regular expression ^[a-zA-Z][a-zA-Z0-9]*$, the struct:

    Copy
    type my_struct struct {...}

    Collection sizes and array length comparisons should make sense

    The size of a collection and the length of an array are always greater than or equal to zero. Testing it doesn’t make sense, since the result is always true.

    Copy
    if len(myArr) >= 0 { ... } // Noncompliant: always true

var result = len(myArr) >= 0 // Noncompliant: always true

    Function names should comply with a naming convention

    For example, with the default provided regular expression ^(_|[a-zA-Z0-9]+)$, the function:

    Copy
    func execute_all() {
...
}

    Unary prefix operators should not be repeated

    The repetition of a unary operator is usually a typo. The second operator invalidates the first one in most cases:

    Copy
    var a int = 1
var flag bool = true

var a1 int = ^^^a // Noncompliant: equivalent to "^a"
var flag2 bool  = !!!flag  // Noncompliant: equivalent to "!flag"

    for loop increment clauses should modify variables from loop conditions

    It can be extremely confusing when a for condition tests a variable which is not updated inside the for post statement.

    Copy
    for i := 1; i <= 10; j++ { // Noncompliant
// ...
}

    Statements should be on separate lines

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

    Unresolved directive in <stdin> - include::{noncompliant}[]

    Write one statement per line to improve readability.

    Unresolved directive in <stdin> - include::{compliant}[]

    Copy
    if condition { doSomething() } // Compliant by exception

    Expressions should not be too complex

    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.

    Copy
    if (((condition1 && condition2) || (condition3 && condition4)) && condition5) { ... }

    Map values should not be replaced unconditionally

    Copy
    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

    Loops with at most one iteration should be refactored

    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.

    Copy
    for i := 0; i < 10; i++ { // noncompliant, loop only executes once
fmt.Println(i)
break
}

    Delivering code in production with debug features activated is security-sensitive

    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.

    Copy
    import "runtime/debug"

_, err := funcThatFails()
if err != nil {
fmt.Printf("Error calling funcThatFails: %v\n", err)
debug.PrintStack() // Sensitive
return
}

    Boolean literals should not be redundant

    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.

    Copy
    if boolFunc() || false { 
// ...
}

flag := x && true

    Octal values should not be used

    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.

    Copy
    func printTen() {                                                                                                                                
myNumber := 010 // Noncompliant. myNumber will hold 8, not 10 - was this really expected?
fmt.Println(myNumber)
}

    Using pseudorandom number generators (PRNGs) is security-sensitive

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

    • CVE-2013-6386

    • CVE-2006-3419

    • CVE-2008-4102

    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.

    Copy
    import "math/rand"

a := make([]byte, 10)
rand.Read(a) // Sensitive

    Mergeable if statements should be combined

    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.

    Copy
    if condition1 {
if condition2 {            // Noncompliant
	fmt.Println("Hello World")
}
}

    Functions should not be empty

    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.

    Copy
    func shouldNotBeEmpty() {  // Noncompliant - method is empty
}

func notImplemented() {  // Noncompliant - method is empty
}

func emptyOnPurpose() {  // Noncompliant - method is empty
}

    Using clear-text protocols is security-sensitive

    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:

    • CVE-2019-6169

    • CVE-2019-12327

    • CVE-2019-11065

    Copy
    import "net/http"

response, err := http.Get("http://www.example.com/") // Sensitive

    Redundant pairs of parentheses should be removed

    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.

    Copy
    func foo(a bool, y int) int {
x := ((y / 2 + 1)) // Noncompliant

if a && ((x+y > 0)) {  // Noncompliant
return ((x + 1))  // Noncompliant
}
}

    Identical expressions should not be used on both sides of a binary operator

    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.

    Copy
    func main() {
v1 := (true && false) && (true && false) // Noncompliant
}

    Related if/else if statements should not have the same condition

    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.

    Copy
    func example(condition1, condition2 bool) {
if condition1 {
} else if condition1 { // Noncompliant
}
}

    Formatting SQL queries is security-sensitive

    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.

    Copy
    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
}

    Unused function parameters should be removed

    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.

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

    All code should be reachable

    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.

    Copy
    func add(x, y int) int {                   
return x + y // Noncompliant
z := x + y // dead code
}

    URIs should not be hardcoded

    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.

    Copy
    file, err := os.Open("accounts.txt") // Noncompliant
if err != nil {
log.Fatal(err)
}

bs, err := ioutil.ReadFile("accounts.txt") // Noncompliant
if err != nil {
return
}

    Variables should not be self-assigned

    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.

    Copy
    func (user *User) rename(name string) {
name = name  // Noncompliant
}

    if ... else if constructs should end with else clauses

    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.

    Copy
    if x == 0 {                                        
doSomething()
} else if x == 1 {
doSomethingElse()
}

    Control flow statements if, for and switch should not be nested too deeply

    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.

    Copy
    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
		}
	}
}
}

    String literals should not be duplicated

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

    Copy
    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")
}

    switch case clauses should not have too many lines

    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.

    Copy
    func foo(tag int) {
switch tag {
case 0:
	methodCall1()
	methodCall2()
	methodCall3()
	methodCall4()
            methodCall5()
            methodCall6()
case 1:
	bar()
}
}

    Functions should not have identical implementations

    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.

    Copy
    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
}

    Using hardcoded IP addresses is security-sensitive

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

    • CVE-2006-5901

    • CVE-2005-3725

    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.

    Copy
    config, err := ReadConfig("properties.ini")

ip := config["ip"]
port := config["ip"]

SocketClient(ip, port)

    Track uses of TODO tags

    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.

    Copy
    func foo() {
// TODO
}

    All branches in a conditional structure should not have exactly the same implementation

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

    In the following code:

    Unresolved directive in <stdin> - include::{example}[]

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

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

    Setting loose POSIX file permissions is security-sensitive

    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.

    Copy
    import (
"os"
)

func main() {
err := os.Chmod("/tmp/fs", 0777) // Sensitive
if err != nil {
    panic(err)
}
}

    Useless if(true) {...} and if(false){...} blocks should be removed

    `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.

    Copy
    if true {
doSomething()
}

if false {
doSomething()
}

    Empty statements should be removed

    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.

    Copy
    func doSomething() {
;                                                       // Noncompliant
}

func doSomethingElse() {
fmt.Println("doSomethingElse");;     // Noncompliant - double useless ;
...
}

    Using weak hashing algorithms is security-sensitive

    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).

    Copy
    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)
}

    switch statements should have default clauses

    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.

    Copy
    switch tag { // Noncompliant - default case is missing
case 0, 1, 2, 3:
foo()
case 4, 5, 6, 7:
bar()
}

    Hard-coded credentials are security-sensitive

    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:

    • CVE-2019-13466

    • CVE-2018-15389

    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”, …​

    Copy
    func connect()  {
user := getEncryptedUser()
password:= getEncryptedPass() // Compliant

url := "login=" + user + "&passwd=" + password
}

    switch statements should not be nested

    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.

    Copy
    func foo(x,y int) {
switch x {
case 0:
	switch y { // Noncompliant; nested switch
	// ...
	}
case 1:
	// ...
default:
	// ...
}
}

    Local variable and function parameter names should comply with a naming convention

    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.

    Copy
    func doSomething(my_param int) { // Noncompliant
var local_ int; // Noncompliant
...
}
    Assistant
    Responses are generated using AI and may contain mistakes.
    twitterlinkedin
    Powered by Mintlify