[CS] Allow contextual types with errors

Previously we would skip type-checking the result expression of a
`return` or the initialization expression of a binding if the contextual
type had an error, but that misses out on useful diagnostics and
prevents code completion and cursor info from working. Change the logic
such that we open ErrorTypes as holes and continue to type-check.

Author: hamishknight

lib/Sema/CSGen.cpp

@@ -4913,16 +4913,27 @@ bool ConstraintSystem::generateConstraints(
         return convertTypeLocator;
       };
 
-      // Substitute type variables in for placeholder types.
-      convertType = convertType.transformRec([&](Type type) -> std::optional<Type> {
-        if (type->is<PlaceholderType>()) {
-          return Type(createTypeVariable(getLocator(type),
-                                         TVO_CanBindToNoEscape |
-                                             TVO_PrefersSubtypeBinding |
-                                             TVO_CanBindToHole));
-        }
-        return std::nullopt;
-      });
+      // If the contextual type has an error, we can't apply the solution.
+      // Record a fix for an invalid AST node.
+      if (convertType->hasError())
+        recordFix(IgnoreInvalidASTNode::create(*this, convertTypeLocator));
+
+      // Substitute type variables in for placeholder and error types.
+      convertType =
+          convertType.transformRec([&](Type type) -> std::optional<Type> {
+            if (!isa<PlaceholderType, ErrorType>(type.getPointer()))
+              return std::nullopt;
+
+            auto flags = TVO_CanBindToNoEscape | TVO_PrefersSubtypeBinding |
+                         TVO_CanBindToHole;
+            auto tv = Type(createTypeVariable(getLocator(type), flags));
+            // For ErrorTypes we want to eagerly bind to a hole since we
+            // know this is where the issue is.
+            if (isa<ErrorType>(type.getPointer())) {
+              recordTypeVariablesAsHoles(tv);
+            }
+            return tv;
+          });
 
       addContextualConversionConstraint(expr, convertType, ctp,
                                         convertTypeLocator);

lib/Sema/CSSyntacticElement.cpp

@@ -845,12 +845,6 @@ class SyntacticElementConstraintGenerator
         ContextualPattern::forPatternBindingDecl(patternBinding, index);
     Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
 
-    // Fail early if pattern couldn't be type-checked.
-    if (!patternType || patternType->hasError()) {
-      hadError = true;
-      return;
-    }
-
     auto target = getTargetForPattern(patternBinding, index, patternType);
     if (!target) {
       hadError = true;

lib/Sema/TypeCheckConstraints.cpp

@@ -876,11 +876,6 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
       auto contextualPattern = ContextualPattern::forRawPattern(pattern, DC);
       patternType = typeCheckPattern(contextualPattern);
     }
-
-    if (patternType->hasError()) {
-      PBD->setInvalid();
-      return true;
-    }
   }
 
   bool hadError = TypeChecker::typeCheckBinding(pattern, init, DC, patternType,

lib/Sema/TypeCheckStmt.cpp

@@ -1075,7 +1075,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
     assert(TheFunc && "Should have bailed from pre-check if this is None");
 
     Type ResultTy = TheFunc->getBodyResultType();
-    if (!ResultTy || ResultTy->hasError())
+    if (!ResultTy)
       return nullptr;
 
     if (!RS->hasResult()) {

lib/Sema/TypeOfReference.cpp

@@ -273,16 +273,32 @@ class InferableTypeOpener final {
     return substTy;
   }
 
+  Type transformErrorType(ErrorType *errTy) {
+    // For ErrorTypes we want to eagerly bind to a hole since we know this is
+    // where the issue is.
+    auto *tv = createTypeVariable(cs.getConstraintLocator(locator));
+    cs.recordTypeVariablesAsHoles(tv);
+    return tv;
+  }
+
   Type transform(Type type) {
     if (!type)
       return type;
-    if (!type->hasUnboundGenericType() && !type->hasPlaceholder())
+    if (!type->hasUnboundGenericType() && !type->hasPlaceholder() &&
+        !type->hasError()) {
       return type;
-
+    }
+    // If the type has an error, we can't apply the solution. Record a fix for
+    // an invalid AST node.
+    if (type->hasError()) {
+      cs.recordFix(
+          IgnoreInvalidASTNode::create(cs, cs.getConstraintLocator(locator)));
+    }
     return type.transformRec([&](Type type) -> std::optional<Type> {
-      if (!type->hasUnboundGenericType() && !type->hasPlaceholder())
+      if (!type->hasUnboundGenericType() && !type->hasPlaceholder() &&
+          !type->hasError()) {
         return type;
-
+      }
       auto *tyPtr = type.getPointer();
       if (auto unbound = dyn_cast<UnboundGenericType>(tyPtr))
         return transformUnboundGenericType(unbound);
@@ -292,6 +308,8 @@ class InferableTypeOpener final {
         return transformBoundGenericType(genericTy);
       if (auto *aliasTy = dyn_cast<TypeAliasType>(tyPtr))
         return transformTypeAliasType(aliasTy);
+      if (auto *errTy = dyn_cast<ErrorType>(tyPtr))
+        return transformErrorType(errTy);
 
       return std::nullopt;
     });

test/ClangImporter/overlay.swift

@@ -19,3 +19,4 @@ Redeclaration.NSStringToNSString(AppKit.NSStringToNSString("abc")) // expected-w
 
 let viaStruct: Redeclaration.FooStruct1 = AppKit.FooStruct1()
 let forwardDecl: Redeclaration.Tribool = AppKit.Tribool() // expected-error {{no type named 'Tribool' in module 'Redeclaration'}}
+// expected-error@-1 {{missing argument for parameter #1 in call}}

test/Constraints/invalid_constraint_lookup.swift

@@ -27,4 +27,5 @@ protocol Collection : _Collection, Sequence {
 }
 func insertionSort<C: Mutable> (_ elements: inout C, i: C.Index) { // expected-error {{cannot find type 'Mutable' in scope}} expected-error {{'Index' is not a member type of type 'C'}}
   var x: C.Iterator.Element = elements[i] // expected-error {{'Iterator' is not a member type of type 'C'}}
+  // expected-error@-1 {{value of type 'C' has no subscripts}}
 }

test/Constraints/patterns.swift

@@ -834,3 +834,10 @@ func testUndefinedTypeInPattern(_ x: Int) {
     }
   }
 }
+
+func testUndefinedInClosureVar() {
+  // Make sure we don't produce "unable to infer closure type without a type annotation"
+  _ = {
+    var x: Undefined // expected-error {{cannot find type 'Undefined' in scope}}
+  }
+}

test/Parse/subscripting.swift

@@ -142,7 +142,7 @@ struct A2 {
   subscript (i : Int) -> // expected-error{{expected subscripting element type}}
      {
     get {
-      return stored
+      return stored // expected-error {{cannot find 'stored' in scope}}
     }
     set {
       stored = newValue // expected-error{{cannot find 'stored' in scope}}

test/SPI/implicit_spi_import.swift

@@ -95,7 +95,9 @@ import Lib
 public func useImplicit() -> _Klass { return _Klass() } // expected-error{{cannot use class '_Klass' here; it is an SPI imported from 'Lib'}}
 
 @_spi(core)
-public func useSPICore() -> CoreStruct { return CoreStruct() } // expected-error{{cannot find type 'CoreStruct' in scope}}
+public func useSPICore() -> CoreStruct { return CoreStruct() }
+// expected-error@-1 {{cannot find type 'CoreStruct' in scope}}
+// expected-error@-2 {{cannot find 'CoreStruct' in scope}}
 
 public func useMain() -> APIProtocol? { return nil }