Merge pull request swiftlang#58654 from tshortli/warn-for-extension-to-potentially-unavailable-type

tshortli · web-flow · commit 6bb7eea90af9 · 2022-05-04T13:56:53.000-07:00
Sema: Warn about potential unavailability of extended type in resilient library API
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}}
 
