From f3d3fa750c0ecf8f209b74741910dc0632fd3009 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 22 May 2024 00:11:18 -0400 Subject: [PATCH] Ignore __slots__ with dynamic values --- .../fixtures/pylint/non_slot_assignment.py | 13 +++++++ .../rules/pylint/rules/non_slot_assignment.rs | 34 +++++++++++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/non_slot_assignment.py b/crates/ruff_linter/resources/test/fixtures/pylint/non_slot_assignment.py index 0c443e773cad9..504374c27001b 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/non_slot_assignment.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/non_slot_assignment.py @@ -82,3 +82,16 @@ def qux(self): @qux.setter def qux(self, value): self.bar = value / 2 + + +class StudentG: + names = ("surname",) + __slots__ = (*names, "a") + + def __init__(self, name, surname): + self.name = name + self.surname = surname # [assigning-non-slot] + self.setup() + + def setup(self): + pass diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs b/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs index 08260ed04c18a..3e2d8a7bc9e52 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs @@ -98,6 +98,8 @@ impl Ranged for AttributeAssignment<'_> { } /// Return a list of attributes that are assigned to but not included in `__slots__`. +/// +/// If the `__slots__` attribute cannot be statically determined, returns an empty vector. fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec { // First, collect all the attributes that are assigned to `__slots__`. let mut slots = FxHashSet::default(); @@ -110,7 +112,13 @@ fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec { }; if id == "__slots__" { - slots.extend(slots_attributes(value)); + for attribute in slots_attributes(value) { + if let Some(attribute) = attribute { + slots.insert(attribute); + } else { + return vec![]; + } + } } } @@ -125,7 +133,13 @@ fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec { }; if id == "__slots__" { - slots.extend(slots_attributes(value)); + for attribute in slots_attributes(value) { + if let Some(attribute) = attribute { + slots.insert(attribute); + } else { + return vec![]; + } + } } } @@ -136,7 +150,13 @@ fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec { }; if id == "__slots__" { - slots.extend(slots_attributes(value)); + for attribute in slots_attributes(value) { + if let Some(attribute) = attribute { + slots.insert(attribute); + } else { + return vec![]; + } + } } } _ => {} @@ -237,12 +257,14 @@ fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec { } /// Return an iterator over the attributes enumerated in the given `__slots__` value. -fn slots_attributes(expr: &Expr) -> impl Iterator { +/// +/// If an attribute can't be statically determined, it will be `None`. +fn slots_attributes(expr: &Expr) -> impl Iterator> { // Ex) `__slots__ = ("name",)` let elts_iter = match expr { Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) - | Expr::Set(ast::ExprSet { elts, .. }) => Some(elts.iter().filter_map(|elt| match elt { + | Expr::Set(ast::ExprSet { elts, .. }) => Some(elts.iter().map(|elt| match elt { Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => Some(value.to_str()), _ => None, })), @@ -251,7 +273,7 @@ fn slots_attributes(expr: &Expr) -> impl Iterator { // Ex) `__slots__ = {"name": ...}` let keys_iter = match expr { - Expr::Dict(dict) => Some(dict.iter_keys().filter_map(|key| match key { + Expr::Dict(dict) => Some(dict.iter_keys().map(|key| match key { Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => Some(value.to_str()), _ => None, })),