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

Add optional_enum_case_matching rule #3002

Merged
merged 1 commit into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
[lakpa](https://github.com/lakpa)
[#2997](https://github.com/realm/SwiftLint/issues/2997)

* Add `optional_enum_case_matching` opt-in rule to validate that
optional enum cases are matched without using `?` when using Swift 5.1 or
above. See [SR-7799](https://bugs.swift.org/browse/SR-7799) for more
details.
[Marcelo Fabri](https://github.com/marcelofabri)

#### Bug Fixes

* Fix crash in `unused_import` rule when unused imports have trailing
Expand Down
61 changes: 61 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
* [Opening Brace Spacing](#opening-brace-spacing)
* [Operator Usage Whitespace](#operator-usage-whitespace)
* [Operator Function Whitespace](#operator-function-whitespace)
* [Optional Enum Case Match](#optional-enum-case-match)
* [Overridden methods call super](#overridden-methods-call-super)
* [Override in Extension](#override-in-extension)
* [Pattern Matching Keywords](#pattern-matching-keywords)
Expand Down Expand Up @@ -15668,6 +15669,66 @@ func abc(lhs: Int, rhs: Int) -> Int {}



## Optional Enum Case Match

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`optional_enum_case_matching` | Disabled | Yes | style | No | 5.1.0

Matching an enum case against an optional enum without '?' is supported on Swift 5.1 and above.

### Examples

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

```swift
switch foo {
case .bar: break
case .baz: break
default: break
}
```

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

```swift
switch foo {
case .bar↓?: break
case .baz: break
default: break
}
```

```swift
switch foo {
case Foo.bar↓?: break
case .baz: break
default: break
}
```

```swift
switch foo {
case .bar↓?, .baz↓?: break
default: break
}
```

```swift
switch foo {
case .bar↓? where x > 1: break
case .baz: break
default: break
}
```

</details>



## Overridden methods call super

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
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 @@ -113,6 +113,7 @@ public let masterRuleList = RuleList(rules: [
OpeningBraceRule.self,
OperatorFunctionWhitespaceRule.self,
OperatorUsageWhitespaceRule.self,
OptionalEnumCaseMatchingRule.self,
OverriddenSuperCallRule.self,
OverrideInExtensionRule.self,
PatternMatchingKeywordsRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import Foundation
import SourceKittenFramework

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

public init() {}

public static let description = RuleDescription(
identifier: "optional_enum_case_matching",
name: "Optional Enum Case Match",
description: "Matching an enum case against an optional enum without '?' is supported on Swift 5.1 and above. ",
kind: .style,
minSwiftVersion: .fiveDotOne,
nonTriggeringExamples: [
"""
switch foo {
case .bar: break
case .baz: break
default: break
}
"""
],
triggeringExamples: [
"""
switch foo {
case .bar↓?: break
case .baz: break
default: break
}
""",
"""
switch foo {
case Foo.bar↓?: break
case .baz: break
default: break
}
""",
"""
switch foo {
case .bar↓?, .baz↓?: break
default: break
}
""",
"""
switch foo {
case .bar↓? where x > 1: break
case .baz: break
default: break
}
"""
],
corrections: [
"""
switch foo {
case .bar↓?: break
case .baz: break
default: break
}
""": """
switch foo {
case .bar: break
case .baz: break
default: break
}
""",
"""
switch foo {
case Foo.bar↓?: break
case .baz: break
default: break
}
""": """
switch foo {
case Foo.bar: break
case .baz: break
default: break
}
""",
"""
switch foo {
case .bar↓?, .baz↓?: break
default: break
}
""": """
switch foo {
case .bar, .baz: break
default: break
}
""",
"""
switch foo {
case .bar↓? where x > 1: break
case .baz: break
default: break
}
""": """
switch foo {
case .bar where x > 1: break
case .baz: break
default: break
}
"""
]
)

// MARK: - ASTRule

public func validate(file: SwiftLintFile,
kind: StatementKind,
dictionary: SourceKittenDictionary) -> [StyleViolation] {
return violationRanges(in: file, kind: kind, dictionary: dictionary).map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: $0.location))
}
}

// MARK: - SubstitutionCorrectableASTRule

public func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? {
return (violationRange, "")
}

public func violationRanges(in file: SwiftLintFile,
kind: StatementKind,
dictionary: SourceKittenDictionary) -> [NSRange] {
guard SwiftVersion.current >= type(of: self).description.minSwiftVersion, kind == .case else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: could've put the cheaper check before the more expensive check

return []
}

let contents = file.contents.bridge()
return dictionary.elements
.filter { $0.kind == "source.lang.swift.structure.elem.pattern" }
.compactMap { dictionary in
guard let offset = dictionary.offset, let length = dictionary.length else {
return nil
}

let tokens = file.syntaxMap
.tokens(inByteRange: NSRange(location: offset, length: length))
.prefix(while: { $0.kind != .keyword })

guard let lastToken = tokens.last else {
return nil
}

let questionMarkByteOffset = lastToken.length + lastToken.offset
guard contents.substringWithByteRange(start: questionMarkByteOffset, length: 1) == "?",
let range = contents.byteRangeToNSRange(start: questionMarkByteOffset, length: 1) else {
return nil
}

return range
}
}
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@
D43B04661E071ED3004016AF /* ColonRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43B04651E071ED3004016AF /* ColonRuleTests.swift */; };
D43B04691E072291004016AF /* ColonConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43B04671E07228D004016AF /* ColonConfiguration.swift */; };
D43B046B1E075905004016AF /* ClosureEndIndentationRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43B046A1E075905004016AF /* ClosureEndIndentationRule.swift */; };
D43CDEDC23BDB8D30074F3EE /* OptionalEnumCaseMatchingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43CDEDB23BDB8D30074F3EE /* OptionalEnumCaseMatchingRule.swift */; };
D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */; };
D44037972132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D44037962132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift */; };
D44254201DB87CA200492EA4 /* ValidIBInspectableRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */; };
Expand Down Expand Up @@ -775,6 +776,7 @@
D43B04651E071ED3004016AF /* ColonRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ColonRuleTests.swift; sourceTree = "<group>"; };
D43B04671E07228D004016AF /* ColonConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ColonConfiguration.swift; sourceTree = "<group>"; };
D43B046A1E075905004016AF /* ClosureEndIndentationRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureEndIndentationRule.swift; sourceTree = "<group>"; };
D43CDEDB23BDB8D30074F3EE /* OptionalEnumCaseMatchingRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptionalEnumCaseMatchingRule.swift; sourceTree = "<group>"; };
D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImplicitGetterRule.swift; sourceTree = "<group>"; };
D44037962132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProhibitedInterfaceBuilderRule.swift; sourceTree = "<group>"; };
D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ValidIBInspectableRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1226,6 +1228,7 @@
692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */,
E5A167C81B25A0B000CF2D03 /* OperatorFunctionWhitespaceRule.swift */,
D4FBADCF1E00DA0400669C73 /* OperatorUsageWhitespaceRule.swift */,
D43CDEDB23BDB8D30074F3EE /* OptionalEnumCaseMatchingRule.swift */,
62DADC471FFF0423002B6319 /* PrefixedTopLevelConstantRule.swift */,
D47F31141EC918B600E3E1CA /* ProtocolPropertyAccessorsOrderRule.swift */,
D4C889701E385B7B00BAE88D /* RedundantDiscardableLetRule.swift */,
Expand Down Expand Up @@ -2113,6 +2116,7 @@
D4EA77CA1F81FACC00C315FB /* LiteralExpressionEndIdentationRule.swift in Sources */,
E86396C51BADAC15002C9E88 /* XcodeReporter.swift in Sources */,
E889D8C51F1D11A200058332 /* Configuration+LintableFiles.swift in Sources */,
D43CDEDC23BDB8D30074F3EE /* OptionalEnumCaseMatchingRule.swift in Sources */,
094385011D5D2894009168CF /* WeakDelegateRule.swift in Sources */,
82F614F22106014500D23904 /* MultilineParametersBracketsRule.swift in Sources */,
6BE79EB12204EC0700B5A2FE /* RequiredDeinitRule.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 @@ -983,6 +983,12 @@ extension OperatorUsageWhitespaceRuleTests {
]
}

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

extension OverriddenSuperCallRuleTests {
static var allTests: [(String, (OverriddenSuperCallRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1711,6 +1717,7 @@ XCTMain([
testCase(OpeningBraceRuleTests.allTests),
testCase(OperatorFunctionWhitespaceRuleTests.allTests),
testCase(OperatorUsageWhitespaceRuleTests.allTests),
testCase(OptionalEnumCaseMatchingRuleTests.allTests),
testCase(OverriddenSuperCallRuleTests.allTests),
testCase(OverrideInExtensionRuleTests.allTests),
testCase(PatternMatchingKeywordsRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,12 @@ class OperatorUsageWhitespaceRuleTests: XCTestCase {
}
}

class OptionalEnumCaseMatchingRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(OptionalEnumCaseMatchingRule.description)
}
}

class OverriddenSuperCallRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(OverriddenSuperCallRule.description)
Expand Down