Cyclomatic complexity ignores case statments #1299
Conversation
Generated by 🚫 danger |
…omplexity_ignores_case_statments
Codecov Report
@@ Coverage Diff @@
## master #1299 +/- ##
==========================================
- Coverage 81.93% 81.93% -0.01%
==========================================
Files 170 173 +3
Lines 8660 8817 +157
==========================================
+ Hits 7096 7224 +128
- Misses 1564 1593 +29
Continue to review full report at Codecov.
|
| case error = "error" | ||
| case ignoresCaseStatements = "ignores_case_statements" | ||
|
|
||
| static func all() -> [ConfigurationKey] { |
| .ignoresCaseStatements | ||
| ] | ||
| } | ||
| static func allValues() -> [String] { |
| .repeatWhile, | ||
| .while, | ||
| .case | ||
| ]) |
| return length.consoleDescription + ", ignores switch statements: \(ignoresCaseStatements)" | ||
| } | ||
|
|
||
| public static let defaultComplexityStatements: Set<StatementKind> = Set([ |
There was a problem hiding this comment.
could use array literal conversion here:
public static let defaultComplexityStatements: Set<StatementKind> = [
.forEach,
...| .case | ||
| ]) | ||
|
|
||
| var length: SeverityLevelsConfiguration |
There was a problem hiding this comment.
Can this be private(set)? We use that pattern a lot in configurations.
|
|
||
| private(set) public var complexityStatements: Set<StatementKind> | ||
|
|
||
| var ignoresCaseStatements: Bool { |
| } | ||
| } | ||
|
|
||
| var params: [RuleParameter<Int>] { |
There was a problem hiding this comment.
This one, no, it's a virtual getter -- returns length.params. The, others yes, updating PR.
There was a problem hiding this comment.
@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.
since the cache is per-version and the version number is only bumped for releases, meaning that during development the cache is likely to be incorrect.
…ty configuration class
…omplexity_ignores_case_statments
|
I've updated to add tests for the configuration class, and made all the requested changes except for adding private(set) to params -- since it's a read only property. LMK if there's anything else needed. Thanks! |
| try configuration.apply(configuration: config3) | ||
| XCTAssertEqual(configuration.length, length2) | ||
| XCTAssertFalse(configuration.ignoresCaseStatements) | ||
| } catch { |
There was a problem hiding this comment.
you can mark the test as throws and avoid the do/catch here
| @@ -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) | |||
There was a problem hiding this comment.
OK, updated LinuxMain + the items mentioned inline.
Tests/LinuxMain.swift
Outdated
| @@ -30,5 +30,7 @@ XCTMain([ | |||
| testCase(TrailingCommaRuleTests.allTests), | |||
| testCase(VerticalWhitespaceRuleTests.allTests), | |||
| testCase(YamlParserTests.allTests), | |||
| testCase(YamlSwiftLintTests.allTests) | |||
| testCase(YamlSwiftLintTests.allTests), | |||
There was a problem hiding this comment.
this should should be kept ordered
|
@jpsim @marcelofabri Anything else needed for this? |
|
Thanks for your hard work and patience with this @mlwelles! 👏 |
Per #1298
I've had a couple of instances where pull requests from my team have include dispatch functions that handle every case in an enum trigger cyclomatic complexity warnings / errors. We've all looked over the code, and agree we like it.
So that we don't trigger with examples we made some changes to have the rule optionally disregard case statements in scoring. I've got a PR ready, will submit if there's any interest.
Is there?