Rewrite implicit_return rule using SwiftSyntax (#5166)

realm · Sep 2, 2023 · 02640a3 · 02640a3
1 parent e82fad9
commit 02640a3
Show file tree

Hide file tree

Showing 7 changed files with 391 additions and 129 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,10 @@
   [mt00chikin](https://github.com/mt00chikin)
   [#3173](https://github.com/realm/SwiftLint/issues/3173)
 
+* The `implicit_return` rule now supports the kinds `subscript` and
+  `initializer` in the `included` configuration list.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+
 * Add `unneeded_override` rule to remove function overrides that only
   call super.  
   [keith](https://github.com/keith)
@@ -85,6 +89,11 @@
 * Document `exclude_ranges` option for `number_separator` rule.  
   [SimplyDanny](https://github.com/SimplyDanny)
 
+* Rewrite `implicit_return` rule with SwiftSyntax fixing a few false positives
+  and false negatives in the process.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+  [#5161](https://github.com/realm/SwiftLint/issues/5161)
+
 * Make sure `severity` is configurable for `type_contents_order` rule.  
   [SimplyDanny](https://github.com/SimplyDanny)
 

diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift
@@ -7,6 +7,8 @@ struct ImplicitReturnConfiguration: SeverityBasedRuleConfiguration, Equatable {
         case closure
         case function
         case getter
+        case `subscript`
+        case initializer
 
         func asOption() -> OptionType { .symbol(rawValue) }
 

diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitReturnRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitReturnRule.swift
@@ -1,7 +1,6 @@
-import Foundation
-import SourceKittenFramework
+import SwiftSyntax
 
-struct ImplicitReturnRule: ConfigurationProviderRule, SubstitutionCorrectableRule, OptInRule {
+struct ImplicitReturnRule: SwiftSyntaxCorrectableRule, ConfigurationProviderRule, OptInRule {
     var configuration = ImplicitReturnConfiguration()
 
     static let description = RuleDescription(
@@ -14,72 +13,78 @@ struct ImplicitReturnRule: ConfigurationProviderRule, SubstitutionCorrectableRul
         corrections: ImplicitReturnRuleExamples.corrections
     )
 
-    func validate(file: SwiftLintFile) -> [StyleViolation] {
-        return violationRanges(in: file).compactMap {
-            StyleViolation(ruleDescription: Self.description,
-                           severity: configuration.severityConfiguration.severity,
-                           location: Location(file: file, characterOffset: $0.location))
-        }
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
+        Visitor(config: configuration)
     }
+}
 
-    func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? {
-        return (violationRange, "")
-    }
+private extension ImplicitReturnRule {
+    private class Visitor: ViolationsSyntaxVisitor {
+        private let config: ConfigurationType
 
-    func violationRanges(in file: SwiftLintFile) -> [NSRange] {
-        let pattern = "(?:\\bin|\\{)\\s+(return\\s+)"
-        let contents = file.stringView
+        override var skippableDeclarations: [DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }
 
-        return file.matchesAndSyntaxKinds(matching: pattern).compactMap { result, kinds in
-            let range = result.range
-            guard kinds == [.keyword, .keyword] || kinds == [.keyword],
-                let byteRange = contents.NSRangeToByteRange(start: range.location, length: range.length),
-                case let kinds = file.structureDictionary.kinds(forByteOffset: byteRange.location),
-                let outerKindString = kinds.lastExcludingBrace()?.kind
-            else {
-                return nil
-            }
+        init(config: ConfigurationType) {
+            self.config = config
+            super.init(viewMode: .sourceAccurate)
+        }
 
-            func isKindIncluded(_ kind: ImplicitReturnConfiguration.ReturnKind) -> Bool {
-                return self.configuration.isKindIncluded(kind)
+        override func visitPost(_ node: AccessorDeclSyntax) {
+            if config.isKindIncluded(.getter),
+               node.accessorSpecifier.tokenKind == .keyword(.get),
+               let body = node.body {
+                collectViolation(in: body.statements)
             }
+        }
 
-            if let outerKind = SwiftExpressionKind(rawValue: outerKindString),
-                isKindIncluded(.closure),
-                [.call, .argument, .closure].contains(outerKind) {
-                    return result.range(at: 1)
+        override func visitPost(_ node: ClosureExprSyntax) {
+            if config.isKindIncluded(.closure) {
+                collectViolation(in: node.statements)
             }
+        }
 
-            if let outerKind = SwiftDeclarationKind(rawValue: outerKindString),
-                (isKindIncluded(.function) && SwiftDeclarationKind.functionKinds.contains(outerKind)) ||
-                (isKindIncluded(.getter) && SwiftDeclarationKind.variableKinds.contains(outerKind)) {
-                    return result.range(at: 1)
+        override func visitPost(_ node: FunctionDeclSyntax) {
+            if config.isKindIncluded(.function),
+               let body = node.body {
+                collectViolation(in: body.statements)
             }
-
-            return nil
         }
-    }
-}
 
-private extension Array where Element == (kind: String, byteRange: ByteRange) {
-    func lastExcludingBrace() -> Element? {
-        guard SwiftVersion.current >= .fiveDotFour else {
-            return last
+        override func visitPost(_ node: InitializerDeclSyntax) {
+            if config.isKindIncluded(.initializer),
+               let body = node.body {
+                collectViolation(in: body.statements)
+            }
         }
 
-        guard let last else {
-            return nil
+        override func visitPost(_ node: PatternBindingSyntax) {
+            if config.isKindIncluded(.getter),
+               case let .getter(itemList) = node.accessorBlock?.accessors {
+                collectViolation(in: itemList)
+            }
         }
 
-        guard last.kind == "source.lang.swift.stmt.brace", count > 1 else {
-            return last
+        override func visitPost(_ node: SubscriptDeclSyntax) {
+            if config.isKindIncluded(.subscript),
+               case let .getter(itemList) = node.accessorBlock?.accessors {
+                collectViolation(in: itemList)
+            }
         }
 
-        let secondLast = self[endIndex - 2]
-        if SwiftExpressionKind(rawValue: secondLast.kind) == .closure {
-            return secondLast
+        private func collectViolation(in itemList: CodeBlockItemListSyntax) {
+            guard let returnStmt = itemList.onlyElement?.item.as(ReturnStmtSyntax.self) else {
+                return
+            }
+            let returnKeyword = returnStmt.returnKeyword
+            violations.append(returnKeyword.positionAfterSkippingLeadingTrivia)
+            violationCorrections.append(
+                ViolationCorrection(
+                    start: returnKeyword.positionAfterSkippingLeadingTrivia,
+                    end: returnKeyword.endPositionBeforeTrailingTrivia
+                            .advanced(by: returnStmt.expression == nil ? 0 : 1),
+                    replacement: ""
+                )
+            )
         }
-
-        return last
     }
 }