Skip to content

Commit b0e12f4

Browse files
jrose-applejrose-apple
committed
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
1 parent 9362bc8 commit b0e12f4

File tree

5 files changed

+204
-31
lines changed

5 files changed

+204
-31
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,22 +3590,23 @@ NOTE(availability_obsoleted, none,
35903590
(DeclName, StringRef, clang::VersionTuple))
35913591

35923592
WARNING(availability_deprecated, none,
3593-
"%0 %select{is|%select{is|was}3}1 deprecated"
3594-
"%select{| %select{on|in}3 %2%select{| %4}3}1",
3595-
(DeclName, bool, StringRef, bool, clang::VersionTuple))
3593+
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
3594+
"deprecated%select{| %select{on|in}4 %3%select{| %5}4}2",
3595+
(unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple))
35963596

35973597
WARNING(availability_deprecated_msg, none,
3598-
"%0 %select{is|%select{is|was}3}1 deprecated"
3599-
"%select{| %select{on|in}3 %2%select{| %4}3}1: %5",
3600-
(DeclName, bool, StringRef, bool, clang::VersionTuple, StringRef))
3598+
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
3599+
"deprecated%select{| %select{on|in}4 %3%select{| %5}4}2: %6",
3600+
(unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple,
3601+
StringRef))
36013602

36023603
WARNING(availability_deprecated_rename, none,
3603-
"%0 %select{is|%select{is|was}3}1 deprecated"
3604-
"%select{| %select{on|in}3 %2%select{| %4}3}1: "
3605-
"%select{renamed to|replaced by}5%" REPLACEMENT_DECL_KIND_SELECT "6 "
3606-
"'%7'",
3607-
(DeclName, bool, StringRef, bool, clang::VersionTuple, bool, unsigned,
3608-
StringRef))
3604+
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
3605+
"deprecated%select{| %select{on|in}4 %3%select{| %5}4}2: "
3606+
"%select{renamed to|replaced by}6%" REPLACEMENT_DECL_KIND_SELECT "7 "
3607+
"'%8'",
3608+
(unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple, bool,
3609+
unsigned, StringRef))
36093610
#undef REPLACEMENT_DECL_KIND_SELECT
36103611

36113612
NOTE(note_deprecated_rename, none,

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,14 +1932,25 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
19321932
}
19331933
}
19341934

1935-
DeclName Name = DeprecatedDecl->getFullName();
1935+
DeclName Name;
1936+
Optional<unsigned> rawAccessorKind;
1937+
if (auto *accessor = dyn_cast<AccessorDecl>(DeprecatedDecl)) {
1938+
Name = accessor->getStorage()->getFullName();
1939+
assert(accessor->isGetterOrSetter());
1940+
rawAccessorKind = static_cast<unsigned>(accessor->getAccessorKind());
1941+
} else {
1942+
Name = DeprecatedDecl->getFullName();
1943+
}
1944+
19361945
StringRef Platform = Attr->prettyPlatformString();
19371946
clang::VersionTuple DeprecatedVersion;
19381947
if (Attr->Deprecated)
19391948
DeprecatedVersion = Attr->Deprecated.getValue();
19401949

1950+
static const unsigned NOT_ACCESSOR_INDEX = 2;
19411951
if (Attr->Message.empty() && Attr->Rename.empty()) {
1942-
diagnose(ReferenceRange.Start, diag::availability_deprecated, Name,
1952+
diagnose(ReferenceRange.Start, diag::availability_deprecated,
1953+
rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
19431954
Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
19441955
DeprecatedVersion)
19451956
.highlight(Attr->getRange());
@@ -1953,21 +1964,23 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
19531964

19541965
if (!Attr->Message.empty()) {
19551966
EncodedDiagnosticMessage EncodedMessage(Attr->Message);
1956-
diagnose(ReferenceRange.Start, diag::availability_deprecated_msg, Name,
1967+
diagnose(ReferenceRange.Start, diag::availability_deprecated_msg,
1968+
rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
19571969
Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
19581970
DeprecatedVersion, EncodedMessage.Message)
19591971
.highlight(Attr->getRange());
19601972
} else {
19611973
unsigned rawReplaceKind = static_cast<unsigned>(
19621974
replacementDeclKind.getValueOr(ReplacementDeclKind::None));
1963-
diagnose(ReferenceRange.Start, diag::availability_deprecated_rename, Name,
1975+
diagnose(ReferenceRange.Start, diag::availability_deprecated_rename,
1976+
rawAccessorKind.getValueOr(NOT_ACCESSOR_INDEX), Name,
19641977
Attr->hasPlatform(), Platform, Attr->Deprecated.hasValue(),
19651978
DeprecatedVersion, replacementDeclKind.hasValue(), rawReplaceKind,
19661979
newName)
19671980
.highlight(Attr->getRange());
19681981
}
19691982

1970-
if (!Attr->Rename.empty()) {
1983+
if (!Attr->Rename.empty() && !rawAccessorKind.hasValue()) {
19711984
auto renameDiag = diagnose(ReferenceRange.Start,
19721985
diag::note_deprecated_rename,
19731986
newName);
@@ -2234,9 +2247,11 @@ class AvailabilityWalker : public ASTWalker {
22342247
return std::make_pair(false, E);
22352248
};
22362249

2237-
if (auto DR = dyn_cast<DeclRefExpr>(E))
2250+
if (auto DR = dyn_cast<DeclRefExpr>(E)) {
22382251
diagAvailability(DR->getDecl(), DR->getSourceRange(),
22392252
getEnclosingApplyExpr());
2253+
maybeDiagStorageAccess(DR->getDecl(), DR->getSourceRange(), DC);
2254+
}
22402255
if (auto MR = dyn_cast<MemberRefExpr>(E)) {
22412256
walkMemberRef(MR);
22422257
return skipChildren();
@@ -2252,8 +2267,10 @@ class AvailabilityWalker : public ASTWalker {
22522267
if (auto DS = dyn_cast<DynamicSubscriptExpr>(E))
22532268
diagAvailability(DS->getMember().getDecl(), DS->getSourceRange());
22542269
if (auto S = dyn_cast<SubscriptExpr>(E)) {
2255-
if (S->hasDecl())
2270+
if (S->hasDecl()) {
22562271
diagAvailability(S->getDecl().getDecl(), S->getSourceRange());
2272+
maybeDiagStorageAccess(S->getDecl().getDecl(), S->getSourceRange(), DC);
2273+
}
22572274
}
22582275
if (auto A = dyn_cast<AssignExpr>(E)) {
22592276
walkAssignExpr(A);
@@ -2336,20 +2353,17 @@ class AvailabilityWalker : public ASTWalker {
23362353
/// Walk a member reference expression, checking for availability.
23372354
void walkMemberRef(MemberRefExpr *E) {
23382355
// Walk the base in a getter context.
2356+
// FIXME: We may need to look at the setter too, if we're going to do
2357+
// writeback. The AST should have this information.
23392358
walkInContext(E, E->getBase(), MemberAccessContext::Getter);
23402359

23412360
ValueDecl *D = E->getMember().getDecl();
23422361
// Diagnose for the member declaration itself.
23432362
if (diagAvailability(D, E->getNameLoc().getSourceRange()))
23442363
return;
23452364

2346-
if (TC.getLangOpts().DisableAvailabilityChecking)
2347-
return;
2348-
2349-
if (auto *ASD = dyn_cast<AbstractStorageDecl>(D)) {
2350-
// Diagnose for appropriate accessors, given the access context.
2351-
diagStorageAccess(ASD, E->getSourceRange(), DC);
2352-
}
2365+
// Diagnose for appropriate accessors, given the access context.
2366+
maybeDiagStorageAccess(D, E->getSourceRange(), DC);
23532367
}
23542368

23552369
/// Walk an inout expression, checking for availability.
@@ -2367,9 +2381,16 @@ class AvailabilityWalker : public ASTWalker {
23672381

23682382
/// Emit diagnostics, if necessary, for accesses to storage where
23692383
/// the accessor for the AccessContext is not available.
2370-
void diagStorageAccess(AbstractStorageDecl *D,
2371-
SourceRange ReferenceRange,
2372-
const DeclContext *ReferenceDC) const {
2384+
void maybeDiagStorageAccess(const ValueDecl *VD,
2385+
SourceRange ReferenceRange,
2386+
const DeclContext *ReferenceDC) const {
2387+
if (TC.getLangOpts().DisableAvailabilityChecking)
2388+
return;
2389+
2390+
auto *D = dyn_cast<AbstractStorageDecl>(VD);
2391+
if (!D)
2392+
return;
2393+
23732394
if (!D->hasAccessorFunctions()) {
23742395
return;
23752396
}
@@ -2397,13 +2418,18 @@ class AvailabilityWalker : public ASTWalker {
23972418
}
23982419

23992420
/// Emit a diagnostic, if necessary for a potentially unavailable accessor.
2400-
/// Returns true if a diagnostic was emitted.
24012421
void diagAccessorAvailability(AccessorDecl *D, SourceRange ReferenceRange,
24022422
const DeclContext *ReferenceDC,
24032423
bool ForInout) const {
24042424
if (!D) {
24052425
return;
24062426
}
2427+
2428+
// Make sure not to diagnose an accessor if we already complained about
2429+
// the property/subscript.
2430+
if (!TypeChecker::getDeprecated(D->getStorage()))
2431+
TC.diagnoseIfDeprecated(ReferenceRange, ReferenceDC, D, /*call*/nullptr);
2432+
24072433
auto MaybeUnavail = TC.checkDeclarationAvailability(D, ReferenceRange.Start,
24082434
DC);
24092435
if (MaybeUnavail.hasValue()) {

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,17 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
9393
NSEnumAddedCasesIn2017ExistingCaseThree,
9494
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)))
9595
};
96+
97+
@interface AccessorDeprecations: NSObject
98+
@property int fullyDeprecated __attribute__((deprecated));
99+
100+
@property int getterDeprecated;
101+
- (int)getterDeprecated __attribute__((deprecated));
102+
@property (class) int getterDeprecatedClass;
103+
+ (int)getterDeprecatedClass __attribute__((deprecated));
104+
105+
@property int setterDeprecated;
106+
- (void)setSetterDeprecated:(int)setterDeprecated __attribute__((deprecated));
107+
@property (class) int setterDeprecatedClass;
108+
+ (void)setSetterDeprecatedClass:(int)setterDeprecated __attribute__((deprecated));
109+
@end

‎test/ClangImporter/availability.swift

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,28 @@ func test_unavailable_func(_ x : NSObject) {
2525
NSDeallocateObject(x) // expected-error {{'NSDeallocateObject' is unavailable}}
2626
}
2727

28-
func test_deprecated_imported_as_unavailable(_ s:UnsafeMutablePointer<CChar>) {
28+
func test_deprecated(_ s:UnsafeMutablePointer<CChar>, _ obj: AccessorDeprecations) {
2929
_ = 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.}}
30+
31+
_ = obj.fullyDeprecated // expected-warning {{'fullyDeprecated' is deprecated}}
32+
obj.fullyDeprecated = 0 // expected-warning {{'fullyDeprecated' is deprecated}}
33+
obj.fullyDeprecated += 1 // expected-warning {{'fullyDeprecated' is deprecated}}
34+
35+
_ = obj.getterDeprecated // expected-warning {{getter for 'getterDeprecated' is deprecated}}
36+
obj.getterDeprecated = 0
37+
obj.getterDeprecated += 1 // expected-warning {{getter for 'getterDeprecated' is deprecated}}
38+
39+
_ = AccessorDeprecations.getterDeprecatedClass // expected-warning {{getter for 'getterDeprecatedClass' is deprecated}}
40+
AccessorDeprecations.getterDeprecatedClass = 0
41+
AccessorDeprecations.getterDeprecatedClass += 1 // expected-warning {{getter for 'getterDeprecatedClass' is deprecated}}
42+
43+
_ = obj.setterDeprecated
44+
obj.setterDeprecated = 0 // expected-warning {{setter for 'setterDeprecated' is deprecated}}
45+
obj.setterDeprecated += 1 // expected-warning {{setter for 'setterDeprecated' is deprecated}}
46+
47+
_ = AccessorDeprecations.setterDeprecatedClass
48+
AccessorDeprecations.setterDeprecatedClass = 0 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
49+
AccessorDeprecations.setterDeprecatedClass += 1 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
3050
}
3151

3252
func test_NSInvocation(_ x: NSInvocation, // expected-error {{'NSInvocation' is unavailable}}

‎test/attr/attr_availability.swift

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,8 +808,120 @@ func rdar32526620_3(a: Int, b: E_32526620, c: String) {} // expected-note {{here
808808
rdar32526620_3(a: 42, b: .bar, c: "question")
809809
// 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=}}
810810

811+
811812
@available(*, unavailable) // expected-warning {{'@available' without an OS is ignored on extensions; apply the attribute to each member instead}} {{1-28=}}
812813
extension DummyType {}
813814

814815
@available(*, deprecated) // expected-warning {{'@available' without an OS is ignored on extensions; apply the attribute to each member instead}} {{1-27=}}
815816
extension DummyType {}
817+
818+
819+
var deprecatedGetter: Int {
820+
@available(*, deprecated) get { return 0 }
821+
set {}
822+
}
823+
var deprecatedGetterOnly: Int {
824+
@available(*, deprecated) get { return 0 }
825+
}
826+
var deprecatedSetter: Int {
827+
get { return 0 }
828+
@available(*, deprecated) set {}
829+
}
830+
var deprecatedBoth: Int {
831+
@available(*, deprecated) get { return 0 }
832+
@available(*, deprecated) set {}
833+
}
834+
var deprecatedMessage: Int {
835+
@available(*, deprecated, message: "bad getter") get { return 0 }
836+
@available(*, deprecated, message: "bad setter") set {}
837+
}
838+
var deprecatedRename: Int {
839+
@available(*, deprecated, renamed: "betterThing()") get { return 0 }
840+
@available(*, deprecated, renamed: "setBetterThing(_:)") set {}
841+
}
842+
@available(*, deprecated, message: "bad variable")
843+
var deprecatedProperty: Int {
844+
@available(*, deprecated, message: "bad getter") get { return 0 }
845+
@available(*, deprecated, message: "bad setter") set {}
846+
}
847+
848+
_ = deprecatedGetter // expected-warning {{getter for 'deprecatedGetter' is deprecated}} {{none}}
849+
deprecatedGetter = 0
850+
deprecatedGetter += 1 // expected-warning {{getter for 'deprecatedGetter' is deprecated}} {{none}}
851+
852+
_ = deprecatedGetterOnly // expected-warning {{getter for 'deprecatedGetterOnly' is deprecated}} {{none}}
853+
854+
_ = deprecatedSetter
855+
deprecatedSetter = 0 // expected-warning {{setter for 'deprecatedSetter' is deprecated}} {{none}}
856+
deprecatedSetter += 1 // expected-warning {{setter for 'deprecatedSetter' is deprecated}} {{none}}
857+
858+
_ = deprecatedBoth // expected-warning {{getter for 'deprecatedBoth' is deprecated}} {{none}}
859+
deprecatedBoth = 0 // expected-warning {{setter for 'deprecatedBoth' is deprecated}} {{none}}
860+
deprecatedBoth += 1 // expected-warning {{getter for 'deprecatedBoth' is deprecated}} {{none}} expected-warning {{setter for 'deprecatedBoth' is deprecated}} {{none}}
861+
862+
_ = deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
863+
deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
864+
deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
865+
866+
_ = deprecatedRename // expected-warning {{getter for 'deprecatedRename' is deprecated: renamed to 'betterThing()'}} {{none}}
867+
deprecatedRename = 0 // expected-warning {{setter for 'deprecatedRename' is deprecated: renamed to 'setBetterThing(_:)'}} {{none}}
868+
deprecatedRename += 1 // expected-warning {{getter for 'deprecatedRename' is deprecated: renamed to 'betterThing()'}} {{none}} expected-warning {{setter for 'deprecatedRename' is deprecated: renamed to 'setBetterThing(_:)'}} {{none}}
869+
870+
_ = deprecatedProperty // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
871+
deprecatedProperty = 0 // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
872+
deprecatedProperty += 1 // expected-warning {{'deprecatedProperty' is deprecated: bad variable}} {{none}}
873+
874+
struct DeprecatedAccessors {
875+
var deprecatedMessage: Int {
876+
@available(*, deprecated, message: "bad getter") get { return 0 }
877+
@available(*, deprecated, message: "bad setter") set {}
878+
}
879+
880+
static var staticDeprecated: Int {
881+
@available(*, deprecated, message: "bad getter") get { return 0 }
882+
@available(*, deprecated, message: "bad setter") set {}
883+
}
884+
885+
@available(*, deprecated, message: "bad property")
886+
var deprecatedProperty: Int {
887+
@available(*, deprecated, message: "bad getter") get { return 0 }
888+
@available(*, deprecated, message: "bad setter") set {}
889+
}
890+
891+
subscript(_: Int) -> Int {
892+
@available(*, deprecated, message: "bad subscript getter") get { return 0 }
893+
@available(*, deprecated, message: "bad subscript setter") set {}
894+
}
895+
896+
@available(*, deprecated, message: "bad subscript!")
897+
subscript(alsoDeprecated _: Int) -> Int {
898+
@available(*, deprecated, message: "bad subscript getter") get { return 0 }
899+
@available(*, deprecated, message: "bad subscript setter") set {}
900+
}
901+
902+
mutating func testAccessors(other: inout DeprecatedAccessors) {
903+
_ = deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
904+
deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
905+
deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
906+
907+
_ = other.deprecatedMessage // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}}
908+
other.deprecatedMessage = 0 // expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
909+
other.deprecatedMessage += 1 // expected-warning {{getter for 'deprecatedMessage' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'deprecatedMessage' is deprecated: bad setter}} {{none}}
910+
911+
_ = other.deprecatedProperty // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
912+
other.deprecatedProperty = 0 // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
913+
other.deprecatedProperty += 1 // expected-warning {{'deprecatedProperty' is deprecated: bad property}} {{none}}
914+
915+
_ = DeprecatedAccessors.staticDeprecated // expected-warning {{getter for 'staticDeprecated' is deprecated: bad getter}} {{none}}
916+
DeprecatedAccessors.staticDeprecated = 0 // expected-warning {{setter for 'staticDeprecated' is deprecated: bad setter}} {{none}}
917+
DeprecatedAccessors.staticDeprecated += 1 // expected-warning {{getter for 'staticDeprecated' is deprecated: bad getter}} {{none}} expected-warning {{setter for 'staticDeprecated' is deprecated: bad setter}} {{none}}
918+
919+
_ = other[0] // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}} {{none}}
920+
other[0] = 0 // expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}} {{none}}
921+
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}}
922+
923+
_ = other[alsoDeprecated: 0] // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
924+
other[alsoDeprecated: 0] = 0 // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
925+
other[alsoDeprecated: 0] += 1 // expected-warning {{'subscript(alsoDeprecated:)' is deprecated: bad subscript!}} {{none}}
926+
}
927+
}

0 commit comments

Comments
 (0)