Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cyclomatic complexity ignores case statments #1299

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#1278](https://github.com/realm/SwiftLint/issues/1278)

* Add `ignores_case_statements` as option to `CyclomaticComplexityRule`.
[Michael L. Welles](https://github.com/mlwelles)
[#1298](https://github.com/realm/SwiftLint/issues/1298)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing new line

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, updated LinuxMain + the items mentioned inline.

* Add correctable `redundant_discardable_let` rule that warns when
`let _ = foo()` is used to discard a result from a function instead of
`_ = foo()`.
Expand Down
20 changes: 5 additions & 15 deletions Source/SwiftLintFramework/Rules/CyclomaticComplexityRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Foundation
import SourceKittenFramework

public struct CyclomaticComplexityRule: ASTRule, ConfigurationProviderRule {
public var configuration = SeverityLevelsConfiguration(warning: 10, error: 20)
public var configuration = CyclomaticComplexityConfiguration(warning: 10, error: 20)

public init() {}

Expand Down Expand Up @@ -51,7 +51,7 @@ public struct CyclomaticComplexityRule: ASTRule, ConfigurationProviderRule {
return [StyleViolation(ruleDescription: type(of: self).description,
severity: parameter.severity,
location: Location(file: file, byteOffset: offset),
reason: "Function should have complexity \(configuration.warning) or less: " +
reason: "Function should have complexity \(configuration.length.warning) or less: " +
"currently complexity equals \(complexity)")]
}

Expand All @@ -78,13 +78,13 @@ public struct CyclomaticComplexityRule: ASTRule, ConfigurationProviderRule {
if statementKind == .switch {
hasSwitchStatements = true
}

let score = configuration.complexityStatements.contains(statementKind) ? 1 : 0
return complexity +
(complexityStatements.contains(statementKind) ? 1 : 0) +
score +
measureComplexity(in: file, dictionary: subDict)
}

if hasSwitchStatements {
if hasSwitchStatements && !configuration.ignoresCaseStatements {
return reduceSwitchComplexity(initialComplexity: complexity, file: file, dictionary: dictionary)
}

Expand All @@ -105,14 +105,4 @@ public struct CyclomaticComplexityRule: ASTRule, ConfigurationProviderRule {
return complexity - fallthroughCount
}

private let complexityStatements: [StatementKind] = [
.forEach,
.if,
.case,
.guard,
.for,
.repeatWhile,
.while
]

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//
// CyclomaticComplexityConfiguration.swift
// SwiftLint
//
// Created by Mike Welles on 2/9/17.
// Copyright © 2017 Realm. All rights reserved.
//
import Foundation
import SourceKittenFramework

private enum ConfigurationKey: String {
case warning = "warning"
case error = "error"
case ignoresCaseStatements = "ignores_case_statements"
}

public struct CyclomaticComplexityConfiguration: RuleConfiguration, Equatable {
public var consoleDescription: String {
return length.consoleDescription + ", ignores switch statements: \(ignoresCaseStatements)"
}

public static let defaultComplexityStatements: Set<StatementKind> = [
.forEach,
.if,
.guard,
.for,
.repeatWhile,
.while,
.case
]

private(set) public var length: SeverityLevelsConfiguration

private(set) public var complexityStatements: Set<StatementKind>

private(set) public var ignoresCaseStatements: Bool {
didSet {
if ignoresCaseStatements {
complexityStatements.remove(.case)
} else {
complexityStatements.insert(.case)
}
}
}

var params: [RuleParameter<Int>] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be private(set)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one, no, it's a virtual getter -- returns length.params. The, others yes, updating PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpsim -- Following up on this, since it's the one remaining issue. I can't add a set to this accessor, it's read-only -- returns length.params, which is itself read-only.

return length.params
}

public init(warning: Int, error: Int?, ignoresCaseStatements: Bool = false) {
self.length = SeverityLevelsConfiguration(warning: warning, error: error)
self.complexityStatements = type(of: self).defaultComplexityStatements
self.ignoresCaseStatements = ignoresCaseStatements
}

public mutating func apply(configuration: Any) throws {
if let configurationArray = [Int].array(of: configuration),
!configurationArray.isEmpty {
let warning = configurationArray[0]
let error = (configurationArray.count > 1) ? configurationArray[1] : nil
length = SeverityLevelsConfiguration(warning: warning, error: error)
} else if let configDict = configuration as? [String: Any], !configDict.isEmpty {
for (string, value) in configDict {
guard let key = ConfigurationKey(rawValue:string) else {
throw ConfigurationError.unknownConfiguration
}
switch (key, value) {
case (.error, let intValue as Int):
length.error = intValue
case (.warning, let intValue as Int):
length.warning = intValue
case (.ignoresCaseStatements, let boolValue as Bool):
ignoresCaseStatements = boolValue
default:
throw ConfigurationError.unknownConfiguration
}
}
} else {
throw ConfigurationError.unknownConfiguration
}
}

}

public func == (lhs: CyclomaticComplexityConfiguration, rhs: CyclomaticComplexityConfiguration) -> Bool {
return lhs.length == rhs.length &&
lhs.ignoresCaseStatements == rhs.ignoresCaseStatements
}
16 changes: 14 additions & 2 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@
4DB7815E1CAD72BA00BC4723 /* LegacyCGGeometryFunctionsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4DB7815C1CAD690100BC4723 /* LegacyCGGeometryFunctionsRule.swift */; };
4DCB8E7F1CBE494E0070FCF0 /* RegexHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4DCB8E7D1CBE43640070FCF0 /* RegexHelpers.swift */; };
57ED827B1CF656E3002B3513 /* JUnitReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57ED82791CF65183002B3513 /* JUnitReporter.swift */; };
67932E2D1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67932E2C1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift */; };
67EB4DFA1E4CC111004E9ACD /* CyclomaticComplexityConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */; };
67EB4DFC1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB4DFB1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift */; };
69F88BF71BDA38A6005E7CAE /* OpeningBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */; };
6C7045441C6ADA450003F15A /* SourceKitCrashTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6C7045431C6ADA450003F15A /* SourceKitCrashTests.swift */; };
6CB514E91C760C6900FA02C4 /* Structure+SwiftLint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */; };
Expand Down Expand Up @@ -94,8 +97,8 @@
D286EC021E02DF6F0003CF72 /* SortedImportsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D286EC001E02DA190003CF72 /* SortedImportsRule.swift */; };
D40AD08A1E032F9700F48C30 /* UnusedClosureParameterRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */; };
D40F83881DE9179200524C62 /* TrailingCommaConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */; };
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */; };
D4130D971E16183F00242361 /* IdentifierNameRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4130D961E16183F00242361 /* IdentifierNameRuleExamples.swift */; };
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */; };
D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */; };
D42D2B381E09CC0D00CD7A2E /* FirstWhereRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D42D2B371E09CC0D00CD7A2E /* FirstWhereRule.swift */; };
D4348EEA1C46122C007707FB /* FunctionBodyLengthRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */; };
Expand Down Expand Up @@ -313,6 +316,9 @@
5499CA971A2394B700783309 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
57ED82791CF65183002B3513 /* JUnitReporter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JUnitReporter.swift; sourceTree = "<group>"; };
65454F451B14D73800319A6C /* ControlStatementRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ControlStatementRule.swift; sourceTree = "<group>"; };
67932E2C1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CyclomaticComplexityConfigurationTests.swift; sourceTree = "<group>"; };
67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CyclomaticComplexityConfiguration.swift; sourceTree = "<group>"; };
67EB4DFB1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CyclomaticComplexityRuleTests.swift; sourceTree = "<group>"; };
692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OpeningBraceRule.swift; sourceTree = "<group>"; };
692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatementPositionRule.swift; sourceTree = "<group>"; };
695BE9CE1BDFD92B0071E985 /* CommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CommaRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -368,8 +374,8 @@
D286EC001E02DA190003CF72 /* SortedImportsRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SortedImportsRule.swift; sourceTree = "<group>"; };
D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedClosureParameterRule.swift; sourceTree = "<group>"; };
D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaConfiguration.swift; sourceTree = "<group>"; };
D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TypeNameRuleExamples.swift; sourceTree = "<group>"; };
D4130D961E16183F00242361 /* IdentifierNameRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IdentifierNameRuleExamples.swift; sourceTree = "<group>"; };
D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TypeNameRuleExamples.swift; sourceTree = "<group>"; };
D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantStringEnumValueRule.swift; sourceTree = "<group>"; };
D42D2B371E09CC0D00CD7A2E /* FirstWhereRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FirstWhereRule.swift; sourceTree = "<group>"; };
D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FunctionBodyLengthRuleTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -545,6 +551,7 @@
children = (
D4C4A34A1DEA4FD700E0E04C /* AttributesConfiguration.swift */,
D43B04671E07228D004016AF /* ColonConfiguration.swift */,
67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */,
D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */,
3B034B6C1E0BE544005D49A9 /* LineLengthConfiguration.swift */,
3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */,
Expand Down Expand Up @@ -744,6 +751,8 @@
D46202201E16002A0027AAD1 /* Swift2RulesTests.swift */,
C9802F2E1E0C8AEE008AB27F /* TrailingCommaRuleTests.swift */,
006204DD1E1E4E0A00FFFBE1 /* VerticalWhitespaceRuleTests.swift */,
67EB4DFB1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift */,
67932E2C1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift */,
);
name = SwiftLintFrameworkTests;
path = Tests/SwiftLintFrameworkTests;
Expand Down Expand Up @@ -1219,6 +1228,7 @@
E88DEA731B0984C400A66CB0 /* String+SwiftLint.swift in Sources */,
E88198591BEA95F100333A11 /* LeadingWhitespaceRule.swift in Sources */,
1EC163521D5992D900DD2928 /* VerticalWhitespaceRule.swift in Sources */,
67EB4DFA1E4CC111004E9ACD /* CyclomaticComplexityConfiguration.swift in Sources */,
57ED827B1CF656E3002B3513 /* JUnitReporter.swift in Sources */,
D43B04691E072291004016AF /* ColonConfiguration.swift in Sources */,
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */,
Expand Down Expand Up @@ -1323,10 +1333,12 @@
E86396C71BADAFE6002C9E88 /* ReporterTests.swift in Sources */,
D43B04661E071ED3004016AF /* ColonRuleTests.swift in Sources */,
3B12C9C71C3361CB000B423F /* RuleTests.swift in Sources */,
67EB4DFC1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift in Sources */,
3B30C4A11C3785B300E04027 /* YamlParserTests.swift in Sources */,
D4998DE71DF191380006E05D /* AttributesRuleTests.swift in Sources */,
E88198631BEA9A5400333A11 /* RulesTests.swift in Sources */,
D46202211E16002A0027AAD1 /* Swift2RulesTests.swift in Sources */,
67932E2D1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift in Sources */,
C9802F2F1E0C8AEE008AB27F /* TrailingCommaRuleTests.swift in Sources */,
3B63D46F1E1F09DF0057BE35 /* LineLengthRuleTests.swift in Sources */,
3BCC04D41C502BAB006073C3 /* RuleConfigurationTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//
// CyclomaticComplexityConfigurationTests.swift
// SwiftLint
//
// Created by Michael Welles on 2/15/17.
// Copyright © 2017 Realm. All rights reserved.
//

import SourceKittenFramework
@testable import SwiftLintFramework
import XCTest

class CyclomaticComplexityConfigurationTests: XCTestCase {
func testCyclomaticComplexityConfigurationInitializerSetsLevels() {
let warning = 10
let error = 30
let level = SeverityLevelsConfiguration(warning: warning, error: error)
let configuration1 = CyclomaticComplexityConfiguration(warning: warning,
error: error)
XCTAssertEqual(configuration1.length, level)

let length2 = SeverityLevelsConfiguration(warning: warning, error: nil)
let configuration2 = CyclomaticComplexityConfiguration(warning: warning,
error: nil)
XCTAssertEqual(configuration2.length, length2)
}

func testCyclomaticComplexityConfigurationInitializerSetsIgnoresCaseStatements() {
let configuration1 = CyclomaticComplexityConfiguration(warning: 10,
error: 30,
ignoresCaseStatements: true)
XCTAssertTrue(configuration1.ignoresCaseStatements)

let configuration2 = CyclomaticComplexityConfiguration(warning:0,
error: 30)
XCTAssertFalse(configuration2.ignoresCaseStatements)
}

func testCyclomaticComplexityConfigurationApplyConfigurationWithDictionary() {
var configuration = CyclomaticComplexityConfiguration(warning: 0, error: 0)

let warning1 = 10
let error1 = 30
let length1 = SeverityLevelsConfiguration(warning: warning1, error: error1)
let config1: [String: Any] = ["warning": warning1,
"error": error1,
"ignores_case_statements": true]

let warning2 = 20
let error2 = 40
let length2 = SeverityLevelsConfiguration(warning: warning2, error: error2)
let config2: [String: Int] = ["warning": warning2, "error": error2]
let config3: [String: Bool] = ["ignores_case_statements": false]

do {
try configuration.apply(configuration: config1)
XCTAssertEqual(configuration.length, length1)
XCTAssertTrue(configuration.ignoresCaseStatements)

try configuration.apply(configuration: config2)
XCTAssertEqual(configuration.length, length2)
XCTAssertTrue(configuration.ignoresCaseStatements)

try configuration.apply(configuration: config3)
XCTAssertEqual(configuration.length, length2)
XCTAssertFalse(configuration.ignoresCaseStatements)
} catch {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can mark the test as throws and avoid the do/catch here

XCTFail()
}
}

func testCyclomaticComplexityConfigurationThrowsOnBadConfigValues() {
let badConfigs: [[String: Any]] = [
["warning": true],
["ignores_case_statements": 300],
["unsupported_key": "unsupported key is unsupported"]
]

for badConfig in badConfigs {
var configuration = CyclomaticComplexityConfiguration(warning: 100, error: 150)
checkError(ConfigurationError.unknownConfiguration) {
try configuration.apply(configuration: badConfig)
}
}
}

func testCyclomaticComplexityConfigurationCompares() {
let config1 = CyclomaticComplexityConfiguration(warning: 10, error: 30)
let config2 = CyclomaticComplexityConfiguration(warning: 10, error: 30, ignoresCaseStatements: true)
let config3 = CyclomaticComplexityConfiguration(warning: 10, error: 30, ignoresCaseStatements: false)
let config4 = CyclomaticComplexityConfiguration(warning: 10, error: 40)
let config5 = CyclomaticComplexityConfiguration(warning: 20, error: 30)
XCTAssertFalse(config1 == config2)
XCTAssertTrue(config1 == config3)
XCTAssertFalse(config1 == config4)
XCTAssertFalse(config1 == config5)
}

}

extension CyclomaticComplexityConfigurationTests {
static var allTests: [(String, (CyclomaticComplexityConfigurationTests) -> () throws -> Void)] {
return [
("testCyclomaticComplexityConfigurationInitializerSetsLevels",
testCyclomaticComplexityConfigurationInitializerSetsLevels),
("testCyclomaticComplexityConfigurationInitializerSetsIgnoresCaseStatements",
testCyclomaticComplexityConfigurationInitializerSetsIgnoresCaseStatements),
("testCyclomaticComplexityConfigurationThrowsOnBadConfigValues",
testCyclomaticComplexityConfigurationThrowsOnBadConfigValues),
("testCyclomaticComplexityConfigurationApplyConfigurationWithDictionary",
testCyclomaticComplexityConfigurationApplyConfigurationWithDictionary),
("testCyclomaticComplexityConfigurationCompares",
testCyclomaticComplexityConfigurationCompares)
]
}
}
Loading