Merge pull request #79858 from tshortli/spi-available-attr-diagnostics

Sema: Diagnose `@_spi_available` on declarations that cannot be unavailable

Author: tshortli

lib/Sema/TypeCheckAttr.cpp

@@ -2338,6 +2338,13 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *parsedAttr) {
       diagnoseAndRemoveAttr(parsedAttr, diag::spi_available_malformed);
       return;
     }
+
+    if (auto diag =
+            TypeChecker::diagnosticIfDeclCannotBeUnavailable(D, *attr)) {
+      diagnoseAndRemoveAttr(const_cast<AvailableAttr *>(attr->getParsedAttr()),
+                            *diag);
+      return;
+    }
   }
 
   if (attr->isNoAsync()) {
@@ -5096,24 +5103,11 @@ void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
   // declaration that is allowed to be unavailable, diagnose.
   if (availabilityConstraint->isUnavailable()) {
     auto attr = availabilityConstraint->getAttr();
-    if (auto diag = TypeChecker::diagnosticIfDeclCannotBeUnavailable(D)) {
-      diagnose(attr.getParsedAttr()->getLocation(), diag.value());
+    if (auto diag = TypeChecker::diagnosticIfDeclCannotBeUnavailable(D, attr)) {
+      diagnoseAndRemoveAttr(const_cast<AvailableAttr *>(attr.getParsedAttr()),
+                            *diag);
       return;
     }
-
-    if (auto *PD = dyn_cast<ProtocolDecl>(D->getDeclContext())) {
-      if (auto *VD = dyn_cast<ValueDecl>(D)) {
-        if (VD->isProtocolRequirement() && !PD->isObjC()) {
-          diagnoseAndRemoveAttr(
-              const_cast<AvailableAttr *>(attr.getParsedAttr()),
-              diag::unavailable_method_non_objc_protocol)
-              .warnInSwiftInterface(D->getDeclContext());
-          // Be lenient in interfaces to accomodate @_spi_available, which has
-          // been accepted historically.
-          return;
-        }
-      }
-    }
   }
 
   // If the decl is potentially unavailable relative to its parent and it's
@@ -5456,7 +5450,8 @@ TypeChecker::diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D) {
 }
 
 std::optional<Diagnostic>
-TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
+TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D,
+                                                 SemanticAvailableAttr attr) {
   auto parentIsUnavailable = [](const Decl *D) -> bool {
     if (auto *parent =
             AvailabilityInference::parentDeclForInferredAvailability(D)) {
@@ -5484,14 +5479,35 @@ TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
       return Diagnostic(diag::availability_decl_no_unavailable, D);
   }
 
+  auto DC = D->getDeclContext();
+  if (auto *PD = dyn_cast<ProtocolDecl>(DC)) {
+    if (auto *VD = dyn_cast<ValueDecl>(D)) {
+      if (VD->isProtocolRequirement() && !PD->isObjC()) {
+        auto diag = Diagnostic(diag::unavailable_method_non_objc_protocol);
+
+        // Be lenient in interfaces to accomodate @_spi_available, which has
+        // been accepted historically.
+        if (attr.isSPI() || DC->isInSwiftinterface())
+          diag.setBehaviorLimit(DiagnosticBehavior::Warning);
+        return diag;
+      }
+    }
+  }
+
   if (auto *VD = dyn_cast<VarDecl>(D)) {
     if (!VD->hasStorageOrWrapsStorage())
       return std::nullopt;
 
     if (parentIsUnavailable(D))
       return std::nullopt;
 
-    // Be lenient in interfaces to accomodate @_spi_available.
+    // @_spi_available does not make the declaration unavailable from the
+    // perspective of the owning module, which is what matters.
+    if (attr.isSPI())
+      return std::nullopt;
+
+    // An unavailable property with storage encountered in a swiftinterface
+    // might have been declared @_spi_available in source.
     if (D->getDeclContext()->isInSwiftinterface())
       return std::nullopt;
 

lib/Sema/TypeChecker.h

@@ -20,6 +20,7 @@
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/AccessScope.h"
 #include "swift/AST/AnyFunctionRef.h"
+#include "swift/AST/Attr.h"
 #include "swift/AST/AvailabilityRange.h"
 #include "swift/AST/AvailabilityScope.h"
 #include "swift/AST/DiagnosticsSema.h"
@@ -1065,7 +1066,8 @@ diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D);
 /// Returns a diagnostic indicating why the declaration cannot be annotated
 /// with an @available() attribute indicating it is unavailable or None if this
 /// is allowed.
-std::optional<Diagnostic> diagnosticIfDeclCannotBeUnavailable(const Decl *D);
+std::optional<Diagnostic>
+diagnosticIfDeclCannotBeUnavailable(const Decl *D, SemanticAvailableAttr attr);
 
 /// Checks whether the required range of versions of the compilation's target
 /// platform are available at the given `SourceRange`. If not, `Diagnose` is

test/Sema/availability_accessors.swift

@@ -569,5 +569,4 @@ struct BadAccessorAvailability<T> {
     @available(swift, introduced: 99) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
     didSet { }
   }
-
 }

test/Sema/availability_deinit.swift

@@ -31,6 +31,11 @@ class DeinitUnavailableMacOS {
   deinit {}
 }
 
+class DeinitSPIAvailableMacOS {
+  @_spi_available(macOS, introduced: 50) // expected-error {{deinitializer cannot be marked unavailable with '@available'}}
+  deinit {}
+}
+
 class AvailableAtDeploymentTargetDeinit {
   @available(macOS 50, *)
   deinit {}

test/Sema/availability_script_mode.swift

@@ -11,10 +11,10 @@ struct Available50 {}
 struct Available51 {}
 
 @available(macOS, unavailable)
-struct UnavailableOnMacOS {}
+struct UnavailableOnMacOS {} // expected-note {{'UnavailableOnMacOS' has been explicitly marked unavailable here}}
 
 @available(*, unavailable)
-struct UnavailableUnconditionally {}
+struct UnavailableUnconditionally {} // expected-note {{'UnavailableUnconditionally' has been explicitly marked unavailable here}}
 
 var alwaysAvailableVar: AlwaysAvailable = .init() // Ok
 
@@ -28,7 +28,7 @@ var availableOn50Var: Available50 = .init() // Ok
 var potentiallyUnavailableVar: Available51 = .init()
 
 @available(macOS, unavailable) // expected-error {{global variable cannot be marked unavailable with '@available' in script mode}}
-var unavailableOnMacOSVar: UnavailableOnMacOS = .init()
+var unavailableOnMacOSVar: UnavailableOnMacOS = .init() // expected-error {{'UnavailableOnMacOS' is unavailable in macOS}}
 
 @available(*, unavailable) // expected-error {{global variable cannot be marked unavailable with '@available' in script mode}}
-var unconditionallyUnavailableVar: UnavailableUnconditionally = .init()
+var unconditionallyUnavailableVar: UnavailableUnconditionally = .init() // expected-error {{'UnavailableUnconditionally' is unavailable}}

test/Sema/availability_stored_unavailable.swift

@@ -3,15 +3,23 @@
 // REQUIRES: OS=macosx
 
 @available(macOS, unavailable)
-struct UnavailableMacOSStruct {}
+struct UnavailableMacOSStruct {} // expected-note 2 {{'UnavailableMacOSStruct' has been explicitly marked unavailable here}}
+
+@available(iOS, introduced: 8.0)
+@_spi_available(macOS, introduced: 10.9)
+public struct SPIAvailableMacOSStruct {}
 
 @available(*, unavailable)
-public struct UniversallyUnavailableStruct {}
+public struct UniversallyUnavailableStruct {} // expected-note {{'UniversallyUnavailableStruct' has been explicitly marked unavailable here}}
 
 // Ok, initialization of globals is lazy and boxed.
 @available(macOS, unavailable)
 var unavailableMacOSGlobal: UnavailableMacOSStruct = .init()
 
+@available(iOS, introduced: 8.0)
+@_spi_available(macOS, introduced: 10.9)
+var spiAvailableMacOSGlobal: SPIAvailableMacOSStruct = .init()
+
 @available(*, unavailable)
 var universallyUnavailableGlobal: UniversallyUnavailableStruct = .init()
 
@@ -22,6 +30,10 @@ struct GoodAvailableStruct {
     get { UnavailableMacOSStruct() }
   }
 
+  // Ok, storage is available to implementation at runtime.
+  @_spi_available(macOS, introduced: 10.9)
+  var spiAvailableMacOS: SPIAvailableMacOSStruct
+
   // Ok, initialization of static vars is lazy and boxed.
   @available(macOS, unavailable)
   static var staticUnavailableMacOS: UnavailableMacOSStruct = .init()
@@ -61,18 +73,21 @@ struct GoodUniversallyUnavailableStruct {
 struct BadStruct {
   // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
   @available(macOS, unavailable)
-  var unavailableMacOS: UnavailableMacOSStruct = .init()
+  var unavailableMacOS: UnavailableMacOSStruct = .init() // expected-error {{'UnavailableMacOSStruct' is unavailable in macOS}}
 
   // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
   @available(macOS, unavailable)
-  lazy var lazyUnavailableMacOS: UnavailableMacOSStruct = .init()
+  lazy var lazyUnavailableMacOS: UnavailableMacOSStruct = .init() // expected-error {{'UnavailableMacOSStruct' is unavailable in macOS}}
 
   // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
   @available(*, unavailable)
-  var universallyUnavailable: UniversallyUnavailableStruct = .init()
+  var universallyUnavailable: UniversallyUnavailableStruct = .init() // expected-error {{'UniversallyUnavailableStruct' is unavailable}}
 }
 
 enum GoodAvailableEnum {
   @available(macOS, unavailable)
   case unavailableMacOS(UnavailableMacOSStruct)
+
+  @_spi_available(macOS, introduced: 10.9)
+  case spiAvailableMacOS(SPIAvailableMacOSStruct)
 }

test/Sema/availability_versions.swift

@@ -1934,4 +1934,13 @@ struct PropertyObservers {
     @available(macOS, obsoleted: 10.9) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
     didSet { }
   }
+
+  var hasSPIAvailableObservers: Int {
+    @_spi_available(macOS, introduced: 10.9) // expected-error {{willSet observer for property cannot be marked unavailable with '@available'}}
+    willSet { }
+
+    @_spi_available(macOS, introduced: 10.9) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
+    didSet { }
+  }
+
 }

test/decl/protocol/req/unavailable.swift

@@ -15,8 +15,13 @@ class Foo : Proto {
 // Reject protocols with 'unavailable' requirements
 // if a protocol is not marked @objc.
 protocol NonObjCProto {
-  @available(*,unavailable) func bad() // expected-error {{protocol members can only be marked unavailable in an @objc protocol}} expected-note {{protocol requires function 'bad()'}}
+  @available(*,unavailable) // expected-error {{protocol members can only be marked unavailable in an @objc protocol}}
+  func bad() // expected-note {{protocol requires function 'bad()'}}
+
   func good()
+
+  @_spi_available(macOS, introduced: 10.9) // expected-warning {{protocol members can only be marked unavailable in an @objc protocol}}
+  func kindaBad() // expected-note {{protocol requires function 'kindaBad()'}}
 }
 
 class Bar : NonObjCProto { // expected-error {{type 'Bar' does not conform to protocol 'NonObjCProto'}} expected-note {{add stubs for conformance}}
@@ -115,4 +120,7 @@ protocol UnavailableAssoc {
 
   @available(swift, obsoleted: 99)
   associatedtype A5
+
+  @_spi_available(macOS, introduced: 11) // expected-error {{associated type cannot be marked unavailable with '@available'}}
+  associatedtype A6
 }