Merge pull request #244 from allevato/distributed-let

Improve pattern recognition in `UseLetInEveryBoundCaseVariable`.

Author: allevato

Sources/SwiftFormatRules/UseLetInEveryBoundCaseVariable.swift

@@ -13,24 +13,60 @@
 import SwiftFormatCore
 import SwiftSyntax
 
-/// Every element bound in a `case` must have its own `let`.
+/// Every variable bound in a `case` pattern must have its own `let/var`.
 ///
-/// e.g. `case let .label(foo, bar)` is forbidden.
+/// For example, `case let .identifier(x, y)` is forbidden. Use
+/// `case .identifier(let x, let y)` instead.
 ///
-/// Lint: `case let ...` will yield a lint error.
+/// Lint: `case let .identifier(...)` will yield a lint error.
 public final class UseLetInEveryBoundCaseVariable: SyntaxLintRule {
 
-  public override func visit(_ node: SwitchCaseLabelSyntax) -> SyntaxVisitorContinueKind {
-    for item in node.caseItems {
-      guard item.pattern.is(ValueBindingPatternSyntax.self) else { continue }
+  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.valuePattern) {
       diagnose(.useLetInBoundCaseVariables, on: node)
     }
-    return .skipChildren
+    return .visitChildren
+  }
+
+  /// 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 }
+
+    // Drill down into any optional patterns that we encounter (e.g., `case let .foo(x)?`).
+    var expression = exprPattern.expression
+    while true {
+      if let optionalExpr = expression.as(OptionalChainingExprSyntax.self) {
+        expression = optionalExpr.expression
+      } else if let forcedExpr = expression.as(ForcedValueExprSyntax.self) {
+        expression = forcedExpr.expression
+      } 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),
+      functionCall.calledExpression.is(MemberAccessExprSyntax.self)
+    {
+      return true
+    }
+
+    // A tuple expression can have the `let/var` distributed into the elements.
+    if expression.is(TupleExprSyntax.self) {
+      return true
+    }
+
+    // Otherwise, we're not sure this is a pattern we can distribute through.
+    return false
   }
 }
 
 extension Diagnostic.Message {
   public static let useLetInBoundCaseVariables = Diagnostic.Message(
     .warning,
-    "distribute 'let' to each bound case variable")
+    "move 'let' keyword to precede each variable bound in the `case` pattern")
 }

Tests/SwiftFormatRulesTests/UseLetInEveryBoundCaseVariableTests.swift

@@ -1,26 +1,115 @@
 import SwiftFormatRules
 
 final class UseLetInEveryBoundCaseVariableTests: LintOrFormatRuleTestCase {
-  func testInvalidLetBoundCase() {
+  override func setUp() {
+    super.setUp()
+    self.shouldCheckForUnassertedDiagnostics = true
+  }
+
+  func testSwitchCase() {
     let input =
       """
       switch DataPoint.labeled("hello", 100) {
-      case let .labeled(label, value):
-        break
+      case let .labeled(label, value): break
+      case .labeled(label, let value): break
+      case .labeled(let label, let value): break
+      case let .labeled(label, value)?: break
+      case let .labeled(label, value)!: break
+      case let .labeled(label, value)??: break
+      case let (label, value): break
+      case let x as SomeType: break
       }
+      """
+    performLint(UseLetInEveryBoundCaseVariable.self, input: input)
 
-      switch DataPoint.labeled("hello", 100) {
-      case .labeled(label, let value):
-        break
-      }
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 2, column: 6)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 5, column: 6)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 6, column: 6)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 7, column: 6)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 8, column: 6)
+  }
 
-      switch DataPoint.labeled("hello", 100) {
-      case .labeled(let label, let value):
-        break
-      }
+  func testIfCase() {
+    let input =
+      """
+      if case 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 let .labeled(label, value)? = DataPoint.labeled("hello", 100) {}
+      if case let .labeled(label, value)! = DataPoint.labeled("hello", 100) {}
+      if case let .labeled(label, value)?? = DataPoint.labeled("hello", 100) {}
+      if case let (label, value) = DataPoint.labeled("hello", 100) {}
+      if case let x as SomeType = someValue {}
       """
     performLint(UseLetInEveryBoundCaseVariable.self, input: input)
-    XCTAssertDiagnosed(.useLetInBoundCaseVariables)
-    XCTAssertNotDiagnosed(.useLetInBoundCaseVariables)
+
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 1, column: 9)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 4, column: 9)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 5, column: 9)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 6, column: 9)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 7, column: 9)
+  }
+
+  func testGuardCase() {
+    let input =
+      """
+      guard case 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 let .labeled(label, value)? = DataPoint.labeled("hello", 100) else {}
+      guard case let .labeled(label, value)! = DataPoint.labeled("hello", 100) else {}
+      guard case let .labeled(label, value)?? = DataPoint.labeled("hello", 100) else {}
+      guard case let (label, value) = DataPoint.labeled("hello", 100) else {}
+      guard case let x as SomeType = someValue else {}
+      """
+    performLint(UseLetInEveryBoundCaseVariable.self, input: input)
+
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 1, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 4, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 5, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 6, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 7, column: 12)
+  }
+
+  func testForCase() {
+    let input =
+      """
+      for case 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 let .labeled(label, value)? in dataPoints {}
+      for case let .labeled(label, value)! in dataPoints {}
+      for case let .labeled(label, value)?? in dataPoints {}
+      for case let (label, value) in dataPoints {}
+      for case let x as SomeType in {}
+      """
+    performLint(UseLetInEveryBoundCaseVariable.self, input: input)
+
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 1, column: 10)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 4, column: 10)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 5, column: 10)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 6, column: 10)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 7, column: 10)
+  }
+
+  func testWhileCase() {
+    let input =
+      """
+      while case 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 let .labeled(label, value)? = iter.next() {}
+      while case let .labeled(label, value)! = iter.next() {}
+      while case let .labeled(label, value)?? = iter.next() {}
+      while case let (label, value) = iter.next() {}
+      while case let x as SomeType = iter.next() {}
+      """
+    performLint(UseLetInEveryBoundCaseVariable.self, input: input)
+
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 1, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 4, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 5, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 6, column: 12)
+    XCTAssertDiagnosed(.useLetInBoundCaseVariables, line: 7, column: 12)
   }
 }