[TypeChecker] Produce a tailored diagnostic for for-in sequence failures

`for-in` "sequence" expression is required to conform to `Sequence`.

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -3397,6 +3397,11 @@ ERROR(unresolved_label_corrected,none,
       "use of unresolved label %0; did you mean %1?",
       (Identifier, Identifier))
 
+ERROR(foreach_sequence_does_not_conform_to_expected_protocol,none,
+      "for-in loop requires %0 to conform to %1"
+      "%select{|; did you mean to unwrap optional?}2",
+      (Type, Type, bool))
+
 // Switch Stmt
 ERROR(no_match_operator,none,
       "no binary '~=' operator available for 'switch' statement", ())

lib/Sema/CSDiagnostics.cpp

@@ -734,6 +734,7 @@ Optional<Diag<Type, Type>> GenericArgumentsMismatchFailure::getDiagnosticFor(
     return diag::cannot_convert_condition_value;
 
   case CTP_ThrowStmt:
+  case CTP_ForEachStmt:
   case CTP_Unused:
   case CTP_CannotFail:
   case CTP_YieldByReference:
@@ -1928,6 +1929,21 @@ bool ContextualFailure::diagnoseAsError() {
     if (diagnoseYieldByReferenceMismatch())
       return true;
 
+    if (CTP == CTP_ForEachStmt) {
+      if (FromType->isAnyExistentialType()) {
+        emitDiagnostic(anchor->getLoc(), diag::type_cannot_conform,
+                       /*isExistentialType=*/true, FromType, ToType);
+        return true;
+      }
+
+      emitDiagnostic(
+          anchor->getLoc(),
+          diag::foreach_sequence_does_not_conform_to_expected_protocol,
+          FromType, ToType, bool(FromType->getOptionalObjectType()))
+          .highlight(anchor->getSourceRange());
+      return true;
+    }
+
     auto contextualType = getToType();
     if (auto msg = getDiagnosticFor(CTP, contextualType->isExistentialType())) {
       diagnostic = *msg;
@@ -1965,6 +1981,7 @@ getContextualNilDiagnostic(ContextualTypePurpose CTP) {
     return diag::cannot_convert_to_return_type_nil;
 
   case CTP_ThrowStmt:
+  case CTP_ForEachStmt:
   case CTP_YieldByReference:
     return None;
 
@@ -2739,6 +2756,7 @@ ContextualFailure::getDiagnosticFor(ContextualTypePurpose context,
     return diag::cannot_convert_condition_value;
 
   case CTP_ThrowStmt:
+  case CTP_ForEachStmt:
   case CTP_Unused:
   case CTP_CannotFail:
   case CTP_YieldByReference:

lib/Sema/CSSimplify.cpp

@@ -3357,6 +3357,25 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
                                          formUnsolvedResult);
         }
       }
+
+      // If the left-hand side of a 'sequence element' constraint
+      // is a dependent member type without any type variables it
+      // means that conformance check has been "fixed".
+      // Let's record other side of the conversion as a "hole"
+      // to give the solver a chance to continue and avoid
+      // producing diagnostics for both missing conformance and
+      // invalid element type.
+      if (shouldAttemptFixes()) {
+        if (auto last = locator.last()) {
+          if (last->is<LocatorPathElt::SequenceElementType>() &&
+              desugar1->is<DependentMemberType>() &&
+              !desugar1->hasTypeVariable()) {
+            recordHole(typeVar2);
+            return getTypeMatchSuccess();
+          }
+        }
+      }
+
       return formUnsolvedResult();
     }
 

lib/Sema/TypeCheckConstraints.cpp

@@ -2908,6 +2908,8 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
     bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
       // Save the locator we're using for the expression.
       Locator = cs.getConstraintLocator(expr);
+      auto *contextualLocator =
+          cs.getConstraintLocator(expr, LocatorPathElt::ContextualType());
 
       // The expression type must conform to the Sequence.
       auto &tc = cs.getTypeChecker();
@@ -2924,11 +2926,16 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
       cs.addConstraint(ConstraintKind::Conversion, cs.getType(expr),
                        SequenceType, Locator);
       cs.addConstraint(ConstraintKind::ConformsTo, SequenceType,
-                       sequenceProto->getDeclaredType(), Locator);
+                       sequenceProto->getDeclaredType(), contextualLocator);
 
-      auto elementLocator =
-        cs.getConstraintLocator(Locator,
-                                ConstraintLocator::SequenceElementType);
+      // Since we are using "contextual type" here, it has to be recorded
+      // in the constraint system for diagnostics to have access to "purpose".
+      cs.setContextualType(
+          expr, TypeLoc::withoutLoc(sequenceProto->getDeclaredType()),
+          CTP_ForEachStmt);
+
+      auto elementLocator = cs.getConstraintLocator(
+          contextualLocator, ConstraintLocator::SequenceElementType);
 
       // Collect constraints from the element pattern.
       auto pattern = Stmt->getPattern();

lib/Sema/TypeChecker.h

@@ -221,6 +221,8 @@ enum ContextualTypePurpose {
                              ///< result type.
   CTP_Condition,        ///< Condition expression of various statements e.g.
                         ///< `if`, `for`, `while` etc.
+  CTP_ForEachStmt,      ///< "expression/sequence" associated with 'for-in' loop
+                        ///< is expected to conform to 'Sequence' protocol.
 
   CTP_CannotFail,       ///< Conversion can never fail. abort() if it does.
 };

test/Constraints/diagnostics.swift

@@ -86,7 +86,7 @@ struct A : P2 {
   func wonka() {}
 }
 let a = A()
-for j in i.wibble(a, a) { // expected-error {{type 'A' does not conform to protocol 'Sequence'}}  expected-error{{variable 'j' is not bound by any pattern}}
+for j in i.wibble(a, a) { // expected-error {{for-in loop requires 'A' to conform to 'Sequence'}}  expected-error{{variable 'j' is not bound by any pattern}}
 }
 
 // Generic as part of function/tuple types

test/Constraints/generic_protocol_witness.swift

@@ -59,5 +59,5 @@ func usesAGenericMethod<U : NeedsAGenericMethod>(_ x: U) {
 struct L<T>: Sequence {} // expected-error {{type 'L<T>' does not conform to protocol 'Sequence'}}
 
 func z(_ x: L<Int>) {
-  for xx in x {} // expected-error {{variable 'xx' is not bound by any pattern}}
+  for xx in x {} // expected-warning {{immutable value 'xx' was never used; consider replacing with '_' or removing it}}
 }

test/Constraints/members.swift

@@ -316,7 +316,7 @@ struct S22490787 {
 func f22490787() {
   var path: S22490787 = S22490787()
 
-  for p in path {  // expected-error {{type 'S22490787' does not conform to protocol 'Sequence'}} expected-error{{variable 'p' is not bound by any pattern}}
+  for p in path {  // expected-error {{for-in loop requires 'S22490787' to conform to 'Sequence'}} expected-error{{variable 'p' is not bound by any pattern}}
   }
 }
 

test/Generics/deduction.swift

@@ -311,7 +311,7 @@ class DeducePropertyParams {
 // SR-69
 struct A {}
 func foo() {
-    for i in min(1,2) { // expected-error{{type 'Int' does not conform to protocol 'Sequence'}} expected-error {{variable 'i' is not bound by any pattern}}
+    for i in min(1,2) { // expected-error{{for-in loop requires 'Int' to conform to 'Sequence'}} expected-error {{variable 'i' is not bound by any pattern}}
     }
     let j = min(Int(3), Float(2.5)) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}}
     let k = min(A(), A()) // expected-error{{global function 'min' requires that 'A' conform to 'Comparable'}}

test/Sema/editor_placeholders.swift

@@ -26,6 +26,6 @@ f(<#T#> + 1) // expected-error{{editor placeholder in source file}}
 f(<#T##Int#>) // expected-error{{editor placeholder in source file}}
 f(<#T##String#>) // expected-error{{editor placeholder in source file}} expected-error{{cannot convert value of type 'String' to expected argument type 'Int'}}
 
-for x in <#T#> { // expected-error{{editor placeholder in source file}} expected-error{{type '()' does not conform to protocol 'Sequence'}}
+for x in <#T#> { // expected-error{{editor placeholder in source file}} expected-error{{for-in loop requires '()' to conform to 'Sequence'}}
   // expected-error@-1{{variable 'x' is not bound by any pattern}}
 }

test/SourceKit/Sema/placeholders.swift.placeholders.response

@@ -27,7 +27,13 @@
     key.column: 19,
     key.filepath: placeholders.swift,
     key.severity: source.diagnostic.severity.error,
-    key.description: "type '()' does not conform to protocol 'Sequence'",
-    key.diagnostic_stage: source.diagnostic.stage.swift.sema
+    key.description: "for-in loop requires '()' to conform to 'Sequence'",
+    key.diagnostic_stage: source.diagnostic.stage.swift.sema,
+    key.ranges: [
+      {
+        key.offset: 75,
+        key.length: 9
+      }
+    ]
   }
 ]

test/attr/attr_dynamic_member_lookup.swift

@@ -600,8 +600,8 @@ func keypath_with_subscripts(_ arr: SubscriptLens<[Int]>,
 
 func keypath_with_incorrect_return_type(_ arr: Lens<Array<Int>>) {
   for idx in 0..<arr.count {
-    // expected-error@-1 {{operator function '..<' requires that 'Lens<Int>' conform to 'Strideable'}}
-    // expected-error@-2 {{operator function '..<' requires that 'Lens<Int>.Stride' conform to 'SignedInteger'}}
+    // expected-error@-1 {{protocol 'Sequence' requires that 'Lens<Int>' conform to 'Strideable'}}
+    // expected-error@-2 {{protocol 'Sequence' requires that 'Lens<Int>.Stride' conform to 'SignedInteger'}}
     // expected-error@-3 {{cannot convert value of type 'Int' to expected argument type 'Lens<Int>'}}
     // expected-error@-4 {{referencing operator function '..<' on 'Comparable' requires that 'Lens<Int>' conform to 'Comparable'}}
     // expected-error@-5 {{variable 'idx' is not bound by any pattern}}

test/stmt/foreach.swift

@@ -5,7 +5,7 @@ struct BadContainer1 {
 }
 
 func bad_containers_1(bc: BadContainer1) {
-  for e in bc { } // expected-error{{type 'BadContainer1' does not conform to protocol 'Sequence'}}
+  for e in bc { } // expected-error{{for-in loop requires 'BadContainer1' to conform to 'Sequence'}}
   // expected-error@-1{{variable 'e' is not bound by any pattern}}
 }
 
@@ -15,7 +15,7 @@ struct BadContainer2 : Sequence { // expected-error{{type 'BadContainer2' does n
 
 func bad_containers_2(bc: BadContainer2) {
   for e in bc { }
-  // expected-error@-1{{variable 'e' is not bound by any pattern}}
+  // expected-warning@-1 {{immutable value 'e' was never used; consider replacing with '_' or removing it}}
 }
 
 struct BadContainer3 : Sequence { // expected-error{{type 'BadContainer3' does not conform to protocol 'Sequence'}}
@@ -24,7 +24,7 @@ struct BadContainer3 : Sequence { // expected-error{{type 'BadContainer3' does n
 
 func bad_containers_3(bc: BadContainer3) {
   for e in bc { }
-  // expected-error@-1{{variable 'e' is not bound by any pattern}}
+  // expected-warning@-1 {{immutable value 'e' was never used; consider replacing with '_' or removing it}}
 }
 
 struct BadIterator1 {}
@@ -36,7 +36,7 @@ struct BadContainer4 : Sequence { // expected-error{{type 'BadContainer4' does n
 
 func bad_containers_4(bc: BadContainer4) {
   for e in bc { }
-  // expected-error@-1{{variable 'e' is not bound by any pattern}}
+  // expected-warning@-1 {{immutable value 'e' was never used; consider replacing with '_' or removing it}}
 }
 
 // Pattern type-checking
@@ -176,10 +176,8 @@ func testMatchingPatterns() {
 // <rdar://problem/21662365> QoI: diagnostic for for-each over an optional sequence isn't great
 func testOptionalSequence() {
   let array : [Int]?
-  for x in array {  // expected-error {{value of optional type '[Int]?' must be unwrapped}}
-    // expected-note@-1{{coalesce}}
-    // expected-note@-2{{force-unwrap}}
-    // expected-error@-3{{variable 'x' is not bound by any pattern}}
+  for x in array {  // expected-error {{for-in loop requires '[Int]?' to conform to 'Sequence'; did you mean to unwrap optional?}}
+    // expected-error@-1{{variable 'x' is not bound by any pattern}}
   }
 }