Merge pull request #30867 from xedin/rdar-59874355

[TypeChecker] Diagnose key paths with contextual type but no leading dot

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -585,6 +585,9 @@ ERROR(expr_swift_keypath_invalid_component,none,
       "invalid component of Swift key path", ())
 ERROR(expr_swift_keypath_not_starting_with_type,none,
       "a Swift key path must begin with a type", ())
+ERROR(expr_swift_keypath_not_starting_with_dot,none,
+      "a Swift key path with contextual root must begin with a leading dot",
+      ())
 ERROR(expr_smart_keypath_value_covert_to_contextual_type,none,
       "key path value type %0 cannot be converted to contextual type %1",
       (Type, Type))

include/swift/AST/Expr.h

@@ -5035,6 +5035,10 @@ class KeyPathExpr : public Expr {
   // The processed/resolved type, like Foo.Bar in \Foo.Bar.?.baz.
   TypeRepr *RootType = nullptr;
 
+  /// Determines whether a key path starts with '.' which denotes necessity for
+  /// a contextual root type.
+  bool HasLeadingDot = false;
+
 public:
   /// A single stored component, which will be one of:
   /// - an unresolved DeclNameRef, which has to be type-checked
@@ -5396,10 +5400,11 @@ class KeyPathExpr : public Expr {
               bool isImplicit = false);
 
   KeyPathExpr(SourceLoc backslashLoc, Expr *parsedRoot, Expr *parsedPath,
-              bool isImplicit = false)
+              bool hasLeadingDot, bool isImplicit = false)
       : Expr(ExprKind::KeyPath, isImplicit), StartLoc(backslashLoc),
         EndLoc(parsedPath ? parsedPath->getEndLoc() : parsedRoot->getEndLoc()),
-        ParsedRoot(parsedRoot), ParsedPath(parsedPath) {
+        ParsedRoot(parsedRoot), ParsedPath(parsedPath),
+        HasLeadingDot(hasLeadingDot) {
     assert((parsedRoot || parsedPath) &&
            "keypath must have either root or path");
     Bits.KeyPathExpr.IsObjC = false;
@@ -5463,6 +5468,9 @@ class KeyPathExpr : public Expr {
   /// True if this is an ObjC key path expression.
   bool isObjC() const { return Bits.KeyPathExpr.IsObjC; }
 
+  /// True if this key path expression has a leading dot.
+  bool expectsContextualRoot() const { return HasLeadingDot; }
+
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::KeyPath;
   }

lib/Parse/ParseExpr.cpp

@@ -570,7 +570,8 @@ ParserResult<Expr> Parser::parseExprKeyPath() {
       return rootResult;
   }
 
-  if (startsWithSymbol(Tok, '.')) {
+  bool hasLeadingDot = startsWithSymbol(Tok, '.');
+  if (hasLeadingDot) {
     SyntaxParsingContext ExprContext(SyntaxContext, SyntaxContextKind::Expr);
 
     auto dotLoc = Tok.getLoc();
@@ -600,8 +601,9 @@ ParserResult<Expr> Parser::parseExprKeyPath() {
   if (rootResult.isNull() && pathResult.isNull())
     return nullptr;
 
-  auto keypath = new (Context) KeyPathExpr(
-      backslashLoc, rootResult.getPtrOrNull(), pathResult.getPtrOrNull());
+  auto keypath =
+      new (Context) KeyPathExpr(backslashLoc, rootResult.getPtrOrNull(),
+                                pathResult.getPtrOrNull(), hasLeadingDot);
 
   // Handle code completion.
   if ((Tok.is(tok::code_complete) && !Tok.isAtStartOfLine()) ||

lib/Sema/CSApply.cpp

@@ -1964,6 +1964,7 @@ namespace {
       auto *keyPath = new (ctx) KeyPathExpr(/*backslashLoc=*/dotLoc,
                                             /*parsedRoot=*/nullptr,
                                             /*parsedPath=*/anchor,
+                                            /*hasLeadingDot=*/false,
                                             /*isImplicit=*/true);
       // Type of the keypath expression we are forming is known
       // in advance, so let's set it right away.

lib/Sema/TypeCheckConstraints.cpp

@@ -1774,16 +1774,31 @@ void PreCheckExpression::resolveKeyPathExpr(KeyPathExpr *KPE) {
   auto traversePath = [&](Expr *expr, bool isInParsedPath,
                           bool emitErrors = true) {
     Expr *outermostExpr = expr;
+    // We can end up in scenarios where the key path has contextual type,
+    // but is missing a leading dot. This can happen when we have an
+    // implicit TypeExpr or an implicit DeclRefExpr.
+    auto diagnoseMissingDot = [&]() {
+      DE.diagnose(expr->getLoc(),
+                  diag::expr_swift_keypath_not_starting_with_dot)
+          .fixItInsert(expr->getStartLoc(), ".");
+    };
     while (1) {
       // Base cases: we've reached the top.
       if (auto TE = dyn_cast<TypeExpr>(expr)) {
         assert(!isInParsedPath);
         rootType = TE->getTypeRepr();
+        if (TE->isImplicit() && !KPE->expectsContextualRoot())
+          diagnoseMissingDot();
         return;
       } else if (isa<KeyPathDotExpr>(expr)) {
         assert(isInParsedPath);
         // Nothing here: the type is either the root, or is inferred.
         return;
+      } else if (!KPE->expectsContextualRoot() && expr->isImplicit() &&
+                 isa<DeclRefExpr>(expr)) {
+        assert(!isInParsedPath);
+        diagnoseMissingDot();
+        return;
       }
 
       // Recurring cases:

lib/Sema/TypeCheckStorage.cpp

@@ -817,19 +817,15 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
     propertyKeyPath = UnresolvedDotExpr::createImplicit(ctx, propertyKeyPath,
         enclosingSelfAccess->accessedProperty->getFullName());
     propertyKeyPath = new (ctx) KeyPathExpr(
-        SourceLoc(), nullptr, propertyKeyPath);
+        SourceLoc(), nullptr, propertyKeyPath, /*hasLeadingDot=*/true);
 
     // Key path referring to the backing storage property.
     Expr *storageKeyPath = new (ctx) KeyPathDotExpr(SourceLoc());
     storageKeyPath = UnresolvedDotExpr::createImplicit(ctx, storageKeyPath,
                                                        storage->getFullName());
-    storageKeyPath = new (ctx) KeyPathExpr(
-        SourceLoc(), nullptr, storageKeyPath);
-    Expr *args[3] = {
-      selfDRE,
-      propertyKeyPath,
-      storageKeyPath
-    };
+    storageKeyPath = new (ctx) KeyPathExpr(SourceLoc(), nullptr, storageKeyPath,
+                                           /*hasLeadingDot=*/true);
+    Expr *args[3] = {selfDRE, propertyKeyPath, storageKeyPath};
 
     SubscriptDecl *subscriptDecl = enclosingSelfAccess->subscript;
     lookupExpr = SubscriptExpr::create(

test/expr/unary/keypath/keypath.swift

@@ -874,6 +874,25 @@ func sr11562() {
   // expected-error@-1 {{subscript index of type '(Int, Int)' in a key path must be Hashable}}
 }
 
+// SR-12290: Ban keypaths with contextual root and without a leading dot
+struct SR_12290 {
+  let property: [Int] = []
+  let kp1: KeyPath<SR_12290, Int> = \property.count // expected-error {{a Swift key path with contextual root must begin with a leading dot}}{{38-38=.}}
+  let kp2: KeyPath<SR_12290, Int> = \.property.count // Ok
+  let kp3: KeyPath<SR_12290, Int> = \SR_12290.property.count // Ok
+
+  func foo1(_: KeyPath<SR_12290, Int> = \property.count) {} // expected-error {{a Swift key path with contextual root must begin with a leading dot}}{{42-42=.}}
+  func foo2(_: KeyPath<SR_12290, Int> = \.property.count) {} // Ok
+  func foo3(_: KeyPath<SR_12290, Int> = \SR_12290.property.count) {} // Ok
+
+  func foo4<T>(_: KeyPath<SR_12290, T>) {}
+  func useFoo4() {
+    foo4(\property.count) // expected-error {{a Swift key path with contextual root must begin with a leading dot}}{{11-11=.}}
+    foo4(\.property.count) // Ok
+    foo4(\SR_12290.property.count) // Ok
+  }
+}
+
 func testSyntaxErrors() { // expected-note{{}}
   _ = \.  ; // expected-error{{expected member name following '.'}}
   _ = \.a ;