Improve message when trying to override an unavailable member.

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -1371,8 +1371,12 @@ NOTE(overridden_here_with_type,none,
      "overridden declaration here has type %0", (Type))
 ERROR(missing_override,none,
       "overriding declaration requires an 'override' keyword", ())
+
 ERROR(override_unavailable,none,
       "cannot override %0 which has been marked unavailable", (Identifier))
+ERROR(override_unavailable_msg, none,
+      "cannot override %0 which has been marked unavailable: %1",
+      (Identifier, StringRef))
 
 ERROR(override_less_available,none,
       "overriding %0 must be as available as declaration it overrides",

lib/Sema/MiscDiagnostics.cpp

@@ -1352,6 +1352,18 @@ bool TypeChecker::diagnoseExplicitUnavailability(const ValueDecl *D,
                                                  SourceRange R,
                                                  const DeclContext *DC,
                                                  const CallExpr *CE) {
+  return diagnoseExplicitUnavailability(D, R, DC,
+                                        [=](InFlightDiagnostic &diag) {
+    fixItAvailableAttrRename(*this, diag, R, AvailableAttr::isUnavailable(D),
+                             CE);
+  });
+}
+
+bool TypeChecker::diagnoseExplicitUnavailability(
+    const ValueDecl *D,
+    SourceRange R,
+    const DeclContext *DC,
+    llvm::function_ref<void(InFlightDiagnostic &)> attachRenameFixIts) {
   auto *Attr = AvailableAttr::isUnavailable(D);
   if (!Attr)
     return false;
@@ -1388,12 +1400,12 @@ bool TypeChecker::diagnoseExplicitUnavailability(const ValueDecl *D,
         auto diag = diagnose(Loc, diag::availability_decl_unavailable_rename,
                              Name, replaceKind.hasValue(), rawReplaceKind,
                              newName);
-        fixItAvailableAttrRename(*this, diag, R, Attr, CE);
+        attachRenameFixIts(diag);
       } else {
         auto diag = diagnose(Loc,diag::availability_decl_unavailable_rename_msg,
                              Name, replaceKind.hasValue(), rawReplaceKind,
                              newName, Attr->Message);
-        fixItAvailableAttrRename(*this, diag, R, Attr, CE);
+        attachRenameFixIts(diag);
       }
     } else if (Attr->Message.empty()) {
       diagnose(Loc, diag::availability_decl_unavailable, Name).highlight(R);

lib/Sema/TypeCheckDecl.cpp

@@ -5478,6 +5478,28 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     return true;
   }
 
+  static void diagnoseUnavailableOverride(TypeChecker &TC,
+                                          const ValueDecl *override,
+                                          const ValueDecl *base,
+                                          const AvailableAttr *attr) {
+    if (attr->Rename.empty()) {
+      if (attr->Message.empty())
+        TC.diagnose(override, diag::override_unavailable, override->getName());
+      else
+        TC.diagnose(override, diag::override_unavailable_msg,
+                    override->getName(), attr->Message);
+      TC.diagnose(base, diag::availability_marked_unavailable,
+                  base->getFullName());
+      return;
+    }
+
+    TC.diagnoseExplicitUnavailability(base, override->getLoc(),
+                                      override->getDeclContext(),
+                                      [](InFlightDiagnostic &diag) {
+      // FIXME: Apply fix-its here.
+    });
+  }
+
   /// Record that the \c overriding declarations overrides the
   /// \c overridden declaration.
   ///
@@ -5580,8 +5602,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     }
 
     // FIXME: Possibly should extend to more availability checking.
-    if (base->getAttrs().isUnavailable(TC.Context)) {
-      TC.diagnose(override, diag::override_unavailable, override->getName());
+    if (auto *attr = base->getAttrs().getUnavailable(TC.Context)) {
+      diagnoseUnavailableOverride(TC, override, base, attr);
     }
 
     if (!TC.getLangOpts().DisableAvailabilityChecking) {

lib/Sema/TypeChecker.h

@@ -1576,6 +1576,14 @@ class TypeChecker final : public LazyResolver {
                                       const DeclContext *DC,
                                       const CallExpr *CE);
 
+  /// Emit a diagnostic for references to declarations that have been
+  /// marked as unavailable, either through "unavailable" or "obsoleted:".
+  bool diagnoseExplicitUnavailability(
+      const ValueDecl *D,
+      SourceRange R,
+      const DeclContext *DC,
+      llvm::function_ref<void(InFlightDiagnostic &)> attachRenameFixIts);
+
   /// @}
 
   /// \name Overload resolution

test/Sema/availability.swift

@@ -19,7 +19,7 @@ func foo(x : NSUInteger) { // expected-error {{'NSUInteger' is unavailable: use
 // Test preventing overrides of unavailable methods.
 class ClassWithUnavailable {
   @available(*, unavailable)
-  func doNotOverride() {}
+  func doNotOverride() {} // expected-note {{'doNotOverride()' has been explicitly marked unavailable here}}
 
   // FIXME: extraneous diagnostic here
   @available(*, unavailable)

test/attr/attr_availability.swift

@@ -496,3 +496,24 @@ func testFactoryMethods() {
   Int.factory(other: 1) // expected-error {{'factory(other:)' has been replaced by 'init(other:)'}} {{6-14=}}
   Int.factory2(other: 1) // expected-error {{'factory2(other:)' has been replaced by 'Int.init(other:)'}} {{3-15=Int}}
 }
+
+class Base {
+  @available(*, unavailable)
+  func bad() {} // expected-note {{here}}
+
+  @available(*, unavailable, message: "it was smelly")
+  func smelly() {} // expected-note {{here}}
+
+  @available(*, unavailable, renamed: "new")
+  func old() {} // expected-note {{here}}
+
+  @available(*, unavailable, renamed: "new", message: "it was smelly")
+  func oldAndSmelly() {} // expected-note {{here}}
+}
+
+class Sub : Base {
+  override func bad() {} // expected-error {{cannot override 'bad' which has been marked unavailable}} {{none}}
+  override func smelly() {} // expected-error {{cannot override 'smelly' which has been marked unavailable: it was smelly}} {{none}}
+  override func old() {} // expected-error {{'old()' has been renamed to 'new'}} {{none}}
+  override func oldAndSmelly() {} // expected-error {{'oldAndSmelly()' has been renamed to 'new': it was smelly}} {{none}}
+}