diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 62b1ad0650..d643a9d6ba 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -19,6 +19,7 @@ public let builtInRules: [any Rule.Type] = [ ClosureEndIndentationRule.self, ClosureParameterPositionRule.self, ClosureSpacingRule.self, + CognitiveComplexityRule.self, CollectionAlignmentRule.self, ColonRule.self, CommaInheritanceRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CognitiveComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CognitiveComplexityRule.swift new file mode 100644 index 0000000000..61d3498bde --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Metrics/CognitiveComplexityRule.swift @@ -0,0 +1,264 @@ +import Foundation +import SwiftSyntax + +@SwiftSyntaxRule +struct CognitiveComplexityRule: Rule { + var configuration = CognitiveComplexityConfiguration() + + static let description = RuleDescription( + identifier: "cognitive_complexity", + name: "Cognitive Complexity", + description: "Cognitive complexity of function bodies should be limited.", + kind: .metrics, + nonTriggeringExamples: [ + Example(""" + func f1(count: Int, buffer: [Int]) -> Int { + if count == 0 + || buffer.count = 0 { + return 0 + } + var sum = 0 + for index in 0.. 0 + && buffer[index] <= 10 { + if buffer.count > 10 { + if buffer[index] % 2 == 0 { + sum += buffer[index] + } else if sum > 0 { + sum -= buffer[index] + } + } + } + } + if sum < 0 { + return -sum + } + return sum + } + """), + Example(""" + func f2(count: Int, buffer: [Int]) -> Int { + var sum = 0 + for index in 0.. 0 && buffer[index] <= 10 { + if buffer.count > 10 { + switch buffer[index] % 2 { + case 0: + if sum > 0 { + sum += buffer[index] + } + default: + if sum > 0 { + sum -= buffer[index] + } + } + } + } + } + return sum + } + """), + ], + triggeringExamples: [ + Example(""" + func f3(count: Int, buffer: [Int]) -> Int { + guard count > 0, + buffer.count > 0 { + return 0 + } + var sum = 0 + for index in 0.. 0 + && buffer[index] <= 10 { + if buffer.count > 10 { + if buffer[index] % 2 == 0 { + sum += buffer[index] + } else if sum > 0 { + sum -= buffer[index] + } else if sum < 0 { + sum += buffer[index] + } + } + } + } + if sum < 0 { + return -sum + } + return sum + } + """), + ] + ) +} + +private extension CognitiveComplexityRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: FunctionDeclSyntax) { + guard let body = node.body else { + return + } + + // for legacy reasons, we try to put the violation in the static or class keyword + let violationToken = node.modifiers.first { element in + let kind = element.name.tokenKind + return kind == .keyword(.static) || kind == .keyword(.class) + }?.name + ?? node.funcKeyword + validate(body: body, violationToken: violationToken) + } + + override func visitPost(_ node: InitializerDeclSyntax) { + guard let body = node.body else { + return + } + + validate(body: body, violationToken: node.initKeyword) + } + + private func validate(body: CodeBlockSyntax, violationToken: TokenSyntax) { + let complexity = ComplexityVisitor( + ignoresLogicalOperatorSequences: configuration.ignoresLogicalOperatorSequences + ).walk(tree: body, handler: \.complexity) + + for parameter in configuration.params where complexity > parameter.value { + let reason = "Function should have cognitive complexity \(configuration.length.warning) or less; " + + "currently complexity is \(complexity)" + + let violation = ReasonedRuleViolation( + position: violationToken.positionAfterSkippingLeadingTrivia, + reason: reason, + severity: parameter.severity + ) + violations.append(violation) + return + } + } + } +} + +private extension ConditionElementListSyntax { + var sequenceCount: Int { + description.components(separatedBy: .newlines).count + } +} + +class ComplexityVisitor: SyntaxVisitor { + private let ignoresLogicalOperatorSequences: Bool + private(set) var complexity = 0 + private var nesting = 0 + + init(ignoresLogicalOperatorSequences: Bool) { + self.ignoresLogicalOperatorSequences = ignoresLogicalOperatorSequences + super.init(viewMode: .sourceAccurate) + } + + func enterNode(_ baseCost: Int? = nil, nesting nestingCost: Int? = nil, other otherCost: Int? = nil) -> SyntaxVisitorContinueKind { + self.complexity += (baseCost ?? 1) + + (nestingCost ?? self.nesting) + + (otherCost ?? 0) + self.nesting += 1 + return .visitChildren + } + + // if + + override func visit(_ node: IfExprSyntax) -> SyntaxVisitorContinueKind { + let baseCost = ignoresLogicalOperatorSequences ? 1 : node.conditions.sequenceCount + + // Only penalize the initial `if`, not the `if else` + let nestingCost = node.parent? + .as(IfExprSyntax.self)? + .elseBody? + .is(IfExprSyntax.self) == true + ? 0 : nesting + + let elseCost = node.elseBody != nil + && !node.elseBody!.is(IfExprSyntax.self) + ? 1 : 0 + + return enterNode(baseCost, nesting: nestingCost, other: elseCost) + } + + override func visitPost(_: IfExprSyntax) { nesting -= 1 } + + // guard + + override func visit(_ node: GuardStmtSyntax) -> SyntaxVisitorContinueKind { + let baseCost = ignoresLogicalOperatorSequences ? 1 : node.conditions.sequenceCount + + return enterNode(baseCost) + } + + override func visitPost(_: GuardStmtSyntax) { nesting -= 1 } + + // ternary + + override func visit(_: TernaryExprSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: TernaryExprSyntax) { nesting -= 1 } + + // switch + + override func visit(_: SwitchExprSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: SwitchExprSyntax) { nesting -= 1 } + + // for + + override func visit(_: ForStmtSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: ForStmtSyntax) { nesting -= 1 } + + // while + + override func visit(_: WhileStmtSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: WhileStmtSyntax) { nesting -= 1 } + + // repeat + + override func visit(_: RepeatStmtSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: RepeatStmtSyntax) { nesting -= 1 } + + // catch + + override func visit(_: CatchClauseSyntax) -> SyntaxVisitorContinueKind { + enterNode() + } + + override func visitPost(_: CatchClauseSyntax) { nesting -= 1 } + + // break & continue + + override func visitPost(_ node: BreakStmtSyntax) { + if node.label != nil { + complexity += 1 + } + } + + override func visitPost(_ node: ContinueStmtSyntax) { + if node.label != nil { + complexity += 1 + } + } + + // closures + + override func visit(_: ClosureExprSyntax) -> SyntaxVisitorContinueKind { + // Only increment nesting + enterNode(0, nesting: 0) + } + + override func visitPost(_: ClosureExprSyntax) { nesting -= 1 } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CognitiveComplexityConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CognitiveComplexityConfiguration.swift new file mode 100644 index 0000000000..13deadb187 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CognitiveComplexityConfiguration.swift @@ -0,0 +1,16 @@ +import SourceKittenFramework +import SwiftLintCore + +@AutoConfigParser +struct CognitiveComplexityConfiguration: RuleConfiguration { + typealias Parent = CognitiveComplexityRule + + @ConfigurationElement(inline: true) + private(set) var length = SeverityLevelsConfiguration(warning: 15, error: 20) + @ConfigurationElement(key: "ignores_logical_operator_sequences") + private(set) var ignoresLogicalOperatorSequences = false + + var params: [RuleParameter] { + length.params + } +} diff --git a/Tests/BuiltInRulesTests/CognitiveComplexityVisitorTests.swift b/Tests/BuiltInRulesTests/CognitiveComplexityVisitorTests.swift new file mode 100644 index 0000000000..77b3c08356 --- /dev/null +++ b/Tests/BuiltInRulesTests/CognitiveComplexityVisitorTests.swift @@ -0,0 +1,208 @@ +@testable import SwiftLintBuiltInRules +import SwiftParser +import TestHelpers +import XCTest + +// FIXME: delete this file once visitor behavior is agreed + +final class CognitiveComplexityVisitorTests: XCTestCase { + func testExamples() { + let tree1 = Parser.parse(source: """ + class c1 { + func f1(b1: Bool, b2: Bool, e1: SomeEnum) -> Int { + if b1 { // +1 + return 1 + } + + var sum = 0 + for i in 0..<10 { // +1 + if i % 2, // +2 (nesting = 1) + i > 3 { // +1 + + switch e1 { // +3 (nesting = 2) + case opt1: + print("abc") + case opt2: + print("def") + default: + continue + } + + do { // +3 (nesting = 2) + print("in do") + } catch { + print(error) + } + } + } + } + } + """) + + XCTAssertEqual(11, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree1, handler: \.complexity)) + + let tree2 = Parser.parse(source: """ + class c2 { + func f1() -> Int { + while true { // +1 + if true { // +1 + 1 + if inner { // +1 + 2 + print("this is internal") + } else { // +1 + print("this is the other") + } + return 1 + } else if false { // +1 + return 0 + } else { // +1 + return 2 + } + } + } + } + """) + + XCTAssertEqual(9, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree2, handler: \.complexity)) + + let tree3 = Parser.parse(source: """ + class c3 { + func overriddenSymbol(from classType: ClassJavaType, + name: String) -> MethodJavaSymbol { + if classType.isUnknown() { // +1 + return Symbols.unknownMethodSymbol + } + + var unknownFound = false + + let symbols = classType.getSymbol().members().lookup(name) + + for overrideSymbol in symbols { // +1 + if overrideSymbol.kind == .method, // +2 (nesting = 1) + !overrideSymbol.isStatic { // +1 + if canOverride(methodSymbol) { // +3 (nesting = 2) + let overriding = checkOverridingParameters(methodSymbol, classType) + if overriding == nil { // +4 (nesting = 3) + if !unknownFound { // +5 (nesting = 4) + unknownFound = true + } + } else if overriding == true { // +1 + return methodSymbol + } + } + } + } + + if unknownFound { // +1 + return Symbols.unknownMethodSymbol + } + + return nil + } + } + // total complexity = 19 + """) + + XCTAssertEqual(19, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree3, handler: \.complexity)) + + let tree4 = Parser.parse(source: """ + class c1 { + func addVersion(entry: inout Entry, + txn: Transaction) throws { + let ti = _persistit.getTransactionIndex() + + while true { // +1 + do { + if first != nil { // +2 (nesting = 1) + if first.getVersion() > entry.getVersion() { // +3 (nesting = 2) + throw RollbackError() + } + if txn.isActive() { // +3 (nesting = 2) + for e in entries { // +4 (nesting = 3) + if depends == TIMED_OUT { // +5 (nesting = 4) + throw RetryException() + } + if depends != 0 // +5 (nesting = 4) + && depends != ABORTED { // +1 + throw RollbackError() + } + } + } + } + entry.setPrevious(first) + first = entry + break + } catch let error as ErrorOne { // +2 (nesting = 1) + do { + let depends = 123 + if depends != 0 // +3 (nesting = 2) + && depends != ABORTED { // +1 + throw RollbackError() + } + } catch { // +3 (nesting = 2) + throw SomeError() + } + } catch { // +2 (nesting = 1) + throw SomeOtherError() + } + } + } + } + // total complexity = 35 + """) + + XCTAssertEqual(35, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree4, handler: \.complexity)) + + let tree5 = Parser.parse(source: """ + class c1 { + func f1() -> Int { + outerLoop: while true { // +1 + for i in 0..<100 { // +2 (nesting = 1) + guard i >= 0, // +3 (nesting = 2) + i != -1 else { // +1 + if i % 2 == 0 { // +4 (nesting = 3) + return 0 + } else { // +1 + return 1 + } + } + + if i > 50 { // +3 (nesting = 2) + break outerLoop // +1 + } + } + } + } + } + """) + + XCTAssertEqual(16, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree5, handler: \.complexity)) + + let tree6 = Parser.parse(source: """ + struct ComplexityView: View { + var body: some View { + VStack(alignment: .center) { // nesting +1 + Text("Stuff") + ForEach(items) { item in // nesting +1 + HStack { // nesting +1 + if let name = item.name { // +4 (nesting = 3) + Text(name) + } else { // +1 + Image(systemName: "exclamation") + } + Image(item.image) + } + } + } + } + } + """) + + XCTAssertEqual(5, ComplexityVisitor(ignoresLogicalOperatorSequences: false) + .walk(tree: tree6, handler: \.complexity)) + } +}