Swift - CodeAnt AI

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

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

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.

var greeting : String!  // Noncompliant

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

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.

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

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.

var greeting: String?

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

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

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.

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

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.

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

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.

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

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.

public class Parent {

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

public class Child extends Parent {

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

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.

foo as! MyClass  // Noncompliant

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

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

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.

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
}

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.

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

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.

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}

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.

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

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 *, +.

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

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
// ...
}

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.

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

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.

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:
// ...
}

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

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

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.

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

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.

@IBOutlet var label: UILabel!  // Noncompliant

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.

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

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.

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
}

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.

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
        /* ... */
      }
    }
  }
}
}

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.

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

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.

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

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.

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

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.

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

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.

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

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`

//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

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

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

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.

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

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

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

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.

class Car {
var color: Int

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

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

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.

public protocol myProtocol {...} // Noncompliant

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.

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

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.

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

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.

Base	Minimum digits
binary	9
decimal	6
octal	9
hexadecimal	9

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

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

enum SomeEnumeration {
case SomeMember  // Non-Compliant
}

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.

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

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

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]*$“

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

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.

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

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

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

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

/*
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 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.

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

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.

if condition {
doSomething()}

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}[]

var x : Int { return 0 } // Compliant by exception

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.

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

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.

for i in 1...10 { // noncompliant, loop only executes once
print("i is \(i)") 
break
}

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.

public class Foo {

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

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

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.

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

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.

if condition1 {
if condition2 {
doSomething()
}
}

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

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.

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

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

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

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.

override doSomething(a: Int, b: Int) {     // no issue reported on b
compute(a)
}

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.

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

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.

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

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.

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

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.

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

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.

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

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}[]

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

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.

var name = "Bob"    // Noncompliant

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

class MyClass {
//...
}

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.

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

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

prepare("a message")  // Noncompliant; duplicated 3 times
execute("a message")
release("a message")

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.

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

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.

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

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

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.

import CryptoSwift

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

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.

public class Foo {
private var foo : String

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

//...

}

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

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.

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

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.

switch myVariable {
case 0: // 6 lines till next case
methodCall1("")
methodCall2("")
methodCall3("")
methodCall4("")
methodCall5("")
case 1:
...
}

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.

func calculate() {
doTheThing()
doOtherThing()
}

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

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

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

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

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

let host = Host(address: configuration.ipAddress)

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.

func doSomething() {
// TODO
}

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.

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

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.

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

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.

class MyClass {
var MyField = 1
}

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.

public protocol Nothing {
}

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

if true {  // Noncompliant
doSomething()
}
...
if false {  // Noncompliant
doSomethingElse()
}

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.

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

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

import CryptoSwift

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

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

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

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

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

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

let postData = "username=\(getEncryptedUser())&password=\(getEncryptedPass())".data(using: .utf8)
//...
var request = URLRequest(url: url)
request.HTTPBody = postData

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.

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

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.

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

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.

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

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

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

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

public class Foo {
public var myField:Int = 0

public func doSomething() {
var myField = 0 /// Noncompliant
// ...
}
}

Get Started