Ignore property assignments in RET504 (#1520)

Author: charliermarsh

resources/test/fixtures/flake8_return/RET504.py

@@ -137,6 +137,14 @@ def x():
     return a
 
 
+# Considered OK, since attribute assignments can have side effects.
+class X:
+    def x(self):
+        a = self.property
+        self.property = None
+        return a
+
+
 # Test cases for using value for assignment then returning it
 # See:https://github.com/afonasev/flake8-return/issues/47
 def resolve_from_url(self, url: str) -> dict:
@@ -238,13 +246,16 @@ def close(self):
             report(traceback.format_exc())
     return any_failed
 
+
 def global_assignment():
     global X
     X = 1
     return X
 
+
 def nonlocal_assignment():
     X = 1
+
     def inner():
         nonlocal X
         X = 1

src/flake8_return/visitor.rs

@@ -38,6 +38,18 @@ impl<'a> ReturnVisitor<'a> {
                     .push(expr.location);
                 return;
             }
+            ExprKind::Attribute { .. } => {
+                // Attribute assignments are often side-effects (e.g., `self.property = value`),
+                // so we conservatively treat them as references to every known
+                // variable.
+                for name in self.stack.assigns.keys() {
+                    self.stack
+                        .refs
+                        .entry(name)
+                        .or_insert_with(Vec::new)
+                        .push(expr.location);
+                }
+            }
             _ => {}
         }
         visitor::walk_expr(self, expr);