From 39dabefd30ef7e76d68398e7d4d63feac14d19e8 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 00:55:32 -0700 Subject: [PATCH 1/6] Rewrite cyclomatic_complexity with SwiftSyntax --- .../Metrics/CyclomaticComplexityRule.swift | 111 ++++++++++-------- 1 file changed, 60 insertions(+), 51 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index 825ef8a388..1b3e309f16 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( @@ -69,70 +70,78 @@ struct CyclomaticComplexityRule: ASTRule { """) ] ) +} - func validate(file: SwiftLintFile, kind: SwiftDeclarationKind, - dictionary: SourceKittenDictionary) -> [StyleViolation] { - guard SwiftDeclarationKind.functionKinds.contains(kind) else { - return [] - } - - let complexity = measureComplexity(in: file, dictionary: dictionary) +private extension CyclomaticComplexityRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: FunctionDeclSyntax) { + 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)] + 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: node.positionAfterSkippingLeadingTrivia, + reason: reason, + severity: parameter.severity + ) + violations.append(violation) + return + } } - - 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 - } - - if let declarationKind = subDict.declarationKind, - SwiftDeclarationKind.functionKinds.contains(declarationKind) { - return complexity - } + init(ignoresCaseStatements: Bool) { + self.ignoresCaseStatements = ignoresCaseStatements + super.init(viewMode: .sourceAccurate) + } - guard let statementKind = subDict.statementKind else { - return complexity + measureComplexity(in: file, dictionary: subDict) - } + override func visitPost(_ node: ForStmtSyntax) { + 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: IfExprSyntax) { + complexity += 1 } - if hasSwitchStatements && !configuration.ignoresCaseStatements { - return reduceSwitchComplexity(initialComplexity: complexity, file: file, dictionary: dictionary) + override func visitPost(_ node: GuardStmtSyntax) { + complexity += 1 } - return complexity - } + override func visitPost(_ node: RepeatStmtSyntax) { + complexity += 1 + } - // Switch complexity is reduced by `fallthrough` cases + override func visitPost(_ node: WhileStmtSyntax) { + 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 + } + } - let fallthroughCount = contents.components(separatedBy: "fallthrough").count - 1 - return complexity - fallthroughCount + override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { + .skipChildren + } } } From 8e20fd8686b816f1ea639e8a1e81bcfb234e0406 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 01:10:05 -0700 Subject: [PATCH 2/6] change position --- .../Rules/Metrics/CyclomaticComplexityRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index 1b3e309f16..4e0321f8e8 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift @@ -88,7 +88,7 @@ private extension CyclomaticComplexityRule { "currently complexity is \(complexity)" let violation = ReasonedRuleViolation( - position: node.positionAfterSkippingLeadingTrivia, + position: node.funcKeyword.positionAfterSkippingLeadingTrivia, reason: reason, severity: parameter.severity ) From 4ab1f4cea685ac480715a80508d99c36d5927c43 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 01:41:50 -0700 Subject: [PATCH 3/6] fixes --- .../Metrics/CyclomaticComplexityRule.swift | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index 4e0321f8e8..6e024fce14 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift @@ -87,8 +87,10 @@ private extension CyclomaticComplexityRule { let reason = "Function should have complexity \(configuration.length.warning) or less; " + "currently complexity is \(complexity)" + // for legacy reasons, we try to put the violation in the static or class keyword + let violationToken = node.modifiers.staticOrClassModifier ?? node.funcKeyword let violation = ReasonedRuleViolation( - position: node.funcKeyword.positionAfterSkippingLeadingTrivia, + position: violationToken.positionAfterSkippingLeadingTrivia, reason: reason, severity: parameter.severity ) @@ -127,6 +129,10 @@ private extension CyclomaticComplexityRule { complexity += 1 } + override func visitPost(_ node: CatchClauseSyntax) { + complexity += 1 + } + override func visitPost(_ node: SwitchCaseSyntax) { if !ignoresCaseStatements { complexity += 1 @@ -145,3 +151,12 @@ private extension CyclomaticComplexityRule { } } } + +private extension DeclModifierListSyntax { + var staticOrClassModifier: TokenSyntax? { + first { element in + let kind = element.name.tokenKind + return kind == .keyword(.static) || kind == .keyword(.class) + }?.name + } +} From 5e3c37d4b7bfbae26efb564ce581d0e5d0759447 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 02:14:05 -0700 Subject: [PATCH 4/6] handle init --- .../Rules/Metrics/CyclomaticComplexityRule.swift | 16 ++++++++++++++-- .../CyclomaticComplexityConfiguration.swift | 13 ------------- .../CyclomaticComplexityRuleTests.swift | 13 ++++++++++++- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index 6e024fce14..ab9ce011c6 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift @@ -79,6 +79,20 @@ private extension CyclomaticComplexityRule { 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) + } + + 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( ignoresCaseStatements: configuration.ignoresCaseStatements ).walk(tree: body, handler: \.complexity) @@ -87,8 +101,6 @@ private extension CyclomaticComplexityRule { let reason = "Function should have complexity \(configuration.length.warning) or less; " + "currently complexity is \(complexity)" - // for legacy reasons, we try to put the violation in the static or class keyword - let violationToken = node.modifiers.staticOrClassModifier ?? node.funcKeyword let violation = ReasonedRuleViolation( position: violationToken.positionAfterSkippingLeadingTrivia, reason: reason, 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) From b30c1cdae0689e0ccfe884cbb918ead2a845742f Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 03:18:06 -0700 Subject: [PATCH 5/6] skip nested init --- .../Metrics/CyclomaticComplexityRule.swift | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift index ab9ce011c6..9b0aaac4d0 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Metrics/CyclomaticComplexityRule.swift @@ -23,14 +23,14 @@ struct CyclomaticComplexityRule: Rule { 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 } } @@ -161,6 +161,10 @@ private extension CyclomaticComplexityRule { override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { .skipChildren } + + override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind { + .skipChildren + } } } From ab7f4c8e8907ccb95a1b89e50b353b90dde3a1ae Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 29 Oct 2023 03:24:13 -0700 Subject: [PATCH 6/6] add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) 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.