Skip to content

Commit

Permalink
Add contains_over_range_not_nil rule, make `contains_over_first_not…
Browse files Browse the repository at this point in the history
…_nil` also match == nil (realm#2811)
  • Loading branch information
cltnschlosser authored and jpsim committed Sep 3, 2019
1 parent ffb2f4f commit 2c07615
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 9 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ This is the last release to support building with Swift 4.2.x.
[Colton Schlosser](https://github.com/cltnschlosser)
[#2807](https://github.com/realm/SwiftLint/issues/2807)

* Add `contains_over_range_nil_comparison` opt-in rule to prefer
using `contains` over comparison of `range(of:)` to `nil`.
[Colton Schlosser](https://github.com/cltnschlosser)
[#2776](https://github.com/realm/SwiftLint/issues/2776)

* Make `contains_over_first_not_nil` rule also match `first(where:) == nil`.
[Colton Schlosser](https://github.com/cltnschlosser)

#### Bug Fixes

* Fixed false positive in `colon` rule inside guard and ternary operator.
Expand Down
106 changes: 106 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* [Contains Over Filter Count](#contains-over-filter-count)
* [Contains Over Filter Is Empty](#contains-over-filter-is-empty)
* [Contains over first not nil](#contains-over-first-not-nil)
* [Contains over range(of:) comparison to nil](#contains-over-rangeof-comparison-to-nil)
* [Control Statement](#control-statement)
* [Convenience Type](#convenience-type)
* [Custom Rules](#custom-rules)
Expand Down Expand Up @@ -2807,6 +2808,36 @@ let firstIndex = myList.firstIndex { $0 % 2 == 0 }

```

```swift
↓myList.first { $0 % 2 == 0 } == nil

```

```swift
↓myList.first(where: { $0 % 2 == 0 }) == nil

```

```swift
↓myList.map { $0 + 1 }.first(where: { $0 % 2 == 0 }) == nil

```

```swift
↓myList.first(where: someFunction) == nil

```

```swift
↓myList.map { $0 + 1 }.first { $0 % 2 == 0 } == nil

```

```swift
(↓myList.first { $0 % 2 == 0 }) == nil

```

```swift
↓myList.firstIndex { $0 % 2 == 0 } != nil

Expand Down Expand Up @@ -2837,6 +2868,81 @@ let firstIndex = myList.firstIndex { $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>



## Contains over range(of:) comparison to nil

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`contains_over_range_nil_comparison` | Disabled | No | performance | No | 3.0.0

Prefer `contains` over `range(of:) != nil` and `range(of:) == nil`.

### Examples

<details>
<summary>Non Triggering Examples</summary>

```swift
let range = myString.range(of: "Test")
```

```swift
myString.contains("Test")
```

```swift
!myString.contains("Test")
```

```swift
resourceString.range(of: rule.regex, options: .regularExpression) != nil
```

</details>
<details>
<summary>Triggering Examples</summary>

```swift
↓myString.range(of: "Test") != nil
```

```swift
↓myString.range(of: "Test") == nil
```

</details>


Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public let masterRuleList = RuleList(rules: [
ContainsOverFilterCountRule.self,
ContainsOverFilterIsEmptyRule.self,
ContainsOverFirstNotNilRule.self,
ContainsOverRangeNilComparisonRule.self,
ControlStatementRule.self,
ConvenienceTypeRule.self,
CustomRules.self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@ public struct ContainsOverFirstNotNilRule: CallPairRule, OptInRule, Configuratio
]
},
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"
]
return ["!=", "=="].flatMap { comparison in
return [
"↓myList.\(method) { $0 % 2 == 0 } \(comparison) nil\n",
"↓myList.\(method)(where: { $0 % 2 == 0 }) \(comparison) nil\n",
"↓myList.map { $0 + 1 }.\(method)(where: { $0 % 2 == 0 }) \(comparison) nil\n",
"↓myList.\(method)(where: someFunction) \(comparison) nil\n",
"↓myList.map { $0 + 1 }.\(method) { $0 % 2 == 0 } \(comparison) nil\n",
"(↓myList.\(method) { $0 % 2 == 0 }) \(comparison) nil\n"
]
}
}
)

public func validate(file: File) -> [StyleViolation] {
let pattern = "[\\}\\)]\\s*!=\\s*nil"
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`")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import SourceKittenFramework

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

public init() {}

public static let description = RuleDescription(
identifier: "contains_over_range_nil_comparison",
name: "Contains over range(of:) comparison to nil",
description: "Prefer `contains` over `range(of:) != nil` and `range(of:) == nil`.",
kind: .performance,
nonTriggeringExamples: [
"let range = myString.range(of: \"Test\")",
"myString.contains(\"Test\")",
"!myString.contains(\"Test\")",
"resourceString.range(of: rule.regex, options: .regularExpression) != nil"
],
triggeringExamples: ["!=", "=="].flatMap { comparison in
return [
"↓myString.range(of: \"Test\") \(comparison) nil"
]
}
)

public func validate(file: File) -> [StyleViolation] {
let pattern = "\\)\\s*(==|!=)\\s*nil"
return validate(file: file, pattern: pattern, patternSyntaxKinds: [.keyword],
callNameSuffix: ".range", severity: configuration.severity,
reason: "Prefer `contains` over range(of:) comparison to nil") { expression in
return expression.enclosedArguments.map { $0.name } == ["of"]
}
}
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
4DCB8E7F1CBE494E0070FCF0 /* RegexHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4DCB8E7D1CBE43640070FCF0 /* RegexHelpers.swift */; };
4E342B4C2215C793008E4EF8 /* ReduceBooleanRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4E342B4A2215C6DF008E4EF8 /* ReduceBooleanRule.swift */; };
550DA0E022DB95AD00B67F03 /* EmptyCollectionLiteralRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 550DA0DE22DB958400B67F03 /* EmptyCollectionLiteralRule.swift */; };
55CE0585231899100023BA72 /* ContainsOverRangeNilComparisonRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 550DA0E122DB9E6B00B67F03 /* ContainsOverRangeNilComparisonRule.swift */; };
57ED827B1CF656E3002B3513 /* JUnitReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57ED82791CF65183002B3513 /* JUnitReporter.swift */; };
584B0D3A2112BA78002F7E25 /* SonarQubeReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 584B0D392112BA78002F7E25 /* SonarQubeReporter.swift */; };
584B0D3C2112E8FB002F7E25 /* CannedSonarQubeReporterOutput.json in Resources */ = {isa = PBXBuildFile; fileRef = 584B0D3B2112E8FB002F7E25 /* CannedSonarQubeReporterOutput.json */; };
Expand Down Expand Up @@ -571,6 +572,7 @@
5499CA961A2394B700783309 /* Components.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Components.plist; sourceTree = "<group>"; };
5499CA971A2394B700783309 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
550DA0DE22DB958400B67F03 /* EmptyCollectionLiteralRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmptyCollectionLiteralRule.swift; sourceTree = "<group>"; };
550DA0E122DB9E6B00B67F03 /* ContainsOverRangeNilComparisonRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContainsOverRangeNilComparisonRule.swift; sourceTree = "<group>"; };
57ED82791CF65183002B3513 /* JUnitReporter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JUnitReporter.swift; sourceTree = "<group>"; };
584B0D392112BA78002F7E25 /* SonarQubeReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SonarQubeReporter.swift; sourceTree = "<group>"; };
584B0D3B2112E8FB002F7E25 /* CannedSonarQubeReporterOutput.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = CannedSonarQubeReporterOutput.json; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1081,6 +1083,7 @@
29AD4C641F6EA16C009B66E1 /* ContainsOverFirstNotNilRule.swift */,
D489B546231233490090BAA0 /* ContainsOverFilterCountRule.swift */,
D489B549231383350090BAA0 /* ContainsOverFilterIsEmptyRule.swift */,
550DA0E122DB9E6B00B67F03 /* ContainsOverRangeNilComparisonRule.swift */,
550DA0DE22DB958400B67F03 /* EmptyCollectionLiteralRule.swift */,
E847F0A81BFBBABD00EA9363 /* EmptyCountRule.swift */,
740DF1AF203F5AFC0081F694 /* EmptyStringRule.swift */,
Expand Down Expand Up @@ -2065,6 +2068,7 @@
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */,
24E17F721B14BB3F008195BE /* File+Cache.swift in Sources */,
C3D23F1D21E3A33700E9BD1B /* UnusedControlFlowLabelRule.swift in Sources */,
55CE0585231899100023BA72 /* ContainsOverRangeNilComparisonRule.swift in Sources */,
6C1D763221A4E69600DEF783 /* Request+DisableSourceKit.swift in Sources */,
47ACC8981E7DC74E0088EEB2 /* ImplicitlyUnwrappedOptionalConfiguration.swift in Sources */,
787CDE39208E7D41005F3D2F /* SwitchCaseAlignmentConfiguration.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ extension ContainsOverFirstNotNilRuleTests {
]
}

extension ContainsOverRangeNilComparisonRuleTests {
static var allTests: [(String, (ContainsOverRangeNilComparisonRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension ControlStatementRuleTests {
static var allTests: [(String, (ControlStatementRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1577,6 +1583,7 @@ XCTMain([
testCase(ContainsOverFilterCountRuleTests.allTests),
testCase(ContainsOverFilterIsEmptyRuleTests.allTests),
testCase(ContainsOverFirstNotNilRuleTests.allTests),
testCase(ContainsOverRangeNilComparisonRuleTests.allTests),
testCase(ControlStatementRuleTests.allTests),
testCase(ConvenienceTypeRuleTests.allTests),
testCase(CustomRulesTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class ContainsOverFilterIsEmptyRuleTests: XCTestCase {
}
}

class ContainsOverRangeNilComparisonRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ContainsOverRangeNilComparisonRule.description)
}
}

class ControlStatementRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ControlStatementRule.description)
Expand Down

0 comments on commit 2c07615

Please sign in to comment.