[pylint] Implement assigning-non-slot (E0237) (#9623)

## Summary

Implement [assigning-non-slot /
E0237](https://pylint.readthedocs.io/en/latest/user_guide/messages/error/assigning-non-slot.html)

related #970

## Test Plan

`cargo test`
This commit is contained in:
Akira Noda 2024-01-24 11:50:22 +09:00 committed by GitHub
parent eab1a6862b
commit 57313d9d63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 338 additions and 0 deletions

View file

@ -0,0 +1,56 @@
class StudentA:
__slots__ = ("name",)
def __init__(self, name, surname):
self.name = name
self.surname = surname # [assigning-non-slot]
self.setup()
def setup(self):
pass
class StudentB:
__slots__ = ("name", "surname")
def __init__(self, name, middle_name):
self.name = name
self.middle_name = middle_name # [assigning-non-slot]
self.setup()
def setup(self):
pass
class StudentC:
__slots__ = ("name", "surname")
def __init__(self, name, surname):
self.name = name
self.surname = surname # OK
self.setup()
def setup(self):
pass
class StudentD(object):
__slots__ = ("name", "surname")
def __init__(self, name, middle_name):
self.name = name
self.middle_name = middle_name # [assigning-non-slot]
self.setup()
def setup(self):
pass
class StudentE(StudentD):
def __init__(self, name, middle_name):
self.name = name
self.middle_name = middle_name # OK
self.setup()
def setup(self):
pass

View file

@ -507,6 +507,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::NoSlotsInNamedtupleSubclass) { if checker.enabled(Rule::NoSlotsInNamedtupleSubclass) {
flake8_slots::rules::no_slots_in_namedtuple_subclass(checker, stmt, class_def); flake8_slots::rules::no_slots_in_namedtuple_subclass(checker, stmt, class_def);
} }
if checker.enabled(Rule::NonSlotAssignment) {
pylint::rules::non_slot_assignment(checker, class_def);
}
if checker.enabled(Rule::SingleStringSlots) { if checker.enabled(Rule::SingleStringSlots) {
pylint::rules::single_string_slots(checker, class_def); pylint::rules::single_string_slots(checker, class_def);
} }

View file

@ -224,6 +224,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0116") => (RuleGroup::Stable, rules::pylint::rules::ContinueInFinally), (Pylint, "E0116") => (RuleGroup::Stable, rules::pylint::rules::ContinueInFinally),
(Pylint, "E0117") => (RuleGroup::Stable, rules::pylint::rules::NonlocalWithoutBinding), (Pylint, "E0117") => (RuleGroup::Stable, rules::pylint::rules::NonlocalWithoutBinding),
(Pylint, "E0118") => (RuleGroup::Stable, rules::pylint::rules::LoadBeforeGlobalDeclaration), (Pylint, "E0118") => (RuleGroup::Stable, rules::pylint::rules::LoadBeforeGlobalDeclaration),
(Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment),
(Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases), (Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases),
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature), (Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType), (Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),

View file

@ -92,6 +92,7 @@ mod tests {
Path::new("named_expr_without_context.py") Path::new("named_expr_without_context.py")
)] )]
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))] #[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))]
#[test_case(Rule::NonSlotAssignment, Path::new("non_slot_assignment.py"))]
#[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))] #[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))]
#[test_case( #[test_case(
Rule::RedefinedArgumentFromLocal, Rule::RedefinedArgumentFromLocal,

View file

@ -41,6 +41,7 @@ pub(crate) use no_method_decorator::*;
pub(crate) use no_self_use::*; pub(crate) use no_self_use::*;
pub(crate) use non_ascii_module_import::*; pub(crate) use non_ascii_module_import::*;
pub(crate) use non_ascii_name::*; pub(crate) use non_ascii_name::*;
pub(crate) use non_slot_assignment::*;
pub(crate) use nonlocal_without_binding::*; pub(crate) use nonlocal_without_binding::*;
pub(crate) use potential_index_error::*; pub(crate) use potential_index_error::*;
pub(crate) use property_with_parameters::*; pub(crate) use property_with_parameters::*;
@ -124,6 +125,7 @@ mod no_method_decorator;
mod no_self_use; mod no_self_use;
mod non_ascii_module_import; mod non_ascii_module_import;
mod non_ascii_name; mod non_ascii_name;
mod non_slot_assignment;
mod nonlocal_without_binding; mod nonlocal_without_binding;
mod potential_index_error; mod potential_index_error;
mod property_with_parameters; mod property_with_parameters;

View file

@ -0,0 +1,242 @@
use rustc_hash::FxHashSet;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for assignments to attributes that are not defined in `__slots__`.
///
/// ## Why is this bad?
/// When using `__slots__`, only the specified attributes are allowed.
/// Attempting to assign to an attribute that is not defined in `__slots__`
/// will result in an `AttributeError` at runtime.
///
/// ## Known problems
/// This rule can't detect `__slots__` implementations in superclasses, and
/// so limits its analysis to classes that inherit from (at most) `object`.
///
/// ## Example
/// ```python
/// class Student:
/// __slots__ = ("name",)
///
/// def __init__(self, name, surname):
/// self.name = name
/// self.surname = surname # [assigning-non-slot]
/// self.setup()
///
/// def setup(self):
/// pass
/// ```
///
/// Use instead:
/// ```python
/// class Student:
/// __slots__ = ("name", "surname")
///
/// def __init__(self, name, surname):
/// self.name = name
/// self.surname = surname
/// self.setup()
///
/// def setup(self):
/// pass
/// ```
#[violation]
pub struct NonSlotAssignment {
name: String,
}
impl Violation for NonSlotAssignment {
#[derive_message_formats]
fn message(&self) -> String {
let NonSlotAssignment { name } = self;
format!("Attribute `{name}` is not defined in class's `__slots__`")
}
}
/// E0237
pub(crate) fn non_slot_assignment(checker: &mut Checker, class_def: &ast::StmtClassDef) {
// If the class inherits from another class (aside from `object`), then it's possible that
// the parent class defines the relevant `__slots__`.
if !class_def.bases().iter().all(|base| {
checker
.semantic()
.resolve_call_path(base)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["", "object"]))
}) {
return;
}
for attribute in is_attributes_not_in_slots(&class_def.body) {
checker.diagnostics.push(Diagnostic::new(
NonSlotAssignment {
name: attribute.name.to_string(),
},
attribute.range(),
));
}
}
#[derive(Debug)]
struct AttributeAssignment<'a> {
/// The name of the attribute that is assigned to.
name: &'a str,
/// The range of the attribute that is assigned to.
range: TextRange,
}
impl Ranged for AttributeAssignment<'_> {
fn range(&self) -> TextRange {
self.range
}
}
/// Return a list of attributes that are assigned to but not included in `__slots__`.
fn is_attributes_not_in_slots(body: &[Stmt]) -> Vec<AttributeAssignment> {
// First, collect all the attributes that are assigned to `__slots__`.
let mut slots = FxHashSet::default();
for statement in body {
match statement {
// Ex) `__slots__ = ("name",)`
Stmt::Assign(ast::StmtAssign { targets, value, .. }) => {
let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() else {
continue;
};
if id == "__slots__" {
slots.extend(slots_attributes(value));
}
}
// Ex) `__slots__: Tuple[str, ...] = ("name",)`
Stmt::AnnAssign(ast::StmtAnnAssign {
target,
value: Some(value),
..
}) => {
let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() else {
continue;
};
if id == "__slots__" {
slots.extend(slots_attributes(value));
}
}
// Ex) `__slots__ += ("name",)`
Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => {
let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() else {
continue;
};
if id == "__slots__" {
slots.extend(slots_attributes(value));
}
}
_ => {}
}
}
if slots.is_empty() {
return vec![];
}
// Second, find any assignments that aren't included in `__slots__`.
let mut assignments = vec![];
for statement in body {
let Stmt::FunctionDef(ast::StmtFunctionDef { name, body, .. }) = statement else {
continue;
};
if name == "__init__" {
for statement in body {
match statement {
// Ex) `self.name = name`
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
let [Expr::Attribute(attribute)] = targets.as_slice() else {
continue;
};
let Expr::Name(ast::ExprName { id, .. }) = attribute.value.as_ref() else {
continue;
};
if id == "self" && !slots.contains(attribute.attr.as_str()) {
assignments.push(AttributeAssignment {
name: &attribute.attr,
range: attribute.range(),
});
}
}
// Ex) `self.name: str = name`
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => {
let Expr::Attribute(attribute) = target.as_ref() else {
continue;
};
let Expr::Name(ast::ExprName { id, .. }) = attribute.value.as_ref() else {
continue;
};
if id == "self" && !slots.contains(attribute.attr.as_str()) {
assignments.push(AttributeAssignment {
name: &attribute.attr,
range: attribute.range(),
});
}
}
// Ex) `self.name += name`
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
let Expr::Attribute(attribute) = target.as_ref() else {
continue;
};
let Expr::Name(ast::ExprName { id, .. }) = attribute.value.as_ref() else {
continue;
};
if id == "self" && !slots.contains(attribute.attr.as_str()) {
assignments.push(AttributeAssignment {
name: &attribute.attr,
range: attribute.range(),
});
}
}
_ => {}
}
}
}
}
assignments
}
/// Return an iterator over the attributes enumerated in the given `__slots__` value.
fn slots_attributes(expr: &Expr) -> impl Iterator<Item = &str> {
// 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::StringLiteral(ast::ExprStringLiteral { value, .. }) => Some(value.to_str()),
_ => None,
})),
_ => None,
};
// Ex) `__slots__ = {"name": ...}`
let keys_iter = match expr {
Expr::Dict(ast::ExprDict { keys, .. }) => Some(keys.iter().filter_map(|key| match key {
Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => Some(value.to_str()),
_ => None,
})),
_ => None,
};
elts_iter
.into_iter()
.flatten()
.chain(keys_iter.into_iter().flatten())
}

View file

@ -0,0 +1,31 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
non_slot_assignment.py:6:9: PLE0237 Attribute `surname` is not defined in class's `__slots__`
|
4 | def __init__(self, name, surname):
5 | self.name = name
6 | self.surname = surname # [assigning-non-slot]
| ^^^^^^^^^^^^ PLE0237
7 | self.setup()
|
non_slot_assignment.py:18:9: PLE0237 Attribute `middle_name` is not defined in class's `__slots__`
|
16 | def __init__(self, name, middle_name):
17 | self.name = name
18 | self.middle_name = middle_name # [assigning-non-slot]
| ^^^^^^^^^^^^^^^^ PLE0237
19 | self.setup()
|
non_slot_assignment.py:42:9: PLE0237 Attribute `middle_name` is not defined in class's `__slots__`
|
40 | def __init__(self, name, middle_name):
41 | self.name = name
42 | self.middle_name = middle_name # [assigning-non-slot]
| ^^^^^^^^^^^^^^^^ PLE0237
43 | self.setup()
|

2
ruff.schema.json generated
View file

@ -3119,6 +3119,8 @@
"PLE0117", "PLE0117",
"PLE0118", "PLE0118",
"PLE02", "PLE02",
"PLE023",
"PLE0237",
"PLE024", "PLE024",
"PLE0241", "PLE0241",
"PLE03", "PLE03",