From 88b8bc421f4d66ff15e9a9d630b4bf82bf4ffdcd Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Sat, 8 Feb 2025 04:57:45 -0600 Subject: [PATCH 1/4] Convert UseLetInEveryBoundCaseVariable to be a formatter Uses the existing logic for determining whether a binding can be moved into a tuple or function call context, and then rewrites the tuple element list or argument list to including the binding keyword. Additionally, improves the diagnostic by using the binding specifier in the source code (i.e. `let` vs `var`). --- Documentation/RuleDocumentation.md | 5 +- .../Core/Pipelines+Generated.swift | 27 ++- .../UseLetInEveryBoundCaseVariable.swift | 154 ++++++++++-- .../UseLetInEveryBoundCaseVariableTests.swift | 226 +++++++++++++----- 4 files changed, 328 insertions(+), 84 deletions(-) diff --git a/Documentation/RuleDocumentation.md b/Documentation/RuleDocumentation.md index 2d0b416cd..0b626d5cf 100644 --- a/Documentation/RuleDocumentation.md +++ b/Documentation/RuleDocumentation.md @@ -505,7 +505,10 @@ For example, `case let .identifier(x, y)` is forbidden. Use Lint: `case let .identifier(...)` will yield a lint error. -`UseLetInEveryBoundCaseVariable` is a linter-only rule. +Format: `case let .identifier(x, y)` will be replaced by +`case .identifier(let x, let y)`. + +`UseLetInEveryBoundCaseVariable` rule can format your code automatically. ### UseShorthandTypeNames diff --git a/Sources/SwiftFormat/Core/Pipelines+Generated.swift b/Sources/SwiftFormat/Core/Pipelines+Generated.swift index 6f22e1384..8b0906e3e 100644 --- a/Sources/SwiftFormat/Core/Pipelines+Generated.swift +++ b/Sources/SwiftFormat/Core/Pipelines+Generated.swift @@ -225,10 +225,12 @@ class LintPipeline: SyntaxVisitor { } override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind { + visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node) visitIfEnabled(UseWhereClausesInForLoops.visit, for: node) return .visitChildren } override func visitPost(_ node: ForStmtSyntax) { + onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node) onVisitPost(rule: UseWhereClausesInForLoops.self, for: node) } @@ -386,6 +388,14 @@ class LintPipeline: SyntaxVisitor { onVisitPost(rule: NoPlaygroundLiterals.self, for: node) } + override func visit(_ node: MatchingPatternConditionSyntax) -> SyntaxVisitorContinueKind { + visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node) + return .visitChildren + } + override func visitPost(_ node: MatchingPatternConditionSyntax) { + onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node) + } + override func visit(_ node: MemberBlockItemListSyntax) -> SyntaxVisitorContinueKind { visitIfEnabled(DoNotUseSemicolons.visit, for: node) return .visitChildren @@ -510,6 +520,14 @@ class LintPipeline: SyntaxVisitor { onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node) } + override func visit(_ node: SwitchCaseItemSyntax) -> SyntaxVisitorContinueKind { + visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node) + return .visitChildren + } + override func visitPost(_ node: SwitchCaseItemSyntax) { + onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node) + } + override func visit(_ node: SwitchCaseLabelSyntax) -> SyntaxVisitorContinueKind { visitIfEnabled(NoLabelsInCasePatterns.visit, for: node) return .visitChildren @@ -568,14 +586,6 @@ class LintPipeline: SyntaxVisitor { onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node) } - override func visit(_ node: ValueBindingPatternSyntax) -> SyntaxVisitorContinueKind { - visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node) - return .visitChildren - } - override func visitPost(_ node: ValueBindingPatternSyntax) { - onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node) - } - override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind { visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node) visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node) @@ -627,6 +637,7 @@ extension FormatPipeline { node = ReturnVoidInsteadOfEmptyTuple(context: context).rewrite(node) node = UseEarlyExits(context: context).rewrite(node) node = UseExplicitNilCheckInConditions(context: context).rewrite(node) + node = UseLetInEveryBoundCaseVariable(context: context).rewrite(node) node = UseShorthandTypeNames(context: context).rewrite(node) node = UseSingleLinePropertyGetter(context: context).rewrite(node) node = UseTripleSlashForDocumentationComments(context: context).rewrite(node) diff --git a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift index 840a8496f..23ddca9ed 100644 --- a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift +++ b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift @@ -18,30 +18,110 @@ import SwiftSyntax /// `case .identifier(let x, let y)` instead. /// /// Lint: `case let .identifier(...)` will yield a lint error. +/// +/// Format: `case let .identifier(x, y)` will be replaced by +/// `case .identifier(let x, let y)`. @_spi(Rules) -public final class UseLetInEveryBoundCaseVariable: SyntaxLintRule { +public final class UseLetInEveryBoundCaseVariable: SyntaxFormatRule { + public override func visit(_ node: MatchingPatternConditionSyntax) -> MatchingPatternConditionSyntax { + if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) { + diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern) + + var result = node + result.pattern = PatternSyntax(replacement) + return result + } + + return super.visit(node) + } + + public override func visit(_ node: SwitchCaseItemSyntax) -> SwitchCaseItemSyntax { + if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) { + diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern) + + var result = node + result.pattern = PatternSyntax(replacement) + return result + } + + return super.visit(node) + } + + public override func visit(_ node: ForStmtSyntax) -> StmtSyntax { + guard node.caseKeyword != nil else { + return super.visit(node) + } + + if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) { + diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern) - public override func visit(_ node: ValueBindingPatternSyntax) -> SyntaxVisitorContinueKind { - // Diagnose a pattern binding if it is a function call and the callee is a member access - // expression (e.g., `case let .x(y)` or `case let T.x(y)`). - if canDistributeLetVarThroughPattern(node.pattern) { - diagnose(.useLetInBoundCaseVariables, on: node) + var result = node + result.pattern = PatternSyntax(replacement) + return StmtSyntax(result) } - return .visitChildren + + return super.visit(node) + } +} + +extension UseLetInEveryBoundCaseVariable { + private enum OptionalPatternKind { + case chained + case forced } - /// Returns true if the given pattern is one that allows a `let/var` to be distributed - /// through to subpatterns. - private func canDistributeLetVarThroughPattern(_ pattern: PatternSyntax) -> Bool { - guard let exprPattern = pattern.as(ExpressionPatternSyntax.self) else { return false } + /// Wraps the given expression in the optional chaining and/or force + /// unwrapping expressions, as described by the specified stack. + private func restoreOptionalChainingAndForcing( + _ expr: ExprSyntax, + patternStack: [(OptionalPatternKind, Trivia)] + ) -> ExprSyntax { + var patternStack = patternStack + var result = expr + + // As we unwind the stack, wrap the expression in optional chaining + // or force unwrap expressions. + while let (kind, trivia) = patternStack.popLast() { + if kind == .chained { + result = ExprSyntax(OptionalChainingExprSyntax( + expression: result, trailingTrivia: trivia)) + } else { + result = ExprSyntax(ForceUnwrapExprSyntax( + expression: result, trailingTrivia: trivia)) + } + } + + return result + } + + /// Returns a rewritten version of the given pattern if bindings can be moved + /// into bound cases. + /// + /// - Parameter pattern: The pattern to rewrite. + /// - Returns: An optional tuple with the rewritten pattern and the binding + /// specifier used in `pattern`, for use in the diagnostic. If `pattern` + /// doesn't qualify for distributing the binding, then the result is `nil`. + private func distributeLetVarThroughPattern( + _ pattern: PatternSyntax + ) -> (ExpressionPatternSyntax, TokenSyntax)? { + guard let bindingPattern = pattern.as(ValueBindingPatternSyntax.self), + let exprPattern = bindingPattern.pattern.as(ExpressionPatternSyntax.self) + else { return nil } + // Grab the `let` or `var` used in the binding pattern. + let specifier = bindingPattern.bindingSpecifier + let identifierBinder = BindIdentifiersRewriter(bindingSpecifier: specifier) + // Drill down into any optional patterns that we encounter (e.g., `case let .foo(x)?`). + var patternStack: [(OptionalPatternKind, Trivia)] = [] var expression = exprPattern.expression while true { if let optionalExpr = expression.as(OptionalChainingExprSyntax.self) { expression = optionalExpr.expression + patternStack.append((.chained, optionalExpr.questionMark.trailingTrivia)) } else if let forcedExpr = expression.as(ForceUnwrapExprSyntax.self) { expression = forcedExpr.expression + patternStack.append((.forced, forcedExpr.exclamationMark.trailingTrivia)) } else { break } @@ -49,23 +129,61 @@ public final class UseLetInEveryBoundCaseVariable: SyntaxLintRule { // Enum cases are written as function calls on member access expressions. The arguments // are the associated values, so the `let/var` can be distributed into those. - if let functionCall = expression.as(FunctionCallExprSyntax.self), + if var functionCall = expression.as(FunctionCallExprSyntax.self), functionCall.calledExpression.is(MemberAccessExprSyntax.self) { - return true + var result = exprPattern + let newArguments = identifierBinder.rewrite(functionCall.arguments) + functionCall.arguments = newArguments.as(LabeledExprListSyntax.self)! + result.expression = restoreOptionalChainingAndForcing( + ExprSyntax(functionCall), + patternStack: patternStack) + return (result, specifier) } // A tuple expression can have the `let/var` distributed into the elements. - if expression.is(TupleExprSyntax.self) { - return true + if var tupleExpr = expression.as(TupleExprSyntax.self) { + var result = exprPattern + let newElements = identifierBinder.rewrite(tupleExpr.elements) + tupleExpr.elements = newElements.as(LabeledExprListSyntax.self)! + result.expression = restoreOptionalChainingAndForcing( + ExprSyntax(tupleExpr), + patternStack: patternStack) + return (result, specifier) } // Otherwise, we're not sure this is a pattern we can distribute through. - return false + return nil } } extension Finding.Message { - fileprivate static let useLetInBoundCaseVariables: Finding.Message = - "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" + fileprivate static func useLetInBoundCaseVariables( + _ specifier: TokenSyntax + ) -> Finding.Message { + "move this '\(specifier.text)' keyword inside the 'case' pattern, before each of the bound variables" + } +} + +/// A syntax rewriter that converts identifier patterns to bindings +/// with the given specifier. +private final class BindIdentifiersRewriter: SyntaxRewriter { + var bindingSpecifier: TokenSyntax + + init(bindingSpecifier: TokenSyntax) { + self.bindingSpecifier = bindingSpecifier + } + + override func visit(_ node: PatternExprSyntax) -> ExprSyntax { + guard let identifier = node.pattern.as(IdentifierPatternSyntax.self) else { + return super.visit(node) + } + + let binding = ValueBindingPatternSyntax( + bindingSpecifier: bindingSpecifier, + pattern: identifier) + var result = node + result.pattern = PatternSyntax(binding) + return ExprSyntax(result) + } } diff --git a/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift b/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift index bf6912635..95c4d58d7 100644 --- a/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift +++ b/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift @@ -3,20 +3,36 @@ import _SwiftFormatTestSupport final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { func testSwitchCase() { - assertLint( + assertFormatting( UseLetInEveryBoundCaseVariable.self, - """ - switch DataPoint.labeled("hello", 100) { - case 1️⃣let .labeled(label, value): break - case .labeled(label, let value): break - case .labeled(let label, let value): break - case 2️⃣let .labeled(label, value)?: break - case 3️⃣let .labeled(label, value)!: break - case 4️⃣let .labeled(label, value)??: break - case 5️⃣let (label, value): break - case let x as SomeType: break - } - """, + input: """ + switch DataPoint.labeled("hello", 100) { + case 1️⃣let .labeled(label, value): break + case .labeled(label, let value): break + case .labeled(let label, let value): break + case 2️⃣let .labeled(label, value)?: break + case 3️⃣let .labeled(label, value)!: break + case 4️⃣let .labeled(label, value)??: break + case 5️⃣let (label, value): break + case let x as SomeType: break + case 6️⃣var .labeled(label, value): break + case 7️⃣var (label, value): break + } + """, + expected: """ + switch DataPoint.labeled("hello", 100) { + case .labeled(let label, let value): break + case .labeled(label, let value): break + case .labeled(let label, let value): break + case .labeled(let label, let value)?: break + case .labeled(let label, let value)!: break + case .labeled(let label, let value)??: break + case (let label, let value): break + case let x as SomeType: break + case .labeled(var label, var value): break + case (var label, var value): break + } + """, findings: [ FindingSpec( "1️⃣", @@ -38,23 +54,45 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { "5️⃣", message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" ), + FindingSpec( + "6️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "7️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), ] ) } func testIfCase() { - assertLint( + assertFormatting( UseLetInEveryBoundCaseVariable.self, - """ - if case 1️⃣let .labeled(label, value) = DataPoint.labeled("hello", 100) {} - if case .labeled(label, let value) = DataPoint.labeled("hello", 100) {} - if case .labeled(let label, let value) = DataPoint.labeled("hello", 100) {} - if case 2️⃣let .labeled(label, value)? = DataPoint.labeled("hello", 100) {} - if case 3️⃣let .labeled(label, value)! = DataPoint.labeled("hello", 100) {} - if case 4️⃣let .labeled(label, value)?? = DataPoint.labeled("hello", 100) {} - if case 5️⃣let (label, value) = DataPoint.labeled("hello", 100) {} - if case let x as SomeType = someValue {} - """, + input: """ + if case 1️⃣let .labeled(label, value) = DataPoint.labeled("hello", 100) {} + if case .labeled(label, let value) = DataPoint.labeled("hello", 100) {} + if case .labeled(let label, let value) = DataPoint.labeled("hello", 100) {} + if case 2️⃣let .labeled(label, value)? = DataPoint.labeled("hello", 100) {} + if case 3️⃣let .labeled(label, value)! = DataPoint.labeled("hello", 100) {} + if case 4️⃣let .labeled(label, value)?? = DataPoint.labeled("hello", 100) {} + if case 5️⃣let (label, value) = DataPoint.labeled("hello", 100) {} + if case let x as SomeType = someValue {} + if case 6️⃣var .labeled(label, value) = DataPoint.labeled("hello", 100) {} + if case 7️⃣var (label, value) = DataPoint.labeled("hello", 100) {} + """, + expected: """ + if case .labeled(let label, let value) = DataPoint.labeled("hello", 100) {} + if case .labeled(label, let value) = DataPoint.labeled("hello", 100) {} + if case .labeled(let label, let value) = DataPoint.labeled("hello", 100) {} + if case .labeled(let label, let value)? = DataPoint.labeled("hello", 100) {} + if case .labeled(let label, let value)! = DataPoint.labeled("hello", 100) {} + if case .labeled(let label, let value)?? = DataPoint.labeled("hello", 100) {} + if case (let label, let value) = DataPoint.labeled("hello", 100) {} + if case let x as SomeType = someValue {} + if case .labeled(var label, var value) = DataPoint.labeled("hello", 100) {} + if case (var label, var value) = DataPoint.labeled("hello", 100) {} + """, findings: [ FindingSpec( "1️⃣", @@ -76,23 +114,45 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { "5️⃣", message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" ), + FindingSpec( + "6️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "7️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), ] ) } func testGuardCase() { - assertLint( + assertFormatting( UseLetInEveryBoundCaseVariable.self, - """ - guard case 1️⃣let .labeled(label, value) = DataPoint.labeled("hello", 100) else {} - guard case .labeled(label, let value) = DataPoint.labeled("hello", 100) else {} - guard case .labeled(let label, let value) = DataPoint.labeled("hello", 100) else {} - guard case 2️⃣let .labeled(label, value)? = DataPoint.labeled("hello", 100) else {} - guard case 3️⃣let .labeled(label, value)! = DataPoint.labeled("hello", 100) else {} - guard case 4️⃣let .labeled(label, value)?? = DataPoint.labeled("hello", 100) else {} - guard case 5️⃣let (label, value) = DataPoint.labeled("hello", 100) else {} - guard case let x as SomeType = someValue else {} - """, + input: """ + guard case 1️⃣let .labeled(label, value) = DataPoint.labeled("hello", 100) else {} + guard case .labeled(label, let value) = DataPoint.labeled("hello", 100) else {} + guard case .labeled(let label, let value) = DataPoint.labeled("hello", 100) else {} + guard case 2️⃣let .labeled(label, value)? = DataPoint.labeled("hello", 100) else {} + guard case 3️⃣let .labeled(label, value)! = DataPoint.labeled("hello", 100) else {} + guard case 4️⃣let .labeled(label, value)?? = DataPoint.labeled("hello", 100) else {} + guard case 5️⃣let (label, value) = DataPoint.labeled("hello", 100) else {} + guard case let x as SomeType = someValue else {} + guard case 6️⃣var .labeled(label, value) = DataPoint.labeled("hello", 100) else {} + guard case 7️⃣var (label, value) = DataPoint.labeled("hello", 100) else {} + """, + expected: """ + guard case .labeled(let label, let value) = DataPoint.labeled("hello", 100) else {} + guard case .labeled(label, let value) = DataPoint.labeled("hello", 100) else {} + guard case .labeled(let label, let value) = DataPoint.labeled("hello", 100) else {} + guard case .labeled(let label, let value)? = DataPoint.labeled("hello", 100) else {} + guard case .labeled(let label, let value)! = DataPoint.labeled("hello", 100) else {} + guard case .labeled(let label, let value)?? = DataPoint.labeled("hello", 100) else {} + guard case (let label, let value) = DataPoint.labeled("hello", 100) else {} + guard case let x as SomeType = someValue else {} + guard case .labeled(var label, var value) = DataPoint.labeled("hello", 100) else {} + guard case (var label, var value) = DataPoint.labeled("hello", 100) else {} + """, findings: [ FindingSpec( "1️⃣", @@ -114,23 +174,45 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { "5️⃣", message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" ), + FindingSpec( + "6️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "7️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), ] ) } func testForCase() { - assertLint( + assertFormatting( UseLetInEveryBoundCaseVariable.self, - """ - for case 1️⃣let .labeled(label, value) in dataPoints {} - for case .labeled(label, let value) in dataPoints {} - for case .labeled(let label, let value) in dataPoints {} - for case 2️⃣let .labeled(label, value)? in dataPoints {} - for case 3️⃣let .labeled(label, value)! in dataPoints {} - for case 4️⃣let .labeled(label, value)?? in dataPoints {} - for case 5️⃣let (label, value) in dataPoints {} - for case let x as SomeType in {} - """, + input: """ + for case 1️⃣let .labeled(label, value) in dataPoints {} + for case .labeled(label, let value) in dataPoints {} + for case .labeled(let label, let value) in dataPoints {} + for case 2️⃣let .labeled(label, value)? in dataPoints {} + for case 3️⃣let .labeled(label, value)! in dataPoints {} + for case 4️⃣let .labeled(label, value)?? in dataPoints {} + for case 5️⃣let (label, value) in dataPoints {} + for case let x as SomeType in {} + for case 6️⃣var .labeled(label, value) in dataPoints {} + for case 7️⃣var (label, value) in dataPoints {} + """, + expected: """ + for case .labeled(let label, let value) in dataPoints {} + for case .labeled(label, let value) in dataPoints {} + for case .labeled(let label, let value) in dataPoints {} + for case .labeled(let label, let value)? in dataPoints {} + for case .labeled(let label, let value)! in dataPoints {} + for case .labeled(let label, let value)?? in dataPoints {} + for case (let label, let value) in dataPoints {} + for case let x as SomeType in {} + for case .labeled(var label, var value) in dataPoints {} + for case (var label, var value) in dataPoints {} + """, findings: [ FindingSpec( "1️⃣", @@ -152,23 +234,45 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { "5️⃣", message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" ), + FindingSpec( + "6️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "7️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), ] ) } func testWhileCase() { - assertLint( + assertFormatting( UseLetInEveryBoundCaseVariable.self, - """ - while case 1️⃣let .labeled(label, value) = iter.next() {} - while case .labeled(label, let value) = iter.next() {} - while case .labeled(let label, let value) = iter.next() {} - while case 2️⃣let .labeled(label, value)? = iter.next() {} - while case 3️⃣let .labeled(label, value)! = iter.next() {} - while case 4️⃣let .labeled(label, value)?? = iter.next() {} - while case 5️⃣let (label, value) = iter.next() {} - while case let x as SomeType = iter.next() {} - """, + input: """ + while case 1️⃣let .labeled(label, value) = iter.next() {} + while case .labeled(label, let value) = iter.next() {} + while case .labeled(let label, let value) = iter.next() {} + while case 2️⃣let .labeled(label, value)? = iter.next() {} + while case 3️⃣let .labeled(label, value)! = iter.next() {} + while case 4️⃣let .labeled(label, value)?? = iter.next() {} + while case 5️⃣let (label, value) = iter.next() {} + while case let x as SomeType = iter.next() {} + while case 6️⃣var .labeled(label, value) = iter.next() + while case 7️⃣var (label, value) = iter.next() + """, + expected: """ + while case .labeled(let label, let value) = iter.next() {} + while case .labeled(label, let value) = iter.next() {} + while case .labeled(let label, let value) = iter.next() {} + while case .labeled(let label, let value)? = iter.next() {} + while case .labeled(let label, let value)! = iter.next() {} + while case .labeled(let label, let value)?? = iter.next() {} + while case (let label, let value) = iter.next() {} + while case let x as SomeType = iter.next() {} + while case .labeled(var label, var value) = iter.next() + while case (var label, var value) = iter.next() + """, findings: [ FindingSpec( "1️⃣", @@ -190,6 +294,14 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { "5️⃣", message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" ), + FindingSpec( + "6️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "7️⃣", + message: "move this 'var' keyword inside the 'case' pattern, before each of the bound variables" + ), ] ) } From b7e3f22e6d932aaf29059267912d2286d7dfc85b Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Sat, 8 Feb 2025 13:47:26 -0600 Subject: [PATCH 2/4] Formatception --- .../UseLetInEveryBoundCaseVariable.swift | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift index 23ddca9ed..7b23546b8 100644 --- a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift +++ b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift @@ -31,27 +31,27 @@ public final class UseLetInEveryBoundCaseVariable: SyntaxFormatRule { result.pattern = PatternSyntax(replacement) return result } - + return super.visit(node) } - + public override func visit(_ node: SwitchCaseItemSyntax) -> SwitchCaseItemSyntax { if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) { diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern) - + var result = node result.pattern = PatternSyntax(replacement) return result } - + return super.visit(node) } - + public override func visit(_ node: ForStmtSyntax) -> StmtSyntax { guard node.caseKeyword != nil else { return super.visit(node) } - + if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) { diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern) @@ -59,7 +59,7 @@ public final class UseLetInEveryBoundCaseVariable: SyntaxFormatRule { result.pattern = PatternSyntax(replacement) return StmtSyntax(result) } - + return super.visit(node) } } @@ -78,22 +78,30 @@ extension UseLetInEveryBoundCaseVariable { ) -> ExprSyntax { var patternStack = patternStack var result = expr - + // As we unwind the stack, wrap the expression in optional chaining // or force unwrap expressions. while let (kind, trivia) = patternStack.popLast() { if kind == .chained { - result = ExprSyntax(OptionalChainingExprSyntax( - expression: result, trailingTrivia: trivia)) + result = ExprSyntax( + OptionalChainingExprSyntax( + expression: result, + trailingTrivia: trivia + ) + ) } else { - result = ExprSyntax(ForceUnwrapExprSyntax( - expression: result, trailingTrivia: trivia)) + result = ExprSyntax( + ForceUnwrapExprSyntax( + expression: result, + trailingTrivia: trivia + ) + ) } } - + return result } - + /// Returns a rewritten version of the given pattern if bindings can be moved /// into bound cases. /// @@ -105,13 +113,13 @@ extension UseLetInEveryBoundCaseVariable { _ pattern: PatternSyntax ) -> (ExpressionPatternSyntax, TokenSyntax)? { guard let bindingPattern = pattern.as(ValueBindingPatternSyntax.self), - let exprPattern = bindingPattern.pattern.as(ExpressionPatternSyntax.self) + let exprPattern = bindingPattern.pattern.as(ExpressionPatternSyntax.self) else { return nil } // Grab the `let` or `var` used in the binding pattern. let specifier = bindingPattern.bindingSpecifier let identifierBinder = BindIdentifiersRewriter(bindingSpecifier: specifier) - + // Drill down into any optional patterns that we encounter (e.g., `case let .foo(x)?`). var patternStack: [(OptionalPatternKind, Trivia)] = [] var expression = exprPattern.expression @@ -137,7 +145,8 @@ extension UseLetInEveryBoundCaseVariable { functionCall.arguments = newArguments.as(LabeledExprListSyntax.self)! result.expression = restoreOptionalChainingAndForcing( ExprSyntax(functionCall), - patternStack: patternStack) + patternStack: patternStack + ) return (result, specifier) } @@ -148,7 +157,8 @@ extension UseLetInEveryBoundCaseVariable { tupleExpr.elements = newElements.as(LabeledExprListSyntax.self)! result.expression = restoreOptionalChainingAndForcing( ExprSyntax(tupleExpr), - patternStack: patternStack) + patternStack: patternStack + ) return (result, specifier) } @@ -169,19 +179,20 @@ extension Finding.Message { /// with the given specifier. private final class BindIdentifiersRewriter: SyntaxRewriter { var bindingSpecifier: TokenSyntax - + init(bindingSpecifier: TokenSyntax) { self.bindingSpecifier = bindingSpecifier } - + override func visit(_ node: PatternExprSyntax) -> ExprSyntax { guard let identifier = node.pattern.as(IdentifierPatternSyntax.self) else { return super.visit(node) } - + let binding = ValueBindingPatternSyntax( bindingSpecifier: bindingSpecifier, - pattern: identifier) + pattern: identifier + ) var result = node result.pattern = PatternSyntax(binding) return ExprSyntax(result) From 1624c93bd0e67f36bd68f2364881a05c277d8c9f Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Mon, 24 Feb 2025 11:36:29 -0600 Subject: [PATCH 3/4] Remove leading trivia on the case item specifier --- .../UseLetInEveryBoundCaseVariable.swift | 4 +- .../UseLetInEveryBoundCaseVariableTests.swift | 46 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift index 7b23546b8..4d7b41636 100644 --- a/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift +++ b/Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift @@ -41,6 +41,7 @@ public final class UseLetInEveryBoundCaseVariable: SyntaxFormatRule { var result = node result.pattern = PatternSyntax(replacement) + result.leadingTrivia = node.leadingTrivia return result } @@ -117,7 +118,8 @@ extension UseLetInEveryBoundCaseVariable { else { return nil } // Grab the `let` or `var` used in the binding pattern. - let specifier = bindingPattern.bindingSpecifier + var specifier = bindingPattern.bindingSpecifier + specifier.leadingTrivia = [] let identifierBinder = BindIdentifiersRewriter(bindingSpecifier: specifier) // Drill down into any optional patterns that we encounter (e.g., `case let .foo(x)?`). diff --git a/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift b/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift index 95c4d58d7..60c95989e 100644 --- a/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift +++ b/Tests/SwiftFormatTests/Rules/UseLetInEveryBoundCaseVariableTests.swift @@ -66,6 +66,52 @@ final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase { ) } + func testSwitchMultipleCases() { + assertFormatting( + UseLetInEveryBoundCaseVariable.self, + input: """ + switch (start.representation, end.representation) { + case 1️⃣let (.element(element), .separator(next: separator)): + return 2 * base.distance(from: element, to: separator) - 1 + case 2️⃣let (.separator(next: separator), .element(element)): + return 2 * base.distance(from: separator, to: element) + 1 + case 3️⃣let (.element(start), .element(end)), + 4️⃣let (.separator(start), .separator(end)): + return 2 * base.distance(from: start, to: end) + } + """, + expected: """ + switch (start.representation, end.representation) { + case (.element(let element), .separator(next: let separator)): + return 2 * base.distance(from: element, to: separator) - 1 + case (.separator(next: let separator), .element(let element)): + return 2 * base.distance(from: separator, to: element) + 1 + case (.element(let start), .element(let end)), + (.separator(let start), .separator(let end)): + return 2 * base.distance(from: start, to: end) + } + """, + findings: [ + FindingSpec( + "1️⃣", + message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "2️⃣", + message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "3️⃣", + message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" + ), + FindingSpec( + "4️⃣", + message: "move this 'let' keyword inside the 'case' pattern, before each of the bound variables" + ), + ] + ) + } + func testIfCase() { assertFormatting( UseLetInEveryBoundCaseVariable.self, From bc0530cb04a4f3b7d650569beeec331e3ff4ed05 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Mon, 24 Feb 2025 11:54:03 -0600 Subject: [PATCH 4/4] Update API breakage list --- api-breakages.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api-breakages.txt b/api-breakages.txt index 0b1c3400a..ec9be6cbc 100644 --- a/api-breakages.txt +++ b/api-breakages.txt @@ -4,3 +4,6 @@ API breakage: constructor FileIterator.init(urls:followSymlinks:) has been removed API breakage: enumelement SwiftFormatError.configurationDumpFailed has been added as a new enum case API breakage: enumelement SwiftFormatError.unsupportedConfigurationVersion has been added as a new enum case +API breakage: class UseLetInEveryBoundCaseVariable has changed its super class from SwiftFormat.SyntaxLintRule to SwiftFormat.SyntaxFormatRule +API breakage: func UseLetInEveryBoundCaseVariable.visit(_:) has return type change from SwiftSyntax.SyntaxVisitorContinueKind to SwiftSyntax.MatchingPatternConditionSyntax +API breakage: func UseLetInEveryBoundCaseVariable.visit(_:) has parameter 0 type change from SwiftSyntax.ValueBindingPatternSyntax to SwiftSyntax.MatchingPatternConditionSyntax