diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e02dec0d..b58a233ae7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/Rules.md b/Rules.md index b9b096c217..32d56f3a02 100644 --- a/Rules.md +++ b/Rules.md @@ -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) @@ -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 + +
+Non Triggering Examples + +```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 } +``` + +
+
+Triggering Examples + +```swift +foo { [↓unowned self] in _ } +``` + +```swift +foo { [↓unowned bar] in _ } +``` + +```swift +foo { [bar, ↓unowned self] in _ } +``` + +
+ + + ## Untyped Error in Catch Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 61c4d27cf9..102cce0ecb 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -155,6 +155,7 @@ public let masterRuleList = RuleList(rules: [ UnavailableFunctionRule.self, UnneededBreakInSwitchRule.self, UnneededParenthesesInClosureArgumentRule.self, + UnownedVariableCaptureRule.self, UntypedErrorInCatchRule.self, UnusedCaptureListRule.self, UnusedClosureParameterRule.self, diff --git a/Source/SwiftLintFramework/Rules/Lint/UnownedVariableCaptureRule.swift b/Source/SwiftLintFramework/Rules/Lint/UnownedVariableCaptureRule.swift new file mode 100644 index 0000000000..7ba961223f --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Lint/UnownedVariableCaptureRule.swift @@ -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 + } +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index a5efe1b7df..f62957d6ef 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -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 */; }; @@ -794,6 +795,7 @@ D4B022B11E10B613007E5297 /* RedundantVoidReturnRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantVoidReturnRule.swift; sourceTree = ""; }; D4B3409C21F16B110038C79A /* UnusedSetterValueRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnusedSetterValueRule.swift; sourceTree = ""; }; D4B472401F66486300BD6EF1 /* FallthroughRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FallthroughRule.swift; sourceTree = ""; }; + D4BED5F72278AECC00D86BCE /* UnownedVariableCaptureRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnownedVariableCaptureRule.swift; sourceTree = ""; }; D4C0E46E1E3D973600C560F2 /* ForWhereRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ForWhereRule.swift; sourceTree = ""; }; D4C27BFD1E12D53F00DF713E /* Version.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Version.swift; sourceTree = ""; }; D4C27BFF1E12DFF500DF713E /* LinterCacheTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LinterCacheTests.swift; sourceTree = ""; }; @@ -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 */, @@ -1732,6 +1735,7 @@ developmentRegion = English; hasScannedForEncodings = 0; knownRegions = ( + English, en, Base, ); @@ -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 */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index d5813e4899..29c77911ca 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -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) @@ -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), diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index 272088cd31..adb224e013 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -672,6 +672,12 @@ class UnneededParenthesesInClosureArgumentRuleTests: XCTestCase { } } +class UnownedVariableCaptureRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(UnownedVariableCaptureRule.description) + } +} + class UntypedErrorInCatchRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(UntypedErrorInCatchRule.description)