diff --git a/crates/ruff/resources/test/fixtures/pylint/unexpected_special_method_signature.py b/crates/ruff/resources/test/fixtures/pylint/unexpected_special_method_signature.py new file mode 100644 index 0000000000000..f5270e04594d6 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/unexpected_special_method_signature.py @@ -0,0 +1,51 @@ +class TestClass: + def __bool__(self): + ... + + def __bool__(self, x): # too many mandatory args + ... + + def __bool__(self, x=1): # additional optional args OK + ... + + def __bool__(self, *args): # varargs OK + ... + + def __bool__(): # ignored; should be caughty by E0211/N805 + ... + + @staticmethod + def __bool__(): + ... + + @staticmethod + def __bool__(x): # too many mandatory args + ... + + @staticmethod + def __bool__(x=1): # additional optional args OK + ... + + def __eq__(self, other): # multiple args + ... + + def __eq__(self, other=1): # expected arg is optional + ... + + def __eq__(self): # too few mandatory args + ... + + def __eq__(self, other, other_other): # too many mandatory args + ... + + def __round__(self): # allow zero additional args. + ... + + def __round__(self, x): # allow one additional args. + ... + + def __round__(self, x, y): # disallow 2 args + ... + + def __round__(self, x, y, z=2): # disallow 3 args even when one is optional + ... \ No newline at end of file diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index b2800250ec64e..ef307237f3a1d 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -590,6 +590,21 @@ where ); } + if self + .settings + .rules + .enabled(Rule::UnexpectedSpecialMethodSignature) + { + pylint::rules::unexpected_special_method_signature( + self, + stmt, + name, + decorator_list, + args, + self.locator, + ); + } + self.check_builtin_shadowing(name, stmt, true); // Visit the decorators and arguments, but avoid the body, which will be diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c4072ceca2a0b..0141f644dae36 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -202,6 +202,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Pylint, "W0711") => Rule::BinaryOpException, (Pylint, "W1508") => Rule::InvalidEnvvarDefault, (Pylint, "W2901") => Rule::RedefinedLoopName, + (Pylint, "E0302") => Rule::UnexpectedSpecialMethodSignature, // flake8-builtins (Flake8Builtins, "001") => Rule::BuiltinVariableShadowing, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 9fda460a5f70d..2e99641d29641 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -187,6 +187,7 @@ ruff_macros::register_rules!( rules::pylint::rules::RedefinedLoopName, rules::pylint::rules::LoggingTooFewArgs, rules::pylint::rules::LoggingTooManyArgs, + rules::pylint::rules::UnexpectedSpecialMethodSignature, // flake8-builtins rules::flake8_builtins::rules::BuiltinVariableShadowing, rules::flake8_builtins::rules::BuiltinArgumentShadowing, diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index b1c4d79a59dd9..2736555b51f30 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -60,6 +60,7 @@ mod tests { #[test_case(Rule::TooManyBranches, Path::new("too_many_branches.py"); "PLR0912")] #[test_case(Rule::TooManyReturnStatements, Path::new("too_many_return_statements.py"); "PLR0911")] #[test_case(Rule::TooManyStatements, Path::new("too_many_statements.py"); "PLR0915")] + #[test_case(Rule::UnexpectedSpecialMethodSignature, Path::new("unexpected_special_method_signature.py"); "PLE0302")] #[test_case(Rule::UnnecessaryDirectLambdaCall, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")] #[test_case(Rule::LoadBeforeGlobalDeclaration, Path::new("load_before_global_declaration.py"); "PLE0118")] #[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"); "PLW0120")] diff --git a/crates/ruff/src/rules/pylint/rules/mod.rs b/crates/ruff/src/rules/pylint/rules/mod.rs index b897495b73fbb..1cbdd566e41f9 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/crates/ruff/src/rules/pylint/rules/mod.rs @@ -34,6 +34,9 @@ pub use too_many_arguments::{too_many_arguments, TooManyArguments}; pub use too_many_branches::{too_many_branches, TooManyBranches}; pub use too_many_return_statements::{too_many_return_statements, TooManyReturnStatements}; pub use too_many_statements::{too_many_statements, TooManyStatements}; +pub use unexpected_special_method_signature::{ + unexpected_special_method_signature, UnexpectedSpecialMethodSignature, +}; pub use unnecessary_direct_lambda_call::{ unnecessary_direct_lambda_call, UnnecessaryDirectLambdaCall, }; @@ -73,6 +76,7 @@ mod too_many_arguments; mod too_many_branches; mod too_many_return_statements; mod too_many_statements; +mod unexpected_special_method_signature; mod unnecessary_direct_lambda_call; mod useless_else_on_loop; mod useless_import_alias; diff --git a/crates/ruff/src/rules/pylint/rules/unexpected_special_method_signature.rs b/crates/ruff/src/rules/pylint/rules/unexpected_special_method_signature.rs new file mode 100644 index 0000000000000..b02f046bf2460 --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/unexpected_special_method_signature.rs @@ -0,0 +1,150 @@ +use std::cmp::Ordering; + +use rustpython_parser::ast::{Arguments, Expr, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::identifier_range; +use ruff_python_ast::source_code::Locator; +use ruff_python_semantic::analyze::visibility::is_staticmethod; +use ruff_python_semantic::scope::ScopeKind; + +use crate::checkers::ast::Checker; + +#[derive(Debug, Eq, PartialEq)] +pub enum ExpectedParams { + Fixed(usize), + Range(usize, usize), +} + +impl ExpectedParams { + fn from_method(name: &str, is_staticmethod: bool) -> Option { + let expected_params = match name { + "__del__" | "__repr__" | "__str__" | "__bytes__" | "__hash__" | "__bool__" + | "__dir__" | "__len__" | "__length_hint__" | "__iter__" | "__reversed__" + | "__neg__" | "__pos__" | "__abs__" | "__invert__" | "__complex__" | "__int__" + | "__float__" | "__index__" | "__trunc__" | "__floor__" | "__ceil__" | "__enter__" + | "__aenter__" | "__getnewargs_ex__" | "__getnewargs__" | "__getstate__" + | "__reduce__" | "__copy__" | "__unicode__" | "__nonzero__" | "__await__" + | "__aiter__" | "__anext__" | "__fspath__" | "__subclasses__" => { + Some(ExpectedParams::Fixed(0)) + } + "__format__" | "__lt__" | "__le__" | "__eq__" | "__ne__" | "__gt__" | "__ge__" + | "__getattr__" | "__getattribute__" | "__delattr__" | "__delete__" + | "__instancecheck__" | "__subclasscheck__" | "__getitem__" | "__missing__" + | "__delitem__" | "__contains__" | "__add__" | "__sub__" | "__mul__" + | "__truediv__" | "__floordiv__" | "__rfloordiv__" | "__mod__" | "__divmod__" + | "__lshift__" | "__rshift__" | "__and__" | "__xor__" | "__or__" | "__radd__" + | "__rsub__" | "__rmul__" | "__rtruediv__" | "__rmod__" | "__rdivmod__" + | "__rpow__" | "__rlshift__" | "__rrshift__" | "__rand__" | "__rxor__" | "__ror__" + | "__iadd__" | "__isub__" | "__imul__" | "__itruediv__" | "__ifloordiv__" + | "__imod__" | "__ilshift__" | "__irshift__" | "__iand__" | "__ixor__" | "__ior__" + | "__ipow__" | "__setstate__" | "__reduce_ex__" | "__deepcopy__" | "__cmp__" + | "__matmul__" | "__rmatmul__" | "__imatmul__" | "__div__" => { + Some(ExpectedParams::Fixed(1)) + } + "__setattr__" | "__get__" | "__set__" | "__setitem__" | "__set_name__" => { + Some(ExpectedParams::Fixed(2)) + } + "__exit__" | "__aexit__" => Some(ExpectedParams::Fixed(3)), + "__round__" => Some(ExpectedParams::Range(0, 1)), + "__pow__" => Some(ExpectedParams::Range(1, 2)), + _ => None, + }?; + + Some(if is_staticmethod { + expected_params + } else { + match expected_params { + ExpectedParams::Fixed(n) => ExpectedParams::Fixed(n + 1), + ExpectedParams::Range(min, max) => ExpectedParams::Range(min + 1, max + 1), + } + }) + } + + fn message(&self) -> String { + match self { + ExpectedParams::Fixed(n) if *n == 1 => "1 parameter".to_string(), + ExpectedParams::Fixed(n) => { + format!("{} parameters", *n) + } + ExpectedParams::Range(min, max) => { + format!("between {} and {} parameters", *min, *max) + } + } + } +} + +#[violation] +pub struct UnexpectedSpecialMethodSignature { + pub method_name: String, + pub expected_params: ExpectedParams, + pub actual_params: usize, +} + +impl Violation for UnexpectedSpecialMethodSignature { + #[derive_message_formats] + fn message(&self) -> String { + let verb = if self.actual_params > 1 { + "were" + } else { + "was" + }; + format!( + "The special method `{}` expects {}, {} {} given", + self.method_name, + self.expected_params.message(), + self.actual_params, + verb + ) + } +} + +/// PLE0302 +pub fn unexpected_special_method_signature( + checker: &mut Checker, + stmt: &Stmt, + name: &str, + decorator_list: &[Expr], + args: &Arguments, + locator: &Locator, +) { + if !matches!(checker.ctx.scope().kind, ScopeKind::Class(_)) { + return; + } + + // Method has no parameter, will be caught by no-method-argument (E0211/N805). + if args.args.is_empty() && args.vararg.is_none() { + return; + } + + let actual_params = args.args.len(); + let optional_params = args.defaults.len(); + let mandatory_params = actual_params - optional_params; + + let Some(expected_params) = ExpectedParams::from_method(name, is_staticmethod(&checker.ctx, decorator_list)) else { + return; + }; + + let emit = match expected_params { + ExpectedParams::Range(min, max) => !(min..=max).contains(&actual_params), + ExpectedParams::Fixed(expected) => match expected.cmp(&mandatory_params) { + Ordering::Less => true, + Ordering::Greater => { + args.vararg.is_none() && optional_params < (expected - mandatory_params) + } + Ordering::Equal => false, + }, + }; + + if emit { + checker.diagnostics.push(Diagnostic::new( + UnexpectedSpecialMethodSignature { + method_name: name.to_owned(), + expected_params, + actual_params, + }, + identifier_range(stmt, locator), + )); + } +} diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE0302_unexpected_special_method_signature.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE0302_unexpected_special_method_signature.py.snap new file mode 100644 index 0000000000000..602b0ad4aa31a --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE0302_unexpected_special_method_signature.py.snap @@ -0,0 +1,57 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +unexpected_special_method_signature.py:5:9: PLE0302 The special method `__bool__` expects 1 parameter, 2 were given + | +5 | ... +6 | +7 | def __bool__(self, x): # too many mandatory args + | ^^^^^^^^ PLE0302 +8 | ... + | + +unexpected_special_method_signature.py:22:9: PLE0302 The special method `__bool__` expects 0 parameters, 1 was given + | +22 | @staticmethod +23 | def __bool__(x): # too many mandatory args + | ^^^^^^^^ PLE0302 +24 | ... + | + +unexpected_special_method_signature.py:35:9: PLE0302 The special method `__eq__` expects 2 parameters, 1 was given + | +35 | ... +36 | +37 | def __eq__(self): # too few mandatory args + | ^^^^^^ PLE0302 +38 | ... + | + +unexpected_special_method_signature.py:38:9: PLE0302 The special method `__eq__` expects 2 parameters, 3 were given + | +38 | ... +39 | +40 | def __eq__(self, other, other_other): # too many mandatory args + | ^^^^^^ PLE0302 +41 | ... + | + +unexpected_special_method_signature.py:47:9: PLE0302 The special method `__round__` expects between 1 and 2 parameters, 3 were given + | +47 | ... +48 | +49 | def __round__(self, x, y): # disallow 2 args + | ^^^^^^^^^ PLE0302 +50 | ... + | + +unexpected_special_method_signature.py:50:9: PLE0302 The special method `__round__` expects between 1 and 2 parameters, 4 were given + | +50 | ... +51 | +52 | def __round__(self, x, y, z=2): # disallow 3 args even when one is optional + | ^^^^^^^^^ PLE0302 +53 | ... + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 37320b5cbf940..e4e5370d28337 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1949,6 +1949,9 @@ "PLE0116", "PLE0117", "PLE0118", + "PLE03", + "PLE030", + "PLE0302", "PLE06", "PLE060", "PLE0604",