From 53647b4e6ed50ddcd9e9704099ef55bd6aa00737 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 26 Aug 2018 03:10:16 -0700 Subject: [PATCH] Add inert_defer rule Fixes #2123 --- CHANGELOG.md | 5 ++ Rules.md | 61 +++++++++++++ .../Models/MasterRuleList.swift | 1 + .../Rules/Lint/InertDeferRule.swift | 89 +++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 4 + Tests/LinuxMain.swift | 7 ++ .../AutomaticRuleTests.generated.swift | 6 ++ 7 files changed, 173 insertions(+) create mode 100644 Source/SwiftLintFramework/Rules/Lint/InertDeferRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 20c9dbcf1..9ceb1d4f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,11 @@ [Marcelo Fabri](https://github.com/marcelofabri) [#2365](https://github.com/realm/SwiftLint/pull/2365) +* Add `inert_defer` rule to validate that `defer` is not used at the end of a + scope. + [Marcelo Fabri](https://github.com/marcelofabri) + [#2123](https://github.com/realm/SwiftLint/issues/2123) + #### Bug Fixes * Fix `comma` rule false positives on object literals (for example, images). diff --git a/Rules.md b/Rules.md index 37bcf6244..7d46d6d72 100644 --- a/Rules.md +++ b/Rules.md @@ -56,6 +56,7 @@ * [Implicit Getter](#implicit-getter) * [Implicit Return](#implicit-return) * [Implicitly Unwrapped Optional](#implicitly-unwrapped-optional) +* [Inert Defer](#inert-defer) * [Is Disjoint](#is-disjoint) * [Joined Default Parameter](#joined-default-parameter) * [Large Tuple](#large-tuple) @@ -8213,6 +8214,66 @@ func foo(int: Int!) {} +## Inert Defer + +Identifier | Enabled by default | Supports autocorrection | Kind | Minimum Swift Compiler Version +--- | --- | --- | --- | --- +`inert_defer` | Enabled | No | lint | 3.0.0 + +If defer is at the end of its parent scope, it will be executed right where it is anyway. + +### Examples + +
+Non Triggering Examples + +```swift +func example3() { + defer { /* deferred code */ } + + print("other code") +} +``` + +```swift +func example4() { + if condition { + defer { /* deferred code */ } + print("other code") + } +} +``` + +
+
+Triggering Examples + +```swift +func example0() { + ↓defer { /* deferred code */ } +} +``` + +```swift +func example1() { + ↓defer { /* deferred code */ } + // comment +} +``` + +```swift +func example2() { + if condition { + ↓defer { /* deferred code */ } + // comment + } +} +``` + +
+ + + ## Is Disjoint Identifier | Enabled by default | Supports autocorrection | Kind | Minimum Swift Compiler Version diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index d6cad323d..f03f64c80 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -57,6 +57,7 @@ public let masterRuleList = RuleList(rules: [ ImplicitGetterRule.self, ImplicitReturnRule.self, ImplicitlyUnwrappedOptionalRule.self, + InertDeferRule.self, IsDisjointRule.self, JoinedDefaultParameterRule.self, LargeTupleRule.self, diff --git a/Source/SwiftLintFramework/Rules/Lint/InertDeferRule.swift b/Source/SwiftLintFramework/Rules/Lint/InertDeferRule.swift new file mode 100644 index 000000000..7bac937b2 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Lint/InertDeferRule.swift @@ -0,0 +1,89 @@ +import Foundation +import SourceKittenFramework + +public struct InertDeferRule: ConfigurationProviderRule, AutomaticTestableRule { + public var configuration = SeverityConfiguration(.warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "inert_defer", + name: "Inert Defer", + description: "If defer is at the end of its parent scope, it will be executed right where it is anyway.", + kind: .lint, + nonTriggeringExamples: [ + """ + func example3() { + defer { /* deferred code */ } + + print("other code") + } + """, + """ + func example4() { + if condition { + defer { /* deferred code */ } + print("other code") + } + } + """ + ], + triggeringExamples: [ + """ + func example0() { + ↓defer { /* deferred code */ } + } + """, + """ + func example1() { + ↓defer { /* deferred code */ } + // comment + } + """, + """ + func example2() { + if condition { + ↓defer { /* deferred code */ } + // comment + } + } + """ + ] + ) + + public func validate(file: File) -> [StyleViolation] { + let defers = file.match(pattern: "defer\\s*\\{", with: [.keyword]) + + return defers.compactMap { range -> StyleViolation? in + let contents = file.contents.bridge() + guard let byteRange = contents.NSRangeToByteRange(start: range.location, length: range.length), + case let kinds = file.structure.kinds(forByteOffset: byteRange.upperBound), + let brace = kinds.enumerated().lazy.reversed().first(where: isBrace), + brace.offset > kinds.startIndex, + case let outerKindIndex = kinds.index(before: brace.offset), + case let outerKind = kinds[outerKindIndex], + case let braceEnd = brace.element.byteRange.upperBound, + case let tokensRange = NSRange(location: braceEnd, length: outerKind.byteRange.upperBound - braceEnd), + case let tokens = file.syntaxMap.tokens(inByteRange: tokensRange), + !tokens.contains(where: isNotComment) else { + return nil + } + + return StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, characterOffset: range.location)) + } + } +} + +private func isBrace(offset: Int, element: (kind: String, byteRange: NSRange)) -> Bool { + return StatementKind(rawValue: element.kind) == .brace +} + +private func isNotComment(token: SyntaxToken) -> Bool { + guard let kind = SyntaxKind(rawValue: token.type) else { + return false + } + + return !SyntaxKind.commentKinds.contains(kind) +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index bac96091d..8fbe9c324 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -213,6 +213,7 @@ D44037972132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D44037962132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift */; }; D44254201DB87CA200492EA4 /* ValidIBInspectableRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */; }; D44254271DB9C15C00492EA4 /* SyntacticSugarRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D44254251DB9C12300492EA4 /* SyntacticSugarRule.swift */; }; + D4441A28213279950020896F /* InertDeferRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4441A27213279950020896F /* InertDeferRule.swift */; }; D4470D571EB69225008A1B2E /* ImplicitReturnRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4470D561EB69225008A1B2E /* ImplicitReturnRule.swift */; }; D4470D591EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4470D581EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift */; }; D4470D5B1EB76F44008A1B2E /* UnusedOptionalBindingRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4470D5A1EB76F44008A1B2E /* UnusedOptionalBindingRuleTests.swift */; }; @@ -637,6 +638,7 @@ D44037962132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProhibitedInterfaceBuilderRule.swift; sourceTree = ""; }; D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ValidIBInspectableRule.swift; sourceTree = ""; }; D44254251DB9C12300492EA4 /* SyntacticSugarRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SyntacticSugarRule.swift; sourceTree = ""; }; + D4441A27213279950020896F /* InertDeferRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InertDeferRule.swift; sourceTree = ""; }; D4470D561EB69225008A1B2E /* ImplicitReturnRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImplicitReturnRule.swift; sourceTree = ""; }; D4470D581EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EmptyEnumArgumentsRule.swift; sourceTree = ""; }; D4470D5A1EB76F44008A1B2E /* UnusedOptionalBindingRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedOptionalBindingRuleTests.swift; sourceTree = ""; }; @@ -947,6 +949,7 @@ E315B83B1DFA4BC500621B44 /* DynamicInlineRule.swift */, 62A3E95B209E078000547A86 /* EmptyXCTestMethodRule.swift */, 626B01B420A1735900D2C42F /* EmptyXCTestMethodRuleExamples.swift */, + D4441A27213279950020896F /* InertDeferRule.swift */, C26330352073DAA200D7B4FD /* LowerACLThanParentRule.swift */, 856651A61D6B395F005E6B29 /* MarkRule.swift */, F9E691272091952E0085B53E /* MissingDocsRule.swift */, @@ -1942,6 +1945,7 @@ D4D5A5FF1E1F3A1C00D15E0C /* ShorthandOperatorRule.swift in Sources */, C3DE5DAC1E7DF9CA00761483 /* FatalErrorMessageRule.swift in Sources */, 626C16E21F948EBC00BB7475 /* QuickDiscouragedFocusedTestRuleExamples.swift in Sources */, + D4441A28213279950020896F /* InertDeferRule.swift in Sources */, B89F3BCF1FD5EE1400931E59 /* RequiredEnumCaseRuleConfiguration.swift in Sources */, D48B51211F4F5DEF0068AB98 /* RuleList+Documentation.swift in Sources */, 8FC9F5111F4B8E48006826C1 /* IsDisjointRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 28c0435e1..d976a8bf9 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -531,6 +531,12 @@ extension ImplicitlyUnwrappedOptionalRuleTests { ] } +extension InertDeferRuleTests { + static var allTests: [(String, (InertDeferRuleTests) -> () throws -> Void)] = [ + ("testWithDefaultConfiguration", testWithDefaultConfiguration) + ] +} + extension IntegrationTests { static var allTests: [(String, (IntegrationTests) -> () throws -> Void)] = [ ("testSwiftLintLints", testSwiftLintLints), @@ -1306,6 +1312,7 @@ XCTMain([ testCase(ImplicitReturnRuleTests.allTests), testCase(ImplicitlyUnwrappedOptionalConfigurationTests.allTests), testCase(ImplicitlyUnwrappedOptionalRuleTests.allTests), + testCase(InertDeferRuleTests.allTests), testCase(IntegrationTests.allTests), testCase(IsDisjointRuleTests.allTests), testCase(JoinedDefaultParameterRuleTests.allTests), diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index 3ced469dc..4f717b04c 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -246,6 +246,12 @@ class ImplicitReturnRuleTests: XCTestCase { } } +class InertDeferRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(InertDeferRule.description) + } +} + class IsDisjointRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(IsDisjointRule.description)