[Diagnostics] Diagnose an attempt to init/use dictionary with array literal in CSGen

It's much easier to detect that contextual type is a dictionary
but expression is an array literal while generating constraints.

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -1891,9 +1891,6 @@ bool ContextualFailure::diagnoseAsError() {
   if (diagnoseMissingFunctionCall())
     return true;
 
-  if (diagnoseConversionToDictionary())
-    return true;
-
   // Special case of some common conversions involving Swift.String
   // indexes, catching cases where people attempt to index them with an integer.
   if (isIntegerToStringIndexConversion()) {
@@ -2403,61 +2400,6 @@ bool ContextualFailure::diagnoseConversionToBool() const {
   return false;
 }
 
-bool ContextualFailure::isInvalidDictionaryConversion(
-    ConstraintSystem &cs, Expr *anchor, Type contextualType) {
-  auto *arrayExpr = dyn_cast<ArrayExpr>(anchor);
-  if (!arrayExpr)
-    return false;
-
-  auto type = contextualType->lookThroughAllOptionalTypes();
-  if (!conformsToKnownProtocol(
-        cs, type, KnownProtocolKind::ExpressibleByDictionaryLiteral))
-    return false;
-
-  return (arrayExpr->getNumElements() & 1) == 0;
-}
-
-bool ContextualFailure::diagnoseConversionToDictionary() const {
-  auto &cs = getConstraintSystem();
-  auto toType = getToType()->lookThroughAllOptionalTypes();
-
-  if (!isInvalidDictionaryConversion(cs, getAnchor(), toType))
-    return false;
-
-  auto *arrayExpr = cast<ArrayExpr>(getAnchor());
-
-  // If the contextual type conforms to ExpressibleByDictionaryLiteral and
-  // this is an empty array, then they meant "[:]".
-  auto numElements = arrayExpr->getNumElements();
-  if (numElements == 0) {
-    emitDiagnostic(arrayExpr->getStartLoc(),
-                   diag::should_use_empty_dictionary_literal)
-        .fixItInsert(arrayExpr->getEndLoc(), ":");
-    return true;
-  }
-
-  // If the contextual type conforms to ExpressibleByDictionaryLiteral, then
-  // they wrote "x = [1,2]" but probably meant "x = [1:2]".
-  bool isIniting = getContextualTypePurpose() == CTP_Initialization;
-  emitDiagnostic(arrayExpr->getStartLoc(), diag::should_use_dictionary_literal,
-                 toType, isIniting);
-
-  auto diagnostic =
-      emitDiagnostic(arrayExpr->getStartLoc(), diag::meant_dictionary_lit);
-
-  // Change every other comma into a colon, only if the number
-  // of commas present matches the number of elements, because
-  // otherwise it might a structural problem with the expression
-  // e.g. ["a""b": 1].
-  const auto commaLocs = arrayExpr->getCommaLocs();
-  if (commaLocs.size() == numElements - 1) {
-    for (unsigned i = 0, e = numElements / 2; i != e; ++i)
-      diagnostic.fixItReplace(commaLocs[i * 2], ":");
-  }
-
-  return true;
-}
-
 bool ContextualFailure::diagnoseThrowsTypeMismatch() const {
   // If this is conversion failure due to a return statement with an argument
   // that cannot be coerced to the result type of the function, emit a
@@ -4857,24 +4799,6 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
   auto *anchor = getAnchor();
   auto *locator = getLocator();
 
-  // Check whether this is situation like `let _: [String: Int] = ["A", 0]`
-  // which attempts to convert an array into a dictionary. We have a tailored
-  // contextual diagnostic for that, so no need to diagnose element mismatches
-  // as well.
-  auto &cs = getConstraintSystem();
-  auto *rawAnchor = getRawAnchor();
-  if (llvm::any_of(cs.getFixes(), [&](const ConstraintFix *fix) -> bool {
-        auto *locator = fix->getLocator();
-        if (!(fix->getKind() == FixKind::ContextualMismatch &&
-              locator->getAnchor() == rawAnchor))
-          return false;
-
-        auto *mismatch = static_cast<const ContextualMismatch *>(fix);
-        return isInvalidDictionaryConversion(cs, rawAnchor,
-                                             mismatch->getToType());
-      }))
-    return false;
-
   auto eltType = getFromType();
   auto contextualType = getToType();
 

lib/Sema/CSDiagnostics.h

@@ -566,10 +566,6 @@ class ContextualFailure : public FailureDiagnostic {
   /// Produce a specialized diagnostic if this is an invalid conversion to Bool.
   bool diagnoseConversionToBool() const;
 
-  /// Produce a specialized diagnostic if this is an attempt to initialize
-  /// or convert an array literal to a dictionary e.g. `let _: [String: Int] = ["A", 0]`
-  bool diagnoseConversionToDictionary() const;
-
   /// Produce a specialized diagnostic if this is an attempt to throw
   /// something with doesn't conform to `Error`.
   bool diagnoseThrowsTypeMismatch() const;
@@ -625,11 +621,6 @@ class ContextualFailure : public FailureDiagnostic {
   /// protocol
   bool tryProtocolConformanceFixIt(InFlightDiagnostic &diagnostic) const;
 
-  /// Check whether this contextual failure represents an invalid
-  /// conversion from array literal to dictionary.
-  static bool isInvalidDictionaryConversion(ConstraintSystem &cs, Expr *anchor,
-                                            Type contextualType);
-
 private:
   Type resolve(Type rawType) const {
     return resolveType(rawType)->getWithoutSpecifierType();

lib/Sema/CSGen.cpp

@@ -1852,7 +1852,56 @@ namespace {
 
         return contextualArrayType;
       }
-      
+
+      // Produce a specialized diagnostic if this is an attempt to initialize
+      // or convert an array literal to a dictionary e.g.
+      // `let _: [String: Int] = ["A", 0]`
+      auto isDictionaryContextualType = [&](Type contextualType) -> bool {
+        if (!contextualType)
+          return false;
+
+        auto type = contextualType->lookThroughAllOptionalTypes();
+        if (conformsToKnownProtocol(
+                CS, type, KnownProtocolKind::ExpressibleByArrayLiteral))
+          return false;
+
+        return conformsToKnownProtocol(
+            CS, type, KnownProtocolKind::ExpressibleByDictionaryLiteral);
+      };
+
+      if (isDictionaryContextualType(contextualType)) {
+        auto &DE = CS.getASTContext().Diags;
+        auto numElements = expr->getNumElements();
+
+        if (numElements == 0) {
+          DE.diagnose(expr->getStartLoc(),
+                      diag::should_use_empty_dictionary_literal)
+              .fixItInsert(expr->getEndLoc(), ":");
+          return nullptr;
+        }
+
+        bool isIniting =
+            CS.getContextualTypePurpose(expr) == CTP_Initialization;
+        DE.diagnose(expr->getStartLoc(), diag::should_use_dictionary_literal,
+                    contextualType->lookThroughAllOptionalTypes(), isIniting);
+
+        auto diagnostic =
+            DE.diagnose(expr->getStartLoc(), diag::meant_dictionary_lit);
+
+        // If there is an even number of elements in the array, let's produce
+        // a fix-it which suggests to replace "," with ":" to form a dictionary
+        // literal.
+        if ((numElements & 1) == 0) {
+          const auto commaLocs = expr->getCommaLocs();
+          if (commaLocs.size() == numElements - 1) {
+            for (unsigned i = 0, e = numElements / 2; i != e; ++i)
+              diagnostic.fixItReplace(commaLocs[i * 2], ":");
+          }
+        }
+
+        return nullptr;
+      }
+
       auto arrayTy = CS.createTypeVariable(locator,
                                            TVO_PrefersSubtypeBinding |
                                            TVO_CanBindToNoEscape);

test/Constraints/dictionary_literal.swift

@@ -63,8 +63,8 @@ var _: MyDictionary<String, Int>? = ["foo", 1]  // expected-error {{dictionary o
 var _: MyDictionary<String, Int>? = ["foo", 1, "bar", 42]  // expected-error {{dictionary of type 'MyDictionary<String, Int>' cannot be initialized with array literal}}
 // expected-note @-1 {{did you mean to use a dictionary literal instead?}} {{43-44=:}} {{53-54=:}}
 
-var _: MyDictionary<String, Int>? = ["foo", 1.0, 2]  // expected-error {{cannot convert value of type '[Double]' to specified type 'MyDictionary<String, Int>?'}}
-// expected-error@-1 {{cannot convert value of type 'String' to expected element type 'Double'}}
+var _: MyDictionary<String, Int>? = ["foo", 1.0, 2]  // expected-error {{dictionary of type 'MyDictionary<String, Int>' cannot be initialized with array literal}}
+// expected-note@-1 {{did you mean to use a dictionary literal instead?}} {{none}}
 
 var _: MyDictionary<String, Int>? = ["foo" : 1.0]  // expected-error {{cannot convert value of type 'Double' to expected dictionary value type 'MyDictionary<String, Int>.Value' (aka 'Int')}}