Sema: Downgrade diagnostics about potential unavailability of the extended type in an extension declaration when the following conditions are met:

tshortli · tshortli · commit 85decfdfdc3b · 2022-05-03T18:24:29.000-07:00
1. The extension is missing explicit availability.
2. The required availability is before the deployment target.

This exception is needed because there are many existing resilient libraries with extensions containing public members and no availability declared for the extension itself. Under existing rules, these decls are not diagnosed because the deployment target of the library is typically high enough that the extended type is implicitly considered available. Now that we're improving availability type checking for resilient libraries, however, these decls are being diagnosed. We need to land the diagnostic without breaking source compatibility so that library authors can identify and fix the issues.

Resolves rdar://92621567
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -5581,6 +5581,10 @@ ERROR(availability_decl_only_version_newer, none,
       "%0 is only available in %1 %2 or newer",
       (DeclName, StringRef, llvm::VersionTuple))
 
+WARNING(availability_decl_only_version_newer_warn, none,
+        "%0 is only available in %1 %2 or newer",
+        (DeclName, StringRef, llvm::VersionTuple))
+
 ERROR(availability_opaque_types_only_version_newer, none,
       "'some' return types are only available in %0 %1 or newer",
       (StringRef, llvm::VersionTuple))
diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp
@@ -1579,7 +1579,7 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
 
   void checkType(Type type, const TypeRepr *typeRepr, const Decl *context,
                  ExportabilityReason reason=ExportabilityReason::General,
-                 bool allowUnavailableProtocol=false) {
+                 DeclAvailabilityFlags flags=None) {
     // Don't bother checking errors.
     if (type && type->hasError())
       return;
@@ -1589,18 +1589,6 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
     if (AvailableAttr::isUnavailable(context))
       return;
 
-    DeclAvailabilityFlags flags = None;
-
-    // We allow a type to conform to a protocol that is less available than
-    // the type itself. This enables a type to retroactively model or directly
-    // conform to a protocol only available on newer OSes and yet still be used on
-    // older OSes.
-    //
-    // To support this, inside inheritance clauses we allow references to
-    // protocols that are unavailable in the current type refinement context.
-    if (allowUnavailableProtocol)
-      flags |= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol;
-
     diagnoseTypeAvailability(typeRepr, type, context->getLoc(),
                              Where.withReason(reason), flags);
   }
@@ -1758,20 +1746,17 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
   void visitNominalTypeDecl(const NominalTypeDecl *nominal) {
     checkGenericParams(nominal, nominal);
 
-    llvm::for_each(nominal->getInherited(),
-                   [&](TypeLoc inherited) {
-      checkType(inherited.getType(), inherited.getTypeRepr(),
-                nominal, ExportabilityReason::General,
-                /*allowUnavailableProtocol=*/true);
+    llvm::for_each(nominal->getInherited(), [&](TypeLoc inherited) {
+      checkType(inherited.getType(), inherited.getTypeRepr(), nominal,
+                ExportabilityReason::General,
+                DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol);
     });
   }
 
   void visitProtocolDecl(ProtocolDecl *proto) {
-    llvm::for_each(proto->getInherited(),
-                  [&](TypeLoc requirement) {
+    llvm::for_each(proto->getInherited(), [&](TypeLoc requirement) {
       checkType(requirement.getType(), requirement.getTypeRepr(), proto,
-                ExportabilityReason::General,
-                /*allowUnavailableProtocol=*/false);
+                ExportabilityReason::General);
     });
 
     if (proto->getTrailingWhereClause()) {
@@ -1846,11 +1831,10 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
     //
     // 1) If the extension defines conformances, the conformed-to protocols
     // must be exported.
-    llvm::for_each(ED->getInherited(),
-                   [&](TypeLoc inherited) {
-      checkType(inherited.getType(), inherited.getTypeRepr(),
-                ED, ExportabilityReason::General,
-                /*allowUnavailableProtocol=*/true);
+    llvm::for_each(ED->getInherited(), [&](TypeLoc inherited) {
+      checkType(inherited.getType(), inherited.getTypeRepr(), ED,
+                ExportabilityReason::General,
+                DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol);
     });
 
     auto wasWhere = Where;
@@ -1866,8 +1850,21 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
     });
 
     Where = wasWhere.withExported(hasExportedMembers);
+
+    // When diagnosing potential unavailability of the extended type, downgrade
+    // the diagnostics to warnings when the extension decl has no declared
+    // availability and the required availability is more than the deployment
+    // target.
+    DeclAvailabilityFlags extendedTypeFlags = None;
+    auto annotatedRange =
+        AvailabilityInference::annotatedAvailableRange(ED, ED->getASTContext());
+    if (!annotatedRange.hasValue())
+      extendedTypeFlags |= DeclAvailabilityFlag::
+          WarnForPotentialUnavailabilityBeforeDeploymentTarget;
+
     checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED,
-              ExportabilityReason::ExtensionWithPublicMembers);
+              ExportabilityReason::ExtensionWithPublicMembers,
+              extendedTypeFlags);
 
     // 3) If the extension contains exported members or defines conformances,
     // the 'where' clause must only name exported types.
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -1881,28 +1881,36 @@ void TypeChecker::checkConcurrencyAvailability(SourceRange ReferenceRange,
   }
 }
 
-void TypeChecker::diagnosePotentialUnavailability(
+bool TypeChecker::diagnosePotentialUnavailability(
     const ValueDecl *D, SourceRange ReferenceRange,
     const DeclContext *ReferenceDC,
-    const UnavailabilityReason &Reason) {
+    const UnavailabilityReason &Reason,
+    bool WarnBeforeDeploymentTarget) {
   ASTContext &Context = ReferenceDC->getASTContext();
 
+  bool AsError = true;
   auto RequiredRange = Reason.getRequiredOSVersionRange();
   {
-    auto Err =
-      Context.Diags.diagnose(
-               ReferenceRange.Start, diag::availability_decl_only_version_newer,
-               D->getName(), prettyPlatformString(targetPlatform(Context.LangOpts)),
-               Reason.getRequiredOSVersionRange().getLowerEndpoint());
+    if (WarnBeforeDeploymentTarget &&
+        !RequiredRange.isContainedIn(
+            AvailabilityContext::forDeploymentTarget(Context).getOSVersion()))
+      AsError = false;
+
+    auto Diag = Context.Diags.diagnose(
+        ReferenceRange.Start,
+        AsError ? diag::availability_decl_only_version_newer
+                : diag::availability_decl_only_version_newer_warn,
+        D->getName(), prettyPlatformString(targetPlatform(Context.LangOpts)),
+        Reason.getRequiredOSVersionRange().getLowerEndpoint());
 
     // Direct a fixit to the error if an existing guard is nearly-correct
-    if (fixAvailabilityByNarrowingNearbyVersionCheck(ReferenceRange,
-                                                     ReferenceDC,
-                                                     RequiredRange, Context, Err))
-      return;
+    if (fixAvailabilityByNarrowingNearbyVersionCheck(
+            ReferenceRange, ReferenceDC, RequiredRange, Context, Diag))
+      return AsError;
   }
 
   fixAvailability(ReferenceRange, ReferenceDC, RequiredRange, Context);
+  return AsError;
 }
 
 void TypeChecker::diagnosePotentialAccessorUnavailability(
@@ -3406,21 +3414,26 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R,
 
   // Diagnose (and possibly signal) for potential unavailability
   auto maybeUnavail = TypeChecker::checkDeclarationAvailability(D, Where);
-  if (maybeUnavail.hasValue()) {
-    auto *DC = Where.getDeclContext();
+  if (!maybeUnavail.hasValue())
+    return false;
 
-    if (accessor) {
-      bool forInout = Flags.contains(DeclAvailabilityFlag::ForInout);
-      TypeChecker::diagnosePotentialAccessorUnavailability(accessor, R, DC,
-                                                           maybeUnavail.getValue(),
-                                                           forInout);
-    } else {
-      TypeChecker::diagnosePotentialUnavailability(D, R, DC, maybeUnavail.getValue());
-    }
-    if (!Flags.contains(DeclAvailabilityFlag::ContinueOnPotentialUnavailability))
-      return true;
+  auto *DC = Where.getDeclContext();
+
+  if (accessor) {
+    bool forInout = Flags.contains(DeclAvailabilityFlag::ForInout);
+    TypeChecker::diagnosePotentialAccessorUnavailability(
+        accessor, R, DC, maybeUnavail.getValue(), forInout);
+  } else {
+    bool downgradeBeforeDeploymentTarget = Flags.contains(
+        DeclAvailabilityFlag::
+            WarnForPotentialUnavailabilityBeforeDeploymentTarget);
+    if (!TypeChecker::diagnosePotentialUnavailability(
+            D, R, DC, maybeUnavail.getValue(), downgradeBeforeDeploymentTarget))
+      return false;
   }
-  return false;
+
+  return !Flags.contains(
+      DeclAvailabilityFlag::ContinueOnPotentialUnavailability);
 }
 
 /// Return true if the specified type looks like an integer of floating point
diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h
@@ -40,8 +40,10 @@ namespace swift {
 
 enum class DeclAvailabilityFlag : uint8_t {
   /// Do not diagnose uses of protocols in versions before they were introduced.
-  /// Used when type-checking protocol conformances, since conforming to a
-  /// protocol that doesn't exist yet is allowed.
+  /// We allow a type to conform to a protocol that is less available than the
+  /// type itself. This enables a type to retroactively model or directly conform
+  /// to a protocol only available on newer OSes and yet still be used on older
+  /// OSes.
   AllowPotentiallyUnavailableProtocol = 1 << 0,
 
   /// Diagnose uses of declarations in versions before they were introduced, but
@@ -54,7 +56,13 @@ enum class DeclAvailabilityFlag : uint8_t {
 
   /// If an error diagnostic would normally be emitted, demote the error to a
   /// warning. Used for ObjC key path components.
-  ForObjCKeyPath = 1 << 3
+  ForObjCKeyPath = 1 << 3,
+  
+  /// Downgrade errors about decl availability to warnings when the fix would be
+  /// to constrain availability to a version that is more available than the
+  /// current deployment target. This is needed for source compatibility in when
+  /// checking public extensions in library modules.
+  WarnForPotentialUnavailabilityBeforeDeploymentTarget = 1 << 4,
 };
 using DeclAvailabilityFlags = OptionSet<DeclAvailabilityFlag>;
 
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -1072,15 +1072,17 @@ checkConformanceAvailability(const RootProtocolConformance *Conf,
 /// expression or statement.
 void checkIgnoredExpr(Expr *E);
 
-// Emits a diagnostic, if necessary, for a reference to a declaration
-// that is potentially unavailable at the given source location.
-void diagnosePotentialUnavailability(const ValueDecl *D,
+// Emits a diagnostic for a reference to a declaration that is potentially
+// unavailable at the given source location. Returns true if an error diagnostic
+// was emitted.
+bool diagnosePotentialUnavailability(const ValueDecl *D,
                                      SourceRange ReferenceRange,
                                      const DeclContext *ReferenceDC,
-                                     const UnavailabilityReason &Reason);
+                                     const UnavailabilityReason &Reason,
+                                     bool WarnBeforeDeploymentTarget);
 
-// Emits a diagnostic, if necessary, for a reference to a declaration
-// that is potentially unavailable at the given source location.
+// Emits a diagnostic for a protocol conformance that is potentially
+// unavailable at the given source location.
 void diagnosePotentialUnavailability(const RootProtocolConformance *rootConf,
                                      const ExtensionDecl *ext,
                                      SourceLoc loc,
diff --git a/test/attr/attr_inlinable_available.swift b/test/attr/attr_inlinable_available.swift
@@ -608,25 +608,32 @@ extension BetweenTargets {
   fileprivate func fileprivateFunc1() {}
 }
 
-// expected-error@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
+// expected-warning@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
 extension BetweenTargets {
   public func publicFunc1() {}
 }
 
-// expected-error@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
+// expected-warning@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
 extension BetweenTargets {
   @usableFromInline
   internal func usableFromInlineFunc1() {}
 }
 
-// expected-error@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
+// expected-warning@+1 {{'BetweenTargets' is only available in}} expected-note@+1 {{add @available attribute to enclosing extension}}
 extension BetweenTargets {
   internal func internalFunc2() {}
   private func privateFunc2() {}
   fileprivate func fileprivateFunc2() {}
   public func publicFunc2() {}
 }
 
+// An extension with more availability than BetweenTargets.
+// expected-error@+2 {{'BetweenTargets' is only available in}}
+@available(macOS 10.10, iOS 8.0, tvOS 9.0, watchOS 2.0, *)
+extension BetweenTargets {
+  public func publicFunc3() {}
+}
+
 // Same availability as BetweenTargets but internal instead of public.
 @available(macOS 10.14.5, iOS 12.3, tvOS 12.3, watchOS 5.3, *)
 internal struct BetweenTargetsInternal {}
@@ -669,7 +676,7 @@ public protocol PublicProto {}
 extension NoAvailable: PublicProto {}
 extension BeforeInliningTarget: PublicProto {}
 extension AtInliningTarget: PublicProto {}
-extension BetweenTargets: PublicProto {} // expected-error {{'BetweenTargets' is only available in}} expected-note {{add @available attribute to enclosing extension}}
+extension BetweenTargets: PublicProto {} // expected-warning {{'BetweenTargets' is only available in}} expected-note {{add @available attribute to enclosing extension}}
 extension AtDeploymentTarget: PublicProto {} // expected-error {{'AtDeploymentTarget' is only available in}} expected-note {{add @available attribute to enclosing extension}}
 extension AfterDeploymentTarget: PublicProto {} // expected-error {{'AfterDeploymentTarget' is only available in}} expected-note {{add @available attribute to enclosing extension}}
 
