From d5f12cbb05567c0d50358d41f36bec04e15532de Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sun, 6 Nov 2022 23:06:29 -0800 Subject: [PATCH] Rewrite `implicit_return` rule with SwiftSyntax --- CHANGELOG.md | 1 + .../ImplicitReturnConfiguration.swift | 4 +- .../Rules/Style/ImplicitReturnRule.swift | 217 +++++++++++++----- .../Style/ImplicitReturnRuleExamples.swift | 8 +- 4 files changed, 167 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f3527de..e0b2d7048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ - `ibinspectable_in_extension` - `identical_operands` - `implicit_getter` + - `implicit_return` - `implicitly_unwrapped_optional` - `inclusive_language` - `inert_defer` diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift index 962619e1b..ee566dee9 100644 --- a/Source/SwiftLintFramework/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/ImplicitReturnConfiguration.swift @@ -1,4 +1,4 @@ -public struct ImplicitReturnConfiguration: RuleConfiguration, Equatable { +public struct ImplicitReturnConfiguration: SeverityBasedRuleConfiguration, Equatable { public enum ReturnKind: String, CaseIterable { case closure case function @@ -7,7 +7,7 @@ public struct ImplicitReturnConfiguration: RuleConfiguration, Equatable { public static let defaultIncludedKinds = Set(ReturnKind.allCases) - private(set) var severityConfiguration = SeverityConfiguration(.warning) + public private(set) var severityConfiguration = SeverityConfiguration(.warning) private(set) var includedKinds = Self.defaultIncludedKinds diff --git a/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRule.swift b/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRule.swift index a6f088577..31b880bce 100644 --- a/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRule.swift +++ b/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRule.swift @@ -1,7 +1,6 @@ -import Foundation -import SourceKittenFramework +import SwiftSyntax -public struct ImplicitReturnRule: ConfigurationProviderRule, SubstitutionCorrectableRule, OptInRule { +public struct ImplicitReturnRule: ConfigurationProviderRule, SwiftSyntaxCorrectableRule, OptInRule { public var configuration = ImplicitReturnConfiguration() public init() {} @@ -16,72 +15,176 @@ public struct ImplicitReturnRule: ConfigurationProviderRule, SubstitutionCorrect corrections: ImplicitReturnRuleExamples.corrections ) - public 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)) - } + public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(includedKinds: configuration.includedKinds) } - public func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? { - return (violationRange, "") - } - - public func violationRanges(in file: SwiftLintFile) -> [NSRange] { - let pattern = "(?:\\bin|\\{)\\s+(return\\s+)" - let contents = file.stringView - - 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 - } - - func isKindIncluded(_ kind: ImplicitReturnConfiguration.ReturnKind) -> Bool { - return self.configuration.isKindIncluded(kind) - } - - if let outerKind = SwiftExpressionKind(rawValue: outerKindString), - isKindIncluded(.closure), - [.call, .argument, .closure].contains(outerKind) { - return result.range(at: 1) - } - - if let outerKind = SwiftDeclarationKind(rawValue: outerKindString), - (isKindIncluded(.function) && SwiftDeclarationKind.functionKinds.contains(outerKind)) || - (isKindIncluded(.getter) && SwiftDeclarationKind.variableKinds.contains(outerKind)) { - return result.range(at: 1) - } - - return nil - } + public func makeRewriter(file: SwiftLintFile) -> ViolationsSyntaxRewriter? { + Rewriter( + includedKinds: configuration.includedKinds, + locationConverter: file.locationConverter, + disabledRegions: disabledRegions(file: file) + ) } } -private extension Array where Element == (kind: String, byteRange: ByteRange) { - func lastExcludingBrace() -> Element? { - guard SwiftVersion.current >= .fiveDotFour else { - return last +private extension ImplicitReturnRule { + final class Visitor: ViolationsSyntaxVisitor { + private let includedKinds: Set + + init(includedKinds: Set) { + self.includedKinds = includedKinds + super.init(viewMode: .sourceAccurate) } - guard let last = last else { - return nil + override func visitPost(_ node: ClosureExprSyntax) { + guard includedKinds.contains(.closure), + let statement = node.statements.onlyElement, + statement.item.is(ReturnStmtSyntax.self) else { + return + } + + violations.append(statement.item.positionAfterSkippingLeadingTrivia) } - guard last.kind == "source.lang.swift.stmt.brace", count > 1 else { - return last + override func visitPost(_ node: FunctionDeclSyntax) { + guard includedKinds.contains(.function), + let statement = node.body?.statements.onlyElement, + statement.item.is(ReturnStmtSyntax.self) else { + return + } + + violations.append(statement.item.positionAfterSkippingLeadingTrivia) } - let secondLast = self[endIndex - 2] - if SwiftExpressionKind(rawValue: secondLast.kind) == .closure { - return secondLast + override func visitPost(_ node: VariableDeclSyntax) { + guard includedKinds.contains(.getter) else { + return + } + + for binding in node.bindings { + switch binding.accessor { + case nil: + continue + case .accessors(let accessors): + if let statement = accessors.getAccessor?.body?.statements.onlyElement, + let returnStmt = statement.item.as(ReturnStmtSyntax.self) { + violations.append(returnStmt.positionAfterSkippingLeadingTrivia) + } + case .getter(let getter): + if let returnStmt = getter.statements.onlyElement?.item.as(ReturnStmtSyntax.self) { + violations.append(returnStmt.positionAfterSkippingLeadingTrivia) + } + } + } + } + } + + final class Rewriter: SyntaxRewriter, ViolationsSyntaxRewriter { + private(set) var correctionPositions: [AbsolutePosition] = [] + private let includedKinds: Set + private let locationConverter: SourceLocationConverter + private let disabledRegions: [SourceRange] + + init(includedKinds: Set, + locationConverter: SourceLocationConverter, + disabledRegions: [SourceRange]) { + self.includedKinds = includedKinds + self.locationConverter = locationConverter + self.disabledRegions = disabledRegions } - return last + override func visit(_ node: ClosureExprSyntax) -> ExprSyntax { + guard includedKinds.contains(.closure), + let statement = node.statements.onlyElement, + let returnStmt = statement.item.as(ReturnStmtSyntax.self), + let expr = returnStmt.expression, + !returnStmt.isContainedIn(regions: disabledRegions, locationConverter: locationConverter) else { + return super.visit(node) + } + + correctionPositions.append(returnStmt.positionAfterSkippingLeadingTrivia) + + let newNode = node.withStatements([ + statement + .withItem(.expr(expr)) + .withLeadingTrivia(returnStmt.leadingTrivia ?? .zero) + ]) + return super.visit(newNode) + } + + override func visit(_ node: FunctionDeclSyntax) -> DeclSyntax { + guard includedKinds.contains(.function), + let statement = node.body?.statements.onlyElement, + let returnStmt = statement.item.as(ReturnStmtSyntax.self), + let expr = returnStmt.expression, + !returnStmt.isContainedIn(regions: disabledRegions, locationConverter: locationConverter) else { + return super.visit(node) + } + + correctionPositions.append(returnStmt.positionAfterSkippingLeadingTrivia) + + let newNode = node.withBody(node.body?.withStatements([ + statement + .withItem(.expr(expr)) + .withLeadingTrivia(returnStmt.leadingTrivia ?? .zero) + ])) + return super.visit(newNode) + } + + override func visit(_ node: VariableDeclSyntax) -> DeclSyntax { + guard includedKinds.contains(.getter) else { + return super.visit(node) + } + + let updatedBindings: [PatternBindingSyntax] = node.bindings.map { binding in + switch binding.accessor { + case nil: + return binding + case .accessors(let accessorBlock): + guard let getAccessor = accessorBlock.getAccessor, + let statement = accessorBlock.getAccessor?.body?.statements.onlyElement, + let returnStmt = statement.item.as(ReturnStmtSyntax.self), + !returnStmt.isContainedIn(regions: disabledRegions, locationConverter: locationConverter), + let expr = returnStmt.expression else { + break + } + + correctionPositions.append(returnStmt.positionAfterSkippingLeadingTrivia) + + let updatedGetAcessor = getAccessor.withBody(getAccessor.body?.withStatements([ + statement + .withItem(.expr(expr)) + .withLeadingTrivia(returnStmt.leadingTrivia ?? .zero) + ])) + let updatedAccessors = accessorBlock + .withAccessors( + accessorBlock.accessors.replacing( + childAt: getAccessor.indexInParent, + with: updatedGetAcessor + ) + ) + return binding.withAccessor(.accessors(updatedAccessors)) + case .getter(let getter): + guard let statement = getter.statements.onlyElement, + let returnStmt = statement.item.as(ReturnStmtSyntax.self), + !returnStmt.isContainedIn(regions: disabledRegions, locationConverter: locationConverter), + let expr = returnStmt.expression else { + break + } + + correctionPositions.append(returnStmt.positionAfterSkippingLeadingTrivia) + return binding.withAccessor(.getter(getter.withStatements([ + statement + .withItem(.expr(expr)) + .withLeadingTrivia(returnStmt.leadingTrivia ?? .zero) + ]))) + } + + return binding + } + + return super.visit(node.withBindings(PatternBindingListSyntax(updatedBindings))) + } } } diff --git a/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRuleExamples.swift b/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRuleExamples.swift index 1f5d0d5cf..3d75e3116 100644 --- a/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRuleExamples.swift +++ b/Source/SwiftLintFramework/Rules/Style/ImplicitReturnRuleExamples.swift @@ -1,9 +1,9 @@ internal struct ImplicitReturnRuleExamples { - internal struct GenericExamples { + internal enum GenericExamples { static let nonTriggeringExamples = [Example("if foo {\n return 0\n}")] } - internal struct ClosureExamples { + internal enum ClosureExamples { static let nonTriggeringExamples = [ Example("foo.map { $0 + 1 }"), Example("foo.map({ $0 + 1 })"), @@ -42,7 +42,7 @@ internal struct ImplicitReturnRuleExamples { ] } - internal struct FunctionExamples { + internal enum FunctionExamples { static let nonTriggeringExamples = [ Example(""" func foo() -> Int { @@ -85,7 +85,7 @@ internal struct ImplicitReturnRuleExamples { ] } - internal struct GetterExamples { + internal enum GetterExamples { static let nonTriggeringExamples = [ Example("var foo: Bool { true }"), Example("""