Skip to content

Commit

Permalink
contains_over_first_not_nil rule now also checks for `firstIndex(wh…
Browse files Browse the repository at this point in the history
…ere:)`

Fixes #2678
  • Loading branch information
marcelofabri committed Apr 9, 2019
1 parent a97487f commit 51d47d4
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 27 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 41 additions & 1 deletion Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 }

```

</details>
<details>
<summary>Triggering Examples</summary>
Expand Down Expand Up @@ -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

```

</details>


Expand Down
5 changes: 4 additions & 1 deletion Source/SwiftLintFramework/Protocols/CallPairRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ 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,
pattern: String,
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()
Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,42 @@
import SourceKittenFramework

public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, ConfigurationProviderRule, AutomaticTestableRule {
public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, ConfigurationProviderRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

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
}
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -817,6 +818,7 @@
D4DABFD81E2C59BC009617B6 /* NotificationCenterDetachmentRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NotificationCenterDetachmentRuleExamples.swift; sourceTree = "<group>"; };
D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NimbleOperatorRule.swift; sourceTree = "<group>"; };
D4DB92241E628898005DE9C1 /* TodoRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TodoRuleTests.swift; sourceTree = "<group>"; };
D4DDFF9722396D62006C3629 /* ContainsOverFirstNotNilRuleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContainsOverFirstNotNilRuleTests.swift; sourceTree = "<group>"; };
D4DE9131207B4731000FFAA8 /* UnavailableFunctionRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnavailableFunctionRule.swift; sourceTree = "<group>"; };
D4E2BA841F6CD77B00E8E184 /* ArrayInitRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArrayInitRule.swift; sourceTree = "<group>"; };
D4E92D1E2137B4C9002EDD48 /* IdenticalOperandsRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdenticalOperandsRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,6 @@ class CommaRuleTests: XCTestCase {
}
}

class ContainsOverFirstNotNilRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ContainsOverFirstNotNilRule.description)
}
}

class ControlStatementRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ControlStatementRule.description)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 51d47d4

Please sign in to comment.