Teach the VarDeclUsageChecker About Variables Bound in Patterns

The VDUC was missing a class of AST nodes that can bind variables:
patterns in switch statements. For these, it was falling back to
requesting a simple replacement of the bound variable name with _. But
for patterns, this means there's a two-step dance the user has to go
through where the first fixit does this:

.pattern(let x) -> .pattern(let _)

Then a second round of compilation would emit a fixit to do this:

.pattern(let _) -> .pattern(_)

Instead, detect "simple" variable bindings - for now, variable patterns
that are immediately preceded by a `let` or `var` binding pattern - and
collapse two steps into one.

Resolves rdar://47240768

Author: CodaFi

lib/Sema/MiscDiagnostics.cpp

@@ -2743,7 +2743,7 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       //    let x = foo()
       //  ->
       //    _ = foo()
-      if (auto *pbd = var->getParentPatternBinding())
+      if (auto *pbd = var->getParentPatternBinding()) {
         if (pbd->getSingleVar() == var && pbd->getInit(0) != nullptr &&
             !isa<TypedPattern>(pbd->getPattern(0))) {
           unsigned varKind = var->isLet();
@@ -2755,7 +2755,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
             .fixItReplace(replaceRange, "_");
           continue;
         }
-      
+      }
+
       // If the variable is defined in a pattern in an if/while/guard statement,
       // see if we can produce a tuned fixit.  When we have something like:
       //
@@ -2808,6 +2809,30 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
               }
         }
       }
+
+      // If the variable is defined in a pattern that isn't one of the usual
+      // conditional statements, try to detect and rewrite "simple" binding
+      // patterns:
+      //    case .pattern(let x):
+      //  ->
+      //    case .pattern(_):
+      if (auto *pattern = var->getParentPattern()) {
+        BindingPattern *foundVP = nullptr;
+        pattern->forEachNode([&](Pattern *P) {
+          if (auto *VP = dyn_cast<BindingPattern>(P))
+            if (VP->getSingleVar() == var)
+              foundVP = VP;
+        });
+
+        if (foundVP) {
+          unsigned varKind = var->isLet();
+          Diags
+              .diagnose(var->getLoc(), diag::variable_never_used,
+                        var->getName(), varKind)
+              .fixItReplace(foundVP->getSourceRange(), "_");
+          continue;
+        }
+      }
 
       // Otherwise, this is something more complex, perhaps
       //    let (a,b) = foo()

test/Constraints/patterns.swift

@@ -131,7 +131,7 @@ default:
 
 // <rdar://problem/19382878> Introduce new x? pattern
 switch Optional(42) {
-case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}}
+case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{10-11=_}}
 case nil: break
 }
 

test/Parse/switch.swift

@@ -28,7 +28,7 @@ func parseError4(x: Int) {
 
 func parseError5(x: Int) {
   switch x {
-  case let z // expected-error {{expected ':' after 'case'}} expected-warning {{immutable value 'z' was never used}} {{12-13=_}}
+  case let z // expected-error {{expected ':' after 'case'}} expected-warning {{immutable value 'z' was never used}} {{8-13=_}}
   }
 }
 

test/decl/var/usage.swift

@@ -304,9 +304,9 @@ func testFixitsInStatementsWithPatterns(_ a : Int?) {
 
 // <rdar://22774938> QoI: "never used" in an "if let" should rewrite expression to use != nil
 func test(_ a : Int?, b : Any) {
-  if true == true, let x = a {   // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{24-25=_}}
+  if true == true, let x = a {   // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{20-25=_}}
   }
-  if let x = a, let y = a {  // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{10-11=_}}
+  if let x = a, let y = a {  // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{6-11=_}}
     _ = y
   }
 
@@ -447,3 +447,51 @@ func testForwardReferenceCapture() {
   innerFunc()
   print(x)
 }
+
+// rdar://47240768 Expand the definition of "simple" pattern to variables bound in patterns
+func testVariablesBoundInPatterns() {
+  enum StringB {
+    case simple(b: Bool)
+    case tuple(b: (Bool, Bool))
+    case optional(b: Bool?)
+  }
+
+  // Because Swift enables all kinds of creative binding forms, make sure that
+  // variable patterns occuring directly under a `let` or `var` have that
+  // introducer stripped by the fixit. All other cases are currently too
+  // complicated for the VarDeclUsageChecker.
+  switch StringB.simple(b: true) {
+  case .simple(b: let b) where false: // expected-warning {{immutable value 'b' was never used; consider replacing with '_' or removing it}} {{19-24=_}}
+    break
+  case .simple(b: var b) where false: // expected-warning {{variable 'b' was never used; consider replacing with '_' or removing it}} {{19-24=_}}
+    break
+  case var .simple(b: b): // expected-warning {{variable 'b' was never used; consider replacing with '_' or removing it}} {{23-24=_}}
+    break
+  case .tuple(b: let (b1, b2)) where false:
+    // expected-warning@-1 {{immutable value 'b1' was never used; consider replacing with '_' or removing it}} {{23-25=_}}
+    // expected-warning@-2 {{immutable value 'b2' was never used; consider replacing with '_' or removing it}} {{27-29=_}}
+    break
+  case .tuple(b: (let b1, let b2)) where false:
+    // expected-warning@-1 {{immutable value 'b1' was never used; consider replacing with '_' or removing it}} {{19-25=_}}
+    // expected-warning@-2 {{immutable value 'b2' was never used; consider replacing with '_' or removing it}} {{27-33=_}}
+    break
+  case .tuple(b: (var b1, var b2)) where false:
+    // expected-warning@-1 {{variable 'b1' was never used; consider replacing with '_' or removing it}} {{19-25=_}}
+    // expected-warning@-2 {{variable 'b2' was never used; consider replacing with '_' or removing it}} {{27-33=_}}
+    break
+  case var .tuple(b: (b1, b2)) where false:
+    // expected-warning@-1 {{variable 'b1' was never used; consider replacing with '_' or removing it}} {{23-25=_}}
+    // expected-warning@-2 {{variable 'b2' was never used; consider replacing with '_' or removing it}} {{27-29=_}}
+    break
+  case .tuple(b: let b): // expected-warning {{immutable value 'b' was never used; consider replacing with '_' or removing it}} {{18-23=_}}
+    break
+  case .optional(b: let x?) where false: // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{25-26=_}}
+    break
+  case .optional(b: let .some(x)) where false: // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{31-32=_}}
+    break
+  case let .optional(b: x?): // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{25-26=_}}
+    break
+  case let .optional(b: .none): // expected-warning {{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{8-12=}}
+    break
+  }
+}