Skip to content

Commit

Permalink
Add unneeded_break_in_switch rule
Browse files Browse the repository at this point in the history
  • Loading branch information
marcelofabri committed Oct 2, 2017
1 parent 41f8b19 commit 192411f
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
or one per line.
[Marcel Jackwerth](https://github.com/sirlantis)

* Add `unneeded_break_in_switch` rule to validate that no extra `break`s are
added in `switch` statements.
[Marcelo Fabri](https://github.com/marcelofabri)
[#1870](https://github.com/realm/SwiftLint/issues/1870)

##### Bug Fixes

* Improve how `opening_brace` rule reports violations locations.
Expand Down
89 changes: 89 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
* [Trailing Whitespace](#trailing-whitespace)
* [Type Body Length](#type-body-length)
* [Type Name](#type-name)
* [Unneeded Break in Switch](#unneeded-break-in-switch)
* [Unneeded Parentheses in Closure Argument](#unneeded-parentheses-in-closure-argument)
* [Unused Closure Parameter](#unused-closure-parameter)
* [Unused Enumerated](#unused-enumerated)
Expand Down Expand Up @@ -14213,6 +14214,94 @@ protocol Foo {



## Unneeded Break in Switch

Identifier | Enabled by default | Supports autocorrection | Kind
--- | --- | --- | ---
`unneeded_break_in_switch` | Enabled | No | idiomatic

Avoid using unneeded break statements.

### Examples

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

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

```swift
switch foo {
default:
break
}
```

```swift
switch foo {
case .bar:
for i in [0, 1, 2] { break }
}
```

```swift
switch foo {
case .bar:
if true { break }
}
```

```swift
switch foo {
case .bar:
something()
}
```

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

```swift
switch foo {
case .bar:
something()
↓break
}
```

```swift
switch foo {
case .bar:
something()
↓break // comment
}
```

```swift
switch foo {
default:
something()
↓break
}
```

```swift
switch foo {
case .foo, .foo2 where condition:
something()
↓break
}
```

</details>



## Unneeded Parentheses in Closure Argument

Identifier | Enabled by default | Supports autocorrection | Kind
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 @@ -108,6 +108,7 @@ public let masterRuleList = RuleList(rules: [
TrailingWhitespaceRule.self,
TypeBodyLengthRule.self,
TypeNameRule.self,
UnneededBreakInSwitchRule.self,
UnneededParenthesesInClosureArgumentRule.self,
UnusedClosureParameterRule.self,
UnusedEnumeratedRule.self,
Expand Down
116 changes: 116 additions & 0 deletions Source/SwiftLintFramework/Rules/UnneededBreakInSwitchRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//
// UnneededBreakInSwitchRule.swift
// SwiftLint
//
// Created by Marcelo Fabri on 10/01/17.
// Copyright © 2017 Realm. All rights reserved.
//

import Foundation
import SourceKittenFramework

private func embedInSwitch(_ text: String, case: String = "case .bar") -> String {
return "switch foo {\n" +
"\(`case`):\n" +
" \(text)\n" +
"}"
}
public struct UnneededBreakInSwitchRule: ConfigurationProviderRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "unneeded_break_in_switch",
name: "Unneeded Break in Switch",
description: "Avoid using unneeded break statements.",
kind: .idiomatic,
nonTriggeringExamples: [
embedInSwitch("break"),
embedInSwitch("break", case: "default"),
embedInSwitch("for i in [0, 1, 2] { break }"),
embedInSwitch("if true { break }"),
embedInSwitch("something()")
],
triggeringExamples: [
embedInSwitch("something()\n ↓break"),
embedInSwitch("something()\n ↓break // comment"),
embedInSwitch("something()\n ↓break", case: "default"),
embedInSwitch("something()\n ↓break", case: "case .foo, .foo2 where condition")
]
)

public func validate(file: File) -> [StyleViolation] {
return file.match(pattern: "break", with: [.keyword]).flatMap { range in
let contents = file.contents.bridge()
guard let byteRange = contents.NSRangeToByteRange(start: range.location, length: range.length),
let innerStructure = file.structure.structures(forByteOffset: byteRange.location).last,
innerStructure.kind.flatMap(StatementKind.init) == .case,
let caseOffset = innerStructure.offset,
let caseLength = innerStructure.length,
let lastPatternEnd = patternEnd(dictionary: innerStructure) else {
return nil
}

let caseRange = NSRange(location: caseOffset, length: caseLength)
let tokens = file.syntaxMap.tokens(inByteRange: caseRange).filter { token in
guard let kind = SyntaxKind(rawValue: token.type),
token.offset > lastPatternEnd else {
return false
}

return !kind.isCommentLike
}

// is the `break` the only token inside `case`? If so, it's valid.
guard tokens.count > 1 else {
return nil
}

// is the `break` found the last (non-comment) token inside `case`?
guard let lastValidToken = tokens.last,
SyntaxKind(rawValue: lastValidToken.type) == .keyword,
lastValidToken.offset == byteRange.location,
lastValidToken.length == byteRange.length else {
return nil
}

return StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: range.location))
}
}

private func patternEnd(dictionary: [String: SourceKitRepresentable]) -> Int? {
let patternEnds = dictionary.elements.flatMap { subDictionary -> Int? in
guard subDictionary.kind == "source.lang.swift.structure.elem.pattern",
let offset = subDictionary.offset,
let length = subDictionary.length else {
return nil
}

return offset + length
}

return patternEnds.max()
}
}

private extension Structure {
func structures(forByteOffset byteOffset: Int) -> [[String: SourceKitRepresentable]] {
var results = [[String: SourceKitRepresentable]]()

func parse(_ dictionary: [String: SourceKitRepresentable]) {
guard let offset = dictionary.offset,
let byteRange = dictionary.length.map({ NSRange(location: offset, length: $0) }),
NSLocationInRange(byteOffset, byteRange) else {
return
}

results.append(dictionary)
dictionary.substructure.forEach(parse)
}
parse(dictionary)
return results
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
// Copyright © 2017 Realm. All rights reserved.
//

import Foundation
import Foundation
import SourceKittenFramework

Expand Down
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
D4DAE8BC1DE14E8F00B0AE7A /* NimbleOperatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */; };
D4DB92251E628898005DE9C1 /* TodoRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DB92241E628898005DE9C1 /* TodoRuleTests.swift */; };
D4E2BA851F6CD77B00E8E184 /* ArrayInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4E2BA841F6CD77B00E8E184 /* ArrayInitRule.swift */; };
D4EA77C81F817FD200C315FB /* UnneededBreakInSwitchRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4EA77C71F817FD200C315FB /* UnneededBreakInSwitchRule.swift */; };
D4FBADD01E00DA0400669C73 /* OperatorUsageWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4FBADCF1E00DA0400669C73 /* OperatorUsageWhitespaceRule.swift */; };
D4FD4C851F2A260A00DD8AA8 /* BlockBasedKVORule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4FD4C841F2A260A00DD8AA8 /* BlockBasedKVORule.swift */; };
D4FD58B21E12A0200019503C /* LinterCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4FD58B11E12A0200019503C /* LinterCache.swift */; };
Expand Down Expand Up @@ -556,6 +557,7 @@
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>"; };
D4E2BA841F6CD77B00E8E184 /* ArrayInitRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArrayInitRule.swift; sourceTree = "<group>"; };
D4EA77C71F817FD200C315FB /* UnneededBreakInSwitchRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnneededBreakInSwitchRule.swift; sourceTree = "<group>"; };
D4FBADCF1E00DA0400669C73 /* OperatorUsageWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OperatorUsageWhitespaceRule.swift; sourceTree = "<group>"; };
D4FD4C841F2A260A00DD8AA8 /* BlockBasedKVORule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BlockBasedKVORule.swift; sourceTree = "<group>"; };
D4FD58B11E12A0200019503C /* LinterCache.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LinterCache.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1082,6 +1084,7 @@
E88DEA8D1B0999CD00A66CB0 /* TypeBodyLengthRule.swift */,
E88DEA911B099B1F00A66CB0 /* TypeNameRule.swift */,
D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */,
D4EA77C71F817FD200C315FB /* UnneededBreakInSwitchRule.swift */,
D46A317E1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift */,
D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */,
D43B04631E0620AB004016AF /* UnusedEnumeratedRule.swift */,
Expand Down Expand Up @@ -1463,6 +1466,7 @@
E8BDE3FF1EDF91B6002EC12F /* RuleList.swift in Sources */,
E889D8C71F1D357B00058332 /* Configuration+Merging.swift in Sources */,
D44254271DB9C15C00492EA4 /* SyntacticSugarRule.swift in Sources */,
D4EA77C81F817FD200C315FB /* UnneededBreakInSwitchRule.swift in Sources */,
006204DC1E1E492F00FFFBE1 /* VerticalWhitespaceConfiguration.swift in Sources */,
E88198441BEA93D200333A11 /* ColonRule.swift in Sources */,
D403A4A31F4DB5510020CA02 /* PatternMatchingKeywordsRule.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ extension RulesTests {
("testTrailingSemicolon", testTrailingSemicolon),
("testTrailingWhitespace", testTrailingWhitespace),
("testTypeBodyLength", testTypeBodyLength),
("testUnneededBreakInSwitch", testUnneededBreakInSwitch),
("testUnneededParenthesesInClosureArgument", testUnneededParenthesesInClosureArgument),
("testUnusedClosureParameter", testUnusedClosureParameter),
("testUnusedEnumerated", testUnusedEnumerated),
Expand Down
4 changes: 4 additions & 0 deletions Tests/SwiftLintFrameworkTests/RulesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,10 @@ class RulesTests: XCTestCase {
verifyRule(TypeBodyLengthRule.description)
}

func testUnneededBreakInSwitch() {
verifyRule(UnneededBreakInSwitchRule.description)
}

func testUnneededParenthesesInClosureArgument() {
verifyRule(UnneededParenthesesInClosureArgumentRule.description)
}
Expand Down

0 comments on commit 192411f

Please sign in to comment.