Check for let vs var bindings of the same variable name in multiple case patterns.

Author: gregomni

include/swift/AST/DiagnosticsSema.def

@@ -2930,6 +2930,9 @@ ERROR(fallthrough_into_case_with_var_binding,none,
 ERROR(unnecessary_cast_over_optionset,none,
       "unnecessary cast over raw value of %0", (Type))
 
+ERROR(mutability_mismatch_multiple_pattern_list,none,
+      "'%select{var|let}0' pattern binding must match previous "
+      "'%select{var|let}1' pattern binding", (bool, bool))
 ERROR(type_mismatch_multiple_pattern_list,none,
       "pattern variable bound to type %0, expected type %1", (Type, Type))
 ERROR(type_mismatch_fallthrough_pattern_list,none,

lib/Sema/TypeCheckStmt.cpp

@@ -871,25 +871,41 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
           // For each variable in the pattern, make sure its type is identical to what it
           // was in the first label item's pattern.
           auto firstPattern = caseBlock->getCaseLabelItems()[0].getPattern();
-          if (pattern != firstPattern) {
-            SmallVector<VarDecl *, 4> vars;
-            firstPattern->collectVariables(vars);
-            pattern->forEachVariable([&](VarDecl *VD) {
-              if (!VD->hasName())
-                return;
-              for (auto *expected : vars) {
-                if (expected->hasName() && expected->getName() == VD->getName()) {
-                  if (!VD->getType()->isEqual(expected->getType())) {
-                    TC.diagnose(VD->getLoc(), diag::type_mismatch_multiple_pattern_list,
-                                VD->getType(), expected->getType());
-                    VD->markInvalid();
-                    expected->markInvalid();
-                  }
-                  return;
+          SmallVector<VarDecl *, 4> vars;
+          firstPattern->collectVariables(vars);
+          pattern->forEachVariable([&](VarDecl *VD) {
+            if (!VD->hasName())
+              return;
+            for (auto *expected : vars) {
+              if (expected->hasName() && expected->getName() == VD->getName()) {
+                if (VD->hasType() && expected->hasType() && !expected->isInvalid() &&
+                    !VD->getType()->isEqual(expected->getType())) {
+                  TC.diagnose(VD->getLoc(), diag::type_mismatch_multiple_pattern_list,
+                              VD->getType(), expected->getType());
+                  VD->markInvalid();
+                  expected->markInvalid();
+                }
+                if (expected->isLet() != VD->isLet()) {
+                  auto diag = TC.diagnose(VD->getLoc(),
+                                          diag::mutability_mismatch_multiple_pattern_list,
+                                          VD->isLet(), expected->isLet());
+
+                  VarPattern *foundVP = nullptr;
+                  VD->getParentPattern()->forEachNode([&](Pattern *P) {
+                    if (auto *VP = dyn_cast<VarPattern>(P))
+                      if (VP->getSingleVar() == VD)
+                        foundVP = VP;
+                  });
+                  if (foundVP)
+                    diag.fixItReplace(foundVP->getLoc(),
+                                      expected->isLet() ? "let" : "var");
+                  VD->markInvalid();
+                  expected->markInvalid();
                 }
+                return;
               }
-            });
-          }
+            }
+          });
         }
         // Check the guard expression, if present.
         if (auto *guard = labelItem.getGuardExpr()) {

test/Parse/switch.swift

@@ -276,6 +276,28 @@ func patternVarDiffType(x: Int, y: Double) {
   }
 }
 
+func patternVarDiffMutability(x: Int, y: Double) {
+  switch x {
+  case let a where a < 5, var a where a > 10: // expected-error {{'var' pattern binding must match previous 'let' pattern binding}}{{27-30=let}}
+    break
+  default:
+    break
+  }
+  switch (x, y) {
+  // Would be nice to have a fixit in the following line if we detect that all bindings in the same pattern have the same problem.
+  case let (a, b) where a < 5, var (a, b) where a > 10: // expected-error 2{{'var' pattern binding must match previous 'let' pattern binding}}{{none}}
+    break
+  case (let a, var b) where a < 5, (let a, let b) where a > 10: // expected-error {{'let' pattern binding must match previous 'var' pattern binding}}{{44-47=var}}
+    break
+  case (let a, let b) where a < 5, (var a, let b) where a > 10, (let a, var b) where a == 8:
+    // expected-error@-1 {{'var' pattern binding must match previous 'let' pattern binding}}{{37-40=let}}
+    // expected-error@-2 {{'var' pattern binding must match previous 'let' pattern binding}}{{73-76=let}}
+    break
+  default:
+    break
+  }
+}
+
 func test_label(x : Int) {
 Gronk: // expected-error {{switch must be exhaustive}} expected-note{{do you want to add a default clause?}}
   switch x {