Skip to content

Check deprecation for getters and setters #15102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 16, 2018
Merged

Check deprecation for getters and setters #15102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9362bc8
Turn several helper methods on TypeChecker into static functions.
jrose-apple Mar 9, 2018
b0e12f4
Check deprecation for getters and setters
jrose-apple Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{| %select{on|in}3 %2%select{| %4}3}1",
(DeclName, bool, StringRef, bool, clang::VersionTuple))
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
"deprecated%select{| %select{on|in}4 %3%select{| %5}4}2",
(unsigned, DeclName, bool, StringRef, bool, clang::VersionTuple))

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

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

NOTE(note_deprecated_rename, none,
Expand Down
100 changes: 64 additions & 36 deletions lib/Sema/TypeCheckAvailability.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,10 +1404,7 @@ const AvailableAttr *TypeChecker::getDeprecated(const Decl *D) {
static bool
someEnclosingDeclMatches(SourceRange ReferenceRange,
const DeclContext *ReferenceDC,
TypeChecker &TC,
llvm::function_ref<bool(const Decl *)> Pred) {
ASTContext &Ctx = TC.Context;

// Climb the DeclContext hierarchy to see if any of the containing
// declarations matches the predicate.
const DeclContext *DC = ReferenceDC;
Expand Down Expand Up @@ -1446,6 +1443,7 @@ someEnclosingDeclMatches(SourceRange ReferenceRange,
if (ReferenceRange.isInvalid())
return false;

ASTContext &Ctx = ReferenceDC->getASTContext();
const Decl *DeclToSearch =
findContainingDeclaration(ReferenceRange, ReferenceDC, Ctx.SourceMgr);

Expand Down Expand Up @@ -1474,28 +1472,32 @@ someEnclosingDeclMatches(SourceRange ReferenceRange,
return false;
}

bool TypeChecker::isInsideImplicitFunction(SourceRange ReferenceRange,
const DeclContext *DC) {
/// Returns true if the reference or any of its parents is an
/// implicit function.
static bool isInsideImplicitFunction(SourceRange ReferenceRange,
const DeclContext *DC) {
auto IsInsideImplicitFunc = [](const Decl *D) {
auto *AFD = dyn_cast<AbstractFunctionDecl>(D);
return AFD && AFD->isImplicit();
};

return someEnclosingDeclMatches(ReferenceRange, DC, *this,
IsInsideImplicitFunc);
return someEnclosingDeclMatches(ReferenceRange, DC, IsInsideImplicitFunc);
}

bool TypeChecker::isInsideUnavailableDeclaration(
SourceRange ReferenceRange, const DeclContext *ReferenceDC) {
/// Returns true if the reference or any of its parents is an
/// unavailable (or obsoleted) declaration.
static bool isInsideUnavailableDeclaration(SourceRange ReferenceRange,
const DeclContext *ReferenceDC) {
auto IsUnavailable = [](const Decl *D) {
return D->getAttrs().getUnavailable(D->getASTContext());
};

return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, *this,
IsUnavailable);
return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, IsUnavailable);
}

bool TypeChecker::isInsideCompatibleUnavailableDeclaration(
/// Returns true if the reference or any of its parents is an
/// unconditional unavailable declaration for the same platform.
static bool isInsideCompatibleUnavailableDeclaration(
SourceRange ReferenceRange, const DeclContext *ReferenceDC,
const AvailableAttr *attr) {
if (!attr->isUnconditionallyUnavailable()) {
Expand All @@ -1512,18 +1514,18 @@ bool TypeChecker::isInsideCompatibleUnavailableDeclaration(
return EnclosingUnavailable && EnclosingUnavailable->Platform == platform;
};

return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, *this,
IsUnavailable);
return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, IsUnavailable);
}

bool TypeChecker::isInsideDeprecatedDeclaration(SourceRange ReferenceRange,
const DeclContext *ReferenceDC){
/// Returns true if the reference is lexically contained in a declaration
/// that is deprecated on all deployment targets.
static bool isInsideDeprecatedDeclaration(SourceRange ReferenceRange,
const DeclContext *ReferenceDC){
auto IsDeprecated = [](const Decl *D) {
return D->getAttrs().getDeprecated(D->getASTContext());
};

return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, *this,
IsDeprecated);
return someEnclosingDeclMatches(ReferenceRange, ReferenceDC, IsDeprecated);
}

static void fixItAvailableAttrRename(TypeChecker &TC,
Expand Down Expand Up @@ -1930,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());
Expand All @@ -1951,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);
Expand Down Expand Up @@ -2232,9 +2247,11 @@ class AvailabilityWalker : public ASTWalker {
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();
Expand All @@ -2250,8 +2267,10 @@ class AvailabilityWalker : public ASTWalker {
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);
Expand Down Expand Up @@ -2334,20 +2353,17 @@ class AvailabilityWalker : public ASTWalker {
/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do something silly here, where we keep track of the operands of InOutExpr nodes and if E is in that set, we also check the setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use LValueAccessKind if Slava doesn't take it out first! But in a separate PR.

walkInContext(E, E->getBase(), MemberAccessContext::Getter);

ValueDecl *D = E->getMember().getDecl();
// Diagnose for the member declaration itself.
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);
}
// Diagnose for appropriate accessors, given the access context.
maybeDiagStorageAccess(D, E->getSourceRange(), DC);
}

/// Walk an inout expression, checking for availability.
Expand All @@ -2365,9 +2381,16 @@ class AvailabilityWalker : public ASTWalker {

/// Emit diagnostics, if necessary, for accesses to storage where
/// the accessor for the AccessContext is not available.
void diagStorageAccess(AbstractStorageDecl *D,
SourceRange ReferenceRange,
const DeclContext *ReferenceDC) const {
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;
}
Expand Down Expand Up @@ -2395,13 +2418,18 @@ class AvailabilityWalker : public ASTWalker {
}

/// 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()) {
Expand Down
21 changes: 0 additions & 21 deletions lib/Sema/TypeChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2404,27 +2404,6 @@ class TypeChecker final : public LazyResolver {
const DeclContext *ReferenceDC, const UnavailabilityReason &Reason,
bool ForInout);

/// Returns true if the reference or any of its parents is an
/// implicit function.
bool isInsideImplicitFunction(SourceRange ReferenceRange,
const DeclContext *DC);

/// Returns true if the reference or any of its parents is an
/// unavailable (or obsoleted) declaration.
bool isInsideUnavailableDeclaration(SourceRange ReferenceRange,
const DeclContext *DC);

/// Returns true if the reference or any of its parents is an
/// unconditional unavailable declaration for the same platform.
bool isInsideCompatibleUnavailableDeclaration(SourceRange ReferenceRange,
const DeclContext *DC,
const AvailableAttr *attr);

/// Returns true if the reference is lexically contained in a declaration
/// that is deprecated on all deployment targets.
bool isInsideDeprecatedDeclaration(SourceRange ReferenceRange,
const DeclContext *DC);

/// Returns the availability attribute indicating deprecation if the
/// declaration is deprecated or null otherwise.
static const AvailableAttr *getDeprecated(const Decl *D);
Expand Down
14 changes: 14 additions & 0 deletions test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 21 additions & 1 deletion test/ClangImporter/availability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
Loading