[Sema] Improve diagnostics for use of self access kind modifier on accessors inside classes (swiftlang#33602)

Author: theblixguy

include/swift/AST/DiagnosticsSema.def

@@ -1294,8 +1294,9 @@ ERROR(nominal_type_not_attribute,none,
 
 ERROR(mutating_invalid_global_scope,none, "%0 is only valid on methods",
       (SelfAccessKind))
-ERROR(mutating_invalid_classes,none, "%0 isn't valid on methods in "
-      "classes or class-bound protocols", (SelfAccessKind))
+ERROR(mutating_invalid_classes,none, "%0 is not valid on %1s in " 
+      "%select{classes|class-bound protocols}2",
+      (SelfAccessKind, DescriptiveDeclKind, bool))
 
 ERROR(functions_mutating_and_not,none,
       "method must not be declared both %0 and %1",

lib/Sema/TypeCheckAttr.cpp

@@ -317,14 +317,17 @@ void AttributeChecker::visitMutationAttr(DeclAttribute *attr) {
     llvm_unreachable("unhandled attribute kind");
   }
 
+  auto DC = FD->getDeclContext();
   // mutation attributes may only appear in type context.
-  if (auto contextTy = FD->getDeclContext()->getDeclaredInterfaceType()) {
+  if (auto contextTy = DC->getDeclaredInterfaceType()) {
     // 'mutating' and 'nonmutating' are not valid on types
     // with reference semantics.
     if (contextTy->hasReferenceSemantics()) {
-      if (attrModifier != SelfAccessKind::Consuming)
+      if (attrModifier != SelfAccessKind::Consuming) {
         diagnoseAndRemoveAttr(attr, diag::mutating_invalid_classes,
-                              attrModifier);
+                              attrModifier, FD->getDescriptiveKind(),
+                              DC->getSelfProtocolDecl() != nullptr);
+      }
     }
   } else {
     diagnoseAndRemoveAttr(attr, diag::mutating_invalid_global_scope,

localization/diagnostics/en.yaml

@@ -3350,7 +3350,7 @@
   msg: "%0 is only valid on methods"
 
 - id: mutating_invalid_classes
-  msg: "%0 isn't valid on methods in classes or class-bound protocols"
+  msg: "%0 is not valid on %1s in %select{classes|class-bound protocols}2"
 
 - id: functions_mutating_and_not
   msg: "method must not be declared both %0 and %1"

test/Sema/immutability.swift

@@ -61,13 +61,13 @@ class FooClass {
 
   mutating init(a : Bool) {}     // expected-error {{'mutating' may only be used on 'func' declarations}} {{3-12=}}
 
-  mutating            // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}} {{3-12=}}
+  mutating            // expected-error {{'mutating' is not valid on instance methods in classes}} {{3-12=}}
   func baz() {}
 
-  nonmutating         // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}} {{3-15=}}
+  nonmutating         // expected-error {{'nonmutating' is not valid on instance methods in classes}} {{3-15=}}
   func bay() {}
 
-  mutating nonmutating // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}} expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
+  mutating nonmutating // expected-error {{'mutating' is not valid on instance methods in classes}} expected-error {{'nonmutating' is not valid on instance methods in classes}}
   func bax() {}
 
   var x : Int {
@@ -86,6 +86,15 @@ class FooClass {
     }
   }
 
+  var computed: Int {
+    mutating get { 0 } // expected-error {{'mutating' is not valid on getters in classes}} {{5-14=}}
+    nonmutating set {} // expected-error {{'nonmutating' is not valid on setters in classes}} {{5-17=}}
+  }
+
+  var storedWithObservers: Int = 0 {
+    mutating willSet {} // expected-error {{'mutating' is not valid on willSet observers in classes}} {{5-14=}}
+    nonmutating didSet {}  // expected-error {{'nonmutating' is not valid on didSet observers in classes}} {{5-17=}}
+  }
 }
 
 
@@ -241,7 +250,7 @@ func test_arguments(_ a : Int,
 
 
 protocol ClassBoundProtocolMutating : class {
-  mutating       // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}} {{3-12=}}
+  mutating       // expected-error {{'mutating' is not valid on instance methods in class-bound protocols}} {{3-12=}}
   func f()
 }
 

test/decl/ext/extensions.swift

@@ -267,14 +267,14 @@ extension ImposeClassReq1 where Self: AnyObject {
 
   var wrappingProperty2: Int {
     get { return someProperty }
-    mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
+    mutating set { someProperty = newValue } // expected-error {{'mutating' is not valid on setters in class-bound protocols}}
   }
 
-  mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
+  mutating func foo() { // expected-error {{mutating' is not valid on instance methods in class-bound protocols}}
     someProperty = 1
   }
 
-  nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
+  nonmutating func bar() { // expected-error {{'nonmutating' is not valid on instance methods in class-bound protocols}}
     someProperty = 2
   }
 
@@ -309,14 +309,14 @@ extension ImposeClassReq2 {
 
   var wrappingProperty2: Int {
     get { return someProperty }
-    mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
+    mutating set { someProperty = newValue } // expected-error {{'mutating' is not valid on setters in class-bound protocols}}
   }
 
-  mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
+  mutating func foo() { // expected-error {{mutating' is not valid on instance methods in class-bound protocols}}
     someProperty = 1
   }
 
-  nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
+  nonmutating func bar() { // expected-error {{'nonmutating' is not valid on instance methods in class-bound protocols}}
     someProperty = 2
   }