Sema: Don't apply access level fix-it to @dynamicMemberLookup subscript.

The adjusted access level for the subscript shouldn't be fixed automatically
since sometimes the diagnostic is a warning and other times automatically
applying the fix-it immediately causes other errors, which can be confusing.

Resolves rdar://158261884.
This commit is contained in:
Allan Shortlidge
2025-08-13 23:11:25 -07:00
parent 31f344de84
commit db117bfefe
5 changed files with 38 additions and 17 deletions

View File

@@ -6471,7 +6471,7 @@ void swift::performStmtDiagnostics(const Stmt *S, DeclContext *DC) {
void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD, void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
AccessLevel desiredAccess, bool isForSetter, AccessLevel desiredAccess, bool isForSetter,
bool shouldUseDefaultAccess) { bool shouldUseDefaultAccess, bool updateAttr) {
StringRef fixItString; StringRef fixItString;
switch (desiredAccess) { switch (desiredAccess) {
case AccessLevel::Private: fixItString = "private "; break; case AccessLevel::Private: fixItString = "private "; break;
@@ -6486,27 +6486,35 @@ void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
AbstractAccessControlAttr *attr; AbstractAccessControlAttr *attr;
if (isForSetter) { if (isForSetter) {
attr = attrs.getAttribute<SetterAccessAttr>(); attr = attrs.getAttribute<SetterAccessAttr>();
cast<AbstractStorageDecl>(VD)->overwriteSetterAccess(desiredAccess); if (updateAttr)
cast<AbstractStorageDecl>(VD)->overwriteSetterAccess(desiredAccess);
} else { } else {
attr = attrs.getAttribute<AccessControlAttr>(); attr = attrs.getAttribute<AccessControlAttr>();
VD->overwriteAccess(desiredAccess); if (updateAttr)
VD->overwriteAccess(desiredAccess);
if (auto *ASD = dyn_cast<AbstractStorageDecl>(VD)) { if (auto *ASD = dyn_cast<AbstractStorageDecl>(VD)) {
if (auto *getter = ASD->getAccessor(AccessorKind::Get)) if (auto *getter = ASD->getAccessor(AccessorKind::Get)) {
getter->overwriteAccess(desiredAccess); if (updateAttr)
getter->overwriteAccess(desiredAccess);
}
if (auto *setterAttr = attrs.getAttribute<SetterAccessAttr>()) { if (auto *setterAttr = attrs.getAttribute<SetterAccessAttr>()) {
if (setterAttr->getAccess() > desiredAccess) if (setterAttr->getAccess() > desiredAccess)
fixItAccess(diag, VD, desiredAccess, true); fixItAccess(diag, VD, desiredAccess, /*isForSetter=*/true,
/*shouldUseDefaultAccess=*/false, updateAttr);
} else { } else {
ASD->overwriteSetterAccess(desiredAccess); if (updateAttr)
ASD->overwriteSetterAccess(desiredAccess);
} }
} }
} }
if (isForSetter && VD->getFormalAccess() == desiredAccess) { if (isForSetter && VD->getFormalAccess() == desiredAccess) {
assert(attr); assert(attr);
attr->setInvalid(); if (updateAttr)
attr->setInvalid();
// Remove the setter attribute. // Remove the setter attribute.
diag.fixItRemove(attr->Range); diag.fixItRemove(attr->Range);
@@ -6526,7 +6534,8 @@ void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
// replace the "(set)" part of a setter attribute. // replace the "(set)" part of a setter attribute.
diag.fixItReplace(attr->getLocation(), fixItString.drop_back()); diag.fixItReplace(attr->getLocation(), fixItString.drop_back());
} }
attr->setInvalid(); if (updateAttr)
attr->setInvalid();
} }
} else if (auto *override = VD->getAttrs().getAttribute<OverrideAttr>()) { } else if (auto *override = VD->getAttrs().getAttribute<OverrideAttr>()) {

View File

@@ -53,10 +53,10 @@ namespace swift {
/// Emit a fix-it to set the access of \p VD to \p desiredAccess. /// Emit a fix-it to set the access of \p VD to \p desiredAccess.
/// ///
/// This actually updates \p VD as well. /// This actually updates \p VD as well if \p updateAttr is true.
void fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD, void fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
AccessLevel desiredAccess, bool isForSetter = false, AccessLevel desiredAccess, bool isForSetter = false,
bool shouldUseDefaultAccess = false); bool shouldUseDefaultAccess = false, bool updateAttr = true);
/// Compute the location of the 'var' keyword for a 'var'-to-'let' Fix-It. /// Compute the location of the 'var' keyword for a 'var'-to-'let' Fix-It.
SourceLoc getFixItLocForVarToLet(VarDecl *var); SourceLoc getFixItLocForVarToLet(VarDecl *var);

View File

@@ -2171,7 +2171,9 @@ visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) {
diag::dynamic_member_lookup_candidate_inaccessible, diag::dynamic_member_lookup_candidate_inaccessible,
inaccessibleCandidate); inaccessibleCandidate);
fixItAccess(diag, inaccessibleCandidate, fixItAccess(diag, inaccessibleCandidate,
requiredAccessScope.requiredAccessForDiagnostics()); requiredAccessScope.requiredAccessForDiagnostics(),
/*isForSetter=*/false, /*useDefaultAccess=*/false,
/*updateAttr=*/false);
diag.warnUntilSwiftVersionIf(!shouldError, futureVersion); diag.warnUntilSwiftVersionIf(!shouldError, futureVersion);
if (shouldError) { if (shouldError) {

View File

@@ -253,9 +253,8 @@ public struct Inaccessible1 {
@dynamicMemberLookup @dynamicMemberLookup
public struct Inaccessible2 { public struct Inaccessible2 {
// expected-non-resilient-warning @+3 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{21-29=public}} // expected-non-resilient-warning @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{21-29=public}}
// expected-resilient-error @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}} // expected-resilient-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}}
// expected-error @+1 {{'@usableFromInline' attribute can only be applied to internal or package declarations, but subscript 'subscript(dynamicMember:)' is public}}
@usableFromInline internal subscript(dynamicMember member: String) -> Int { @usableFromInline internal subscript(dynamicMember member: String) -> Int {
return 42 return 42
} }
@@ -279,6 +278,18 @@ private struct Inaccessible4 {
} }
} }
@dynamicMemberLookup
public struct Inaccessible5 {
internal struct Nested { }
var nested: Nested
// expected-non-resilient-warning @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{3-11=public}}
// expected-resilient-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-11=public}}
internal subscript<Value>(dynamicMember keyPath: KeyPath<Nested, Value>) -> Value {
nested[keyPath: keyPath]
}
}
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// Existentials // Existentials
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//

View File

@@ -50,8 +50,7 @@ public struct Inaccessible1 {
@dynamicMemberLookup @dynamicMemberLookup
public struct Inaccessible2 { public struct Inaccessible2 {
// expected-error @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}} // expected-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}}
// expected-error @+1 {{'@usableFromInline' attribute can only be applied to internal or package declarations, but subscript 'subscript(dynamicMember:)' is public}}
@usableFromInline internal subscript(dynamicMember member: String) -> Int { @usableFromInline internal subscript(dynamicMember member: String) -> Int {
return 42 return 42
} }