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`).

Author: natecook1000

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
 

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)

Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift

@@ -18,54 +18,172 @@ 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
       }
     }
 
     // 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)
+  }
 }