Merge pull request #37350 from artemcm/DeployTargetIndependentAvailabilityDiagnostic

artemcm · web-flow · commit 364f154fff5f · 2021-05-12T15:39:45.000-07:00
Warn about unnecessary availability independently from the deployment target
diff --git a/include/swift/AST/TypeRefinementContext.h b/include/swift/AST/TypeRefinementContext.h
@@ -154,13 +154,21 @@ class TypeRefinementContext {
 
   SourceRange SrcRange;
 
+  /// A canonical availiability info for this context, computed top-down from the root
+  /// context (compilation deployment target).
   AvailabilityContext AvailabilityInfo;
 
+  /// If this context was annotated with an availability attribute, this property captures that.
+  /// It differs from the above `AvailabilityInfo` by being independent of the deployment target,
+  /// and is used for providing availability attribute redundancy warning diagnostics.
+  AvailabilityContext ExplicitAvailabilityInfo;
+
   std::vector<TypeRefinementContext *> Children;
 
   TypeRefinementContext(ASTContext &Ctx, IntroNode Node,
                         TypeRefinementContext *Parent, SourceRange SrcRange,
-                        const AvailabilityContext &Info);
+                        const AvailabilityContext &Info,
+                        const AvailabilityContext &ExplicitInfo);
 
 public:
   
@@ -172,6 +180,7 @@ class TypeRefinementContext {
   static TypeRefinementContext *createForDecl(ASTContext &Ctx, Decl *D,
                                               TypeRefinementContext *Parent,
                                               const AvailabilityContext &Info,
+                                              const AvailabilityContext &ExplicitInfo,
                                               SourceRange SrcRange);
   
   /// Create a refinement context for the Then branch of the given IfStmt.
@@ -245,6 +254,12 @@ class TypeRefinementContext {
     return AvailabilityInfo;
   }
 
+  /// Returns the information on what availability was specified by the programmer
+  /// on this context (if any).
+  const AvailabilityContext &getExplicitAvailabilityInfo() const {
+    return ExplicitAvailabilityInfo;
+  }
+
   /// Adds a child refinement context.
   void addChild(TypeRefinementContext *Child) {
     assert(Child->getSourceRange().isValid());
diff --git a/lib/AST/TypeRefinementContext.cpp b/lib/AST/TypeRefinementContext.cpp
@@ -28,8 +28,10 @@ using namespace swift;
 TypeRefinementContext::TypeRefinementContext(ASTContext &Ctx, IntroNode Node,
                                              TypeRefinementContext *Parent,
                                              SourceRange SrcRange,
-                                             const AvailabilityContext &Info)
-    : Node(Node), SrcRange(SrcRange), AvailabilityInfo(Info) {
+                                             const AvailabilityContext &Info,
+                                             const AvailabilityContext &ExplicitInfo)
+    : Node(Node), SrcRange(SrcRange),
+      AvailabilityInfo(Info), ExplicitAvailabilityInfo(ExplicitInfo) {
   if (Parent) {
     assert(SrcRange.isValid());
     Parent->addChild(this);
@@ -46,18 +48,20 @@ TypeRefinementContext::createRoot(SourceFile *SF,
   ASTContext &Ctx = SF->getASTContext();
   return new (Ctx)
       TypeRefinementContext(Ctx, SF,
-                            /*Parent=*/nullptr, SourceRange(), Info);
+                            /*Parent=*/nullptr, SourceRange(),
+                            Info, AvailabilityContext::alwaysAvailable());
 }
 
 TypeRefinementContext *
 TypeRefinementContext::createForDecl(ASTContext &Ctx, Decl *D,
                                      TypeRefinementContext *Parent,
                                      const AvailabilityContext &Info,
+                                     const AvailabilityContext &ExplicitInfo,
                                      SourceRange SrcRange) {
   assert(D);
   assert(Parent);
   return new (Ctx)
-      TypeRefinementContext(Ctx, D, Parent, SrcRange, Info);
+      TypeRefinementContext(Ctx, D, Parent, SrcRange, Info, ExplicitInfo);
 }
 
 TypeRefinementContext *
@@ -68,7 +72,8 @@ TypeRefinementContext::createForIfStmtThen(ASTContext &Ctx, IfStmt *S,
   assert(Parent);
   return new (Ctx)
       TypeRefinementContext(Ctx, IntroNode(S, /*IsThen=*/true), Parent,
-                            S->getThenStmt()->getSourceRange(), Info);
+                            S->getThenStmt()->getSourceRange(),
+                            Info, /* ExplicitInfo */Info);
 }
 
 TypeRefinementContext *
@@ -79,7 +84,8 @@ TypeRefinementContext::createForIfStmtElse(ASTContext &Ctx, IfStmt *S,
   assert(Parent);
   return new (Ctx)
       TypeRefinementContext(Ctx, IntroNode(S, /*IsThen=*/false), Parent,
-                            S->getElseStmt()->getSourceRange(), Info);
+                            S->getElseStmt()->getSourceRange(),
+                            Info, /* ExplicitInfo */Info);
 }
 
 TypeRefinementContext *
@@ -92,7 +98,7 @@ TypeRefinementContext::createForConditionFollowingQuery(ASTContext &Ctx,
   assert(Parent);
   SourceRange Range(PAI->getEndLoc(), LastElement.getEndLoc());
   return new (Ctx) TypeRefinementContext(Ctx, PAI, Parent, Range,
-                                         Info);
+                                         Info, /* ExplicitInfo */Info);
 }
 
 TypeRefinementContext *
@@ -107,7 +113,7 @@ TypeRefinementContext::createForGuardStmtFallthrough(ASTContext &Ctx,
   SourceRange Range(RS->getEndLoc(), ContainingBraceStmt->getEndLoc());
   return new (Ctx) TypeRefinementContext(Ctx,
                                          IntroNode(RS, /*IsFallthrough=*/true),
-                                         Parent, Range, Info);
+                                         Parent, Range, Info, /* ExplicitInfo */Info);
 }
 
 TypeRefinementContext *
@@ -118,7 +124,7 @@ TypeRefinementContext::createForGuardStmtElse(ASTContext &Ctx, GuardStmt *RS,
   assert(Parent);
   return new (Ctx)
       TypeRefinementContext(Ctx, IntroNode(RS, /*IsFallthrough=*/false), Parent,
-                            RS->getBody()->getSourceRange(), Info);
+                            RS->getBody()->getSourceRange(), Info, /* ExplicitInfo */Info);
 }
 
 TypeRefinementContext *
@@ -128,7 +134,7 @@ TypeRefinementContext::createForWhileStmtBody(ASTContext &Ctx, WhileStmt *S,
   assert(S);
   assert(Parent);
   return new (Ctx) TypeRefinementContext(
-      Ctx, S, Parent, S->getBody()->getSourceRange(), Info);
+      Ctx, S, Parent, S->getBody()->getSourceRange(), Info, /* ExplicitInfo */Info);
 }
 
 // Only allow allocation of TypeRefinementContext using the allocator in
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -400,13 +400,15 @@ class TypeRefinementContextBuilder : private ASTWalker {
     // The potential versions in the declaration are constrained by both
     // the declared availability of the declaration and the potential versions
     // of its lexical context.
-    AvailabilityContext DeclInfo =
+    AvailabilityContext ExplicitDeclInfo =
         swift::AvailabilityInference::availableRange(D, Context);
+    AvailabilityContext DeclInfo = ExplicitDeclInfo;
     DeclInfo.intersectWith(getCurrentTRC()->getAvailabilityInfo());
 
     TypeRefinementContext *NewTRC =
         TypeRefinementContext::createForDecl(Context, D, getCurrentTRC(),
                                              DeclInfo,
+                                             ExplicitDeclInfo,
                                              refinementSourceRangeForDecl(D));
     
     // Record the TRC for this storage declaration so that
@@ -646,6 +648,8 @@ class TypeRefinementContextBuilder : private ASTWalker {
     for (StmtConditionElement Element : Cond) {
       TypeRefinementContext *CurrentTRC = getCurrentTRC();
       AvailabilityContext CurrentInfo = CurrentTRC->getAvailabilityInfo();
+      AvailabilityContext CurrentExplicitInfo =
+        CurrentTRC->getExplicitAvailabilityInfo();
 
       // If the element is not a condition, walk it in the current TRC.
       if (Element.getKind() != StmtConditionElement::CK_Availability) {
@@ -708,24 +712,17 @@ class TypeRefinementContextBuilder : private ASTWalker {
         continue;
       }
 
-
-      // If the version range for the current TRC 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.
-      if (CurrentInfo.isContainedIn(NewConstraint)) {
+      // If the explicitly-specified (via #availability) version range for the
+      // current TRC 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.
+      if (CurrentExplicitInfo.isContainedIn(NewConstraint)) {
         DiagnosticEngine &Diags = Context.Diags;
-        // Some availability checks will always pass because the minimum
-        // deployment target guarantees they will never be false. We don't
-        // diagnose these checks as useless because the source file may
-        // be shared with other projects/targets having older deployment
-        // targets. We don't currently have a mechanism for the user to
-        // suppress these warnings (for example, by indicating when the
-        // required compatibility version is different than the deployment
-        // target).
         if (CurrentTRC->getReason() != TypeRefinementContext::Reason::Root) {
           PlatformKind BestPlatform = targetPlatform(Context.LangOpts);
           auto *PlatformSpec =
               dyn_cast<PlatformVersionConstraintAvailabilitySpec>(Spec);
+
           // 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
@@ -738,7 +735,9 @@ class TypeRefinementContextBuilder : private ASTWalker {
           Diags.diagnose(CurrentTRC->getIntroductionLoc(),
                          diag::availability_query_useless_enclosing_scope_here);
         }
+      }
 
+      if (CurrentInfo.isContainedIn(NewConstraint)) {
         // No need to actually create the refinement context if we know it is
         // useless.
         continue;
diff --git a/test/Sema/availability_implicitly_unnecessary.swift b/test/Sema/availability_implicitly_unnecessary.swift
@@ -0,0 +1,9 @@
+// RUN: %target-typecheck-verify-swift -verify -target %target-cpu-apple-macosx11.2 -disable-objc-attr-requires-foundation-module
+// REQUIRES: OS=macosx
+
+@available(macOS 11.0, *)
+class Foo {
+    func foo() {
+        if #available(macOS 11.1, *) {}
+    }
+}
diff --git a/test/Sema/availability_unnecessary.swift b/test/Sema/availability_unnecessary.swift
@@ -0,0 +1,14 @@
+// Ensure that the `unnecessary check` availability warning is emitted when unnecessary due to
+// scope's explicit annotation
+
+// RUN: %target-typecheck-verify-swift -verify -target %target-cpu-apple-macosx11.2 -disable-objc-attr-requires-foundation-module
+// REQUIRES: OS=macosx
+
+@available(macOS 11.1, *)
+class Foo {
+    // expected-note@-1 {{enclosing scope here}}
+    func foo() {
+        // expected-warning@+1 {{unnecessary check for 'macOS'; enclosing scope ensures guard will always be true}}
+        if #available(macOS 11.0, *) {}
+    }
+}
