Skip to content

Commit

Permalink
Merge pull request #2739 from realm/marcelo/unowned_variable_capture
Browse files Browse the repository at this point in the history
Add `unowned_variable_capture` opt-in rule
  • Loading branch information
marcelofabri authored May 2, 2019
2 parents eecef72 + a3aa367 commit c33fc83
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
[Frederick Pietschmann](https://github.com/fredpi)
[#2717](https://github.com/realm/SwiftLint/issues/2717)

* Add `unowned_variable_capture` opt-in rule to warn against unowned captures
in closures when using Swift 5.
[Marcelo Fabri](https://github.com/marcelofabri)
[#2097](https://github.com/realm/SwiftLint/issues/2097)

#### Bug Fixes

* Don't trigger `redundant_void_return` violations when using `subscript` as the
Expand Down
58 changes: 58 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
* [Unavailable Function](#unavailable-function)
* [Unneeded Break in Switch](#unneeded-break-in-switch)
* [Unneeded Parentheses in Closure Argument](#unneeded-parentheses-in-closure-argument)
* [Unowned Variable Capture](#unowned-variable-capture)
* [Untyped Error in Catch](#untyped-error-in-catch)
* [Unused Capture List](#unused-capture-list)
* [Unused Closure Parameter](#unused-closure-parameter)
Expand Down Expand Up @@ -22950,6 +22951,63 @@ foo.bar { [weak self] ↓(x, y) in }



## Unowned Variable Capture

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`unowned_variable_capture` | Disabled | No | lint | No | 5.0.0

Prefer capturing references as weak to avoid potential crashes.

### Examples

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

```swift
foo { [weak self] in _ }
```

```swift
foo { [weak self] param in _ }
```

```swift
foo { [weak bar] in _ }
```

```swift
foo { [weak bar] param in _ }
```

```swift
foo { bar in _ }
```

```swift
foo { $0 }
```

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

```swift
foo { [↓unowned self] in _ }
```

```swift
foo { [↓unowned bar] in _ }
```

```swift
foo { [bar, ↓unowned self] in _ }
```

</details>



## Untyped Error in Catch

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: [
UnavailableFunctionRule.self,
UnneededBreakInSwitchRule.self,
UnneededParenthesesInClosureArgumentRule.self,
UnownedVariableCaptureRule.self,
UntypedErrorInCatchRule.self,
UnusedCaptureListRule.self,
UnusedClosureParameterRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import Foundation
import SourceKittenFramework

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

public init() {}

public static let description = RuleDescription(
identifier: "unowned_variable_capture",
name: "Unowned Variable Capture",
description: "Prefer capturing references as weak to avoid potential crashes.",
kind: .lint,
minSwiftVersion: .five,
nonTriggeringExamples: [
"foo { [weak self] in _ }",
"foo { [weak self] param in _ }",
"foo { [weak bar] in _ }",
"foo { [weak bar] param in _ }",
"foo { bar in _ }",
"foo { $0 }"
],
triggeringExamples: [
"foo { [↓unowned self] in _ }",
"foo { [↓unowned bar] in _ }",
"foo { [bar, ↓unowned self] in _ }"
]
)

public func validate(file: File, kind: SwiftExpressionKind,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
guard kind == .closure, let bodyOffset = dictionary.bodyOffset, let bodyLength = dictionary.bodyLength,
case let contents = file.contents.bridge(),
let closureRange = contents.byteRangeToNSRange(start: bodyOffset, length: bodyLength),
let inTokenRange = file.match(pattern: "\\bin\\b", with: [.keyword], range: closureRange).first,
let inTokenByteRange = contents.NSRangeToByteRange(start: inTokenRange.location,
length: inTokenRange.length) else {
return []
}

let length = inTokenByteRange.location - bodyOffset
let variables = localVariableDeclarations(inByteRange: NSRange(location: bodyOffset, length: length),
structure: file.structure)
let unownedVariableOffsets = variables.compactMap { dictionary in
return dictionary.swiftAttributes.first { attributeDict in
guard attributeDict.attribute.flatMap(SwiftDeclarationAttributeKind.init) == .weak,
let offset = attributeDict.offset, let length = attributeDict.length else {
return false
}

return contents.substringWithByteRange(start: offset, length: length) == "unowned"
}?.offset
}

return unownedVariableOffsets.map { offset in
return StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, byteOffset: offset))
}
}

private func localVariableDeclarations(inByteRange byteRange: NSRange,
structure: Structure) -> [[String: SourceKitRepresentable]] {
var results = [[String: SourceKitRepresentable]]()

func parse(dictionary: [String: SourceKitRepresentable]) {
if let kindString = (dictionary.kind),
SwiftDeclarationKind(rawValue: kindString) == .varLocal,
let offset = dictionary.offset,
let length = dictionary.length {
let variableByteRange = NSRange(location: offset, length: length)

if byteRange.intersects(variableByteRange) {
results.append(dictionary)
}
}
dictionary.substructure.forEach(parse)
}
parse(dictionary: structure.dictionary)
return results
}
}
5 changes: 5 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@
D4B022B21E10B613007E5297 /* RedundantVoidReturnRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4B022B11E10B613007E5297 /* RedundantVoidReturnRule.swift */; };
D4B3409D21F16B110038C79A /* UnusedSetterValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4B3409C21F16B110038C79A /* UnusedSetterValueRule.swift */; };
D4B472411F66486300BD6EF1 /* FallthroughRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4B472401F66486300BD6EF1 /* FallthroughRule.swift */; };
D4BED5F82278AECC00D86BCE /* UnownedVariableCaptureRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4BED5F72278AECC00D86BCE /* UnownedVariableCaptureRule.swift */; };
D4C0E46F1E3D973600C560F2 /* ForWhereRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C0E46E1E3D973600C560F2 /* ForWhereRule.swift */; };
D4C27BFE1E12D53F00DF713E /* Version.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C27BFD1E12D53F00DF713E /* Version.swift */; };
D4C27C001E12DFF500DF713E /* LinterCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C27BFF1E12DFF500DF713E /* LinterCacheTests.swift */; };
Expand Down Expand Up @@ -794,6 +795,7 @@
D4B022B11E10B613007E5297 /* RedundantVoidReturnRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantVoidReturnRule.swift; sourceTree = "<group>"; };
D4B3409C21F16B110038C79A /* UnusedSetterValueRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnusedSetterValueRule.swift; sourceTree = "<group>"; };
D4B472401F66486300BD6EF1 /* FallthroughRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FallthroughRule.swift; sourceTree = "<group>"; };
D4BED5F72278AECC00D86BCE /* UnownedVariableCaptureRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnownedVariableCaptureRule.swift; sourceTree = "<group>"; };
D4C0E46E1E3D973600C560F2 /* ForWhereRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ForWhereRule.swift; sourceTree = "<group>"; };
D4C27BFD1E12D53F00DF713E /* Version.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Version.swift; sourceTree = "<group>"; };
D4C27BFF1E12DFF500DF713E /* LinterCacheTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LinterCacheTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1113,6 +1115,7 @@
D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */,
D4D7320C21E15ED4001C07D9 /* UnusedControlFlowLabelRule.swift */,
8F715B82213B528B00427BD9 /* UnusedImportRule.swift */,
D4BED5F72278AECC00D86BCE /* UnownedVariableCaptureRule.swift */,
8F6B3153213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift */,
D4B3409C21F16B110038C79A /* UnusedSetterValueRule.swift */,
D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */,
Expand Down Expand Up @@ -1732,6 +1735,7 @@
developmentRegion = English;
hasScannedForEncodings = 0;
knownRegions = (
English,
en,
Base,
);
Expand Down Expand Up @@ -2023,6 +2027,7 @@
756B585D2138ECD300D1A4E9 /* CollectionAlignmentRule.swift in Sources */,
621C8EA420CBC7A10007DA74 /* RedundantTypeAnnotationRule.swift in Sources */,
62DADC481FFF0423002B6319 /* PrefixedTopLevelConstantRule.swift in Sources */,
D4BED5F82278AECC00D86BCE /* UnownedVariableCaptureRule.swift in Sources */,
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */,
24E17F721B14BB3F008195BE /* File+Cache.swift in Sources */,
C3D23F1D21E3A33700E9BD1B /* UnusedControlFlowLabelRule.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 @@ -1341,6 +1341,12 @@ extension UnneededParenthesesInClosureArgumentRuleTests {
]
}

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

extension UntypedErrorInCatchRuleTests {
static var allTests: [(String, (UntypedErrorInCatchRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1683,6 +1689,7 @@ XCTMain([
testCase(UnavailableFunctionRuleTests.allTests),
testCase(UnneededBreakInSwitchRuleTests.allTests),
testCase(UnneededParenthesesInClosureArgumentRuleTests.allTests),
testCase(UnownedVariableCaptureRuleTests.allTests),
testCase(UntypedErrorInCatchRuleTests.allTests),
testCase(UnusedCaptureListRuleTests.allTests),
testCase(UnusedClosureParameterRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,12 @@ class UnneededParenthesesInClosureArgumentRuleTests: XCTestCase {
}
}

class UnownedVariableCaptureRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(UnownedVariableCaptureRule.description)
}
}

class UntypedErrorInCatchRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(UntypedErrorInCatchRule.description)
Expand Down

0 comments on commit c33fc83

Please sign in to comment.