unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

[arphaman]

Summary

The fixit is given only if the API_AVAILABLE macro is defined.

[erik.pilkington]

Thanks for working on this! This looks like it would be very useful.

lib/Sema/SemaDeclAttr.cpp

7230

Its unfortunate to loop over every macro. Can we use Preprocessor::getMacroDefinition()?

7231

It would be nice if we could recommend using this macro even if it isn't defined, as users might not have included the <os/availability.h> header. Maybe we can do that on apple platforms, noting that the the macro is declared in os/availability.h if it isn't already defined?

7238

OffendingDecl, right? Thats the one with the offending availability attribute

7240

I was somewhat uncertain about adding a fixit for this because its difficult to determine where exactly the availability attribute should go, it looks like this doesn't emit an attribute at the right place for this, for example. (It should go after the meth)

@interface X
-(new_int)meth;
@end

Maybe we can do better if we look at what Enclosing actually declares? It's not worth it to emit an incorrect fixit.

[arphaman]

lib/Sema/SemaDeclAttr.cpp

7230

Good call.

7231

I think we either have to go the full way (i.e. have an #include <os/availability.h> fixit inserted as well) or just avoid any fixits. Note that these fixits will be used pretty much only in Xcode which doesn't show any notes that don't have fixits, so an additional note wouldn't make sense. We could potentially change the message of note_partial_availability_silence but I doubt it's that useful.

7240

Yeah, Good point.

[arphaman]

Addressed Erik's comments

[erik.pilkington]

LGTM, thanks for working on this!

lib/Sema/SemaDeclAttr.cpp

7063

TagDecls also have a special rule: struct __attribute__(...) X;.

7230

Looks like there is a convenience function: isMacroDefined(StringRef)

7231

Ok, I think this is fine.

lib/Sema/SemaDeclAttr.cpp

static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
   SourceLocation NoteLocation = OffendingDecl->getLocation();
 
   // Matches 'diag::note_property_attribute' options.
   unsigned property_note_select;
 
   // Matches diag::note_availability_specified_here.
   unsigned available_here_select_kind;
 
   VersionTuple DeclVersion;
   if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, OffendingDecl))
     DeclVersion = AA->getIntroduced();
 
   if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))
     return;
 
   switch (K) {
   case AR_Deprecated:
static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
   if (K == AR_NotYetIntroduced)
     if (const auto *Enclosing = findEnclosingDeclToAnnotate(Ctx)) {
       if (auto *TD = dyn_cast<TagDecl>(Enclosing))
         if (TD->getDeclName().isEmpty()) {
           S.Diag(TD->getLocation(), diag::note_partial_availability_silence)
               << /*Anonymous*/1 << TD->getKindName();
           return;
         }
-      S.Diag(Enclosing->getLocation(), diag::note_partial_availability_silence)
-          << /*Named*/0 << Enclosing;
+      auto FixitNoteDiag = S.Diag(Enclosing->getLocation(),
+                                  diag::note_partial_availability_silence)
+                           << /*Named*/ 0 << Enclosing;
+      // Don't offer a fixit for declarations with availability attributes.
+      if (Enclosing->hasAttr<AvailabilityAttr>())
+        return;
+      for (const auto &M : S.getPreprocessor().macros()) {
+        if (M.first->getName() != "API_AVAILABLE")
+          continue;
+        std::string PlatformName =
+            AvailabilityAttr::getPlatformNameSourceSpelling(
+                S.getASTContext().getTargetInfo().getPlatformName())
+                .lower();
+        std::string Introduced =
+            ReferringDecl->getVersionIntroduced().getAsString();
+        FixitNoteDiag << FixItHint::CreateInsertion(
+            Enclosing->getLocStart(), (llvm::Twine("API_AVAILABLE(") +
+                                       PlatformName + "(" + Introduced + "))\n")
+                                          .str());
+        break;
+      }
     }
 }
 
 static void handleDelayedAvailabilityCheck(Sema &S, DelayedDiagnostic &DD,
                                            Decl *Ctx) {
   assert(DD.Kind == DelayedDiagnostic::Availability &&
          "Expected an availability diagnostic here");
 

test/FixIt/fixit-availability.c

 // RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -fdiagnostics-parseable-fixits -triple x86_64-apple-darwin9 %s 2>&1 | FileCheck %s
 
 __attribute__((availability(macos, introduced=10.12)))
 int function(void);
 
 void use() {
   function();
 // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (__builtin_available(macOS 10.12, *)) {\n      "
 // CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n  } else {\n      // Fallback on earlier versions\n  }"
 }
+
+__attribute__((availability(macos, introduced=10.12)))
+struct New { };
+
+struct NoFixit {
+  struct New field;
+};
+// CHECK-NOT: API_AVAILABLE

test/FixIt/fixit-availability.mm

// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:6-[[@LINE-2]]:6}:"\n    } else {\n        // Fallback on earlier versions\n    }"
   }
   if (y)
     int z = function();
 // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macOS 10.12, *)) {\n        "
 // CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:24-[[@LINE-2]]:24}:"\n    } else {\n        // Fallback on earlier versions\n    }"
   anotherFunction(y);
   anotherFunction(x);
 }
+
+#define API_AVAILABLE(X) __attribute__((availability(macos, introduced=10.12))) // dummy macro
+
+API_AVAILABLE(macos(10.12))
+@interface NewClass
+@end
+
+@interface OldButOfferFixit
+@property(copy) NewClass *prop;
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:1-[[@LINE-2]]:1}:"API_AVAILABLE(macos(10.12))\n"
+@end
+
+// Avoid a fixit for declarations that already have an attribute:
+__attribute__((availability(macos, introduced=10.11)))
+@interface WithoutFixit
+@property(copy) NewClass *prop;
+// CHECK-NOT: API_AVAILABLE
+@end