CodeAnt AI home pagelight logodark logo
  • Support
  • Dashboard
  • Dashboard
  • Join Community
Start Here
  • What is CodeAnt?
Setup
  • Github
  • Bitbucket
  • Gitlab
  • Azure Devops
Pull Request Review
  • Features
  • Customize Review
  • Quality Gates
  • Integrations
Scan center
  • Code Security
  • Code Quality
  • Cloud Security
  • Engineering Productivity
Integrations
  • Jira
  • Test Coverage
  • CI/CD
IDE
  • Setup
  • Review
  • Enhancements
Rule Reference
  • Compliance
  • Anti-Patterns
    • Pyspark
    • Python
    • Java
    • C / CPP
    • C #
    • JavaScript
    • Jcl
    • Kotlin
    • Kubernetes
    • Abap
    • Apex
    • Azure Source Manager
    • Php
    • Pli
    • Plsql
    • Secrets
    • Swift
    • Terraform
    • Text
    • Tsql
    • Rpg
    • Ruby
    • Scala
    • Vb6
    • Vbnet
    • Xml
    • Flex
    • Go
    • Html
    • Docker
    • Css
    • Cobol
    • Common
  • Code Governance
  • Infrastructure Security Database
  • Application Security Database
Resources
  • Open Source
  • Blogs
Anti-Patterns

Swift

IBInspectable should be used correctly

Adding `IBInspectable to a properly-typed variable makes it available in Xcode’s Interface Builder. But that only works if variable type is declared explicitly as one of the following:

  • Int types, Double, Float, Bool

  • String (or its optional)

  • CGFloat, CGPoint, CGSize, CGRect

  • UIColor, UIImage (and their optionals)

  • NSString, NSColor, NSImage (and their optionals)

  • NSRect, NSPoint, NSSize`,

Copy
@IBInspectable  // Noncompliant; type is implicit
public var cornerRadius = 2.0 {
didSet {
 //...
}
}

Implicitly unwrapped optionals should not be used

The point of using an optional is to signal that the value may be nil and to provide graceful ways of dealing with it if it is nil. While implicitly unwrapped optionals still provide means of dealing with nil values, they also signal that the value won’t be nil, and unwrap it automatically. In addition to sending a decidedly mixed signal, this could lead to runtime errors if the value ever is nil.

It is safest, and clearest to use either an optional or a plain type and avoid the boggy middle ground of implicitly unwrapped optionals.

Copy
var greeting : String!  // Noncompliant

println(greeting)  // At this point the value is nil. Runtime error results

Operator functions should call existing functions

Making an operator a convenience wrapper around an existing function or method provides additional flexibility to users in how the functionality is called and in what options are passed in.

This rule raises an issue when the function that defines the operation of a operator consists of something other than a single function call.

Copy
infix operator >< { associativity right precedence 90 }
func >< (left: Double, right: Double) -> Double {  // Noncompliant
let leftD = (left % 1) * 100
let rightD = (right % 1) * 100
let leftW = (left - leftD) / 100
let rightW = (right - rightD) / 100
return (leftD + leftW) * (rightD + rightW)
}

Optionals should not be force-unwrapped

The point of declaring an optional variable is to make explicit the fact that it might contain no valid value, i.e. nil. Force-unwrapping an optional will lead to a runtime error if the optional does contain nil. Even if the value is tested first, it’s still considered a bad practice to use force-unwrapping. Instead, optional binding or optional chaining should be used.

Copy
var greeting: String?

// ...
println( \(greeting!))  // Noncompliant; could cause a runtime error

if greeting != nil {
println( \(greeting!))  // Noncompliant; better but still not great
}

Trailing closure syntax should not be used when multiple parameters are of function type

Using trailing closure syntax for the last parameter in a call is often the most elegant way to handle it. But if the call requires multiple function-type arguments, the use of a trailing closure can be messy and confusing. In such cases, it’s better to pass closure expressions as normal arguments.

Copy
var x = complexOperation(
arg: 2,
op1: {$0 + 10}
) {$0 * $0}

Classes should use weak delegate references

Classes should only hold `weak references to delegate fields with class type. Otherwise, the owning class will have a strong reference to its delegate, and vice versa, and the OS won’t be able to deallocate either of them..

Note that this only applies to non-computed delegate fields in classes, and not to fields in structs and enum`s.

Copy
class MyClass {
var delegate: ConventionDelegate?  // Noncompliant
}

Conditional compilation should not be used

Conditional compilation is generally recognized as a bad practice that is occasionally necessary when dealing with platform-specific code. As much as possible, code should be refactored to minimize or eliminate conditionally-compiled, platform-specific code because even when necessary and well-intentioned, such code segments can leave your codebase in a hopeless tangle.

Copy
#if os(OSX) // Noncompliant 
let a = 2 
#else 
let a = 3 
#endif

Statements should end with semicolons

In Swift, the semicolon (;) is optional as a statement separator, but omitting semicolons can be confusing.

Copy
var x = 1

Overrides should match their parent class methods in visibility

When a method doesn’t match it’s super method in visibility (public, protected, …​), malicious callers could take advantage of the over-broad access offered by the child class to undermine the application.

Copy
public class Parent {

protected void foo() {
//...
}
}

public class Child extends Parent {

public void foo() {  // Noncompliant
// ...
super.foo();
}
}

Implicit getters should be used

In the interests of clean code, getters should be implicit, rather than explicit for properties that have only getters.

Copy
var luckyNumber: Int {
get {  // Noncompliant
return 7
}
}

Force casts should not be used

Because force casting (as!) does not perform any type safety validations, it is capable of performing dangerous conversions between unrelated types. When the types are truly unrelated, the cast will cause a system crash.

Copy
foo as! MyClass  // Noncompliant

Boolean literals should not be redundant

Redundant boolean literals should be removed from expressions to improve readability.

Copy
if condition == true  { /* ... */ } // Noncompliant
if condition != false { /* ... */ } // Noncompliant
if condition && true  { /* ... */ } // Noncompliant
if condition || false { /* ... */ } // Noncompliant
doSomething(!false)                 // Noncompliant
doSomething(condition == true)      // Noncompliant

v = condition ? true  : false   // Noncompliant
v = condition ? true  : exp     // Noncompliant
v = condition ? false : exp     // Noncompliant
v = condition ? exp   : true    // Noncompliant
v = condition ? exp   : false   // Noncompliant

Methods and field names should not be the same or differ only by capitalization

Looking at the set of methods in a class, struct, enum, or extension and finding two methods that differ only by capitalization is confusing to users of the class. It is similarly confusing to have a method and a field or a case which differ only in capitalization.

Copy
class SomeClass {
var lookUp = false
func lookup(){ }        // Noncompliant; method name differs from field name only by capitalization
func lookUP(){ }        // Noncompliant; method name differs from field and another method name only by capitalization
}

Function type parameters should come at the end of the parameter list

Trailing closure syntax can only be used with the last argument to a function call. Place a function type parameter anywhere else in the list and you limit the options of the caller.

Copy
func foo(p1: Int->Int, p2: Int){  // Noncompliant; p1 should come at the end
print(p1(p2))
}

foo({a in a * 2}, 42) // Trailing closure syntax can't be used here

Trailing closure syntax should be used for all closure parameters at the end of a parameter list

The use of trailing closure syntax can make the code clearer, but it should only be used when all the closure arguments can be passed as trailing closures. The use of a trailing-closure syntax only for some of them can be messy and confusing.

If the function takes a closure parameter in the middle of the parameter list, followed by a non-closure parameter, these parameters cannot use trailing closure syntax. In such a case, passing all the closures consistently as regular parameters inside the parameter list improves readability.

This rule raises an issue when trailing closure syntax is not used or is used partially for a call with all the closure arguments at the end of the parameter list.

Copy
UIView.animateWithDuration(1.0, animations: {  // Noncompliant
self.myView.alpha = 0
})

UIView.animateWithDuration(1.0, animations: {  // Noncompliant, only one closure uses the syntax
self.myView.alpha = 0
}) {
self.blurBg.hidden = true
}

complexAction( // Noncompliant, only one closure uses the syntax
first: {$0 + $1},
repeat: 15
) {$0 * 2}

Redundant pairs of parentheses should be removed

Useless parentheses can sometimes be misleading and so should be removed.

Copy
return ((x + 1))       // Noncompliant
var x = ((y / 2 + 1))  // Noncompliant
if ((x > 0)) { ... }   // Noncompliant

Forbidden super calls should not be made

It is often considered good practice at the end of an override to invoke `super, but there are cases where according to the Apple developer documentation this should not be done.

  • updateLayer - optimize the rendering of your view

  • loadView - provide a view when view is nil

  • providePlaceholder - provide a placeholder for a document returned by the Document Picker but not yet stored locally

In all cases, these are actions that should happen once and only once. Subsequently invoking super` would see your desired result replaced (at best) by less specialized results.

Copy
class VC: UIMyViewController {
override func loadView() {
// ...  
super.loadView()
}
}

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

Using the same value on either side of a binary operator is almost always a mistake. In the case of logical operators, it is either a copy/paste error and therefore a bug, or it is simply wasted 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.

This rule ignores *, +.

Copy
if a == a { // always true
doZ()
}
if  a != a  { // always false
doY()
}
if a == b && a == b { // if the first one is true, the second one is too
doX()
}
if a == b || a == b { // if the first one is true, the second one is too
doW()
}

var j = 5 / 5 //always 1
var k = 5 - 5 //always 0

Floating point numbers should not be tested for equality

Floating point math is imprecise because of the challenges of storing such values in a binary representation. Even worse, floating point math is not associative; push a `Float or a Double through a series of simple mathematical operations and the answer will be different based on the order of those operation because of the rounding that takes place at each step.

Even simple floating point assignments are not simple:

var f: Float = 0.1 // 0.1000000014901161193847656
var d: Double = 0.1 // 0.1000000000000000055511151

Therefore, the use of the equality (==) and inequality (!=) operators on Float or Double` values is almost always an error.

This rule checks for the use of direct and indirect equality/inequailty tests on floats and doubles.

Copy
var myNumber: Float = 0.3 + 0.6

if myNumber == 0.9 { // Noncompliant. Because of floating point imprecision, this will be false
// ...
}

if myNumber <= 0.9 && myNumber >= 0.9 { // Noncompliant indirect equality test
// ...
}

if myNumber < 0.9 || myNumber > 0.9 { // Noncompliant indirect inequality test
// ...
}

try! should not be used

The use of Swift 2.0’s try! lets you execute code that might throw an exception without using the do and catch syntax normally required for such code. By using it, you’re guaranteeing that the executed code will never fail. Murphy’s Law guarantees you’re wrong. And when it does fail, the program will exit abruptly, probably without cleaning up after itself.

Copy
let myvar = try! dangerousCode(foo);  // Noncompliant
// ...

Related if/else if statements and cases in a switch should not have the same condition

A switch and 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
if param == 1 {
openWindow()
} else if param == 2 {
closeWindow()
} else if param == 1 {            // Noncompliant
moveWindowToTheBackground()
}

switch i {
case 1:
//...
case 3:
//...
case 1:                         // Noncompliant
//...
default:
// ...
}

Operators should be surrounded by whitespace in function definitions

Surrounding your operators with whitespace in operator declarations will help maintainers derive meaning from what might otherwise look like a meaningless jumble of punctuation.

Copy
func <*>(a: MyClass, b: MyClass) -> Boolean { // Noncompliant

All code should be reachable

Jump statements (return, break, continue, and fallthrough) move control flow out of the current code block. So any statements that come after a jump are dead code.

Copy
func fun(a:Int)->Int{
var i = 10;
return i + a;
i++;             // this is never executed
}

IBOutlet variables should be private

Marking a variable with IBOutlet allows it to be connected with a Storyboard component through the Interface Builder. Allowing such a variable to be accessed outside the class, may result in other classes making assignments that override the automatic dependency injection from the Storyboard itself.

Copy
@IBOutlet var label: UILabel!  // Noncompliant

Collection sizes comparisons should make sense

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

Copy
if (myArray.count >= 0) { ... } // Noncompliant always true

Infix operators that end with = should update their left operands

The conventional expectation of operators that end with =, such as +=, -=, *=, and so on, is that the result of the operation will be assigned to the operand on the left-hand side of the operator.

Define any other behavior and you almost guarantee that the users of your code will misunderstand and therefore misuse your operator.

Copy
func **= (p1:Int, p2:Int) -> Int {   // Noncompliant. Change operator name or update value of first parameter
return p1 ** p2
}

func => (p1:Int, p2:Int) -> Int {  // Compliant; doesn't end with '='
return p1 ** p1 ** p2
}

Function names should comply with a naming convention

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

Copy
func DoSomething() { // Noncompliant
// ...
}

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

Nested control flow statements if, for, for in, while, do while and switch 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 var i = 0; i < 10; i++ {  // Compliant - depth = 3, not exceeding the limit
    /* ... */
    if condition4 {            // Non-Compliant - depth = 4
      if condition5 {          // Depth = 5, exceeding the limit, but issues are only reported on depth = 4
        /* ... */
      }
    }
  }
}
}

Precedence and associativity of standard operators should not be changed

It is acceptable to override standard operators to provide appropriate behaviors for your classes. But it is not appropriate to change those operators’ associativity or precedence from the standard. Doing so will inevitably lead to misuse and mistakes for users of the class.

Instead of overriding an existing operator’s associativity or precedence, you should either let them use the default values or define a completely new operator.

Copy
infix operator - : CustomAdditionPrecedence   // Noncompliant. For a different behavior create a different operator

precedencegroup CustomAdditionPrecedence {
associativity: right
}

func - (lhs: MyInt, rhs: MyInt) -> MyInt {
// ...
}

var a = MyInt(10), b = MyInt(5), c = MyInt(5)
print(a - b - c) // against expectations, this outputs 10

Access control should be specified for top-level definitions

The access level defaults to internal if left unspecified. Since that doesn’t make sense for most top-level declarations, access levels should always be specified explicitly, even when internal is what’s intended.

This rule raises an issue when the access level is not specified on any top-level declaration.

Copy
class Foo {  // Noncompliant
// ...
}

Trailing closures should not begin on new lines

When the last arguments to a function are closures, it’s possible and often desirable to write these closures after the function’s parentheses. These are called trailing closure arguments. In order to help distinguish trailing closure arguments from independent code blocks, it is best to begin the first closure argument on the same line as the function call and each following closure argument on the last line of the preceding one.

Copy
funWithClosureArgument()
{ // Noncompliant; looks like an independent code block
print("Hello world")
}

funWith3ClosureArguments {
print("Hello world")
} b:
{  // Noncompliant; looks like an independent code block
print("Hello world")
}
c: {  // Noncompliant; looks like an independent code block
print("Hello world")
}

get should be omitted in read-only computed properties and subscripts

For read-only computed properties and subscript declarations, the get keyword and its braces are optional, and should be omitted for the sake of brevity.

Copy
struct Magic {
var number:Int {
get {  // Noncompliant
  return 42
}
}
}

Parentheses should be omitted when trailing closure is the only argument

If a closure expression is provided as the function or method’s only argument and you provide that expression as a trailing closure, you do not need to write a pair of parentheses () after the function or method’s name when you call the function. This makes the code somewhat easier to read.

Copy
reversedNames = names.sorted() { $0 > $1 } // Noncompliant

MARK comments should be formatted correctly

A properly formatted (book)mark comment adds an entry in Xcode’s quick jump bar. But if the formatting is incorrect, then it’s just a comment. A `MARK comment must be formatted in one of the following ways

  • // MARK: text - Adds text to quick jump bar

  • // MARK: - - Adds hr to quick jump bar

  • // MARK: - text - Adds HR, followed by text to quick jump bar

  • // BOOKMARK

  • // BOOKMARKS

  • //BOOKMARK`

Copy
//MARK: -  // Noncompliant; leading space missing
//&nbsp;&nbsp;MARK: - // Noncompliant; too many leading spaces
// MARK -  // Noncompliant; missing colon
// MARK:  // Noncompliant; must contain text or dash
// MARK:&nbsp;&nbsp;foo  // Noncompliant; too many spaces after colon

Multiple variables should not be declared on the same line

Declaring multiple variables on one line is difficult to read.

Copy
var i = 1, j = 2

isEmpty should be used to test for emptiness

Using Int.count to test for emptiness works, but using Int.isEmpty makes the code more readable and can be more performant. The time complexity of any isEmpty implementation should be O(1) whereas some implementations of count() can be O(n).

Copy
if (arr.count == 0) {  // Noncompliant
/* ... */
}

let uses should be minimized

Brevity may be the soul of wit, but concise (yet readable!) code is the soul of good programming. For that reason, you should never use a let or var keyword that can be left out with the same effect.

Copy
if let x = x, let y = y {  // Noncompliant
// ...
}

if let p = p, var q = q {
// ...
}

if case (let x?, let y?) = foo {  // Noncompliant 
// ...
}

self should only be used when required

The use of self is optional except when in closure expressions, and when it’s needed to distinguish between property names and arguments. For the sake of brevity, self should be omitted when it’s not strictly required.

Copy
class Car {
var color: Int

init(color: Int) {
self.color = color
}

func fade() {
self.color--  // Noncompliant
}
}

Protocol names should comply with a naming convention

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 protocol names match a provided regular expression.

Copy
public protocol myProtocol {...} // Noncompliant

Functions should not return constants

There’s no point in forcing the overhead of a function or method call for a function that always returns the same constant value. Even worse, the fact that a function call must be made will likely mislead developers who call the method thinking that something more is done. Declare a constant instead.

This rule raises an issue on functions that contain only one statement: the return of a constant value.

Copy
func getBestNumber() -> Int {
return 12  // Noncompliant
}

break should be the only statement in a case

Because case statements in a Swift switch do not fall through, there is no need to use break at the end of a case unless it would otherwise be empty. Since an empty case isn’t allowed, an explicit break is needed to make such code compilable. There is no other reason to use break in a case.

Copy
switch weekday {
case sunday:
break
case monday:
getUpEarly()
break  // Noncompliant
case tuesday
// ...
}

Underscores should be used to make large numbers readable

In Swift it is possible to add underscores (’_’) to numeric literals to enhance readability. The addition of underscores in this manner has no semantic meaning, but makes it easier for maintainers to understand the code.

The number of digits to the left of a decimal point needed to trigger this rule varies by base.

BaseMinimum digits

binary

9

decimal

6

octal

9

hexadecimal

9

Copy
let i = 10000000  // Noncompliant; is this 10 million or 100 million?
let j = 0b01101001010011011110010101011110  // Noncompliant
let l = 0x7fffffffffffffff  // Noncompliant

Enumeration members should comply with a naming convention

Shared coding conventions allow teams to collaborate efficiently. This rule checks that all enumeration member names match a provided regular expression.

Copy
enum SomeEnumeration {
case SomeMember  // Non-Compliant
}

Backticks should not be used around symbol names

You can’t create a variable named “for”. Unless you put backticks (`) around it.

Since that would be the first step down a slippery slope of hopeless confusion, backticks should be removed from identifier names - whether they’re keywords or not - and the identifiers renamed as required.

Copy
var `for` = 1   // Noncompliant
for (var `in` = 0; `in` < 10 && `for` > 0; `in`++) {  // Noncompliant
// ...
}

var `x` = "hello"  // Noncompliant; why would you do this?

struct names should comply with a naming convention

Sharing some naming conventions enables teams to collaborate more efficiently. This rule checks that all struct names match a provided regular expression.

Using the default regular expression: ”^[A-Z][a-zA-Z0-9]*$“

Copy
struct my_struct {
var one : Int
var two : Int
}

return should be omitted from single-expression closures

When a closure contains only a return statement, the return itself can be omitted.

Copy
someList.sort { a, b in
return a > b
}

Filter conditions should be used as predicates to first

If you only want one instance that matches certain criteria out of a collection, it’s far more efficient to grab the first matching item than it is to fully filter the collection for your criteria and then only use a single value.

Copy
let one = arr.filter { $0.containsString("yo") }.first  // Noncompliant

Classes should only remove themselves as observers in deinit

Classes can register themselves to receive notifications using NotificationCenter.add. Having done so, it seems suspicious that a class would opt to stop receiving notifications before de-initialization. For that reason, this rule raises an issue when NotificationCenter.default.removeObserver(self) is called anywhere but in deinit

Copy
class MyClass {
func doTheThing() {
//...
NotificationCenter.default.removeObserver(self)  // Noncompliant
}
}

Comments should not be nested

Nested comments are confusing and can lead maintainers to misunderstand which code is active.

Copy
/*
This is a comment block.
It may be difficult to figure out that the following line of code is actually commented


variable = function_call();
/* variable contains the result. Noncompliant; inner comment */
*/

Jump statements should not be redundant

Jump statements such as return and continue let you change the default flow of program execution, but jump statements that direct the control flow to the original direction are just a waste of keystrokes.

Copy
var i:Int
for i in 1...10 {
print("i is \(i)")
continue  // this is meaningless; the loop would continue anyway
}

Boolean checks should not be inverted

It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.

Copy
if !(a == 2) {...}  // Noncompliant
let b = !(i < 10)  // Noncompliant

A close curly brace should be located at the beginning of a line

Shared coding conventions make it possible for a team to efficiently collaborate. This rule makes it mandatory to place a close curly brace at the beginning of a line.

Copy
if condition {
doSomething()}

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
var x : Int { return 0 } // 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 { ... }

Collection elements should not be replaced unconditionally

Copy
letters["a"] = "Apple"
letters["a"] = "Boy"  // Noncompliant

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 in 1...10 { // noncompliant, loop only executes once
print("i is \(i)") 
break
}

Deprecated code should be removed

This rule is meant to be used as a way to track code which is marked as being deprecated. Deprecated code should eventually be removed.

Copy
public class Foo {

@availability(*, deprecated=1.1)   // Noncompliant
public func bar() {
}

@availability(*, obsoleted=1.1)  // Noncompliant
public func baz() {
}
}

Sequential tests should not check the same condition

When the same condition is checked twice in a row, it is either confusing - why have separate checks? - or an error - some other condition should have been checked in the second test.

Copy
if a == b { // Compliant; a reassigned in previous block
doSomething(b)
}
if a == b {  // Noncompliant; is this really what was intended?
doTheThing(c)
}

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 {
doSomething()
}
}

if let y = someOptional {
if x > 0 {
doSomething()
} 
}

Functions and closures 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
}

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
override doSomething(a: Int, b: Int) {     // no issue reported on b
compute(a)
}

switch statements should have at least 3 case clauses

switch statements are useful when there are many different cases depending on the value of the same expression.

For just one or two cases, however, the code will be more readable with if statements.

Copy
switch (variable) {
case 0:
doSomething();
default:
doSomethingElse();
}

An open curly brace should be located at the end of a line

Shared naming conventions allow teams to collaborate effectively. This rule raises an issue when an open curly brace is not placed at the end of a line of code.

Copy
if condition
{
doSomething()
}

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
public class Foo {
public func listUsers() -> [User] {
    var users:[User]
    let location = "/home/mylogin/Dev/users.txt"     // Non-Compliant
    let fileContent = NSString(contentsOfFile: location, encoding: NSUTF8StringEncoding, error: nil)
    users = parse(fileContent!)
    return users
}
}

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

Enumeration types should comply with a naming convention

Shared naming conventions allow teams to collaborate efficiently. This rule checks that all enum names match a provided regular expression.

Copy
enum someEnumeration { // Non-Compliant
case Bar
}

Unused labels should be removed

If a label is declared but not used in the program, it can be considered as dead code and should therefore be removed.

This will improve maintainability as developers will not wonder what this label is used for.

Copy
whileLoopLabel: while x > 0 {    // Noncompliant
x -= 1
}

Functions should not contain too many return statements

Having too many return statements in a function increases the function’s essential complexity because the flow of execution is broken each time a return statement is encountered. This makes it harder to read and understand the logic of the function.

Copy
func myMethod() -> Bool { // Noncompliant as there are 4 return statements
if condition1 {
return true
} else {
if condition2 {
  return false
} else {
  return true
}
}
return false
}

Ternary operators should not be nested

Nested ternaries are hard to read and can make the order of operations complex to understand.

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

Instead, use another line to express the nested operation in a separate statement.

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

Copy
func getReadableStatus(job: Job) -> String {
return job.isRunning ? "Running" : job.hasErrors ? "Failed" : "Succeeded";  // Noncompliant
}

Functions and variables should not be defined outside of classes

Defining and using global variables and global functions, when the convention dictates OOP can be confusing and difficult to use properly for multiple reasons:

  • You run the risk of name clashes.

  • Global functions must be stateless, or they can cause difficult-to-track bugs.

  • Global variables can be updated from anywhere and may no longer hold the value you expect.

  • It is difficult to properly test classes that use global functions.

Instead of being declared globally, such variables and functions should be moved into a class, potentially marked static, so they can be used without a class instance.

This rule checks that only object-oriented programming is used and that no functions or procedures are declared outside of a class.

Copy
var name = "Bob"    // Noncompliant

func doSomething() {   // Noncompliant
//...
}

class MyClass {
//...
}

Increment (++) and decrement (--) operators should not be used in a method call or mixed with other operators in an expression

The use of increment and decrement operators in method calls or in combination with other arithmetic operators is not recommended, because:

  • It can significantly impair the readability of the code.

  • It introduces additional side effects into a statement, with the potential for undefined behavior.

  • It is safer to use these operators in isolation from any other arithmetic operators.

Copy
u8a = ++u8b + u8c--
foo = bar++ / 4

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
prepare("a message")  // Noncompliant; duplicated 3 times
execute("a message")
release("a message")

Modulus results should not be checked for direct equality

When the modulus of a negative number is calculated, the result will either be negative or zero. Thus, comparing the modulus of a variable for equality with a positive number (or a negative one) could result in unexpected results.

Copy
func isOdd(x:Int) -> Bool {
return x % 2 == 1  // Noncompliant; if x is negative, x % 2 == -1
}

Neither DES (Data Encryption Standard) nor DESede (3DES) should be used

According to the US National Institute of Standards and Technology (NIST), the Data Encryption Standard (DES) is no longer considered secure:

For similar reasons, RC2 should also be avoided.

Copy
let cryptor = try Cryptor(operation: .encrypt, algorithm: .des, options: .none, key: key, iv: []) // Noncompliant

let crypt = CkoCrypt2()
crypt.CryptAlgorithm = "3des" // Noncompliant

SHA-1 and Message-Digest hash algorithms should not be used in secure contexts

The MD5 algorithm and its successor, SHA-1, are no longer considered secure, because it is too easy to create hash collisions with them. That is, it takes too little computational effort to come up with a different input that produces the same MD5 or SHA-1 hash, and using the new, same-hash value gives an attacker the same access as if he had the originally-hashed value. This applies as well to the other Message-Digest algorithms: MD2, MD4, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160.

Consider using safer alternatives, such as SHA-256, SHA-512 or SHA-3.

Copy
import CryptoSwift

let bytes:Array<UInt8> = [0x01, 0x02, 0x03]
let digest = input.md5() // Noncompliant

A field should not duplicate the name of its containing class

It’s confusing to have a class member with the same name (case differences aside) as its enclosing class. This is particularly so when you consider the common practice of naming a class instance for the class itself.

Best practice dictates that any field or member with the same name as the enclosing class be renamed to be more descriptive of the particular aspect of the class it represents or holds.

Copy
public class Foo {
private var foo : String

public func getFoo() -> String {
 return foo
}

//...

}

var foo = Foo()
foo.getFoo() // what does this return?

Comments should not be located at the end of lines of code

This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.

Copy
var a1 = b + c // This is a trailing comment that can be very very long

switch case clauses should not have too many lines of code

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
switch myVariable {
case 0: // 6 lines till next case
methodCall1("")
methodCall2("")
methodCall3("")
methodCall4("")
methodCall5("")
case 1:
...
}

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 calculate() {
doTheThing()
doOtherThing()
}

func doEverything() {  // Noncompliant: duplicates calculate
doTheThing()
doOtherThing()
}

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
let host = Host(address: configuration.ipAddress)

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 doSomething() {
// 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()
}

Type parameter names should comply with a naming convention

Shared naming conventions make it possible for a team to collaborate efficiently. Following the established convention of single-letter type parameter names helps users and maintainers of your code quickly see the difference between a type parameter and a poorly named class.

This rule check that all type parameter names match a provided regular expression. The following code snippets use the default regular expression.

Copy
public class MyClass<TYPE> {    // Noncompliant
func method<TYPE>(t : TYPE) { // Noncompliant
}
}

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

The goal of a naming convention is 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 field names match a provided regular expression.

Copy
class MyClass {
var MyField = 1
}

Unused private functions should be removed

This rule raises an issue when a {visibility} {operationName} is never referenced in the code.

Copy
class Foo {    
private func unusedMethod() {...} // Noncompliant
}

Classes should not be empty

There is no good excuse for an empty class. If it’s being used simply as a common extension point, it should be replaced with an interface. If it was stubbed in as a placeholder for future development it should be fleshed-out. In any other case, it should be eliminated.

Copy
public protocol Nothing {
}

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 {  // Noncompliant
doSomething()
}
...
if false {  // Noncompliant
doSomethingElse()
}

The ternary operator should not return the same value regardless of the condition

When the second and third operands of a ternary operator are the same, the operator will always return the same value regardless of the condition. Either the operator itself is pointless, or a mistake was made in coding it.

Copy
func canVote(person:Person) -> Bool {
return person.age > 18 ? true : true // Noncompliant; is this what was intended?
}

Files should not be empty

Files with no lines of code clutter a project and should be removed.

Copy
//import Foundation
//
//public class Bar {}

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 CryptoSwift

let bytes:Array<UInt8> = [0x01, 0x02, 0x03]
let digest = input.sha512() // Compliant

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
let postData = "username=\(getEncryptedUser())&password=\(getEncryptedPass())".data(using: .utf8)
//...
var request = URLRequest(url: url)
request.HTTPBody = postData

Duplicate values should not be passed as arguments

There are valid cases for passing a variable multiple times into the same method call, but usually doing so is a mistake, and something else was intended for one of the arguments.

Copy
if equal(myPoint.x, myPoint.x) {  // Noncompliant
//...
}

Unused local variables should be removed

An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.

Copy
for i in 1...10 {  // Ignored
print("Hello! ");
}

Unused assignments should be removed

Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are unnecessary and don’t contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code cleanliness and readability. Even if the unnecessary operations do not do any harm in terms of the program’s correctness, they are - at best - a waste of computing resources.

Copy
func foo(y: Int) -> Int {
var x = 100  // Noncompliant: dead store
x = 150      // Noncompliant: dead store
x = 200
return x + y
}

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 printSomething(param_:String){ // Noncompliant
var Text = "df" // Noncompliant
Text = Text + param_
print(Text)
}

Local variables should not have the same name as fields or enum cases

Local variables should not have the same name as fields or “enum” cases

Copy
public class Foo {
public var myField:Int = 0

public func doSomething() {
var myField = 0 /// Noncompliant
// ...
}
}
SecretsTerraform
twitterlinkedin
Powered by Mintlify
Assistant
Responses are generated using AI and may contain mistakes.