Rewrite cyclomatic_complexity with SwiftSyntax (#5308)

CHANGELOG.md

@@ -23,6 +23,9 @@
 * Speed up `closure_parameter_position` rule when there are no violations.  
   [Marcelo Fabri](https://github.com/marcelofabri)
 
+* Rewrite `cyclomatic_complexity` rule using SwiftSyntax.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+
 #### Bug Fixes
 
 * Fix correction of `explicit_init` rule by keeping significant trivia.  

Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift

@@ -1,7 +1,8 @@
 import Foundation
-import SourceKittenFramework
+import SwiftSyntax
 
-struct CyclomaticComplexityRule: ASTRule {
+@SwiftSyntaxRule
+struct CyclomaticComplexityRule: Rule {
     var configuration = CyclomaticComplexityConfiguration()
 
     static let description = RuleDescription(
@@ -22,14 +23,14 @@ struct CyclomaticComplexityRule: ASTRule {
             func f(code: Int) -> Int {
                 switch code {
                 case 0: fallthrough
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
-                case 0: return 1
+                case 1: return 1
+                case 2: return 1
+                case 3: return 1
+                case 4: return 1
+                case 5: return 1
+                case 6: return 1
+                case 7: return 1
+                case 8: return 1
                 default: return 1
                 }
             }
@@ -69,70 +70,109 @@ struct CyclomaticComplexityRule: ASTRule {
             """)
         ]
     )
+}
+
+private extension CyclomaticComplexityRule {
+    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
+        override func visitPost(_ node: FunctionDeclSyntax) {
+            guard let body = node.body else {
+                return
+            }
 
-    func validate(file: SwiftLintFile, kind: SwiftDeclarationKind,
-                  dictionary: SourceKittenDictionary) -> [StyleViolation] {
-        guard SwiftDeclarationKind.functionKinds.contains(kind) else {
-            return []
+            // for legacy reasons, we try to put the violation in the static or class keyword
+            let violationToken = node.modifiers.staticOrClassModifier ?? node.funcKeyword
+            validate(body: body, violationToken: violationToken)
         }
 
-        let complexity = measureComplexity(in: file, dictionary: dictionary)
+        override func visitPost(_ node: InitializerDeclSyntax) {
+            guard let body = node.body else {
+                return
+            }
 
-        for parameter in configuration.params where complexity > parameter.value {
-            let offset = dictionary.offset ?? 0
-            let reason = "Function should have complexity \(configuration.length.warning) or less; " +
-                         "currently complexity is \(complexity)"
-            return [StyleViolation(ruleDescription: Self.description,
-                                   severity: parameter.severity,
-                                   location: Location(file: file, byteOffset: offset),
-                                   reason: reason)]
+            validate(body: body, violationToken: node.initKeyword)
         }
 
-        return []
+        private func validate(body: CodeBlockSyntax, violationToken: TokenSyntax) {
+            let complexity = ComplexityVisitor(
+                ignoresCaseStatements: configuration.ignoresCaseStatements
+            ).walk(tree: body, handler: \.complexity)
+
+            for parameter in configuration.params where complexity > parameter.value {
+                let reason = "Function should have 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 func measureComplexity(in file: SwiftLintFile, dictionary: SourceKittenDictionary) -> Int {
-        var hasSwitchStatements = false
+    private class ComplexityVisitor: SyntaxVisitor {
+        private(set) var complexity = 0
+        let ignoresCaseStatements: Bool
 
-        let complexity = dictionary.substructure.reduce(0) { complexity, subDict in
-            guard subDict.kind != nil else {
-                return complexity
-            }
+        init(ignoresCaseStatements: Bool) {
+            self.ignoresCaseStatements = ignoresCaseStatements
+            super.init(viewMode: .sourceAccurate)
+        }
 
-            if let declarationKind = subDict.declarationKind,
-                SwiftDeclarationKind.functionKinds.contains(declarationKind) {
-                return complexity
-            }
+        override func visitPost(_ node: ForStmtSyntax) {
+            complexity += 1
+        }
 
-            guard let statementKind = subDict.statementKind else {
-                return complexity + measureComplexity(in: file, dictionary: subDict)
-            }
+        override func visitPost(_ node: IfExprSyntax) {
+            complexity += 1
+        }
 
-            if statementKind == .switch {
-                hasSwitchStatements = true
-            }
-            let score = configuration.complexityStatements.contains(statementKind) ? 1 : 0
-            return complexity +
-                score +
-                measureComplexity(in: file, dictionary: subDict)
+        override func visitPost(_ node: GuardStmtSyntax) {
+            complexity += 1
         }
 
-        if hasSwitchStatements && !configuration.ignoresCaseStatements {
-            return reduceSwitchComplexity(initialComplexity: complexity, file: file, dictionary: dictionary)
+        override func visitPost(_ node: RepeatStmtSyntax) {
+            complexity += 1
         }
 
-        return complexity
-    }
+        override func visitPost(_ node: WhileStmtSyntax) {
+            complexity += 1
+        }
 
-    // Switch complexity is reduced by `fallthrough` cases
+        override func visitPost(_ node: CatchClauseSyntax) {
+            complexity += 1
+        }
 
-    private func reduceSwitchComplexity(initialComplexity complexity: Int, file: SwiftLintFile,
-                                        dictionary: SourceKittenDictionary) -> Int {
-        let bodyRange = dictionary.bodyByteRange ?? ByteRange(location: 0, length: 0)
+        override func visitPost(_ node: SwitchCaseSyntax) {
+            if !ignoresCaseStatements {
+                complexity += 1
+            }
+        }
 
-        let contents = file.stringView.substringWithByteRange(bodyRange) ?? ""
+        override func visitPost(_ node: FallThroughStmtSyntax) {
+            // Switch complexity is reduced by `fallthrough` cases
+            if !ignoresCaseStatements {
+                complexity -= 1
+            }
+        }
+
+        override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
+            .skipChildren
+        }
+
+        override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+            .skipChildren
+        }
+    }
+}
 
-        let fallthroughCount = contents.components(separatedBy: "fallthrough").count - 1
-        return complexity - fallthroughCount
+private extension DeclModifierListSyntax {
+    var staticOrClassModifier: TokenSyntax? {
+        first { element in
+            let kind = element.name.tokenKind
+            return kind == .keyword(.static) || kind == .keyword(.class)
+        }?.name
     }
 }

Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CyclomaticComplexityConfiguration.swift

@@ -5,15 +5,6 @@ import SwiftLintCore
 struct CyclomaticComplexityConfiguration: RuleConfiguration {
     typealias Parent = CyclomaticComplexityRule
 
-    private static let defaultComplexityStatements: Set<StatementKind> = [
-        .forEach,
-        .if,
-        .guard,
-        .for,
-        .repeatWhile,
-        .while
-    ]
-
     @ConfigurationElement
     private(set) var length = SeverityLevelsConfiguration<Parent>(warning: 10, error: 20)
     @ConfigurationElement(key: "ignores_case_statements")
@@ -22,8 +13,4 @@ struct CyclomaticComplexityConfiguration: RuleConfiguration {
     var params: [RuleParameter<Int>] {
         return length.params
     }
-
-    var complexityStatements: Set<StatementKind> {
-        Self.defaultComplexityStatements.union(ignoresCaseStatements ? [] : [.case])
-    }
 }

Tests/SwiftLintFrameworkTests/CyclomaticComplexityRuleTests.swift

@@ -12,6 +12,17 @@ class CyclomaticComplexityRuleTests: SwiftLintTestCase {
         return Example(example)
     }()
 
+    private lazy var complexSwitchInitExample: Example = {
+        var example = "init() {\n"
+        example += "    switch foo {\n"
+        for index in (0...30) {
+            example += "  case \(index):   print(\"\(index)\")\n"
+        }
+        example += "    }\n"
+        example += "}\n"
+        return Example(example)
+    }()
+
     private lazy var complexIfExample: Example = {
         let nest = 22
         var example = "func nestThoseIfs() {\n"
@@ -47,7 +58,7 @@ class CyclomaticComplexityRuleTests: SwiftLintTestCase {
 
     func testIgnoresCaseStatementsConfigurationDisabled() {
         let baseDescription = CyclomaticComplexityRule.description
-        let triggeringExamples = baseDescription.triggeringExamples + [complexSwitchExample]
+        let triggeringExamples = baseDescription.triggeringExamples + [complexSwitchExample, complexSwitchInitExample]
         let nonTriggeringExamples = baseDescription.nonTriggeringExamples
 
         let description = baseDescription.with(nonTriggeringExamples: nonTriggeringExamples)