Merge pull request #83718 from tshortli/dont-automatically-apply-dynamic-member-lookup-subscript-access-fix-it

Author: tshortli

lib/Sema/MiscDiagnostics.cpp

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

lib/Sema/MiscDiagnostics.h

@@ -53,10 +53,10 @@ namespace swift {
 
   /// 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,
                    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.
   SourceLoc getFixItLocForVarToLet(VarDecl *var);

lib/Sema/TypeCheckAttr.cpp

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

test/attr/attr_dynamic_member_lookup.swift

@@ -253,9 +253,8 @@ public struct Inaccessible1 {
 
 @dynamicMemberLookup
 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-resilient-error @+2 {{'@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}}
+  // 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 @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}}
   @usableFromInline internal subscript(dynamicMember member: String) -> Int {
     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
 //===----------------------------------------------------------------------===//

test/attr/attr_dynamic_member_lookup_swift7.swift

@@ -50,8 +50,7 @@ public struct Inaccessible1 {
 
 @dynamicMemberLookup
 public struct Inaccessible2 {
-  // expected-error @+2 {{'@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}}
+  // expected-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}}
   @usableFromInline internal subscript(dynamicMember member: String) -> Int {
     return 42
   }