Implement Pylint single-string-used-for-slots (C0205) as single-string-slots (PLC0205) (#5399)

## Summary

Implement Pylint rule `single-string-used-for-slots` (`C0205`) as
`single-string-slots` (`PLC0205`). This rule checks for single strings
being assigned to `__slots__`. For example

```python
class Foo:
    __slots__: str = "bar"

    def __init__(self, bar: str) -> None:
        self.bar = bar
```

should be

```python
class Foo:
    __slots__: tuple[str, ...] = ("bar",)

    def __init__(self, bar: str) -> None:
        self.bar = bar
```

Related to #970. Includes documentation.

## Test Plan

`cargo test`
This commit is contained in:
Tom Kuson 2023-06-27 19:33:58 +01:00 committed by GitHub
parent 035f8993f4
commit a0a93a636f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 182 additions and 0 deletions

View file

@ -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

View file

@ -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) {

View file

@ -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),

View file

@ -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"))]

View file

@ -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;

View file

@ -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()));
}
}
}
}
_ => {}
}
}
}

View file

@ -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):
|

1
ruff.schema.json generated
View file

@ -2094,6 +2094,7 @@
"PLC0",
"PLC02",
"PLC020",
"PLC0205",
"PLC0208",
"PLC04",
"PLC041",