diff --git a/resources/test/fixtures/flake8_return/RET504.py b/resources/test/fixtures/flake8_return/RET504.py index 6f0e6f9b9c0fe7..92bd411d8d0abc 100644 --- a/resources/test/fixtures/flake8_return/RET504.py +++ b/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 diff --git a/src/flake8_return/visitor.rs b/src/flake8_return/visitor.rs index 7f3120dbf026e4..c8a95a09dcf9d9 100644 --- a/src/flake8_return/visitor.rs +++ b/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);