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 unowned_variable_capture opt-in rule #2739

Merged
merged 1 commit into from
May 2, 2019
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
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