From 51d47d492c27b1e069f1c9ca6415f3277fe7dc72 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Wed, 13 Mar 2019 10:03:53 -0700 Subject: [PATCH] `contains_over_first_not_nil` rule now also checks for `firstIndex(where:)` Fixes #2678 --- CHANGELOG.md | 4 ++ Rules.md | 42 ++++++++++++++++- .../Protocols/CallPairRule.swift | 5 +- .../ContainsOverFirstNotNilRule.swift | 46 +++++++++++-------- SwiftLint.xcodeproj/project.pbxproj | 4 ++ .../AutomaticRuleTests.generated.swift | 6 --- .../ContainsOverFirstNotNilRuleTests.swift | 36 +++++++++++++++ 7 files changed, 116 insertions(+), 27 deletions(-) create mode 100644 Tests/SwiftLintFrameworkTests/ContainsOverFirstNotNilRuleTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ae643b861..2fd76b018e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,10 @@ [Matthew Healy](https://github.com/matthew-healy) [#2595](https://github.com/realm/SwiftLint/2595) +* `contains_over_first_not_nil` rule now also checks for `firstIndex(where:)`. + [Marcelo Fabri](https://github.com/marcelofabri) + [#2678](https://github.com/realm/SwiftLint/issues/2678) + #### Bug Fixes * Fix bug where SwiftLint ignores excluded files list in a nested configuration diff --git a/Rules.md b/Rules.md index d157789f1a..f7e23bf86f 100644 --- a/Rules.md +++ b/Rules.md @@ -2519,7 +2519,7 @@ Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Mi --- | --- | --- | --- | --- | --- `contains_over_first_not_nil` | Disabled | No | performance | No | 3.0.0 -Prefer `contains` over `first(where:) != nil` +Prefer `contains` over `first(where:) != nil` and `firstIndex(where:) != nil`. ### Examples @@ -2536,6 +2536,16 @@ let first = myList.first { $0 % 2 == 0 } ``` +```swift +let firstIndex = myList.firstIndex(where: { $0 % 2 == 0 }) + +``` + +```swift +let firstIndex = myList.firstIndex { $0 % 2 == 0 } + +``` +
Triggering Examples @@ -2570,6 +2580,36 @@ let first = myList.first { $0 % 2 == 0 } ``` +```swift +↓myList.firstIndex { $0 % 2 == 0 } != nil + +``` + +```swift +↓myList.firstIndex(where: { $0 % 2 == 0 }) != nil + +``` + +```swift +↓myList.map { $0 + 1 }.firstIndex(where: { $0 % 2 == 0 }) != nil + +``` + +```swift +↓myList.firstIndex(where: someFunction) != nil + +``` + +```swift +↓myList.map { $0 + 1 }.firstIndex { $0 % 2 == 0 } != nil + +``` + +```swift +(↓myList.firstIndex { $0 % 2 == 0 }) != nil + +``` +
diff --git a/Source/SwiftLintFramework/Protocols/CallPairRule.swift b/Source/SwiftLintFramework/Protocols/CallPairRule.swift index 6968ad7704..ce17bcf49a 100644 --- a/Source/SwiftLintFramework/Protocols/CallPairRule.swift +++ b/Source/SwiftLintFramework/Protocols/CallPairRule.swift @@ -23,6 +23,7 @@ extension CallPairRule { - patternSyntaxKinds: Syntax kinds matches should have - callNameSuffix: Suffix of the first method call name - severity: Severity of violations + - reason: The reason of the generated violations - predicate: Predicate to apply after checking callNameSuffix */ internal func validate(file: File, @@ -30,6 +31,7 @@ extension CallPairRule { patternSyntaxKinds: [SyntaxKind], callNameSuffix: String, severity: ViolationSeverity, + reason: String? = nil, predicate: ([String: SourceKitRepresentable]) -> Bool = { _ in true }) -> [StyleViolation] { let firstRanges = file.match(pattern: pattern, with: patternSyntaxKinds) let contents = file.contents.bridge() @@ -59,7 +61,8 @@ extension CallPairRule { return violatingLocations.map { StyleViolation(ruleDescription: type(of: self).description, severity: severity, - location: Location(file: file, byteOffset: $0)) + location: Location(file: file, byteOffset: $0), + reason: reason) } } diff --git a/Source/SwiftLintFramework/Rules/Performance/ContainsOverFirstNotNilRule.swift b/Source/SwiftLintFramework/Rules/Performance/ContainsOverFirstNotNilRule.swift index 7a24ccf450..e574d1c6b9 100644 --- a/Source/SwiftLintFramework/Rules/Performance/ContainsOverFirstNotNilRule.swift +++ b/Source/SwiftLintFramework/Rules/Performance/ContainsOverFirstNotNilRule.swift @@ -1,6 +1,6 @@ import SourceKittenFramework -public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, ConfigurationProviderRule, AutomaticTestableRule { +public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, ConfigurationProviderRule { public var configuration = SeverityConfiguration(.warning) public init() {} @@ -8,27 +8,35 @@ public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, Configuratio public static let description = RuleDescription( identifier: "contains_over_first_not_nil", name: "Contains over first not nil", - description: "Prefer `contains` over `first(where:) != nil`", + description: "Prefer `contains` over `first(where:) != nil` and `firstIndex(where:) != nil`.", kind: .performance, - nonTriggeringExamples: [ - "let first = myList.first(where: { $0 % 2 == 0 })\n", - "let first = myList.first { $0 % 2 == 0 }\n" - ], - triggeringExamples: [ - "↓myList.first { $0 % 2 == 0 } != nil\n", - "↓myList.first(where: { $0 % 2 == 0 }) != nil\n", - "↓myList.map { $0 + 1 }.first(where: { $0 % 2 == 0 }) != nil\n", - "↓myList.first(where: someFunction) != nil\n", - "↓myList.map { $0 + 1 }.first { $0 % 2 == 0 } != nil\n", - "(↓myList.first { $0 % 2 == 0 }) != nil\n" - ] + nonTriggeringExamples: ["first", "firstIndex"].flatMap { method in + return [ + "let \(method) = myList.\(method)(where: { $0 % 2 == 0 })\n", + "let \(method) = myList.\(method) { $0 % 2 == 0 }\n" + ] + }, + triggeringExamples: ["first", "firstIndex"].flatMap { method in + return [ + "↓myList.\(method) { $0 % 2 == 0 } != nil\n", + "↓myList.\(method)(where: { $0 % 2 == 0 }) != nil\n", + "↓myList.map { $0 + 1 }.\(method)(where: { $0 % 2 == 0 }) != nil\n", + "↓myList.\(method)(where: someFunction) != nil\n", + "↓myList.map { $0 + 1 }.\(method) { $0 % 2 == 0 } != nil\n", + "(↓myList.\(method) { $0 % 2 == 0 }) != nil\n" + ] + } ) public func validate(file: File) -> [StyleViolation] { - return validate(file: file, - pattern: "[\\}\\)]\\s*!=\\s*nil", - patternSyntaxKinds: [.keyword], - callNameSuffix: ".first", - severity: configuration.severity) + let pattern = "[\\}\\)]\\s*!=\\s*nil" + let firstViolations = validate(file: file, pattern: pattern, patternSyntaxKinds: [.keyword], + callNameSuffix: ".first", severity: configuration.severity, + reason: "Prefer `contains` over `first(where:) != nil`") + let firstIndexViolations = validate(file: file, pattern: pattern, patternSyntaxKinds: [.keyword], + callNameSuffix: ".firstIndex", severity: configuration.severity, + reason: "Prefer `contains` over `firstIndex(where:) != nil`") + + return firstViolations + firstIndexViolations } } diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 66fb60dc23..7b923861a0 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -332,6 +332,7 @@ D4DABFD91E2C59BC009617B6 /* NotificationCenterDetachmentRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DABFD81E2C59BC009617B6 /* NotificationCenterDetachmentRuleExamples.swift */; }; D4DAE8BC1DE14E8F00B0AE7A /* NimbleOperatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */; }; D4DB92251E628898005DE9C1 /* TodoRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DB92241E628898005DE9C1 /* TodoRuleTests.swift */; }; + D4DDFF9822396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DDFF9722396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift */; }; D4DE9133207B4750000FFAA8 /* UnavailableFunctionRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DE9131207B4731000FFAA8 /* UnavailableFunctionRule.swift */; }; D4E2BA851F6CD77B00E8E184 /* ArrayInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4E2BA841F6CD77B00E8E184 /* ArrayInitRule.swift */; }; D4E92D1F2137B4C9002EDD48 /* IdenticalOperandsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4E92D1E2137B4C9002EDD48 /* IdenticalOperandsRule.swift */; }; @@ -817,6 +818,7 @@ D4DABFD81E2C59BC009617B6 /* NotificationCenterDetachmentRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NotificationCenterDetachmentRuleExamples.swift; sourceTree = ""; }; D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NimbleOperatorRule.swift; sourceTree = ""; }; D4DB92241E628898005DE9C1 /* TodoRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TodoRuleTests.swift; sourceTree = ""; }; + D4DDFF9722396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContainsOverFirstNotNilRuleTests.swift; sourceTree = ""; }; D4DE9131207B4731000FFAA8 /* UnavailableFunctionRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnavailableFunctionRule.swift; sourceTree = ""; }; D4E2BA841F6CD77B00E8E184 /* ArrayInitRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArrayInitRule.swift; sourceTree = ""; }; D4E92D1E2137B4C9002EDD48 /* IdenticalOperandsRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdenticalOperandsRule.swift; sourceTree = ""; }; @@ -1415,6 +1417,7 @@ E809EDA21B8A73FB00399043 /* ConfigurationTests.swift */, F480DC7E1F26090000099465 /* ConfigurationTests+Nested.swift */, F480DC821F2609D700099465 /* ConfigurationTests+ProjectMock.swift */, + D4DDFF9722396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift */, 3BB47D861C51DE6E00AE6A10 /* CustomRulesTests.swift */, 67932E2C1E54AF4B00CB0629 /* CyclomaticComplexityConfigurationTests.swift */, 67EB4DFB1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift */, @@ -2198,6 +2201,7 @@ 3B12C9C31C320A53000B423F /* YamlSwiftLintTests.swift in Sources */, D41985ED21FAD033003BE2B7 /* DeploymentTargetConfigurationTests.swift in Sources */, D450D1E221F19B1E00E60010 /* TrailingClosureConfigurationTests.swift in Sources */, + D4DDFF9822396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift in Sources */, E832F10D1B17E725003F265F /* IntegrationTests.swift in Sources */, D4C27C001E12DFF500DF713E /* LinterCacheTests.swift in Sources */, D45255C81F0932F8003C9B56 /* RuleDescription+Examples.swift in Sources */, diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index 159531ebf3..c6572ba314 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -66,12 +66,6 @@ class CommaRuleTests: XCTestCase { } } -class ContainsOverFirstNotNilRuleTests: XCTestCase { - func testWithDefaultConfiguration() { - verifyRule(ContainsOverFirstNotNilRule.description) - } -} - class ControlStatementRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(ControlStatementRule.description) diff --git a/Tests/SwiftLintFrameworkTests/ContainsOverFirstNotNilRuleTests.swift b/Tests/SwiftLintFrameworkTests/ContainsOverFirstNotNilRuleTests.swift new file mode 100644 index 0000000000..50e7f03959 --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/ContainsOverFirstNotNilRuleTests.swift @@ -0,0 +1,36 @@ +import SwiftLintFramework +import XCTest + +class ContainsOverFirstNotNilRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(ContainsOverFirstNotNilRule.description) + } + + // MARK: - Reasons + + func testFirstReason() { + let string = "↓myList.first { $0 % 2 == 0 } != nil" + let violations = self.violations(string) + + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations.first?.reason, "Prefer `contains` over `first(where:) != nil`") + } + + func testFirstIndexReason() { + let string = "↓myList.firstIndex { $0 % 2 == 0 } != nil" + let violations = self.violations(string) + + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations.first?.reason, "Prefer `contains` over `firstIndex(where:) != nil`") + } + + // MARK: - Private + + private func violations(_ string: String, config: Any? = nil) -> [StyleViolation] { + guard let config = makeConfig(config, ContainsOverFirstNotNilRule.description.identifier) else { + return [] + } + + return SwiftLintFrameworkTests.violations(string, config: config) + } +}