Diagnostics Sema: add fix-it about narrowing nearby version check

Before, this fix-it was attached to the error. This resulted in error messages like "'foo' is only available in macOS 10.12.2 or newer" with a fix-it like "replace ''10.12.1' with '10.12.2'". There was no clear information to the user why this fix-it was suggested, and it was not even clear what it would do, because the change would almost always be in a different line than the error.

Now, the fix-it will appear as a separate note. This way, it can have its own (hopefully more helpful) description. It will now also co-exist with other fix-it options, like "add 'if #available' version check" and "add @available attribute to enclosing global function".

Unit tests have been updated accordingly.

Author: cbjeukendrup

include/swift/AST/DiagnosticsSema.def

@@ -5423,6 +5423,10 @@ FIXIT(insert_available_attr,
       "@available(%0 %1, *)\n%2",
       (StringRef, StringRef, StringRef))
 
+NOTE(availability_narrow_nearby_version_check, none,
+     "narrow nearby version check from '%0' to '%1'",
+     (StringRef, StringRef))
+
 ERROR(availability_accessor_only_version_newer, none,
       "%select{getter|setter}0 for %1 is only available in %2 %3"
       " or newer",

lib/Sema/TypeCheckAvailability.cpp

@@ -1523,11 +1523,8 @@ static void fixAvailabilityForDecl(SourceRange ReferenceRange, const Decl *D,
 /// version), emit a diagnostic and fixit that narrows the existing TRC
 /// condition to the required range.
 static bool fixAvailabilityByNarrowingNearbyVersionCheck(
-    SourceRange ReferenceRange,
-    const DeclContext *ReferenceDC,
-    const VersionRange &RequiredRange,
-    ASTContext &Context,
-    InFlightDiagnostic &Err) {
+    SourceRange ReferenceRange, const DeclContext *ReferenceDC,
+    const VersionRange &RequiredRange, ASTContext &Context) {
   const TypeRefinementContext *TRC = nullptr;
   AvailabilityContext RunningOSOverApprox =
     TypeChecker::overApproximateAvailabilityAtLocation(ReferenceRange.Start,
@@ -1559,8 +1556,13 @@ static bool fixAvailabilityByNarrowingNearbyVersionCheck(
       Platform, RunningVers);
     if (!FixRange.isValid())
       return false;
+
     // Have found a nontrivial type refinement context-introducer to narrow.
-    Err.fixItReplace(FixRange, RequiredVers.getAsString());
+    Context.Diags
+        .diagnose(FixRange.Start,
+                  diag::availability_narrow_nearby_version_check,
+                  RunningVers.getAsString(), RequiredVers.getAsString())
+        .fixItReplace(FixRange, RequiredVers.getAsString());
     return true;
   }
   return false;
@@ -1625,14 +1627,17 @@ static void fixAvailabilityByAddingVersionCheck(
 }
 
 /// Emit suggested Fix-Its for a reference with to an unavailable symbol
-/// requiting the given OS version range.
+/// requiring the given OS version range.
 static void fixAvailability(SourceRange ReferenceRange,
                             const DeclContext *ReferenceDC,
                             const VersionRange &RequiredRange,
                             ASTContext &Context) {
   if (ReferenceRange.isInvalid())
     return;
 
+  fixAvailabilityByNarrowingNearbyVersionCheck(ReferenceRange, ReferenceDC,
+                                               RequiredRange, Context);
+
   Optional<ASTNode> NodeToWrapInVersionCheck;
   const Decl *FoundMemberDecl = nullptr;
   const Decl *FoundTypeLevelDecl = nullptr;
@@ -1665,19 +1670,12 @@ void TypeChecker::diagnosePotentialOpaqueTypeUnavailability(
   ASTContext &Context = ReferenceDC->getASTContext();
 
   auto RequiredRange = Reason.getRequiredOSVersionRange();
-  {
-    auto Err =
-      Context.Diags.diagnose(
-               ReferenceRange.Start, diag::availability_opaque_types_only_version_newer,
-               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;
-  }
+
+  Context.Diags.diagnose(ReferenceRange.Start,
+                         diag::availability_opaque_types_only_version_newer,
+                         prettyPlatformString(targetPlatform(Context.LangOpts)),
+                         RequiredRange.getLowerEndpoint());
+
   fixAvailability(ReferenceRange, ReferenceDC, RequiredRange, Context);
 }
 
@@ -1687,20 +1685,12 @@ static void diagnosePotentialConcurrencyUnavailability(
   ASTContext &Context = ReferenceDC->getASTContext();
 
   auto RequiredRange = Reason.getRequiredOSVersionRange();
-  {
-    auto Err =
-      Context.Diags.diagnose(
-          ReferenceRange.Start,
-          diag::availability_concurrency_only_version_newer,
-          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;
-  }
+
+  Context.Diags.diagnose(ReferenceRange.Start,
+                         diag::availability_concurrency_only_version_newer,
+                         prettyPlatformString(targetPlatform(Context.LangOpts)),
+                         RequiredRange.getLowerEndpoint());
+
   fixAvailability(ReferenceRange, ReferenceDC, RequiredRange, Context);
 }
 
@@ -1729,19 +1719,11 @@ void TypeChecker::diagnosePotentialUnavailability(
   ASTContext &Context = ReferenceDC->getASTContext();
 
   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());
-
-    // Direct a fixit to the error if an existing guard is nearly-correct
-    if (fixAvailabilityByNarrowingNearbyVersionCheck(ReferenceRange,
-                                                     ReferenceDC,
-                                                     RequiredRange, Context, Err))
-      return;
-  }
+
+  Context.Diags.diagnose(
+      ReferenceRange.Start, diag::availability_decl_only_version_newer,
+      D->getName(), prettyPlatformString(targetPlatform(Context.LangOpts)),
+      RequiredRange.getLowerEndpoint());
 
   fixAvailability(ReferenceRange, ReferenceDC, RequiredRange, Context);
 }
@@ -1757,25 +1739,15 @@ void TypeChecker::diagnosePotentialAccessorUnavailability(
   const AbstractStorageDecl *ASD = Accessor->getStorage();
   DeclName Name = ASD->getName();
 
+  auto RequiredRange = Reason.getRequiredOSVersionRange();
   auto &diag = ForInout ? diag::availability_inout_accessor_only_version_newer
                         : diag::availability_accessor_only_version_newer;
 
-  auto RequiredRange = Reason.getRequiredOSVersionRange();
-  {
-    auto Err =
-      Context.Diags.diagnose(
-               ReferenceRange.Start, diag,
-               static_cast<unsigned>(Accessor->getAccessorKind()), Name,
-               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;
-  }
+  Context.Diags.diagnose(ReferenceRange.Start, diag,
+                         static_cast<unsigned>(Accessor->getAccessorKind()),
+                         Name,
+                         prettyPlatformString(targetPlatform(Context.LangOpts)),
+                         RequiredRange.getLowerEndpoint());
 
   fixAvailability(ReferenceRange, ReferenceDC, RequiredRange, Context);
 }
@@ -1797,36 +1769,30 @@ behaviorLimitForExplicitUnavailability(
 }
 
 void TypeChecker::diagnosePotentialUnavailability(
-    const RootProtocolConformance *rootConf,
-    const ExtensionDecl *ext,
-    SourceLoc loc,
-    const DeclContext *dc,
-    const UnavailabilityReason &reason) {
-  ASTContext &ctx = dc->getASTContext();
+    const RootProtocolConformance *rootConf, const ExtensionDecl *ext,
+    SourceLoc loc, const DeclContext *ReferenceDC,
+    const UnavailabilityReason &Reason) {
+  ASTContext &Context = ReferenceDC->getASTContext();
 
-  auto requiredRange = reason.getRequiredOSVersionRange();
+  auto RequiredRange = Reason.getRequiredOSVersionRange();
   {
     auto type = rootConf->getType();
     auto proto = rootConf->getProtocol()->getDeclaredInterfaceType();
 
-    auto diagID = (ctx.LangOpts.EnableConformanceAvailabilityErrors
-                   ? diag::conformance_availability_only_version_newer
-                   : diag::conformance_availability_only_version_newer_warn);
-    auto behavior = behaviorLimitForExplicitUnavailability(rootConf, dc);
-    auto err =
-      ctx.Diags.diagnose(
-               loc, diagID,
-               type, proto, prettyPlatformString(targetPlatform(ctx.LangOpts)),
-               reason.getRequiredOSVersionRange().getLowerEndpoint());
+    const auto &diag =
+        (Context.LangOpts.EnableConformanceAvailabilityErrors
+             ? diag::conformance_availability_only_version_newer
+             : diag::conformance_availability_only_version_newer_warn);
+    auto behavior =
+        behaviorLimitForExplicitUnavailability(rootConf, ReferenceDC);
+    auto err = Context.Diags.diagnose(
+        loc, diag, type, proto,
+        prettyPlatformString(targetPlatform(Context.LangOpts)),
+        RequiredRange.getLowerEndpoint());
     err.limitBehavior(behavior);
-
-    // Direct a fixit to the error if an existing guard is nearly-correct
-    if (fixAvailabilityByNarrowingNearbyVersionCheck(loc, dc,
-                                                     requiredRange, ctx, err))
-      return;
   }
 
-  fixAvailability(loc, dc, requiredRange, ctx);
+  fixAvailability(loc, ReferenceDC, RequiredRange, Context);
 }
 
 const AvailableAttr *TypeChecker::getDeprecated(const Decl *D) {

test/Sema/availability_versions_multi.swift

@@ -23,7 +23,7 @@ let ignored2: Int = globalAvailableOn99_51 // expected-error {{'globalAvailableO
 let ignored3: Int = globalAvailableOn99_52 // expected-error {{'globalAvailableOn99_52' is only available in macOS 99.52 or newer}}
     // expected-note@-1 {{add @available attribute to enclosing let}}
 
-@available(OSX, introduced: 99.51)
+@available(OSX, introduced: 99.51) // expected-note 4 {{narrow nearby version check}} {{29-34=99.52}}
 func useFromOtherOn99_51() {
   // This will trigger validation of OtherIntroduced99_51 in
   // in availability_multi_other.swift
@@ -32,29 +32,33 @@ func useFromOtherOn99_51() {
 
   let o10_9 = OtherIntroduced10_9()
   o10_9.extensionMethodOnOtherIntroduced10_9AvailableOn99_51(o99_51)
-  _ = o99_51.returns99_52Introduced99_52() // expected-error {{'returns99_52Introduced99_52()' is only available in macOS 99.52 or newer}}
-      // expected-note@-1 {{add 'if #available' version check}}
+
+  _ = o99_51.returns99_52Introduced99_52()
+      // expected-error@-1 {{'returns99_52Introduced99_52()' is only available in macOS 99.52 or newer}}
+      // expected-note@-2 {{add 'if #available' version check}}
 
   _ = OtherIntroduced99_52()
       // expected-error@-1 {{'OtherIntroduced99_52' is only available in macOS 99.52 or newer}}
       // expected-note@-2 {{add 'if #available' version check}}
 
-  o99_51.extensionMethodOnOtherIntroduced99_51AvailableOn99_52() // expected-error {{'extensionMethodOnOtherIntroduced99_51AvailableOn99_52()' is only available in macOS 99.52 or newer}}
-      // expected-note@-1 {{add 'if #available' version check}}
+  o99_51.extensionMethodOnOtherIntroduced99_51AvailableOn99_52()
+      // expected-error@-1 {{'extensionMethodOnOtherIntroduced99_51AvailableOn99_52()' is only available in macOS 99.52 or newer}}
+      // expected-note@-2 {{add 'if #available' version check}}
 
   _ = OtherIntroduced99_51.NestedIntroduced99_52()
       // expected-error@-1 {{'NestedIntroduced99_52' is only available in macOS 99.52 or newer}}
       // expected-note@-2 {{add 'if #available' version check}}
 }
 
-@available(OSX, introduced: 99.52)
+@available(OSX, introduced: 99.52) // expected-note {{narrow nearby version check}} {{29-34=99.53}}
 func useFromOtherOn99_52() {
   _ = OtherIntroduced99_52()
 
   let n99_52 = OtherIntroduced99_51.NestedIntroduced99_52()
   _ = n99_52.returns99_52()
-  _ = n99_52.returns99_53() // expected-error {{'returns99_53()' is only available in macOS 99.53 or newer}}
-      // expected-note@-1 {{add 'if #available' version check}}
+  _ = n99_52.returns99_53()
+      // expected-error@-1 {{'returns99_53()' is only available in macOS 99.53 or newer}}
+      // expected-note@-2 {{add 'if #available' version check}}
 
   // This will trigger validation of the global in availability_in_multi_other.swift
   _ = globalFromOtherOn99_52

test/attr/attr_availability_narrow.swift

@@ -7,15 +7,17 @@ import Foundation
 @available(macOS 10.50.2, *)
 func foo() { }
 
-func useFoo() {
-  if #available(macOS 10.50.1, *) {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{23-30=10.50.2}}
+func useFoo() { // expected-note {{add @available attribute to enclosing global function}}
+  if #available(macOS 10.50.1, *) { // expected-note {{narrow nearby version check from '10.50.1' to '10.50.2'}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
-func useFooDifferentSpelling() {
-  if #available(OSX 10.50.1, *) {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{21-28=10.50.2}}
+func useFooDifferentSpelling() { // expected-note {{add @available attribute to enclosing global function}}
+  if #available(OSX 10.50.1, *) { // expected-note {{narrow nearby version check from '10.50.1' to '10.50.2'}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
@@ -25,39 +27,42 @@ func useFooAlreadyOkRange() {
   }
 }
 
-func useFooUnaffectedSimilarText() {
-  if #available(iOS 10.50.10, OSX 10.50.1, *) {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{35-42=10.50.2}}
+func useFooUnaffectedSimilarText() { // expected-note {{add @available attribute to enclosing global function}}
+  if #available(iOS 10.50.10, OSX 10.50.1, *) { // expected-note {{narrow nearby version check from '10.50.1' to '10.50.2'}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
-func useFooWayOff() {
-    // expected-note@-1{{add @available attribute to enclosing global function}}
+func useFooWayOff() { // expected-note {{add @available attribute to enclosing global function}}
   if #available(OSX 10.10, *) {
     foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
-    // expected-note@-1{{add 'if #available' version check}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
-@available(OSX 10.50, *)
+@available(OSX 10.50, *) // expected-note {{narrow nearby version check from '10.50' to '10.50.2'}}
 class FooUser {
-  func useFoo() {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{16-21=10.50.2}}
+  func useFoo() { // expected-note {{add @available attribute to enclosing instance method}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
-@available(OSX, introduced: 10.50, obsoleted: 10.50.4)
+@available(OSX, introduced: 10.50, obsoleted: 10.50.4) // expected-note {{narrow nearby version check from '10.50' to '10.50.2'}}
 class FooUser2 {
-  func useFoo() {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{29-34=10.50.2}}
+  func useFoo() { // expected-note {{add @available attribute to enclosing instance method}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 
-@available(OSX, introduced: 10.50, obsoleted: 10.50.4)
+@available(OSX, introduced: 10.50, obsoleted: 10.50.4) // expected-note {{narrow nearby version check from '10.50' to '10.50.2'}}
 @objc
 class FooUser3 : NSObject {
-  func useFoo() {
-    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}} {{29-34=10.50.2}}
+  func useFoo() { // expected-note {{add @available attribute to enclosing instance method}}
+    foo() // expected-error {{'foo()' is only available in macOS 10.50.2 or newer}}
+    // expected-note@-1 {{add 'if #available' version check}}
   }
 }
 

test/attr/attr_availability_tvos.swift

@@ -58,8 +58,9 @@ if #available(iOS 9.3, *) {
       // expected-note@-1 {{add 'if #available' version check}}
 }
 
-if #available(iOS 9.3, tvOS 9.1, *) {
-  functionIntroducedOntvOS9_2() // expected-error {{'functionIntroducedOntvOS9_2()' is only available in tvOS 9.2 or newer}} {{29-32=9.2}}
+if #available(iOS 9.3, tvOS 9.1, *) { // expected-note {{narrow nearby version check from '9.1' to '9.2'}}
+  functionIntroducedOntvOS9_2() // expected-error {{'functionIntroducedOntvOS9_2()' is only available in tvOS 9.2 or newer}}
+      // expected-note@-1 {{add 'if #available' version check}}
 }
 
 if #available(iOS 9.1, tvOS 9.2, *) {

test/attr/attr_availability_watchos.swift

@@ -58,8 +58,9 @@ if #available(iOS 9.3, *) {
       // expected-note@-1 {{add 'if #available' version check}}
 }
 
-if #available(iOS 9.3, watchOS 2.1, *) {
-  functionIntroducedOnwatchOS2_2() // expected-error {{'functionIntroducedOnwatchOS2_2()' is only available in watchOS 2.2 or newer}} {{32-35=2.2}}
+if #available(iOS 9.3, watchOS 2.1, *) { // expected-note {{narrow nearby version check from '2.1' to '2.2'}}
+  functionIntroducedOnwatchOS2_2() // expected-error {{'functionIntroducedOnwatchOS2_2()' is only available in watchOS 2.2 or newer}}
+      // expected-note@-1 {{add 'if #available' version check}}
 }
 
 if #available(iOS 9.1, watchOS 2.2, *) {