Merge pull request #24277 from AnthonyLatsis/mutability-proto-stubs

ASTPrinter: Mutability fixes for protocol stubs

Author: jrose-apple

lib/AST/ASTPrinter.cpp

@@ -950,6 +950,9 @@ void PrintAST::printAttributes(const Decl *D) {
 #define CONTEXTUAL_SIMPLE_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
 #define CONTEXTUAL_DECL_ATTR_ALIAS(X, Class) EXCLUDE_ATTR(Class)
 #include "swift/AST/Attr.def"
+    } else if (isa<FuncDecl>(D)) {
+      Options.ExcludeAttrList.push_back(DAK_Mutating);
+      Options.ExcludeAttrList.push_back(DAK_NonMutating);
     }
 
     // If the declaration is implicitly @objc, print the attribute now.
@@ -978,12 +981,6 @@ void PrintAST::printAttributes(const Decl *D) {
     }
   }
 
-  // Explicitly print 'mutating' and 'nonmutating' before getters and setters
-  // for which that is true.
-  if (auto accessor = dyn_cast<AccessorDecl>(D)) {
-    printMutatingModifiersIfNeeded(accessor);
-  }
-
   Options.ExcludeAttrList.resize(originalExcludeAttrCount);
 }
 
@@ -1706,9 +1703,11 @@ void PrintAST::printBodyIfNecessary(const AbstractFunctionDecl *decl) {
 }
 
 void PrintAST::printMutatingModifiersIfNeeded(const AccessorDecl *accessor) {
-  if (accessor->isAssumedNonMutating() && accessor->isMutating()) {
+  if (accessor->isAssumedNonMutating() && accessor->isMutating() &&
+      !Options.excludeAttrKind(DAK_Mutating)) {
     Printer.printKeyword("mutating", Options, " ");
-  } else if (accessor->isExplicitNonMutating()) {
+  } else if (accessor->isExplicitNonMutating() &&
+             !Options.excludeAttrKind(DAK_NonMutating)) {
     Printer.printKeyword("nonmutating", Options, " ");
   }
 }
@@ -2717,6 +2716,10 @@ bool PrintAST::printASTNodes(const ArrayRef<ASTNode> &Elements,
 void PrintAST::visitAccessorDecl(AccessorDecl *decl) {
   printDocumentationComment(decl);
   printAttributes(decl);
+
+  // Explicitly print 'mutating' and 'nonmutating' before getters and setters
+  // for which that is true.
+  printMutatingModifiersIfNeeded(decl);
   switch (auto kind = decl->getAccessorKind()) {
   case AccessorKind::Get:
   case AccessorKind::Address:
@@ -2773,7 +2776,7 @@ void PrintAST::visitFuncDecl(FuncDecl *decl) {
     if (!Options.SkipIntroducerKeywords) {
       if (decl->isStatic() && Options.PrintStaticKeyword)
         printStaticKeyword(decl->getCorrectStaticSpelling());
-      if (decl->isMutating() && !decl->getAttrs().hasAttribute<MutatingAttr>()) {
+      if (decl->isMutating() && !Options.excludeAttrKind(DAK_Mutating)) {
         Printer.printKeyword("mutating", Options, " ");
       } else if (decl->isConsuming() && !decl->getAttrs().hasAttribute<ConsumingAttr>()) {
         Printer.printKeyword("__consuming", Options, " ");

lib/Sema/TypeCheckProtocol.cpp

@@ -2627,6 +2627,15 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
     Options.FunctionDefinitions = true;
     Options.PrintAccessorBodiesInProtocols = true;
 
+    // Skip 'mutating' only inside classes: mutating methods usually
+    // don't have a sensible non-mutating implementation.
+    bool isClass = Adopter->getSelfClassDecl() != nullptr;
+    if (isClass)
+      Options.ExcludeAttrList.push_back(DAK_Mutating);
+    // 'nonmutating' is only meaningful on value type member accessors.
+    if (isClass || !isa<AbstractStorageDecl>(Requirement))
+      Options.ExcludeAttrList.push_back(DAK_NonMutating);
+
     // FIXME: Once we support move-only types, remove this if the
     //        conforming type is move-only. Until then, don't suggest printing
     //        __consuming on a protocol requirement.

test/decl/protocol/conforms/Inputs/fixit_stub_mutability_proto_module.swift

@@ -0,0 +1,4 @@
+public protocol ExternalMutabilityProto {
+    mutating func foo()
+    subscript() -> Int { mutating get nonmutating set }
+}

test/decl/protocol/conforms/fixit_stub_editor.swift

@@ -1,4 +1,7 @@
-// RUN: %target-typecheck-verify-swift -diagnostics-editor-mode
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %S/Inputs/fixit_stub_mutability_proto_module.swift -emit-module -parse-as-library -o %t
+
+// RUN: %target-swift-frontend -typecheck %s -I %t -verify -diagnostics-editor-mode
 
 protocol P1 {
   @available(*, deprecated)
@@ -26,3 +29,31 @@ protocol P4 : P3 {
 }
 
 class C2 : P4 {} // expected-error{{type 'C2' does not conform to protocol 'P4'}} expected-error{{type 'C2' does not conform to protocol 'P3'}} expected-note{{do you want to add protocol stubs?}}{{16-16=\n    typealias T1 = <#type#>\n\n    typealias T2 = <#type#>\n\n    typealias T3 = <#type#>\n}}
+
+
+// =============================================================================
+// Test how we print stubs for mutating and non-mutating requirements.
+//
+// - Test that we don't print 'mutating' in classes.
+// - Test that we print 'non-mutating' for non-mutating setters
+//   in structs.
+// =============================================================================
+
+protocol LocalMutabilityProto {
+    mutating func foo()
+    subscript() -> Int { get nonmutating set }
+}
+
+class Class1: LocalMutabilityProto { // expected-error{{type 'Class1' does not conform to protocol 'LocalMutabilityProto'}} expected-note{{do you want to add protocol stubs?}} {{37-37=\n    func foo() {\n        <#code#>\n    \}\n\n    subscript() -> Int {\n        get {\n            <#code#>\n        \}\n        set {\n            <#code#>\n        \}\n    \}\n}}
+}
+
+struct Struct1: LocalMutabilityProto { // expected-error{{type 'Struct1' does not conform to protocol 'LocalMutabilityProto'}} expected-note{{do you want to add protocol stubs?}} {{39-39=\n    mutating func foo() {\n        <#code#>\n    \}\n\n    subscript() -> Int {\n        get {\n            <#code#>\n        \}\n        nonmutating set {\n            <#code#>\n        \}\n    \}\n}}
+}
+
+import fixit_stub_mutability_proto_module
+
+class Class2: ExternalMutabilityProto { // expected-error{{type 'Class2' does not conform to protocol 'ExternalMutabilityProto'}} expected-note{{do you want to add protocol stubs?}} {{40-40=\n    func foo() {\n        <#code#>\n    \}\n\n    subscript() -> Int {\n        get {\n            <#code#>\n        \}\n        set(newValue) {\n            <#code#>\n        \}\n    \}\n}}
+}
+
+struct Struct2: ExternalMutabilityProto { // expected-error{{type 'Struct2' does not conform to protocol 'ExternalMutabilityProto'}} expected-note{{do you want to add protocol stubs?}} {{42-42=\n    mutating func foo() {\n        <#code#>\n    \}\n\n    subscript() -> Int {\n        mutating get {\n            <#code#>\n        \}\n        nonmutating set(newValue) {\n            <#code#>\n        \}\n    \}\n}}
+}