[Diagnostics] Demote availability errors to warning for decls in ObjC keypaths to prevent source breakage.

The change to resolve ObjC #keyPath expression components caused some source
breakage as they are now being checked for availability issues. This change
updates availability checking to demote error diagnostics to warnings
within #keyPath expressions. There were cases in the source compat suite where
unavailble properites were used in #keyPath expressions, but they caused no
issues at runtime because the properties' ObjC runtime name was still correct
(e.g. the same as its renamed-to property in Swift).

Author: Nathan Hawes

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))

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;
       }
 
@@ -2628,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

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

localization/diagnostics/en.yaml

@@ -9232,11 +9232,21 @@
     %select{getter for |setter for |}0%1 is unavailable%select{ in
     %3|}2%select{|: %4}4
 
+- id: availability_decl_unavailable_warn
+  msg: >-
+    %select{getter for |setter for |}0%1 is unavailable%select{ in
+    %3|}2%select{|: %4}4
+
 - id: availability_decl_unavailable_rename
   msg: >-
     %select{getter for |setter for |}0%1 has been %select{renamed to|replaced
     by}2%select{| instance method| property}3 '%4'%select{|: %5}5
 
+- id: availability_decl_unavailable_rename_warn
+  msg: >-
+    %select{getter for |setter for |}0%1 has been %select{renamed to|replaced
+    by}2%select{| instance method| property}3 '%4'%select{|: %5}5
+
 - id: availability_marked_unavailable
   msg: >-
     %select{getter for |setter for |}0%1 has been explicitly marked

test/expr/primary/keypath/keypath-objc.swift

@@ -57,7 +57,7 @@ func testKeyPath(a: A, b: B) {
   let _: String = #keyPath(A.propString)
 
   // Property of String property (which looks on NSString)
-  let _: String = #keyPath(A.propString.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propString.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // String property with a suffix
   let _: String = #keyPath(A.propString).description
@@ -72,36 +72,36 @@ func testKeyPath(a: A, b: B) {
 
   // Array property (make sure we look at the array element).
   let _: String = #keyPath(A.propArray)
-  let _: String = #keyPath(A.propArray.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propArray.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // Dictionary property (make sure we look at the value type).
   let _: String = #keyPath(A.propDict.anyKeyName)
   let _: String = #keyPath(A.propDict.anyKeyName.propA)
 
   // Set property (make sure we look at the set element).
   let _: String = #keyPath(A.propSet)
-  let _: String = #keyPath(A.propSet.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propSet.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // AnyObject property
-  let _: String = #keyPath(A.propAnyObject.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propAnyObject.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
   let _: String = #keyPath(A.propAnyObject.propA)  
   let _: String = #keyPath(A.propAnyObject.propB)  
   let _: String = #keyPath(A.propAnyObject.description)  
 
   // NSString property
-  let _: String = #keyPath(A.propNSString.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propNSString.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // NSArray property (AnyObject array element).
   let _: String = #keyPath(A.propNSArray)
-  let _: String = #keyPath(A.propNSArray.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propNSArray.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // NSDictionary property (AnyObject value type).
   let _: String = #keyPath(A.propNSDict.anyKeyName)
   let _: String = #keyPath(A.propNSDict.anyKeyName.propA)
 
   // NSSet property (AnyObject set element).
   let _: String = #keyPath(A.propNSSet)
-  let _: String = #keyPath(A.propNSSet.URLsInText) // expected-error{{'URLsInText' has been renamed to 'urlsInText'}}
+  let _: String = #keyPath(A.propNSSet.URLsInText) // expected-warning{{'URLsInText' has been renamed to 'urlsInText'}}
 
   // Property with keyword name.
   let _: String = #keyPath(A.repeat)