Merge pull request swiftlang#33320 from nathawes/index-key-paths

Nathan Hawes · web-flow · commit 69abfc1e6cb9 · 2020-08-06T15:39:36.000-07:00
[Sema/Index] Resolve #keyPath components so they get handled by indexing, semantic highlighting, etc
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -4657,13 +4657,24 @@ ERROR(availability_decl_unavailable, none,
       "%select{ in %3|}2%select{|: %4}4",
       (unsigned, DeclName, bool, StringRef, StringRef))
 
+WARNING(availability_decl_unavailable_warn, none,
+        "%select{getter for |setter for |}0%1 is unavailable"
+        "%select{ in %3|}2%select{|: %4}4",
+        (unsigned, DeclName, bool, StringRef, StringRef))
+
 #define REPLACEMENT_DECL_KIND_SELECT "select{| instance method| property}"
 ERROR(availability_decl_unavailable_rename, none,
       "%select{getter for |setter for |}0%1 has been "
       "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
       "'%4'%select{|: %5}5",
       (unsigned, DeclName, bool, unsigned, StringRef, StringRef))
 
+WARNING(availability_decl_unavailable_rename_warn, none,
+        "%select{getter for |setter for |}0%1 has been "
+        "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
+        "'%4'%select{|: %5}5",
+        (unsigned, DeclName, bool, unsigned, StringRef, StringRef))
+
 NOTE(availability_marked_unavailable, none,
      "%select{getter for |setter for |}0%1 has been explicitly marked "
      "unavailable here", (unsigned, DeclName))
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -5259,6 +5259,7 @@ class KeyPathExpr : public Expr {
       OptionalWrap,
       Identity,
       TupleElement,
+      DictionaryKey,
     };
   
   private:
@@ -5367,6 +5368,16 @@ class KeyPathExpr : public Expr {
                        propertyType,
                        loc);
     }
+
+    /// Create a component for a dictionary key (#keyPath only).
+    static Component forDictionaryKey(DeclNameRef UnresolvedName,
+                                      Type valueType,
+                                      SourceLoc loc) {
+      return Component(nullptr, UnresolvedName, nullptr, {}, {},
+                       Kind::DictionaryKey,
+                       valueType,
+                       loc);
+    }
     
     /// Create a component for a subscript.
     static Component forSubscript(ASTContext &ctx,
@@ -5457,6 +5468,7 @@ class KeyPathExpr : public Expr {
       case Kind::Property:
       case Kind::Identity:
       case Kind::TupleElement:
+      case Kind::DictionaryKey:
         return true;
 
       case Kind::UnresolvedSubscript:
@@ -5481,6 +5493,7 @@ class KeyPathExpr : public Expr {
       case Kind::Property:
       case Kind::Identity:
       case Kind::TupleElement:
+      case Kind::DictionaryKey:
         return nullptr;
       }
       llvm_unreachable("unhandled kind");
@@ -5500,6 +5513,7 @@ class KeyPathExpr : public Expr {
       case Kind::Property:
       case Kind::Identity:
       case Kind::TupleElement:
+      case Kind::DictionaryKey:
         llvm_unreachable("no subscript labels for this kind");
       }
       llvm_unreachable("unhandled kind");
@@ -5522,6 +5536,7 @@ class KeyPathExpr : public Expr {
       case Kind::Property:
       case Kind::Identity:
       case Kind::TupleElement:
+      case Kind::DictionaryKey:
         return {};
       }
       llvm_unreachable("unhandled kind");
@@ -5533,6 +5548,7 @@ class KeyPathExpr : public Expr {
     DeclNameRef getUnresolvedDeclName() const {
       switch (getKind()) {
       case Kind::UnresolvedProperty:
+      case Kind::DictionaryKey:
         return Decl.UnresolvedName;
 
       case Kind::Invalid:
@@ -5563,6 +5579,7 @@ class KeyPathExpr : public Expr {
       case Kind::OptionalForce:
       case Kind::Identity:
       case Kind::TupleElement:
+      case Kind::DictionaryKey:
         llvm_unreachable("no decl ref for this kind");
       }
       llvm_unreachable("unhandled kind");
@@ -5582,6 +5599,7 @@ class KeyPathExpr : public Expr {
         case Kind::Identity:
         case Kind::Property:
         case Kind::Subscript:
+        case Kind::DictionaryKey:
           llvm_unreachable("no field number for this kind");
       }
       llvm_unreachable("unhandled kind");
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
@@ -2829,6 +2829,11 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
         PrintWithColorRAII(OS, DiscriminatorColor)
           << "#" << component.getTupleIndex();
         break;
+      case KeyPathExpr::Component::Kind::DictionaryKey:
+        PrintWithColorRAII(OS, ASTNodeColor) << "dict_key";
+        PrintWithColorRAII(OS, IdentifierColor)
+          << "  key='" << component.getUnresolvedDeclName() << "'";
+        break;
       }
       PrintWithColorRAII(OS, TypeColor)
         << " type='" << GetTypeOfKeyPathComponent(E, i) << "'";
diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp
@@ -1133,6 +1133,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
       case KeyPathExpr::Component::Kind::Invalid:
       case KeyPathExpr::Component::Kind::Identity:
       case KeyPathExpr::Component::Kind::TupleElement:
+      case KeyPathExpr::Component::Kind::DictionaryKey:
         // No subexpr to visit.
         break;
       }
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -2383,6 +2383,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
   case Kind::Property:
   case Kind::Identity:
   case Kind::TupleElement:
+  case Kind::DictionaryKey:
     llvm_unreachable("no hashable conformances for this kind");
   }
 }
diff --git a/lib/IDE/SourceEntityWalker.cpp b/lib/IDE/SourceEntityWalker.cpp
@@ -412,6 +412,7 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
       case KeyPathExpr::Component::Kind::OptionalWrap:
       case KeyPathExpr::Component::Kind::OptionalForce:
       case KeyPathExpr::Component::Kind::Identity:
+      case KeyPathExpr::Component::Kind::DictionaryKey:
         break;
       }
     }
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -3734,6 +3734,11 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
     case KeyPathExpr::Component::Kind::UnresolvedProperty:
     case KeyPathExpr::Component::Kind::UnresolvedSubscript:
       llvm_unreachable("not resolved");
+      break;
+
+    case KeyPathExpr::Component::Kind::DictionaryKey:
+      llvm_unreachable("DictionaryKey only valid in #keyPath");
+      break;
     }
   }
   
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -237,6 +237,9 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
       // Don't bother building the key path string if the key path didn't even
       // resolve.
       return false;
+    case KeyPathExpr::Component::Kind::DictionaryKey:
+      llvm_unreachable("DictionaryKey only valid in #keyPath expressions.");
+      return false;
     }
   }
   
@@ -4690,6 +4693,10 @@ namespace {
         case KeyPathExpr::Component::Kind::OptionalWrap:
         case KeyPathExpr::Component::Kind::TupleElement:
           llvm_unreachable("already resolved");
+          break;
+        case KeyPathExpr::Component::Kind::DictionaryKey:
+          llvm_unreachable("DictionaryKey only valid in #keyPath");
+          break;
         }
 
         // Update "componentTy" with the result type of the last component.
@@ -7745,9 +7752,8 @@ namespace {
             componentType = solution.simplifyType(cs.getType(kp, i));
             assert(!componentType->hasTypeVariable() &&
                    "Should not write type variable into key-path component");
+            kp->getMutableComponents()[i].setComponentType(componentType);
           }
-
-          kp->getMutableComponents()[i].setComponentType(componentType);
         }
       }
 
diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -3487,6 +3487,9 @@ namespace {
         }
         case KeyPathExpr::Component::Kind::Identity:
           continue;
+        case KeyPathExpr::Component::Kind::DictionaryKey:
+          llvm_unreachable("DictionaryKey only valid in #keyPath");
+          break;
         }
 
         // By now, `base` is the result type of this component. Set it in the
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -8242,6 +8242,9 @@ ConstraintSystem::simplifyKeyPathConstraint(
     case KeyPathExpr::Component::Kind::TupleElement:
       llvm_unreachable("not implemented");
       break;
+    case KeyPathExpr::Component::Kind::DictionaryKey:
+      llvm_unreachable("DictionaryKey only valid in #keyPath");
+      break;
     }
   }
 
diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -493,6 +493,7 @@ ConstraintLocator *ConstraintSystem::getCalleeLocator(
     case ComponentKind::OptionalChain:
     case ComponentKind::OptionalWrap:
     case ComponentKind::Identity:
+    case ComponentKind::DictionaryKey:
       // These components don't have any callee associated, so just continue.
       break;
     }
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -2067,6 +2067,7 @@ void swift::diagnoseUnavailableOverride(ValueDecl *override,
 
   diagnoseExplicitUnavailability(base, override->getLoc(),
                                  override->getDeclContext(),
+                                 /*Flags*/None,
                                  [&](InFlightDiagnostic &diag) {
     ParsedDeclName parsedName = parseDeclName(attr->Rename);
     if (!parsedName || parsedName.isPropertyAccessor() ||
@@ -2097,10 +2098,11 @@ void swift::diagnoseUnavailableOverride(ValueDecl *override,
 /// Emit a diagnostic for references to declarations that have been
 /// marked as unavailable, either through "unavailable" or "obsoleted:".
 bool swift::diagnoseExplicitUnavailability(const ValueDecl *D,
-                                                 SourceRange R,
-                                                 const DeclContext *DC,
-                                                 const ApplyExpr *call) {
-  return diagnoseExplicitUnavailability(D, R, DC,
+                                           SourceRange R,
+                                           const DeclContext *DC,
+                                           const ApplyExpr *call,
+                                           DeclAvailabilityFlags Flags) {
+  return diagnoseExplicitUnavailability(D, R, DC, Flags,
                                         [=](InFlightDiagnostic &diag) {
     fixItAvailableAttrRename(diag, R, D, AvailableAttr::isUnavailable(D),
                              call);
@@ -2172,6 +2174,7 @@ bool swift::diagnoseExplicitUnavailability(
     const ValueDecl *D,
     SourceRange R,
     const DeclContext *DC,
+    DeclAvailabilityFlags Flags,
     llvm::function_ref<void(InFlightDiagnostic &)> attachRenameFixIts) {
   auto *Attr = AvailableAttr::isUnavailable(D);
   if (!Attr)
@@ -2229,6 +2232,14 @@ bool swift::diagnoseExplicitUnavailability(
     break;
   }
 
+  // TODO: Consider removing this.
+  // ObjC keypaths components weren't checked previously, so errors are demoted
+  // to warnings to avoid source breakage. In some cases unavailable or
+  // obsolete decls still map to valid ObjC runtime names, so behave correctly
+  // at runtime, even though their use would produce an error outside of a
+  // #keyPath expression.
+  bool warnInObjCKeyPath = Flags.contains(DeclAvailabilityFlag::ForObjCKeyPath);
+
   if (!Attr->Rename.empty()) {
     SmallString<32> newNameBuf;
     Optional<ReplacementDeclKind> replaceKind =
@@ -2238,7 +2249,9 @@ bool swift::diagnoseExplicitUnavailability(
     StringRef newName = replaceKind ? newNameBuf.str() : Attr->Rename;
       EncodedDiagnosticMessage EncodedMessage(Attr->Message);
       auto diag =
-          diags.diagnose(Loc, diag::availability_decl_unavailable_rename,
+          diags.diagnose(Loc, warnInObjCKeyPath
+                         ? diag::availability_decl_unavailable_rename_warn
+                         : diag::availability_decl_unavailable_rename,
                          RawAccessorKind, Name, replaceKind.hasValue(),
                          rawReplaceKind, newName, EncodedMessage.Message);
       attachRenameFixIts(diag);
@@ -2253,7 +2266,9 @@ bool swift::diagnoseExplicitUnavailability(
   } else {
     EncodedDiagnosticMessage EncodedMessage(Attr->Message);
     diags
-        .diagnose(Loc, diag::availability_decl_unavailable, RawAccessorKind,
+        .diagnose(Loc, warnInObjCKeyPath
+                  ? diag::availability_decl_unavailable_warn
+                  : diag::availability_decl_unavailable, RawAccessorKind,
                   Name, platform.empty(), platform, EncodedMessage.Message)
         .highlight(R);
   }
@@ -2501,14 +2516,18 @@ class AvailabilityWalker : public ASTWalker {
   /// Walk a keypath expression, checking all of its components for
   /// availability.
   void maybeDiagKeyPath(KeyPathExpr *KP) {
+    auto flags = DeclAvailabilityFlags();
+    if (KP->isObjC())
+      flags = DeclAvailabilityFlag::ForObjCKeyPath;
+
     for (auto &component : KP->getComponents()) {
       switch (component.getKind()) {
       case KeyPathExpr::Component::Kind::Property:
       case KeyPathExpr::Component::Kind::Subscript: {
         auto *decl = component.getDeclRef().getDecl();
         auto loc = component.getLoc();
         SourceRange range(loc, loc);
-        diagAvailability(decl, range, nullptr);
+        diagAvailability(decl, range, nullptr, flags);
         break;
       }
 
@@ -2522,6 +2541,7 @@ class AvailabilityWalker : public ASTWalker {
       case KeyPathExpr::Component::Kind::OptionalWrap:
       case KeyPathExpr::Component::Kind::OptionalForce:
       case KeyPathExpr::Component::Kind::Identity:
+      case KeyPathExpr::Component::Kind::DictionaryKey:
         break;
       }
     }
@@ -2627,7 +2647,7 @@ AvailabilityWalker::diagAvailability(ConcreteDeclRef declRef, SourceRange R,
       if (TypeChecker::diagnoseInlinableDeclRef(R.Start, declRef, DC, FragileKind))
         return true;
 
-  if (diagnoseExplicitUnavailability(D, R, DC, call))
+  if (diagnoseExplicitUnavailability(D, R, DC, call, Flags))
     return true;
 
   // Make sure not to diagnose an accessor's deprecation if we already
diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h
@@ -50,6 +50,10 @@ enum class DeclAvailabilityFlag : uint8_t {
   /// Do not diagnose uses of declarations in versions before they were
   /// introduced. Used to work around availability-checker bugs.
   AllowPotentiallyUnavailable = 1 << 3,
+
+  /// If an error diagnostic would normally be emitted, demote the error to a
+  /// warning. Used for ObjC key path components.
+  ForObjCKeyPath = 1 << 4
 };
 using DeclAvailabilityFlags = OptionSet<DeclAvailabilityFlag>;
 
@@ -70,14 +74,16 @@ void diagnoseUnavailableOverride(ValueDecl *override,
 bool diagnoseExplicitUnavailability(const ValueDecl *D,
                                     SourceRange R,
                                     const DeclContext *DC,
-                                    const ApplyExpr *call);
+                                    const ApplyExpr *call,
+                                    DeclAvailabilityFlags Flags = None);
 
 /// Emit a diagnostic for references to declarations that have been
 /// marked as unavailable, either through "unavailable" or "obsoleted:".
 bool diagnoseExplicitUnavailability(
     const ValueDecl *D,
     SourceRange R,
     const DeclContext *DC,
+    DeclAvailabilityFlags Flags,
     llvm::function_ref<void(InFlightDiagnostic &)> attachRenameFixIts);
 
 /// Check if \p decl has a introduction version required by -require-explicit-availability
diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp
@@ -621,9 +621,21 @@ static Optional<Type> getTypeOfCompletionContextExpr(
 
   case CompletionTypeCheckKind::KeyPath:
     referencedDecl = nullptr;
-    if (auto keyPath = dyn_cast<KeyPathExpr>(parsedExpr))
-      return TypeChecker::checkObjCKeyPathExpr(DC, keyPath,
-                                               /*requireResultType=*/true);
+    if (auto keyPath = dyn_cast<KeyPathExpr>(parsedExpr)) {
+      auto components = keyPath->getComponents();
+      if (!components.empty()) {
+        auto &last = components.back();
+        if (last.isResolved()) {
+          if (last.getKind() == KeyPathExpr::Component::Kind::Property)
+            referencedDecl = last.getDeclRef();
+          Type lookupTy = last.getComponentType();
+          ASTContext &Ctx = DC->getASTContext();
+          if (auto bridgedClass = Ctx.getBridgedToObjC(DC, lookupTy))
+            return bridgedClass;
+          return lookupTy;
+        }
+      }
+    }
 
     return None;
   }
diff --git a/lib/Sema/TypeCheckExprObjC.cpp b/lib/Sema/TypeCheckExprObjC.cpp
diff --git a/localization/diagnostics/en.yaml b/localization/diagnostics/en.yaml
diff --git a/test/IDE/complete_pound_keypath.swift b/test/IDE/complete_pound_keypath.swift
diff --git a/test/Index/index_keypaths.swift b/test/Index/index_keypaths.swift
diff --git a/test/expr/primary/keypath/keypath-objc.swift b/test/expr/primary/keypath/keypath-objc.swift

Original file line number	Diff line number	Diff line change
`@@ -1133,6 +1133,7 @@ class Traversal : public ASTVisitor<Traversal, Expr, Stmt,`
`1133`	`1133`	`case KeyPathExpr::Component::Kind::Invalid:`
`1134`	`1134`	`case KeyPathExpr::Component::Kind::Identity:`
`1135`	`1135`	`case KeyPathExpr::Component::Kind::TupleElement:`
	`1136`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
`1136`	`1137`	`// No subexpr to visit.`
`1137`	`1138`	`break;`
`1138`	`1139`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2383,6 +2383,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(`
`2383`	`2383`	`case Kind::Property:`
`2384`	`2384`	`case Kind::Identity:`
`2385`	`2385`	`case Kind::TupleElement:`
	`2386`	`+ case Kind::DictionaryKey:`
`2386`	`2387`	`llvm_unreachable("no hashable conformances for this kind");`
`2387`	`2388`	`}`
`2388`	`2389`	`}`
Original file line number	Diff line number	Diff line change
`@@ -412,6 +412,7 @@ std::pair<bool, Expr > SemaAnnotator::walkToExprPre(Expr E) {`
`412`	`412`	`case KeyPathExpr::Component::Kind::OptionalWrap:`
`413`	`413`	`case KeyPathExpr::Component::Kind::OptionalForce:`
`414`	`414`	`case KeyPathExpr::Component::Kind::Identity:`
	`415`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
`415`	`416`	`break;`
`416`	`417`	`}`
`417`	`418`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3734,6 +3734,11 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {`
`3734`	`3734`	`case KeyPathExpr::Component::Kind::UnresolvedProperty:`
`3735`	`3735`	`case KeyPathExpr::Component::Kind::UnresolvedSubscript:`
`3736`	`3736`	`llvm_unreachable("not resolved");`
	`3737`	`+ break;`
	`3738`	`+`
	`3739`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
	`3740`	`+ llvm_unreachable("DictionaryKey only valid in #keyPath");`
	`3741`	`+ break;`
`3737`	`3742`	`}`
`3738`	`3743`	`}`
`3739`	`3744`
Original file line number	Diff line number	Diff line change
`@@ -237,6 +237,9 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,`
`237`	`237`	`// Don't bother building the key path string if the key path didn't even`
`238`	`238`	`// resolve.`
`239`	`239`	`return false;`
	`240`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
	`241`	`+ llvm_unreachable("DictionaryKey only valid in #keyPath expressions.");`
	`242`	`+ return false;`
`240`	`243`	`}`
`241`	`244`	`}`
`242`	`245`
`@@ -4690,6 +4693,10 @@ namespace {`
`4690`	`4693`	`case KeyPathExpr::Component::Kind::OptionalWrap:`
`4691`	`4694`	`case KeyPathExpr::Component::Kind::TupleElement:`
`4692`	`4695`	`llvm_unreachable("already resolved");`
	`4696`	`+ break;`
	`4697`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
	`4698`	`+ llvm_unreachable("DictionaryKey only valid in #keyPath");`
	`4699`	`+ break;`
`4693`	`4700`	`}`
`4694`	`4701`
`4695`	`4702`	`// Update "componentTy" with the result type of the last component.`
`@@ -7745,9 +7752,8 @@ namespace {`
`7745`	`7752`	`componentType = solution.simplifyType(cs.getType(kp, i));`
`7746`	`7753`	`assert(!componentType->hasTypeVariable() &&`
`7747`	`7754`	`"Should not write type variable into key-path component");`
	`7755`	`+ kp->getMutableComponents()[i].setComponentType(componentType);`
`7748`	`7756`	`}`
`7749`		`-`
`7750`		`- kp->getMutableComponents()[i].setComponentType(componentType);`
`7751`	`7757`	`}`
`7752`	`7758`	`}`
`7753`	`7759`
Original file line number	Diff line number	Diff line change
`@@ -3487,6 +3487,9 @@ namespace {`
`3487`	`3487`	`}`
`3488`	`3488`	`case KeyPathExpr::Component::Kind::Identity:`
`3489`	`3489`	`continue;`
	`3490`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
	`3491`	`+ llvm_unreachable("DictionaryKey only valid in #keyPath");`
	`3492`	`+ break;`
`3490`	`3493`	`}`
`3491`	`3494`
`3492`	`3495`	// By now, `base` is the result type of this component. Set it in the
Original file line number	Diff line number	Diff line change
`@@ -8242,6 +8242,9 @@ ConstraintSystem::simplifyKeyPathConstraint(`
`8242`	`8242`	`case KeyPathExpr::Component::Kind::TupleElement:`
`8243`	`8243`	`llvm_unreachable("not implemented");`
`8244`	`8244`	`break;`
	`8245`	`+ case KeyPathExpr::Component::Kind::DictionaryKey:`
	`8246`	`+ llvm_unreachable("DictionaryKey only valid in #keyPath");`
	`8247`	`+ break;`
`8245`	`8248`	`}`
`8246`	`8249`	`}`
`8247`	`8250`