Check deprecation for getters and setters

This handles the case where just one accessor is deprecated but not
the other, which does come up sometimes in Apple's frameworks.

rdar://problem/18633725

include/swift/AST/DiagnosticsSema.def

@@ -3590,22 +3590,23 @@ NOTE(availability_obsoleted, none,
     (DeclName, StringRef, clang::VersionTuple))
 
 WARNING(availability_deprecated, none,
-        "%0 %select{is|%select{is|was}3}1 deprecated"
+        "%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
-        "%select{| %select{on|in}3 %2%select{| %4}3}1",
+        "deprecated%select{| %select{on|in}4 %3%select{| %5}4}2",
-        (DeclName, bool, StringRef, bool, clang::VersionTuple))
+        (unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple))
 
 WARNING(availability_deprecated_msg, none,
-        "%0 %select{is|%select{is|was}3}1 deprecated"
+        "%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
-        "%select{| %select{on|in}3 %2%select{| %4}3}1: %5",
+        "deprecated%select{| %select{on|in}4 %3%select{| %5}4}2: %6",
-        (DeclName, bool, StringRef, bool, clang::VersionTuple, StringRef))
+        (unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple,
+         StringRef))
 
 WARNING(availability_deprecated_rename, none,
-        "%0 %select{is|%select{is|was}3}1 deprecated"
+        "%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
-        "%select{| %select{on|in}3 %2%select{| %4}3}1: "
+        "deprecated%select{| %select{on|in}4 %3%select{| %5}4}2: "
-        "%select{renamed to|replaced by}5%" REPLACEMENT_DECL_KIND_SELECT "6 "
+        "%select{renamed to|replaced by}6%" REPLACEMENT_DECL_KIND_SELECT "7 "
-        "'%7'",
+        "'%8'",
-        (DeclName, bool, StringRef, bool, clang::VersionTuple, bool, unsigned,
+        (unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple, bool,
-         StringRef))
+         unsigned, StringRef))
 #undef REPLACEMENT_DECL_KIND_SELECT
 
 NOTE(note_deprecated_rename, none,

lib/Sema/TypeCheckAvailability.cpp

@@ -1932,14 +1932,25 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
     }
   }
 
-  DeclName Name = DeprecatedDecl->getFullName();
+  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();
+  }
+
   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, Name,
+    diagnose(ReferenceRange.Start, diag::availability_deprecated,
+             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
              Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
              DeprecatedVersion)
       .highlight(Attr->getRange());
@@ -1953,21 +1964,23 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
 
   if (!Attr->Message.empty()) {
     EncodedDiagnosticMessage EncodedMessage(Attr->Message);
-    diagnose(ReferenceRange.Start, diag::availability_deprecated_msg, Name,
+    diagnose(ReferenceRange.Start, diag::availability_deprecated_msg,
+             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), 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, Name,
+    diagnose(ReferenceRange.Start, diag::availability_deprecated_rename,
+             rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
              Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
              DeprecatedVersion, replacementDeclKind.hasValue(), rawReplaceKind,
              newName)
       .highlight(Attr->getRange());
   }
 
-  if (!Attr->Rename.empty()) {
+  if (!Attr->Rename.empty() && !rawAccessorKind.hasValue()) {
     auto renameDiag = diagnose(ReferenceRange.Start,
                                diag::note_deprecated_rename,
                                newName);
@@ -2234,9 +2247,11 @@ public:
       return std::make_pair(false, E);
     };
 
-    if (auto DR = dyn_cast<DeclRefExpr>(E))
+    if (auto DR = dyn_cast<DeclRefExpr>(E)) {
       diagAvailability(DR->getDecl(), DR->getSourceRange(),
                        getEnclosingApplyExpr());
+      maybeDiagStorageAccess(DR->getDecl(), DR->getSourceRange(), DC);
+    }
     if (auto MR = dyn_cast<MemberRefExpr>(E)) {
       walkMemberRef(MR);
       return skipChildren();
@@ -2252,8 +2267,10 @@ public:
     if (auto DS = dyn_cast<DynamicSubscriptExpr>(E))
       diagAvailability(DS->getMember().getDecl(), DS->getSourceRange());
     if (auto S = dyn_cast<SubscriptExpr>(E)) {
-      if (S->hasDecl())
+      if (S->hasDecl()) {
         diagAvailability(S->getDecl().getDecl(), S->getSourceRange());
+        maybeDiagStorageAccess(S->getDecl().getDecl(), S->getSourceRange(), DC);
+      }
     }
     if (auto A = dyn_cast<AssignExpr>(E)) {
       walkAssignExpr(A);
@@ -2336,6 +2353,8 @@ private:
   /// Walk a member reference expression, checking for availability.
   void walkMemberRef(MemberRefExpr *E) {
     // Walk the base in a getter context.
+    // FIXME: We may need to look at the setter too, if we're going to do
+    // writeback. The AST should have this information.
     walkInContext(E, E->getBase(), MemberAccessContext::Getter);
 
     ValueDecl *D = E->getMember().getDecl();
@@ -2343,13 +2362,8 @@ private:
     if (diagAvailability(D, E->getNameLoc().getSourceRange()))
       return;
 
-    if (TC.getLangOpts().DisableAvailabilityChecking)
-      return;
-
-    if (auto *ASD = dyn_cast<AbstractStorageDecl>(D)) {
     // Diagnose for appropriate accessors, given the access context.
-      diagStorageAccess(ASD, E->getSourceRange(), DC);
+    maybeDiagStorageAccess(D, E->getSourceRange(), DC);
-    }
   }
   
   /// Walk an inout expression, checking for availability.
@@ -2367,9 +2381,16 @@ private:
 
   /// Emit diagnostics, if necessary, for accesses to storage where
   /// the accessor for the AccessContext is not available.
-  void diagStorageAccess(AbstractStorageDecl *D,
+  void maybeDiagStorageAccess(const ValueDecl *VD,
                               SourceRange ReferenceRange,
                               const DeclContext *ReferenceDC) const {
+    if (TC.getLangOpts().DisableAvailabilityChecking)
+      return;
+
+    auto *D = dyn_cast<AbstractStorageDecl>(VD);
+    if (!D)
+      return;
+
     if (!D->hasAccessorFunctions()) {
       return;
     }
@@ -2397,13 +2418,18 @@ private:
   }
 
   /// Emit a diagnostic, if necessary for a potentially unavailable accessor.
-  /// Returns true if a diagnostic was emitted.
   void diagAccessorAvailability(AccessorDecl *D, SourceRange ReferenceRange,
                                 const DeclContext *ReferenceDC,
                                 bool ForInout) const {
     if (!D) {
       return;
     }
+
+    // Make sure not to diagnose an accessor if we already complained about
+    // the property/subscript.
+    if (!TypeChecker::getDeprecated(D->getStorage()))
+      TC.diagnoseIfDeprecated(ReferenceRange, ReferenceDC, D, /*call*/nullptr);
+
     auto MaybeUnavail = TC.checkDeclarationAvailability(D, ReferenceRange.Start,
                                                         DC);
     if (MaybeUnavail.hasValue()) {

test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h

@@ -93,3 +93,17 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
     NSEnumAddedCasesIn2017ExistingCaseThree,
     NSEnumAddedCasesIn2017NewCaseOne __attribute__((availability(macosx,introduced=10.13))) __attribute__((availability(ios,introduced=11.0))) __attribute__((availability(tvos,introduced=11.0))) __attribute__((availability(watchos,introduced=4.0)))
 };
+
+@interface AccessorDeprecations: NSObject
+@property int fullyDeprecated __attribute__((deprecated));
+
+@property int getterDeprecated;
+- (int)getterDeprecated __attribute__((deprecated));
+@property (class) int getterDeprecatedClass;
++ (int)getterDeprecatedClass __attribute__((deprecated));
+
+@property int setterDeprecated;
+- (void)setSetterDeprecated:(int)setterDeprecated __attribute__((deprecated));
+@property (class) int setterDeprecatedClass;
++ (void)setSetterDeprecatedClass:(int)setterDeprecated __attribute__((deprecated));
+@end

test/ClangImporter/availability.swift

@@ -25,8 +25,28 @@ func test_unavailable_func(_ x : NSObject) {
   NSDeallocateObject(x) // expected-error {{'NSDeallocateObject' is unavailable}}
 }
 
-func test_deprecated_imported_as_unavailable(_ s:UnsafeMutablePointer<CChar>) {
+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.}}
+
+  _ = obj.fullyDeprecated // expected-warning {{'fullyDeprecated' is deprecated}}
+  obj.fullyDeprecated = 0 // expected-warning {{'fullyDeprecated' is deprecated}}
+  obj.fullyDeprecated += 1 // expected-warning {{'fullyDeprecated' is deprecated}}
+
+  _ = obj.getterDeprecated // expected-warning {{getter for 'getterDeprecated' is deprecated}}
+  obj.getterDeprecated = 0
+  obj.getterDeprecated += 1 // expected-warning {{getter for 'getterDeprecated' is deprecated}}
+
+  _ = AccessorDeprecations.getterDeprecatedClass // expected-warning {{getter for 'getterDeprecatedClass' is deprecated}}
+  AccessorDeprecations.getterDeprecatedClass = 0
+  AccessorDeprecations.getterDeprecatedClass += 1 // expected-warning {{getter for 'getterDeprecatedClass' is deprecated}}
+
+  _ = obj.setterDeprecated
+  obj.setterDeprecated = 0 // expected-warning {{setter for 'setterDeprecated' is deprecated}}
+  obj.setterDeprecated += 1 // expected-warning {{setter for 'setterDeprecated' is deprecated}}
+
+  _ = AccessorDeprecations.setterDeprecatedClass
+  AccessorDeprecations.setterDeprecatedClass = 0 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
+  AccessorDeprecations.setterDeprecatedClass += 1 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
 }
 
 func test_NSInvocation(_ x: NSInvocation,         // expected-error {{'NSInvocation' is unavailable}}

test/attr/attr_availability.swift

@@ -808,8 +808,120 @@ func rdar32526620_3(a: Int, b: E_32526620, c: String) {} // expected-note {{here
 rdar32526620_3(a: 42, b: .bar, c: "question")
 // expected-error@-1 {{'rdar32526620_3(a:b:c:)' has been replaced by instance method 'E_32526620.set(a:c:)'}} {{1-15=E_32526620.bar.set}} {{23-32=}}
 
+
 @available(*, unavailable) // expected-warning {{'@available' without an OS is ignored on extensions; apply the attribute to each member instead}} {{1-28=}}
 extension DummyType {}
 
 @available(*, deprecated) // expected-warning {{'@available' without an OS is ignored on extensions; apply the attribute to each member instead}} {{1-27=}}
 extension DummyType {}
+
+
+var deprecatedGetter: Int {
+  @available(*, deprecated) get { return 0 }
+  set {}
+}
+var deprecatedGetterOnly: Int {
+  @available(*, deprecated) get { return 0 }
+}
+var deprecatedSetter: Int {
+  get { return 0 }
+  @available(*, deprecated) set {}
+}
+var deprecatedBoth: Int {
+  @available(*, deprecated) get { return 0 }
+  @available(*, deprecated) set {}
+}
+var deprecatedMessage: Int {
+  @available(*, deprecated, message: "bad getter") get { return 0 }
+  @available(*, deprecated, message: "bad setter") set {}
+}
+var deprecatedRename: Int {
+  @available(*, deprecated, renamed: "betterThing()") get { return 0 }
+  @available(*, deprecated, renamed: "setBetterThing(_:)") set {}
+}
+@available(*, deprecated, message: "bad variable")
+var deprecatedProperty: Int {
+  @available(*, deprecated, message: "bad getter") get { return 0 }
+  @available(*, deprecated, message: "bad setter") set {}
+}
+
+_ = deprecatedGetter // expected-warning {{getter for 'deprecatedGetter' is deprecated}} {{none}}
+deprecatedGetter = 0
+deprecatedGetter += 1 // expected-warning {{getter for 'deprecatedGetter' is deprecated}} {{none}}
+
+_ = deprecatedGetterOnly // expected-warning {{getter for 'deprecatedGetterOnly' is deprecated}} {{none}}
+
+_ = deprecatedSetter
+deprecatedSetter = 0 // expected-warning {{setter for 'deprecatedSetter' is deprecated}} {{none}}
+deprecatedSetter += 1 // expected-warning {{setter for 'deprecatedSetter' is deprecated}} {{none}}
+
+_ = deprecatedBoth // expected-warning {{getter for 'deprecatedBoth' is deprecated}} {{none}}
+deprecatedBoth = 0 // expected-warning {{setter for 'deprecatedBoth' is deprecated}} {{none}}
+deprecatedBoth += 1 // expected-warning {{getter for 'deprecatedBoth' is deprecated}} {{none}} expected-warning {{setter for 'deprecatedBoth' is deprecated}} {{none}}
+
+_ = deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
+deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+
+_ = deprecatedRename // expected-warning {{getter for 'deprecatedRename' is deprecated: renamed to 'betterThing()'}} {{none}}
+deprecatedRename = 0  // expected-warning {{setter for 'deprecatedRename' is deprecated: renamed to 'setBetterThing(_:)'}} {{none}}
+deprecatedRename += 1 // expected-warning {{getter for 'deprecatedRename' is deprecated: renamed to 'betterThing()'}} {{none}} expected-warning {{setter for 'deprecatedRename' is deprecated: renamed to 'setBetterThing(_:)'}} {{none}}
+
+_ = deprecatedProperty // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
+deprecatedProperty = 0 // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
+deprecatedProperty += 1 // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
+
+struct DeprecatedAccessors {
+  var deprecatedMessage: Int {
+    @available(*, deprecated, message: "bad getter") get { return 0 }
+    @available(*, deprecated, message: "bad setter") set {}
+  }
+
+  static var staticDeprecated: Int {
+    @available(*, deprecated, message: "bad getter") get { return 0 }
+    @available(*, deprecated, message: "bad setter") set {}
+  }
+
+  @available(*, deprecated, message: "bad property")
+  var deprecatedProperty: Int {
+    @available(*, deprecated, message: "bad getter") get { return 0 }
+    @available(*, deprecated, message: "bad setter") set {}
+  }
+
+  subscript(_: Int) -> Int {
+    @available(*, deprecated, message: "bad subscript getter") get { return 0 }
+    @available(*, deprecated, message: "bad subscript setter") set {}
+  }
+
+  @available(*, deprecated, message: "bad subscript!")
+  subscript(alsoDeprecated _: Int) -> Int {
+    @available(*, deprecated, message: "bad subscript getter") get { return 0 }
+    @available(*, deprecated, message: "bad subscript setter") set {}
+  }
+
+  mutating func testAccessors(other: inout DeprecatedAccessors) {
+    _ = deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
+    deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+    deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+
+    _ = other.deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
+    other.deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+    other.deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
+
+    _ = other.deprecatedProperty // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
+    other.deprecatedProperty = 0 // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
+    other.deprecatedProperty += 1 // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
+
+    _ = DeprecatedAccessors.staticDeprecated // expected-warning {{getter for 'staticDeprecated' is deprecated: bad getter}} {{none}}
+    DeprecatedAccessors.staticDeprecated = 0 // expected-warning {{setter for 'staticDeprecated' is deprecated: bad setter}} {{none}}
+    DeprecatedAccessors.staticDeprecated += 1 // expected-warning {{getter for 'staticDeprecated' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'staticDeprecated' is deprecated: bad setter}} {{none}}
+
+    _ = other[0] // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}} {{none}}
+    other[0] = 0 // expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}} {{none}}
+    other[0] += 1 // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}} {{none}} expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}} {{none}}
+
+    _ = other[alsoDeprecated: 0] // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
+    other[alsoDeprecated: 0] = 0 // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
+    other[alsoDeprecated: 0] += 1 // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
+  }
+}