AST: Emit warnings for redundant availability checks in custom domains.

Author: tshortli

include/swift/AST/AvailabilityConstraint.h

@@ -188,12 +188,21 @@ enum class AvailabilityConstraintFlag : uint8_t {
 };
 using AvailabilityConstraintFlags = OptionSet<AvailabilityConstraintFlag>;
 
-/// Returns the set of availability constraints that restrict use of \p decl
+/// Returns the set of availability constraints that restricts use of \p decl
 /// when it is referenced from the given context. In other words, it is the
-/// collection of of `@available` attributes with unsatisfied conditions.
+/// collection of `@available` attributes with unsatisfied conditions.
 DeclAvailabilityConstraints getAvailabilityConstraintsForDecl(
     const Decl *decl, const AvailabilityContext &context,
     AvailabilityConstraintFlags flags = std::nullopt);
+
+/// Returns the availability constraints that restricts use of \p decl
+/// in \p domain when it is referenced from the given context. In other words,
+/// it is the unsatisfied `@available` attribute  that applies to \p domain in
+/// the given context.
+std::optional<AvailabilityConstraint> getAvailabilityConstraintForDeclInDomain(
+    const Decl *decl, const AvailabilityContext &context,
+    AvailabilityDomain domain,
+    AvailabilityConstraintFlags flags = std::nullopt);
 } // end namespace swift
 
 namespace llvm {

include/swift/AST/AvailabilityScope.h

@@ -272,8 +272,11 @@ class AvailabilityScope : public ASTAllocated<AvailabilityScope> {
       AvailabilityDomain Domain, const llvm::VersionTuple &Version) const;
 
   /// Returns the availability version range that was explicitly written in
-  /// source, if applicable. Otherwise, returns null.
-  std::optional<const AvailabilityRange> getExplicitAvailabilityRange() const;
+  /// source for the given domain, if applicable. Otherwise, returns
+  /// `std::nullopt`.
+  std::optional<const AvailabilityRange>
+  getExplicitAvailabilityRange(AvailabilityDomain Domain,
+                               ASTContext &Ctx) const;
 
   /// Returns the source range this scope represents.
   SourceRange getSourceRange() const { return SrcRange; }

lib/AST/AvailabilityConstraint.cpp

@@ -331,3 +331,15 @@ swift::getAvailabilityConstraintsForDecl(const Decl *decl,
   return constraints;
 }
 
+std::optional<AvailabilityConstraint>
+swift::getAvailabilityConstraintForDeclInDomain(
+    const Decl *decl, const AvailabilityContext &context,
+    AvailabilityDomain domain, AvailabilityConstraintFlags flags) {
+  auto constraints = getAvailabilityConstraintsForDecl(decl, context, flags);
+  for (auto const &constraint : constraints) {
+    if (constraint.getDomain().isRelated(domain))
+      return constraint;
+  }
+
+  return std::nullopt;
+}

lib/AST/AvailabilityScope.cpp

@@ -17,6 +17,7 @@
 #include "swift/AST/AvailabilityScope.h"
 
 #include "swift/AST/ASTContext.h"
+#include "swift/AST/AvailabilityConstraint.h"
 #include "swift/AST/AvailabilityInference.h"
 #include "swift/AST/AvailabilitySpec.h"
 #include "swift/AST/Decl.h"
@@ -356,15 +357,17 @@ SourceRange AvailabilityScope::getAvailabilityConditionVersionSourceRange(
 }
 
 std::optional<const AvailabilityRange>
-AvailabilityScope::getExplicitAvailabilityRange() const {
+AvailabilityScope::getExplicitAvailabilityRange(AvailabilityDomain domain,
+                                                ASTContext &ctx) const {
   switch (getReason()) {
   case Reason::Root:
     return std::nullopt;
 
   case Reason::Decl: {
     auto decl = Node.getAsDecl();
-    if (auto attr = decl->getAvailableAttrForPlatformIntroduction())
-      return attr->getIntroducedRange(decl->getASTContext());
+    if (auto constraint = swift::getAvailabilityConstraintForDeclInDomain(
+            decl, AvailabilityContext::forAlwaysAvailable(ctx), domain))
+      return constraint->getAttr().getIntroducedRange(ctx);
 
     return std::nullopt;
   }
@@ -379,22 +382,12 @@ AvailabilityScope::getExplicitAvailabilityRange() const {
   case Reason::GuardStmtElseBranch:
   case Reason::WhileStmtBody:
     // Availability is inherently explicit for all of these nodes.
-    return getPlatformAvailabilityRange();
+    return getAvailabilityContext().getAvailabilityRange(domain, ctx);
   }
 
   llvm_unreachable("Unhandled Reason in switch.");
 }
 
-static std::string
-stringForAvailability(const AvailabilityRange &availability) {
-  if (availability.isAlwaysAvailable())
-    return "all";
-  if (availability.isKnownUnreachable())
-    return "none";
-
-  return availability.getVersionString();
-}
-
 void AvailabilityScope::print(raw_ostream &OS, SourceManager &SrcMgr,
                               unsigned Indent) const {
   OS.indent(Indent);
@@ -446,9 +439,6 @@ void AvailabilityScope::print(raw_ostream &OS, SourceManager &SrcMgr,
     }
   }
 
-  if (auto explicitAvailability = getExplicitAvailabilityRange())
-    OS << " explicit_version=" << stringForAvailability(*explicitAvailability);
-
   for (AvailabilityScope *Child : Children) {
     OS << '\n';
     Child->print(OS, SrcMgr, Indent + 2);

lib/AST/AvailabilityScopeBuilder.cpp

@@ -1077,9 +1077,9 @@ class AvailabilityScopeBuilder : private ASTWalker {
         // current scope is completely contained in the range for the spec, then
         // a version query can never be false, so the spec is useless.
         // If so, report this.
-        // FIXME: [availability] Diagnose non-platform queries as useless too.
-        auto explicitRange = currentScope->getExplicitAvailabilityRange();
-        if (domain.isPlatform() && explicitRange && trueRange &&
+        auto explicitRange =
+            currentScope->getExplicitAvailabilityRange(domain, Context);
+        if (explicitRange && trueRange &&
             explicitRange->isContainedIn(*trueRange)) {
           // Platform unavailability queries never refine availability so don't
           // diangose them.
@@ -1088,17 +1088,9 @@ class AvailabilityScopeBuilder : private ASTWalker {
 
           DiagnosticEngine &diags = Context.Diags;
           if (currentScope->getReason() != AvailabilityScope::Reason::Root) {
-            PlatformKind bestPlatform = targetPlatform(Context.LangOpts);
-
-            // If possible, try to report the diagnostic in terms for the
-            // platform the user uttered in the '#available()'. For a platform
-            // that inherits availability from another platform it may be
-            // different from the platform specified in the target triple.
-            if (domain.getPlatformKind() != PlatformKind::none)
-              bestPlatform = domain.getPlatformKind();
             diags.diagnose(query->getLoc(),
                            diag::availability_query_useless_enclosing_scope,
-                           platformString(bestPlatform));
+                           domain.getNameForAttributePrinting());
             diags.diagnose(
                 currentScope->getIntroductionLoc(),
                 diag::availability_query_useless_enclosing_scope_here);

lib/Sema/TypeCheckAvailability.cpp

@@ -685,7 +685,8 @@ static bool fixAvailabilityByNarrowingNearbyVersionCheck(
     return false;
 
   // FIXME: [availability] Support fixing availability for versionless domains.
-  auto ExplicitAvailability = scope->getExplicitAvailabilityRange();
+  auto ExplicitAvailability =
+      scope->getExplicitAvailabilityRange(Domain, Context);
   if (ExplicitAvailability && !RequiredAvailability.isAlwaysAvailable() &&
       scope->getReason() != AvailabilityScope::Reason::Root &&
       RequiredAvailability.isContainedIn(*ExplicitAvailability)) {

test/Availability/availability_custom_domains.swift

@@ -41,7 +41,7 @@ func testDeployment() {
 }
 
 func testIfAvailable(_ truthy: Bool) {
-  if #available(EnabledDomain) {
+  if #available(EnabledDomain) { // expected-note {{enclosing scope here}}
     availableInEnabledDomain()
     unavailableInEnabledDomain() // expected-error {{'unavailableInEnabledDomain()' is unavailable}}
     availableInDynamicDomain() // expected-error {{'availableInDynamicDomain()' is only available in DynamicDomain}}
@@ -59,7 +59,9 @@ func testIfAvailable(_ truthy: Bool) {
       unavailableInDynamicDomain()
     }
 
-    if #unavailable(EnabledDomain) {
+    if #available(EnabledDomain) {} // expected-warning {{unnecessary check for 'EnabledDomain'; enclosing scope ensures guard will always be true}}
+
+    if #unavailable(EnabledDomain) { // FIXME: [availability] Diagnose as unreachable
       // Unreachable.
       availableInEnabledDomain()
       unavailableInEnabledDomain() // expected-error {{'unavailableInEnabledDomain()' is unavailable}}
@@ -112,19 +114,25 @@ func testIfAvailable(_ truthy: Bool) {
 }
 
 func testWhileAvailable() {
-  while #available(EnabledDomain) {
+  while #available(EnabledDomain) { // expected-note {{enclosing scope here}}
     availableInEnabledDomain()
     unavailableInEnabledDomain() // expected-error {{'unavailableInEnabledDomain()' is unavailable}}
+
+    if #available(EnabledDomain) {} // expected-warning {{unnecessary check for 'EnabledDomain'; enclosing scope ensures guard will always be true}}
+    if #unavailable(EnabledDomain) {} // FIXME: [availability] Diagnose as unreachable
   }
 
   while #unavailable(EnabledDomain) {
     availableInEnabledDomain() // expected-error {{'availableInEnabledDomain()' is only available in EnabledDomain}}
     unavailableInEnabledDomain()
+
+    if #available(EnabledDomain) {} // FIXME: [availability] Diagnose as unreachable
+    if #unavailable(EnabledDomain) {} // FIXME: [availability] Diagnose as redundant
   }
 }
 
 func testGuardAvailable() {
-  guard #available(EnabledDomain) else {
+  guard #available(EnabledDomain) else { // expected-note {{enclosing scope here}}
     availableInEnabledDomain() // expected-error {{'availableInEnabledDomain()' is only available in EnabledDomain}}
     unavailableInEnabledDomain()
     availableInDynamicDomain() // expected-error {{'availableInDynamicDomain()' is only available in DynamicDomain}}
@@ -135,14 +143,17 @@ func testGuardAvailable() {
   availableInEnabledDomain()
   unavailableInEnabledDomain() // expected-error {{'unavailableInEnabledDomain()' is unavailable}}
   availableInDynamicDomain() // expected-error {{'availableInDynamicDomain()' is only available in DynamicDomain}}
+
+  if #available(EnabledDomain) {} // expected-warning {{unnecessary check for 'EnabledDomain'; enclosing scope ensures guard will always be true}}
+  if #unavailable(EnabledDomain) {} // FIXME: [availability] Diagnose as unreachable
 }
 
 @available(EnabledDomain)
-func testEnabledDomainAvailable() {
+func testEnabledDomainAvailable() { // expected-note {{enclosing scope here}}
   availableInEnabledDomain()
   unavailableInEnabledDomain() // expected-error {{'unavailableInEnabledDomain()' is unavailable}}
 
-  if #available(EnabledDomain) {} // FIXME: [availability] Diagnose as redundant
+  if #available(EnabledDomain) {} // expected-warning {{unnecessary check for 'EnabledDomain'; enclosing scope ensures guard will always be true}}
   if #unavailable(EnabledDomain) {} // FIXME: [availability] Diagnose as unreachable
 
   alwaysAvailable()