Skip to content

Commit

Permalink
Merge pull request #2719 from daltonclaybrook/dc-unused-capture-list-…
Browse files Browse the repository at this point in the history
…rule

Add unused_capture_list rule
  • Loading branch information
marcelofabri authored Apr 15, 2019
2 parents c80ffd3 + 5999012 commit 87d9097
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 16 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#2678](https://github.com/realm/SwiftLint/issues/2678)

* Add `unused_capture_list` rule to ensure that all references in a closure
capture list are used.
[Dalton Claybrook](https://github.com/daltonclaybrook)
[#2715](https://github.com/realm/SwiftLint/issues/2715)

#### Bug Fixes

* Fix bug where SwiftLint ignores excluded files list in a nested configuration
Expand Down
78 changes: 78 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
* [Unneeded Break in Switch](#unneeded-break-in-switch)
* [Unneeded Parentheses in Closure Argument](#unneeded-parentheses-in-closure-argument)
* [Untyped Error in Catch](#untyped-error-in-catch)
* [Unused Capture List](#unused-capture-list)
* [Unused Closure Parameter](#unused-closure-parameter)
* [Unused Control Flow Label](#unused-control-flow-label)
* [Unused Enumerated](#unused-enumerated)
Expand Down Expand Up @@ -22910,6 +22911,83 @@ do {



## Unused Capture List

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`unused_capture_list` | Enabled | No | lint | No | 4.2.0

Unused reference in a capture list should be removed.

### Examples

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

```swift
[1, 2].map { [weak self] num in
self?.handle(num)
}
```

```swift
let failure: Failure = { [weak self, unowned delegate = self.delegate!] foo in
delegate.handle(foo, self)
}
```

```swift
numbers.forEach({
[weak handler] in
handler?.handle($0)
})
```

```swift
{ [foo] in foo.bar() }()
```

```swift
sizes.max().flatMap { [(offset: offset, size: $0)] } ?? []
```

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

```swift
[1, 2].map { [↓weak self] num in
print(num)
}
```

```swift
let failure: Failure = { [weak self, ↓unowned delegate = self.delegate!] foo in
self?.handle(foo)
}
```

```swift
let failure: Failure = { [↓weak self, ↓unowned delegate = self.delegate!] foo in
print(foo)
}
```

```swift
numbers.forEach({
[weak handler] in
print($0)
})
```

```swift
{ [↓foo] in _ }()
```

</details>



## Unused Closure Parameter

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 @@ -155,6 +155,7 @@ public let masterRuleList = RuleList(rules: [
UnneededBreakInSwitchRule.self,
UnneededParenthesesInClosureArgumentRule.self,
UntypedErrorInCatchRule.self,
UnusedCaptureListRule.self,
UnusedClosureParameterRule.self,
UnusedControlFlowLabelRule.self,
UnusedEnumeratedRule.self,
Expand Down
142 changes: 142 additions & 0 deletions Source/SwiftLintFramework/Rules/Lint/UnusedCaptureListRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import Foundation
import SourceKittenFramework

public struct UnusedCaptureListRule: ASTRule, ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static var description = RuleDescription(
identifier: "unused_capture_list",
name: "Unused Capture List",
description: "Unused reference in a capture list should be removed.",
kind: .lint,
minSwiftVersion: .fourDotTwo,
nonTriggeringExamples: [
"""
[1, 2].map { [weak self] num in
self?.handle(num)
}
""",
"""
let failure: Failure = { [weak self, unowned delegate = self.delegate!] foo in
delegate.handle(foo, self)
}
""",
"""
numbers.forEach({
[weak handler] in
handler?.handle($0)
})
""",
"{ [foo] in foo.bar() }()",
"sizes.max().flatMap { [(offset: offset, size: $0)] } ?? []"
],
triggeringExamples: [
"""
[1, 2].map { [↓weak self] num in
print(num)
}
""",
"""
let failure: Failure = { [weak self, ↓unowned delegate = self.delegate!] foo in
self?.handle(foo)
}
""",
"""
let failure: Failure = { [↓weak self, ↓unowned delegate = self.delegate!] foo in
print(foo)
}
""",
"""
numbers.forEach({
[weak handler] in
print($0)
})
""",
"{ [↓foo] in _ }()"
]
)

private let captureListRegex = regex("^\\{\\s*\\[([^\\]]+)\\].*\\bin\\b")

public func validate(file: File, kind: SwiftExpressionKind,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
let contents = file.contents.bridge()
guard kind == .closure,
let offset = dictionary.offset,
let length = dictionary.length,
let closureRange = contents.byteRangeToNSRange(start: offset, length: length),
let match = captureListRegex.firstMatch(in: file.contents, options: [], range: closureRange)
else { return [] }

let captureListRange = match.range(at: 1)
guard captureListRange.location != NSNotFound,
captureListRange.length > 0 else { return [] }

let captureList = contents.substring(with: captureListRange)
let references = referencesAndLocationsFromCaptureList(captureList)

let restOfClosureLocation = captureListRange.location + captureListRange.length + 1
let restOfClosureLength = closureRange.length - (restOfClosureLocation - closureRange.location)
let restOfClosureRange = NSRange(location: restOfClosureLocation, length: restOfClosureLength)
guard let restOfClosureByteRange = contents
.NSRangeToByteRange(start: restOfClosureRange.location, length: restOfClosureRange.length)
else { return [] }

let identifiers = identifierStrings(in: file, byteRange: restOfClosureByteRange)
return violations(in: file, references: references,
identifiers: identifiers, captureListRange: captureListRange)
}

// MARK: - Private

private func referencesAndLocationsFromCaptureList(_ captureList: String) -> [(String, Int)] {
var locationOffset = 0
return captureList.components(separatedBy: ",")
.reduce(into: [(String, Int)]()) { referencesAndLocations, item in
let item = item.bridge()
let range = item.rangeOfCharacter(from: CharacterSet.whitespacesAndNewlines.inverted)
guard range.location != NSNotFound else { return }

let location = range.location + locationOffset
locationOffset += item.length + 1 // 1 for comma
let reference = item.components(separatedBy: "=")
.first?
.trimmingCharacters(in: .whitespacesAndNewlines)
.components(separatedBy: .whitespaces)
.last
if let reference = reference {
referencesAndLocations.append((reference, location))
}
}
}

private func identifierStrings(in file: File, byteRange: NSRange) -> Set<String> {
let contents = file.contents.bridge()
let identifiers = file.syntaxMap
.tokens(inByteRange: byteRange)
.compactMap { token -> String? in
guard token.type == SyntaxKind.identifier.rawValue || token.type == SyntaxKind.keyword.rawValue,
let range = contents.byteRangeToNSRange(start: token.offset, length: token.length)
else { return nil }
return contents.substring(with: range)
}
return Set(identifiers)
}

private func violations(in file: File, references: [(String, Int)],
identifiers: Set<String>, captureListRange: NSRange) -> [StyleViolation] {
return references.compactMap { reference, location -> StyleViolation? in
guard !identifiers.contains(reference) else { return nil }
let offset = captureListRange.location + location
let reason = "Unused reference \(reference) in a capture list should be removed."
return StyleViolation(
ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: offset),
reason: reason
)
}
}
}
Loading

0 comments on commit 87d9097

Please sign in to comment.