Merge pull request #31671 from LucianoPAlmeida/SR-12745-diagnostics-keypath

[Diagnostics] Improve keypath diagnostic  when cannot be inferred by context

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -71,6 +71,11 @@ ERROR(could_not_find_value_dynamic_member_corrected,none,
 ERROR(could_not_find_value_dynamic_member,none,
       "value of type %0 has no dynamic member %2 using key path from root type %1",
       (Type, Type, DeclNameRef))
+ERROR(cannot_infer_contextual_keypath_type_specify_root,none,
+      "cannot infer key path type from context; consider explicitly specifying a root type", ())
+ERROR(cannot_infer_keypath_root_anykeypath_context,none,
+     "'AnyKeyPath' does not provide enough context for root type to be inferred; "
+     "consider explicitly specifying a root type", ())
 
 ERROR(could_not_find_type_member,none,
       "type %0 has no member %1", (Type, DeclNameRef))

lib/Sema/CSBindings.cpp

@@ -1107,6 +1107,11 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
             cs, TypeVar->getImpl().getLocator());
         if (cs.recordFix(fix))
           return true;
+      } else if (srcLocator->isKeyPathRoot()) {
+        auto *fix =
+            SpecifyKeyPathRootType::create(cs, TypeVar->getImpl().getLocator());
+        if (cs.recordFix(fix))
+          return true;
       }
     }
   }

lib/Sema/CSDiagnostics.cpp

@@ -6271,3 +6271,25 @@ bool MultiArgFuncKeyPathFailure::diagnoseAsError() {
                  resolveType(functionType));
   return true;
 }
+
+bool UnableToInferKeyPathRootFailure::diagnoseAsError() {
+  assert(isExpr<KeyPathExpr>(getAnchor()) && "Expected key path expression");
+  auto &ctx = getASTContext();
+  auto contextualType = getContextualType(getAnchor());
+  auto *keyPathExpr = castToExpr<KeyPathExpr>(getAnchor());
+
+  auto emitKeyPathDiagnostic = [&]() {
+    if (contextualType &&
+        contextualType->getAnyNominal() == ctx.getAnyKeyPathDecl()) {
+      return emitDiagnostic(
+          diag::cannot_infer_keypath_root_anykeypath_context);
+    }
+    return emitDiagnostic(
+        diag::cannot_infer_contextual_keypath_type_specify_root);
+  };
+
+  emitKeyPathDiagnostic()
+      .highlight(keyPathExpr->getLoc())
+      .fixItInsertAfter(keyPathExpr->getStartLoc(), "<#Root#>");
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -2058,6 +2058,21 @@ class MultiArgFuncKeyPathFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose a failure to infer a KeyPath type by context.
+///
+/// ```swift
+/// _ = \.x
+/// let _ : AnyKeyPath = \.x
+/// ```
+class UnableToInferKeyPathRootFailure final : public FailureDiagnostic {
+public:
+  UnableToInferKeyPathRootFailure(const Solution &solution,
+                                  ConstraintLocator *locator)
+  : FailureDiagnostic(solution, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -1323,3 +1323,17 @@ AllowKeyPathRootTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
   return new (cs.getAllocator())
       AllowKeyPathRootTypeMismatch(cs, lhs, rhs, locator);
 }
+
+SpecifyKeyPathRootType *
+SpecifyKeyPathRootType::create(ConstraintSystem &cs,
+                               ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      SpecifyKeyPathRootType(cs, locator);
+}
+
+bool SpecifyKeyPathRootType::diagnose(const Solution &solution,
+                                      bool asNote) const {
+  UnableToInferKeyPathRootFailure failure(solution, getLocator());
+  
+  return failure.diagnose(asNote);
+}

lib/Sema/CSFix.h

@@ -259,7 +259,11 @@ enum class FixKind : uint8_t {
   AllowKeyPathRootTypeMismatch,
 
   /// Allow key path to be bound to a function type with more than 1 argument
-  AllowMultiArgFuncKeyPathMismatch
+  AllowMultiArgFuncKeyPathMismatch,
+  
+  /// Specify key path root type when it cannot be infered from context.
+  SpecifyKeyPathRootType,
+
 };
 
 class ConstraintFix {
@@ -1805,7 +1809,7 @@ class AllowCoercionToForceCast final : public ContextualMismatch {
 ///   bar[keyPath: keyPath]
 /// }
 /// \endcode
-class AllowKeyPathRootTypeMismatch : public ContextualMismatch {
+class AllowKeyPathRootTypeMismatch final : public ContextualMismatch {
 protected:
   AllowKeyPathRootTypeMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
                                ConstraintLocator *locator)
@@ -1823,6 +1827,21 @@ class AllowKeyPathRootTypeMismatch : public ContextualMismatch {
   create(ConstraintSystem &cs, Type lhs, Type rhs, ConstraintLocator *locator);
 };
 
+class SpecifyKeyPathRootType final : public ConstraintFix {
+    SpecifyKeyPathRootType(ConstraintSystem &cs, ConstraintLocator *locator)
+        : ConstraintFix(cs, FixKind::SpecifyKeyPathRootType, locator) {}
+
+  public:
+    std::string getName() const {
+      return "specify key path root type";
+    }
+
+    bool diagnose(const Solution &solution, bool asNote = false) const;
+
+    static SpecifyKeyPathRootType *create(ConstraintSystem &cs,
+                                          ConstraintLocator *locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSGen.cpp

@@ -3398,7 +3398,8 @@ namespace {
       auto rootLocator =
           CS.getConstraintLocator(E, ConstraintLocator::KeyPathRoot);
       auto locator = CS.getConstraintLocator(E);
-      Type root = CS.createTypeVariable(rootLocator, TVO_CanBindToNoEscape);
+      Type root = CS.createTypeVariable(rootLocator, TVO_CanBindToNoEscape |
+                                        TVO_CanBindToHole);
 
       // If a root type was explicitly given, then resolve it now.
       if (auto rootRepr = E->getRootType()) {
@@ -3540,7 +3541,8 @@ namespace {
       // path components.
       auto typeLoc =
           CS.getConstraintLocator(locator, ConstraintLocator::KeyPathType);
-      Type kpTy = CS.createTypeVariable(typeLoc, TVO_CanBindToNoEscape);
+      Type kpTy = CS.createTypeVariable(typeLoc, TVO_CanBindToNoEscape |
+                                        TVO_CanBindToHole);
       CS.addKeyPathConstraint(kpTy, root, rvalueBase, componentTypeVars,
                               locator);
       return kpTy;

lib/Sema/CSSimplify.cpp

@@ -7663,6 +7663,12 @@ ConstraintSystem::simplifyKeyPathConstraint(
 
     return true;
   };
+  
+  // We have a hole, the solver can't infer the key path type. So let's
+  // just assume this is solved.
+  if (shouldAttemptFixes() && keyPathTy->isHole()) {
+    return SolutionKind::Solved;
+  }
 
   // If we're fixed to a bound generic type, trying harvesting context from it.
   // However, we don't want a solution that fixes the expression type to
@@ -9482,7 +9488,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::CoerceToCheckedCast:
   case FixKind::SpecifyObjectLiteralTypeImport:
   case FixKind::AllowKeyPathRootTypeMismatch:
-  case FixKind::AllowCoercionToForceCast: {
+  case FixKind::AllowCoercionToForceCast:
+  case FixKind::SpecifyKeyPathRootType: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
 

test/expr/unary/keypath/keypath.swift

@@ -195,9 +195,9 @@ func testKeyPath(sub: Sub, optSub: OptSub,
 
   let _: AnyKeyPath = \A.property
   let _: AnyKeyPath = \C<A>.value
-  let _: AnyKeyPath = \.property // expected-error{{ambiguous}}
+  let _: AnyKeyPath = \.property // expected-error {{'AnyKeyPath' does not provide enough context for root type to be inferred; consider explicitly specifying a root type}} {{24-24=<#Root#>}}
   let _: AnyKeyPath = \C.value // expected-error{{generic parameter 'T' could not be inferred}}
-  let _: AnyKeyPath = \.value // expected-error{{ambiguous}}
+  let _: AnyKeyPath = \.value // expected-error {{'AnyKeyPath' does not provide enough context for root type to be inferred; consider explicitly specifying a root type}} {{24-24=<#Root#>}}
 
   let _ = \Prop.[nonHashableSub] // expected-error{{subscript index of type 'NonHashableSub' in a key path must be Hashable}}
   let _ = \Prop.[sub, sub]
@@ -893,6 +893,15 @@ struct SR_12290 {
   }
 }
 
+func testKeyPathHole() {
+  _ = \.x // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{8-8=<#Root#>}}
+  let _ : AnyKeyPath = \.x 
+  // expected-error@-1 {{'AnyKeyPath' does not provide enough context for root type to be inferred; consider explicitly specifying a root type}} {{25-25=<#Root#>}}
+
+  func f(_ i: Int) {}
+  f(\.x) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{6-6=<#Root#>}}
+}
+
 func testSyntaxErrors() { // expected-note{{}}
   _ = \.  ; // expected-error{{expected member name following '.'}}
   _ = \.a ;