[CS] Diagnose misuse of CheckedCastExpr with ~=

Using an unwrap operator with 'as' or the wrong keyword (i.e. `is`)
when already checking a cast via ~= results in error:
'pattern variable binding cannot appear in an expression'.
Add a diagnostic that provides more guidance and a fix-it
for the removal of ?/! or replacement of 'is' with 'as'.

Author: jameesbrown

include/swift/AST/DiagnosticsSema.def

@@ -1419,6 +1419,15 @@ ERROR(optional_chain_isnt_chaining,none,
       ())
 ERROR(pattern_in_expr,none,
       "%0 cannot appear in an expression", (DescriptivePatternKind))
+ERROR(conditional_cast_in_type_casting_pattern,none,
+      "cannot conditionally downcast in a type-casting pattern",
+      ())
+ERROR(force_cast_in_type_casting_pattern,none,
+      "cannot force downcast in a type-casting pattern",
+      ())
+ERROR(cannot_bind_value_with_is,none,
+      "use 'as' keyword to bind a matched value",
+      ())
 NOTE(note_call_to_operator,none,
      "in call to operator %0", (const ValueDecl *))
 NOTE(note_call_to_func,none,

include/swift/Sema/CSFix.h

@@ -3094,11 +3094,12 @@ class IgnoreUnresolvedPatternVar final : public ConstraintFix {
 
 class IgnoreInvalidPatternInExpr final : public ConstraintFix {
   Pattern *P;
+  bool IsInvalidCast;
 
   IgnoreInvalidPatternInExpr(ConstraintSystem &cs, Pattern *pattern,
-                             ConstraintLocator *locator)
+                             bool isInvalidCast, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::IgnoreInvalidPatternInExpr, locator),
-        P(pattern) {}
+        P(pattern), IsInvalidCast(isInvalidCast) {}
 
 public:
   std::string getName() const override {
@@ -3111,8 +3112,10 @@ class IgnoreInvalidPatternInExpr final : public ConstraintFix {
     return diagnose(*commonFixes.front().first);
   }
 
-  static IgnoreInvalidPatternInExpr *
-  create(ConstraintSystem &cs, Pattern *pattern, ConstraintLocator *locator);
+  static IgnoreInvalidPatternInExpr *create(ConstraintSystem &cs,
+                                            Pattern *pattern,
+                                            bool isInvalidCast,
+                                            ConstraintLocator *locator);
 
   static bool classof(const ConstraintFix *fix) {
     return fix->getKind() == FixKind::IgnoreInvalidPatternInExpr;

lib/Sema/CSDiagnostics.cpp

@@ -8699,8 +8699,33 @@ bool InvalidPatternInExprFailure::diagnoseAsError() {
       E = parent;
     }
   }
-  emitDiagnostic(diag::pattern_in_expr, P->getDescriptiveKind());
-  return true;
+  if (IsInvalidCast) {
+    return diagnoseInvalidCheckedCast();
+  } else {
+    emitDiagnostic(diag::pattern_in_expr, P->getDescriptiveKind());
+    return true;
+  }
+}
+
+bool InvalidPatternInExprFailure::diagnoseInvalidCheckedCast() const {
+  auto *castExpr = cast<CheckedCastExpr>(getAsExpr(getAnchor()));
+  // Emit the appropriate diagnostic based on the cast kind.
+  if (auto *forced = dyn_cast<ForcedCheckedCastExpr>(castExpr)) {
+    emitDiagnostic(diag::force_cast_in_type_casting_pattern)
+        .fixItRemove(forced->getExclaimLoc());
+    return true;
+  }
+  if (auto *conditional = dyn_cast<ConditionalCheckedCastExpr>(castExpr)) {
+    emitDiagnostic(diag::conditional_cast_in_type_casting_pattern)
+        .fixItRemove(conditional->getQuestionLoc());
+    return true;
+  }
+  if (auto *isExpr = dyn_cast<IsExpr>(castExpr)) {
+    emitDiagnostic(diag::cannot_bind_value_with_is)
+        .fixItReplace(isExpr->getAsLoc(), "as");
+    return true;
+  }
+  return false;
 }
 
 bool MissingContextualTypeForNil::diagnoseAsError() {

lib/Sema/CSDiagnostics.h

@@ -2597,13 +2597,27 @@ class InvalidEmptyKeyPathFailure final : public FailureDiagnostic {
 /// \endcode
 class InvalidPatternInExprFailure final : public FailureDiagnostic {
   Pattern *P;
+  bool IsInvalidCast;
 
 public:
   InvalidPatternInExprFailure(const Solution &solution, Pattern *pattern,
-                              ConstraintLocator *locator)
-      : FailureDiagnostic(solution, locator), P(pattern) {}
+                              bool isInvalidCast, ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), P(pattern),
+        IsInvalidCast(isInvalidCast) {}
 
   bool diagnoseAsError() override;
+
+private:
+  /// Diagnose situations where a type-casting pattern that binds a value
+  /// expects 'as' but is given 'as!', 'as?' or 'is' instead
+  /// e.g:
+  ///
+  /// \code
+  /// case let x as? Int = y
+  /// case let x as! Int = y
+  /// case let x is Int = y
+  /// \endcode
+  bool diagnoseInvalidCheckedCast() const;
 };
 
 /// Diagnose situations where there is no context to determine a

lib/Sema/CSFix.cpp

@@ -2124,15 +2124,16 @@ IgnoreInvalidASTNode *IgnoreInvalidASTNode::create(ConstraintSystem &cs,
 
 bool IgnoreInvalidPatternInExpr::diagnose(const Solution &solution,
                                           bool asNote) const {
-  InvalidPatternInExprFailure failure(solution, P, getLocator());
+  InvalidPatternInExprFailure failure(solution, P, IsInvalidCast, getLocator());
   return failure.diagnose(asNote);
 }
 
 IgnoreInvalidPatternInExpr *
 IgnoreInvalidPatternInExpr::create(ConstraintSystem &cs, Pattern *pattern,
+                                   bool isInvalidCast,
                                    ConstraintLocator *locator) {
   return new (cs.getAllocator())
-      IgnoreInvalidPatternInExpr(cs, pattern, locator);
+      IgnoreInvalidPatternInExpr(cs, pattern, isInvalidCast, locator);
 }
 
 bool SpecifyContextualTypeForNil::diagnose(const Solution &solution,

lib/Sema/CSGen.cpp

@@ -3543,16 +3543,36 @@ namespace {
 
     Type visitUnresolvedPatternExpr(UnresolvedPatternExpr *expr) {
       // Encountering an UnresolvedPatternExpr here means we have an invalid
-      // ExprPattern with a Pattern node like 'let x' nested in it. Record a
-      // fix, and assign ErrorTypes to any VarDecls bound.
-      auto *locator = CS.getConstraintLocator(expr);
+      // ExprPattern with a Pattern node like 'let x' nested in it. Assign
+      // ErrorTypes to any VarDecls bound, and record a fix after finding the
+      // correct anchor.
       auto *P = expr->getSubPattern();
-      CS.recordFix(IgnoreInvalidPatternInExpr::create(CS, P, locator));
-
       P->forEachVariable([&](VarDecl *VD) {
         CS.setType(VD, ErrorType::get(CS.getASTContext()));
       });
-
+      Expr *anchor = expr;
+      bool isInvalidCast = false;
+      // If we are in a CheckedCastExpr, it's possible that is the problem.
+      // E.g: `if case let x as? Int = y {}`. In which case, we want to anchor
+      // the fix there and set `isInvalidCast` = true.
+      auto *E = CS.getParentExpr(expr);
+      while (E && !isa<CheckedCastExpr>(E))
+        E = CS.getParentExpr(E);
+      auto *castExpr = cast_or_null<CheckedCastExpr>(E);
+      // Check if we are in an argument of `~=` that is _not_ inside a CallExpr,
+      // avoiding cases like: `case let f(x as? Int) = y {}`,
+      // where resolving the CheckedCastExpr would not be a fix.
+      auto *parent = castExpr ? CS.getParentExpr(castExpr) : nullptr;
+      while (parent && !isa<BinaryExpr>(parent))
+        parent = isa<CallExpr>(parent) ? nullptr : CS.getParentExpr(parent);
+      auto *BE = cast_or_null<BinaryExpr>(parent);
+      if (BE && isPatternMatchingOperator(BE->getFn())) {
+        anchor = castExpr;
+        isInvalidCast = true;
+      }
+      auto *locator = CS.getConstraintLocator(anchor);
+      CS.recordFix(
+          IgnoreInvalidPatternInExpr::create(CS, P, isInvalidCast, locator));
       return CS.createTypeVariable(locator, TVO_CanBindToHole);
     }
 

test/Constraints/rdar106598067.swift

@@ -3,10 +3,35 @@
 enum E: Error { case e }
 
 // rdar://106598067 – Make sure we don't crash.
-// FIXME: We ought to have a tailored diagnostic to change to 'as' instead of 'as?'
 let fn = {
   do {} catch let x as? E {}
-  // expected-error@-1 {{pattern variable binding cannot appear in an expression}}
+  // expected-error@-1 {{cannot conditionally downcast in a type-casting pattern}}{{23-24=}}
   // expected-error@-2 {{expression pattern of type 'E?' cannot match values of type 'any Error'}}
   // expected-warning@-3 {{'catch' block is unreachable because no errors are thrown in 'do' block}}
 }
+
+// https://github.com/swiftlang/swift/issues/44631
+let maybeInt: Any = 1
+switch maybeInt {
+case let intValue as? Int: _ = intValue
+  // expected-error@-1 {{cannot conditionally downcast in a type-casting pattern}}{{21-22=}}
+  // expected-error@-2 {{expression pattern of type 'Int?' cannot match values of type 'Any'}}
+case let intValue as! Int: _ = intValue
+  // expected-error@-1 {{cannot force downcast in a type-casting pattern}}{{21-22=}}
+case let intValue is Int: _ = intValue
+  // expected-error@-1 {{use 'as' keyword to bind a matched value}}{{19-21=as}}
+default: break
+}
+
+let maybeIntTuple: (Any, (Any, Any)) = (1, (2, 3))
+switch maybeIntTuple {
+case let (intValue, (intValue2, intValue3)) as? Int: _ = intValue; _ = intValue2; _ = intValue3
+  // expected-error@-1 {{cannot conditionally downcast in a type-casting pattern}}{{47-48=}}
+  // expected-error@-2 {{expression pattern of type 'Int?' cannot match values of type '(Any, (Any, Any))'}}
+case let (intValue, (intValue2, intValue3)) as! Int: _ = intValue; _ = intValue2; _ = intValue3
+  // expected-error@-1 {{cannot force downcast in a type-casting pattern}}{{47-48=}}
+  // expected-error@-2 {{expression pattern of type 'Int' cannot match values of type '(Any, (Any, Any))'}}
+case let (intValue, (intValue2, intValue3)) is Int: _ = intValue; _ = intValue2; _ = intValue3
+  // expected-error@-1 {{use 'as' keyword to bind a matched value}}{{45-47=as}}
+  // expected-error@-2 {{expression pattern of type 'Bool' cannot match values of type '(Any, (Any, Any))'}}
+}