diff --git a/crates/ruff/resources/test/fixtures/pylint/single_string_slots.py b/crates/ruff/resources/test/fixtures/pylint/single_string_slots.py new file mode 100644 index 0000000000..b7a2dac915 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/single_string_slots.py @@ -0,0 +1,35 @@ +# Errors. +class Foo: + __slots__ = "bar" + + def __init__(self, bar): + self.bar = bar + + +class Foo: + __slots__: str = "bar" + + def __init__(self, bar): + self.bar = bar + + +class Foo: + __slots__: str = f"bar" + + def __init__(self, bar): + self.bar = bar + + +# Non-errors. +class Foo: + __slots__ = ("bar",) + + def __init__(self, bar): + self.bar = bar + + +class Foo: + __slots__: tuple[str, ...] = ("bar",) + + def __init__(self, bar): + self.bar = bar diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 32eab592f3..b7b0d1e925 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -770,6 +770,9 @@ where if self.enabled(Rule::NoSlotsInNamedtupleSubclass) { flake8_slots::rules::no_slots_in_namedtuple_subclass(self, stmt, class_def); } + if self.enabled(Rule::SingleStringSlots) { + pylint::rules::single_string_slots(self, class_def); + } } Stmt::Import(ast::StmtImport { names, range: _ }) => { if self.enabled(Rule::MultipleImportsOnOneLine) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index b63fea0b74..15b334e884 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -156,6 +156,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyflakes, "901") => (RuleGroup::Unspecified, rules::pyflakes::rules::RaiseNotImplemented), // pylint + (Pylint, "C0205") => (RuleGroup::Unspecified, rules::pylint::rules::SingleStringSlots), (Pylint, "C0414") => (RuleGroup::Unspecified, rules::pylint::rules::UselessImportAlias), (Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString), (Pylint, "C3002") => (RuleGroup::Unspecified, rules::pylint::rules::UnnecessaryDirectLambdaCall), diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index c99c843d06..4f5a5b2f6a 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -33,6 +33,7 @@ mod tests { )] #[test_case(Rule::ComparisonWithItself, Path::new("comparison_with_itself.py"))] #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))] + #[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_1.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_2.py"))] diff --git a/crates/ruff/src/rules/pylint/rules/mod.rs b/crates/ruff/src/rules/pylint/rules/mod.rs index dbbca73e1a..84251fb061 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/crates/ruff/src/rules/pylint/rules/mod.rs @@ -31,6 +31,7 @@ pub(crate) use property_with_parameters::*; pub(crate) use redefined_loop_name::*; pub(crate) use repeated_isinstance_calls::*; pub(crate) use return_in_init::*; +pub(crate) use single_string_slots::*; pub(crate) use sys_exit_alias::*; pub(crate) use too_many_arguments::*; pub(crate) use too_many_branches::*; @@ -77,6 +78,7 @@ mod property_with_parameters; mod redefined_loop_name; mod repeated_isinstance_calls; mod return_in_init; +mod single_string_slots; mod sys_exit_alias; mod too_many_arguments; mod too_many_branches; diff --git a/crates/ruff/src/rules/pylint/rules/single_string_slots.rs b/crates/ruff/src/rules/pylint/rules/single_string_slots.rs new file mode 100644 index 0000000000..964e5512bd --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/single_string_slots.rs @@ -0,0 +1,107 @@ +use rustpython_parser::ast::{self, Constant, Expr, Stmt, StmtClassDef}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::identifier::Identifier; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for single strings assigned to `__slots__`. +/// +/// ## Why is this bad? +/// In Python, the `__slots__` attribute allows you to explicitly define the +/// attributes (instance variables) that a class can have. By default, Python +/// uses a dictionary to store an object's attributes, which incurs some memory +/// overhead. However, when `__slots__` is defined, Python uses a more compact +/// internal structure to store the object's attributes, resulting in memory +/// savings. +/// +/// Any string iterable may be assigned to `__slots__` (most commonly, a +/// `tuple` of strings). If a string is assigned to `__slots__`, it is +/// interpreted as a single attribute name, rather than an iterable of attribute +/// names. This can cause confusion, as users that iterate over the `__slots__` +/// value may expect to iterate over a sequence of attributes, but would instead +/// iterate over the characters of the string. +/// +/// To use a single string attribute in `__slots__`, wrap the string in an +/// iterable container type, like a `tuple`. +/// +/// ## Example +/// ```python +/// class Person: +/// __slots__: str = "name" +/// +/// def __init__(self, name: string) -> None: +/// self.name = name +/// ``` +/// +/// Use instead: +/// ```python +/// class Person: +/// __slots__: tuple[str, ...] = ("name",) +/// +/// def __init__(self, name: string) -> None: +/// self.name = name +/// ``` +/// +/// ## References +/// - [Python documentation: `__slots__`](https://docs.python.org/3/reference/datamodel.html#slots) +#[violation] +pub struct SingleStringSlots; + +impl Violation for SingleStringSlots { + #[derive_message_formats] + fn message(&self) -> String { + format!("Class `__slots__` should be a non-string iterable") + } +} + +/// PLC0205 +pub(crate) fn single_string_slots(checker: &mut Checker, class: &StmtClassDef) { + for stmt in &class.body { + match stmt { + Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { + for target in targets { + if let Expr::Name(ast::ExprName { id, .. }) = target { + if id.as_str() == "__slots__" { + if matches!( + value.as_ref(), + Expr::Constant(ast::ExprConstant { + value: Constant::Str(_), + .. + }) | Expr::JoinedStr(_) + ) { + checker + .diagnostics + .push(Diagnostic::new(SingleStringSlots, stmt.identifier())); + } + } + } + } + } + Stmt::AnnAssign(ast::StmtAnnAssign { + target, + value: Some(value), + .. + }) => { + if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { + if id.as_str() == "__slots__" { + if matches!( + value.as_ref(), + Expr::Constant(ast::ExprConstant { + value: Constant::Str(_), + .. + }) | Expr::JoinedStr(_) + ) { + checker + .diagnostics + .push(Diagnostic::new(SingleStringSlots, stmt.identifier())); + } + } + } + } + _ => {} + } + } +} diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0205_single_string_slots.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0205_single_string_slots.py.snap new file mode 100644 index 0000000000..b378d106c9 --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0205_single_string_slots.py.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +single_string_slots.py:3:5: PLC0205 Class `__slots__` should be a non-string iterable + | +1 | # Errors. +2 | class Foo: +3 | __slots__ = "bar" + | ^^^^^^^^^^^^^^^^^ PLC0205 +4 | +5 | def __init__(self, bar): + | + +single_string_slots.py:10:5: PLC0205 Class `__slots__` should be a non-string iterable + | + 9 | class Foo: +10 | __slots__: str = "bar" + | ^^^^^^^^^^^^^^^^^^^^^^ PLC0205 +11 | +12 | def __init__(self, bar): + | + +single_string_slots.py:17:5: PLC0205 Class `__slots__` should be a non-string iterable + | +16 | class Foo: +17 | __slots__: str = f"bar" + | ^^^^^^^^^^^^^^^^^^^^^^^ PLC0205 +18 | +19 | def __init__(self, bar): + | + + diff --git a/ruff.schema.json b/ruff.schema.json index b145c5ad36..c6d3d474b7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2094,6 +2094,7 @@ "PLC0", "PLC02", "PLC020", + "PLC0205", "PLC0208", "PLC04", "PLC041",