Merge pull request swiftlang#21983 from KingOfBrian/bugfix/SR-964-As-Member-master

[Sema] Report unused newValue when getter is accessed in custom setter

Author: jrose-apple

lib/Sema/MiscDiagnostics.cpp

@@ -2201,7 +2201,7 @@ class VarDeclUsageChecker : public ASTWalker {
   const VarDecl *AssociatedGetter = nullptr;
 
   /// The first reference to the associated getter.
-  const DeclRefExpr *AssociatedGetterDeclRef = nullptr;
+  const Expr *AssociatedGetterRefExpr = nullptr;
 
   /// This is a mapping from VarDecls to the if/while/guard statement that they
   /// occur in, when they are in a pattern in a StmtCondition.
@@ -2468,7 +2468,7 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       if (FD && FD->getAccessorKind() == AccessorKind::Set) {
         auto getter = dyn_cast<VarDecl>(FD->getStorage());
         if ((access & RK_Read) == 0 && AssociatedGetter == getter) {
-          if (auto DRE = AssociatedGetterDeclRef) {
+          if (auto DRE = AssociatedGetterRefExpr) {
             Diags.diagnose(DRE->getLoc(), diag::unused_setter_parameter,
                            var->getName());
             Diags.diagnose(DRE->getLoc(), diag::fixit_for_unused_setter_parameter,
@@ -2764,10 +2764,18 @@ std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
   if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
     addMark(DRE->getDecl(), RK_Read);
 
-    // If the Decl is a read of a getter, track the first DRE for diagnostics
+    // If the Expression is a read of a getter, track for diagnostics
     if (auto VD = dyn_cast<VarDecl>(DRE->getDecl())) {
-      if (AssociatedGetter == VD && AssociatedGetterDeclRef == nullptr)
-        AssociatedGetterDeclRef = DRE;
+      if (AssociatedGetter == VD && AssociatedGetterRefExpr == nullptr)
+        AssociatedGetterRefExpr = DRE;
+    }
+  }
+  // If the Expression is a member reference, see if it is a read of the getter
+  // to track for diagnostics.
+  if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
+    if (auto VD = dyn_cast<VarDecl>(MRE->getMember().getDecl())) {
+      if (AssociatedGetter == VD && AssociatedGetterRefExpr == nullptr)
+        AssociatedGetterRefExpr = MRE;
     }
   }
   // If this is an AssignExpr, see if we're mutating something that we know

test/Sema/diag_self_assign.swift

@@ -50,7 +50,7 @@ class SA3 {
       return foo // expected-warning {{attempting to access 'foo' within its own getter}} expected-note{{access 'self' explicitly to silence this warning}} {{14-14=self.}}
     }
     set {
-      foo = foo // expected-error {{assigning a property to itself}} expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}}
+      foo = foo // expected-error {{assigning a property to itself}} expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}} expected-warning{{setter argument 'newValue' was never used, but the property was accessed}} expected-note{{did you mean to use 'newValue' instead of accessing the property's current value?}}
       self.foo = self.foo // expected-error {{assigning a property to itself}}
       foo = self.foo // expected-error {{assigning a property to itself}} expected-warning {{attempting to modify 'foo' within its own setter}} expected-note{{access 'self' explicitly to silence this warning}} {{7-7=self.}}
       self.foo = foo // expected-error {{assigning a property to itself}}

test/decl/var/usage.swift

@@ -287,6 +287,22 @@ func sr964() {
       print(suspiciousSetter) // expected-warning {{setter argument 'newValue' was never used, but the property was accessed}} expected-note {{did you mean to use 'newValue' instead of accessing the property's current value?}} {{13-29=newValue}}
     }
   }
+  struct MemberGetterStruct {
+    var suspiciousSetter: String {
+      get { return "" }
+      set {
+        print(suspiciousSetter) // expected-warning {{setter argument 'newValue' was never used, but the property was accessed}} expected-note {{did you mean to use 'newValue' instead of accessing the property's current value?}} {{15-31=newValue}}
+      }
+    }
+  }
+  class MemberGetterClass {
+    var suspiciousSetter: String {
+      get { return "" }
+      set {
+        print(suspiciousSetter) // expected-warning {{setter argument 'newValue' was never used, but the property was accessed}} expected-note {{did you mean to use 'newValue' instead of accessing the property's current value?}} {{15-31=newValue}}
+      }
+    }
+  }
   var namedSuspiciousSetter: String {
     get { return "" }
     set(parameter) {
@@ -305,3 +321,12 @@ func sr964() {
     }
   }
 }
+struct MemberGetterExtension {}
+extension MemberGetterExtension {
+  var suspiciousSetter: String {
+    get { return "" }
+    set {
+      print(suspiciousSetter) // expected-warning {{setter argument 'newValue' was never used, but the property was accessed}} expected-note {{did you mean to use 'newValue' instead of accessing the property's current value?}} {{13-29=newValue}}
+    }
+  }
+}