From 715198ae3f8bbb6f31cdf055ad013872d96c5d33 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 11:17:45 -0700 Subject: [PATCH] Rewrite cyclomatic_complexity with SwiftSyntax (#5308) --- CHANGELOG.md | 3 + .../Metrics/CyclomaticComplexityRule.swift | 150 +++++++++++------- .../CyclomaticComplexityConfiguration.swift | 13 -- .../CyclomaticComplexityRuleTests.swift | 13 +- 4 files changed, 110 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbd1252c63..4da3be8be7 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index 825ef8a388..9b0aaac4d0 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift +++ b/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 { + 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 } } diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CyclomaticComplexityConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CyclomaticComplexityConfiguration.swift index 46cc15cc75..c7fc2d380a 100644 --- a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CyclomaticComplexityConfiguration.swift +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/CyclomaticComplexityConfiguration.swift @@ -5,15 +5,6 @@ import SwiftLintCore struct CyclomaticComplexityConfiguration: RuleConfiguration { typealias Parent = CyclomaticComplexityRule - private static let defaultComplexityStatements: Set = [ - .forEach, - .if, - .guard, - .for, - .repeatWhile, - .while - ] - @ConfigurationElement private(set) var length = SeverityLevelsConfiguration(warning: 10, error: 20) @ConfigurationElement(key: "ignores_case_statements") @@ -22,8 +13,4 @@ struct CyclomaticComplexityConfiguration: RuleConfiguration { var params: [RuleParameter] { return length.params } - - var complexityStatements: Set { - Self.defaultComplexityStatements.union(ignoresCaseStatements ? [] : [.case]) - } } diff --git a/Tests/SwiftLintFrameworkTests/CyclomaticComplexityRuleTests.swift b/Tests/SwiftLintFrameworkTests/CyclomaticComplexityRuleTests.swift index 85dc74679d..be4233ccab 100644 --- a/Tests/SwiftLintFrameworkTests/CyclomaticComplexityRuleTests.swift +++ b/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)