[Diagnostics] Do not offer a mutating fix-it if we have a mutating protocol requirement

Do not offer a mutating fix-it if we have a mutating protocol requirement and we're assigning to it from an implicitly nonmutating setter inside a protocol extension

Author: theblixguy

include/swift/AST/Decl.h

@@ -5131,7 +5131,8 @@ class VarDecl : public AbstractStorageDecl {
   /// If this is a simple 'let' constant, emit a note with a fixit indicating
   /// that it can be rewritten to a 'var'.  This is used in situations where the
   /// compiler detects obvious attempts to mutate a constant.
-  void emitLetToVarNoteIfSimple(DeclContext *UseDC) const;
+  void emitLetToVarNoteIfSimple(DeclContext *UseDC,
+                                ValueDecl *Anchor = nullptr) const;
 
   /// Returns true if the name is the self identifier and is implicit.
   bool isSelfParameter() const;

lib/AST/Decl.cpp

@@ -5689,7 +5689,8 @@ ObjCSelector VarDecl::getDefaultObjCSetterSelector(ASTContext &ctx,
 /// If this is a simple 'let' constant, emit a note with a fixit indicating
 /// that it can be rewritten to a 'var'.  This is used in situations where the
 /// compiler detects obvious attempts to mutate a constant.
-void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
+void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC,
+                                       ValueDecl *Anchor) const {
   // If it isn't a 'let', don't touch it.
   if (!isLet()) return;
 
@@ -5703,12 +5704,31 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
     if (FD && !FD->isMutating() && !FD->isImplicit() && FD->isInstanceMember()&&
         !FD->getDeclContext()->getDeclaredInterfaceType()
                  ->hasReferenceSemantics()) {
-      // Do not suggest the fix it in implicit getters
+      // Do not suggest the fix-it in implicit getters
       if (auto AD = dyn_cast<AccessorDecl>(FD)) {
         if (AD->isGetter() && !AD->getAccessorKeywordLoc().isValid())
           return;
+
+        // Do not suggest the fix-it if we have an implicitly nonmutating
+        // setter in a protocol extension and we're assigning to a mutating
+        // protocol requirement.
+        if (Anchor && Anchor->isProtocolRequirement() && isa<VarDecl>(Anchor)) {
+          auto requirementVar = cast<VarDecl>(Anchor);
+          auto innermostTyCtx = AD->getInnermostTypeContext();
+          bool isRequirementSetterMutating = requirementVar->isSetterMutating();
+          bool isProtoExtension =
+              innermostTyCtx
+                  ? innermostTyCtx->getExtendedProtocolDecl() != nullptr
+                  : false;
+          bool isImplicitlyNonMutatingSetter =
+              AD->isSetter() && AD->isNonMutating() &&
+              !AD->getAttrs().hasAttribute<NonMutatingAttr>();
+          if (isRequirementSetterMutating && isProtoExtension &&
+              isImplicitlyNonMutatingSetter)
+            return;
+        }
       }
-                   
+
       auto &d = getASTContext().Diags;
       d.diagnose(FD->getFuncLoc(), diag::change_to_mutating,
                  isa<AccessorDecl>(FD))

lib/Sema/CSDiagnostics.cpp

@@ -1468,9 +1468,18 @@ bool AssignmentFailure::diagnoseAsError() {
         }
       }
 
-      // If this is a simple variable marked with a 'let', emit a note to fixit
-      // hint it to 'var'.
-      VD->emitLetToVarNoteIfSimple(DC);
+      // If this is a simple variable marked with a 'let', emit a note to change
+      // it to 'var'.
+      //
+      // We also need a reference to the overload for the anchor, because it's
+      // possible that we're assigning to a mutating protocol property from an
+      // implicitly nonmutating setter in a protocol extension. In that case,
+      // we want to drop the fix-it to add 'mutating' as it's gonna re-trigger
+      // this error.
+      auto overload =
+          getConstraintSystem().findSelectedOverloadFor(getAnchor());
+      auto anchor = overload ? overload->Choice.getDeclOrNull() : nullptr;
+      VD->emitLetToVarNoteIfSimple(DC, anchor);
       return true;
     }
 

test/decl/ext/extensions.swift

@@ -151,7 +151,6 @@ extension DoesNotImposeClassReq_2 where Self : AnyObject {
   var wrappingProperty: String {
     get { property }
     set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
-    // expected-note@-1 {{mark accessor 'mutating' to make 'self' mutable}}
   }
 }