Merge pull request #15864 from jrose-apple/get-set-stop

jrose-apple · web-flow · commit ab9f62dcd813 · 2018-04-12T09:52:05.000-07:00
Diagnose unavailable getters and setters
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -3622,38 +3622,45 @@ ERROR(dynamic_requires_objc,none,
 //------------------------------------------------------------------------------
 
 ERROR(availability_decl_unavailable, none,
-  "%0 is unavailable", (DeclName))
+      "%select{getter for |setter for |}0%1 is unavailable",
+      (unsigned, DeclName))
 
 #define REPLACEMENT_DECL_KIND_SELECT "select{| instance method| property}"
 ERROR(availability_decl_unavailable_rename, none,
-      "%0 has been %select{renamed to|replaced by}1"
-      "%" REPLACEMENT_DECL_KIND_SELECT "2 '%3'",
-      (DeclName, bool, unsigned, StringRef))
+      "%select{getter for |setter for |}0%1 has been "
+      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
+      "'%4'",
+      (unsigned, DeclName, bool, unsigned, StringRef))
 
 ERROR(availability_decl_unavailable_rename_msg, none,
-      "%0 has been %select{renamed to|replaced by}1"
-      "%" REPLACEMENT_DECL_KIND_SELECT "2 '%3': %4",
-      (DeclName, bool, unsigned, StringRef, StringRef))
+      "%select{getter for |setter for |}0%1 has been "
+      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
+      "'%4': %5",
+      (unsigned, DeclName, bool, unsigned, StringRef, StringRef))
 
 ERROR(availability_decl_unavailable_msg, none,
-  "%0 is unavailable: %1", (DeclName, StringRef))
+      "%select{getter for |setter for |}0%1 is unavailable: %2",
+      (unsigned, DeclName, StringRef))
 
 ERROR(availability_decl_unavailable_in_swift, none,
-  "%0 is unavailable in Swift", (DeclName))
+      "%select{getter for |setter for |}0%1 is unavailable in Swift",
+      (unsigned, DeclName))
 
 ERROR(availability_decl_unavailable_in_swift_msg, none,
-  "%0 is unavailable in Swift: %1", (DeclName, StringRef))
+      "%select{getter for |setter for |}0%1 is unavailable in Swift: %2",
+      (unsigned, DeclName, StringRef))
 
 NOTE(availability_marked_unavailable, none,
-  "%0 has been explicitly marked unavailable here", (DeclName))
+     "%select{getter for |setter for |}0%1 has been explicitly marked "
+     "unavailable here", (unsigned, DeclName))
 
 NOTE(availability_introduced_in_swift, none,
-     "%0 was introduced in Swift %1",
-    (DeclName, clang::VersionTuple))
+     "%select{getter for |setter for |}0%1 was introduced in Swift %2",
+     (unsigned, DeclName, clang::VersionTuple))
 
 NOTE(availability_obsoleted, none,
-     "%0 was obsoleted in %1 %2",
-    (DeclName, StringRef, clang::VersionTuple))
+     "%select{getter for |setter for |}0%1 was obsoleted in %2 %3",
+     (unsigned, DeclName, StringRef, clang::VersionTuple))
 
 WARNING(availability_deprecated, none,
         "%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -1534,6 +1534,9 @@ static void fixItAvailableAttrRename(TypeChecker &TC,
                                      const ValueDecl *renamedDecl,
                                      const AvailableAttr *attr,
                                      const ApplyExpr *call) {
+  if (isa<AccessorDecl>(renamedDecl))
+    return;
+
   ParsedDeclName parsed = swift::parseDeclName(attr->Rename);
   if (!parsed)
     return;
@@ -1906,6 +1909,26 @@ describeRename(ASTContext &ctx, const AvailableAttr *attr, const ValueDecl *D,
   return ReplacementDeclKind::None;
 }
 
+/// Returns a value that can be used to select between accessor kinds in
+/// diagnostics.
+///
+/// This is correlated with diag::availability_deprecated and others.
+static std::pair<unsigned, DeclName>
+getAccessorKindAndNameForDiagnostics(const ValueDecl *D) {
+  // This should always be one more than the last AccessorKind supported in
+  // the diagnostics. If you need to change it, change the assertion below as
+  // well.
+  static const unsigned NOT_ACCESSOR_INDEX = 2;
+
+  if (auto *accessor = dyn_cast<AccessorDecl>(D)) {
+    DeclName Name = accessor->getStorage()->getFullName();
+    assert(accessor->isGetterOrSetter());
+    return {static_cast<unsigned>(accessor->getAccessorKind()), Name};
+  }
+
+  return {NOT_ACCESSOR_INDEX, D->getFullName()};
+}
+
 void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
                                        const DeclContext *ReferenceDC,
                                        const ValueDecl *DeprecatedDecl,
@@ -1933,26 +1956,19 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
   }
 
   DeclName Name;
-  Optional<unsigned> rawAccessorKind;
-  if (auto *accessor = dyn_cast<AccessorDecl>(DeprecatedDecl)) {
-    Name = accessor->getStorage()->getFullName();
-    assert(accessor->isGetterOrSetter());
-    rawAccessorKind = static_cast<unsigned>(accessor->getAccessorKind());
-  } else {
-    Name = DeprecatedDecl->getFullName();
-  }
+  unsigned RawAccessorKind;
+  std::tie(RawAccessorKind, Name) =
+      getAccessorKindAndNameForDiagnostics(DeprecatedDecl);
 
   StringRef Platform = Attr->prettyPlatformString();
   clang::VersionTuple DeprecatedVersion;
   if (Attr->Deprecated)
     DeprecatedVersion = Attr->Deprecated.getValue();
 
-  static const unsigned NOT_ACCESSOR_INDEX = 2;
   if (Attr->Message.empty() && Attr->Rename.empty()) {
     diagnose(ReferenceRange.Start, diag::availability_deprecated,
-             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
-             Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
-             DeprecatedVersion)
+             RawAccessorKind, Name, Attr->hasPlatform(), Platform,
+             Attr->Deprecated.hasValue(), DeprecatedVersion)
       .highlight(Attr->getRange());
     return;
   }
@@ -1965,22 +1981,21 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
   if (!Attr->Message.empty()) {
     EncodedDiagnosticMessage EncodedMessage(Attr->Message);
     diagnose(ReferenceRange.Start, diag::availability_deprecated_msg,
-             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
-             Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
-             DeprecatedVersion, EncodedMessage.Message)
+             RawAccessorKind, Name, Attr->hasPlatform(), Platform,
+             Attr->Deprecated.hasValue(), DeprecatedVersion,
+             EncodedMessage.Message)
       .highlight(Attr->getRange());
   } else {
     unsigned rawReplaceKind = static_cast<unsigned>(
         replacementDeclKind.getValueOr(ReplacementDeclKind::None));
     diagnose(ReferenceRange.Start, diag::availability_deprecated_rename,
-             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
-             Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
-             DeprecatedVersion, replacementDeclKind.hasValue(), rawReplaceKind,
-             newName)
+             RawAccessorKind, Name, Attr->hasPlatform(), Platform,
+             Attr->Deprecated.hasValue(), DeprecatedVersion,
+             replacementDeclKind.hasValue(), rawReplaceKind, newName)
       .highlight(Attr->getRange());
   }
 
-  if (!Attr->Rename.empty() && !rawAccessorKind.hasValue()) {
+  if (!Attr->Rename.empty() && !isa<AccessorDecl>(DeprecatedDecl)) {
     auto renameDiag = diagnose(ReferenceRange.Start,
                                diag::note_deprecated_rename,
                                newName);
@@ -1999,8 +2014,13 @@ void TypeChecker::diagnoseUnavailableOverride(ValueDecl *override,
     else
       diagnose(override, diag::override_unavailable_msg,
                override->getBaseName(), attr->Message);
+
+    DeclName name;
+    unsigned rawAccessorKind;
+    std::tie(rawAccessorKind, name) =
+        getAccessorKindAndNameForDiagnostics(base);
     diagnose(base, diag::availability_marked_unavailable,
-             base->getFullName());
+             rawAccessorKind, name);
     return;
   }
 
@@ -2127,7 +2147,9 @@ bool TypeChecker::diagnoseExplicitUnavailability(
   }
 
   SourceLoc Loc = R.Start;
-  auto Name = D->getFullName();
+  DeclName Name;
+  unsigned RawAccessorKind;
+  std::tie(RawAccessorKind, Name) = getAccessorKindAndNameForDiagnostics(D);
 
   switch (Attr->getPlatformAgnosticAvailability()) {
   case PlatformAgnosticAvailabilityKind::Deprecated:
@@ -2150,14 +2172,14 @@ bool TypeChecker::diagnoseExplicitUnavailability(
 
       if (Attr->Message.empty()) {
         auto diag = diagnose(Loc, diag::availability_decl_unavailable_rename,
-                             Name, replaceKind.hasValue(), rawReplaceKind,
-                             newName);
+                             RawAccessorKind, Name, replaceKind.hasValue(),
+                             rawReplaceKind, newName);
         attachRenameFixIts(diag);
       } else {
         EncodedDiagnosticMessage EncodedMessage(Attr->Message);
         auto diag = diagnose(Loc, diag::availability_decl_unavailable_rename_msg,
-                             Name, replaceKind.hasValue(), rawReplaceKind,
-                             newName, EncodedMessage.Message);
+                             RawAccessorKind, Name, replaceKind.hasValue(),
+                             rawReplaceKind, newName, EncodedMessage.Message);
         attachRenameFixIts(diag);
       }
     } else if (isSubscriptReturningString(D, Context)) {
@@ -2171,12 +2193,13 @@ bool TypeChecker::diagnoseExplicitUnavailability(
     } else if (Attr->Message.empty()) {
       diagnose(Loc, inSwift ? diag::availability_decl_unavailable_in_swift
                             : diag::availability_decl_unavailable,
-               Name).highlight(R);
+               RawAccessorKind, Name)
+        .highlight(R);
     } else {
       EncodedDiagnosticMessage EncodedMessage(Attr->Message);
       diagnose(Loc, inSwift ? diag::availability_decl_unavailable_in_swift_msg
                             : diag::availability_decl_unavailable_msg,
-               Name, EncodedMessage.Message)
+               RawAccessorKind, Name, EncodedMessage.Message)
         .highlight(R);
     }
     break;
@@ -2191,20 +2214,23 @@ bool TypeChecker::diagnoseExplicitUnavailability(
   case AvailableVersionComparison::Unavailable:
     if (Attr->isLanguageVersionSpecific()
         && Attr->Introduced.hasValue())
-      diagnose(D, diag::availability_introduced_in_swift, Name,
-               *Attr->Introduced).highlight(Attr->getRange());
+      diagnose(D, diag::availability_introduced_in_swift,
+               RawAccessorKind, Name, *Attr->Introduced)
+        .highlight(Attr->getRange());
     else
-      diagnose(D, diag::availability_marked_unavailable, Name)
+      diagnose(D, diag::availability_marked_unavailable, RawAccessorKind, Name)
         .highlight(Attr->getRange());
     break;
 
   case AvailableVersionComparison::Obsoleted:
     // FIXME: Use of the platformString here is non-awesome for application
     // extensions.
-    diagnose(D, diag::availability_obsoleted, Name,
+    diagnose(D, diag::availability_obsoleted,
+             RawAccessorKind, Name,
              (Attr->isLanguageVersionSpecific() ?
               "Swift" : Attr->prettyPlatformString()),
-             *Attr->Obsoleted).highlight(Attr->getRange());
+             *Attr->Obsoleted)
+      .highlight(Attr->getRange());
     break;
   }
   return true;
@@ -2433,8 +2459,18 @@ class AvailabilityWalker : public ASTWalker {
       return;
     }
 
-    // Make sure not to diagnose an accessor if we already complained about
-    // the property/subscript.
+    // If the property/subscript is unconditionally unavailable, don't bother
+    // with any of the rest of this.
+    if (AvailableAttr::isUnavailable(D->getStorage()))
+      return;
+
+    if (TC.diagnoseExplicitUnavailability(D, ReferenceRange, ReferenceDC,
+                                          /*call*/nullptr)) {
+      return;
+    }
+
+    // Make sure not to diagnose an accessor's deprecation if we already
+    // complained about the property/subscript.
     if (!TypeChecker::getDeprecated(D->getStorage()))
       TC.diagnoseIfDeprecated(ReferenceRange, ReferenceDC, D, /*call*/nullptr);
 
@@ -2549,9 +2585,14 @@ bool AvailabilityWalker::diagnoseIncDecRemoval(const ValueDecl *D,
   }
   
   if (!replacement.empty()) {
+    DeclName Name;
+    unsigned RawAccessorKind;
+    std::tie(RawAccessorKind, Name) = getAccessorKindAndNameForDiagnostics(D);
+
     // If we emit a deprecation diagnostic, produce a fixit hint as well.
     auto diag = TC.diagnose(R.Start, diag::availability_decl_unavailable_msg,
-                            D->getFullName(), "it has been removed in Swift 3");
+                            RawAccessorKind, Name,
+                            "it has been removed in Swift 3");
     if (isa<PrefixUnaryExpr>(call)) {
       // Prefix: remove the ++ or --.
       diag.fixItRemove(call->getFn()->getSourceRange());
@@ -2594,9 +2635,13 @@ bool AvailabilityWalker::diagnoseMemoryLayoutMigration(const ValueDecl *D,
   if (!args)
     return false;
 
+  DeclName Name;
+  unsigned RawAccessorKind;
+  std::tie(RawAccessorKind, Name) = getAccessorKindAndNameForDiagnostics(D);
+
   EncodedDiagnosticMessage EncodedMessage(Attr->Message);
   auto diag = TC.diagnose(R.Start, diag::availability_decl_unavailable_msg,
-                          D->getFullName(), EncodedMessage.Message);
+                          RawAccessorKind, Name, EncodedMessage.Message);
   diag.highlight(R);
 
   auto subject = args->getSubExpr();
diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp
@@ -1383,9 +1383,10 @@ bool TypeChecker::coercePatternToType(Pattern *&P, DeclContext *dc, Type type,
                 EEP->getName().str() == "Some") {
               SmallString<4> Rename;
               camel_case::toLowercaseWord(EEP->getName().str(), Rename);
-              diagnose(EEP->getLoc(), diag::availability_decl_unavailable_rename,
-                          EEP->getName(), /*replaced*/false,
-                          /*special kind*/0, Rename.str())
+              diagnose(EEP->getLoc(),
+                       diag::availability_decl_unavailable_rename,
+                       /*"getter" prefix*/2, EEP->getName(), /*replaced*/false,
+                       /*special kind*/0, Rename.str())
                 .fixItReplace(EEP->getLoc(), Rename.str());
 
               return true;
diff --git a/test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h b/test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h
@@ -107,3 +107,18 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
 @property (class) int setterDeprecatedClass;
 + (void)setSetterDeprecatedClass:(int)setterDeprecated __attribute__((deprecated));
 @end
+
+
+@interface UnavailableAccessors: NSObject
+@property NSInteger fullyUnavailable __attribute__((unavailable));
+
+@property NSInteger getterUnavailable;
+- (NSInteger)getterUnavailable __attribute__((unavailable));
+@property (class) NSInteger getterUnavailableClass;
++ (NSInteger)getterUnavailableClass __attribute__((unavailable));
+
+@property NSInteger setterUnavailable;
+- (void)setSetterUnavailable:(NSInteger)setterUnavailable __attribute__((unavailable));
+@property (class) NSInteger setterUnavailableClass;
++ (void)setSetterUnavailableClass:(NSInteger)setterUnavailable __attribute__((unavailable));
+@end
diff --git a/test/ClangImporter/availability.swift b/test/ClangImporter/availability.swift
@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -verify -I %S/Inputs/custom-modules %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -verify -I %S/Inputs/custom-modules %s -verify-ignore-unknown
 
 // REQUIRES: objc_interop
 
@@ -25,6 +25,28 @@ func test_unavailable_func(_ x : NSObject) {
   NSDeallocateObject(x) // expected-error {{'NSDeallocateObject' is unavailable}}
 }
 
+func test_unavailable_accessors(_ obj: UnavailableAccessors) {
+  _ = obj.fullyUnavailable // expected-error {{'fullyUnavailable' is unavailable}}
+  obj.fullyUnavailable = 0 // expected-error {{'fullyUnavailable' is unavailable}}
+  obj.fullyUnavailable += 1 // expected-error {{'fullyUnavailable' is unavailable}}
+
+  _ = obj.getterUnavailable // expected-error {{getter for 'getterUnavailable' is unavailable}}
+  obj.getterUnavailable = 0
+  obj.getterUnavailable += 1 // expected-error {{getter for 'getterUnavailable' is unavailable}}
+
+  _ = UnavailableAccessors.getterUnavailableClass // expected-error {{getter for 'getterUnavailableClass' is unavailable}}
+  UnavailableAccessors.getterUnavailableClass = 0
+  UnavailableAccessors.getterUnavailableClass += 1 // expected-error {{getter for 'getterUnavailableClass' is unavailable}}
+
+  _ = obj.setterUnavailable
+  obj.setterUnavailable = 0 // expected-error {{setter for 'setterUnavailable' is unavailable}}
+  obj.setterUnavailable += 1 // expected-error {{setter for 'setterUnavailable' is unavailable}}
+
+  _ = UnavailableAccessors.setterUnavailableClass
+  UnavailableAccessors.setterUnavailableClass = 0 // expected-error {{setter for 'setterUnavailableClass' is unavailable}}
+  UnavailableAccessors.setterUnavailableClass += 1 // expected-error {{setter for 'setterUnavailableClass' is unavailable}}
+}
+
 func test_deprecated(_ s:UnsafeMutablePointer<CChar>, _ obj: AccessorDeprecations) {
   _ = tmpnam(s) // expected-warning {{'tmpnam' is deprecated: Due to security concerns inherent in the design of tmpnam(3), it is highly recommended that you use mkstemp(3) instead.}}
 
diff --git a/test/attr/attr_availability.swift b/test/attr/attr_availability.swift
diff --git a/test/attr/attr_availability_osx.swift b/test/attr/attr_availability_osx.swift
