[pylint] Implement PLE0302 unexpected-special-method-signature (#…

…4075)

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
+        ...

crates/ruff/src/checkers/ast/mod.rs

@@ -602,6 +602,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

crates/ruff/src/codes.rs

@@ -202,6 +202,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Pylint, "W0711") => Rule::BinaryOpException,
         (Pylint, "W1508") => Rule::InvalidEnvvarDefault,
         (Pylint, "W2901") => Rule::RedefinedLoopName,
+        (Pylint, "E0302") => Rule::UnexpectedSpecialMethodSignature,
 
         // flake8-builtins
         (Flake8Builtins, "001") => Rule::BuiltinVariableShadowing,

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,

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

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;

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<ExpectedParams> {
+        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),
+        ));
+    }
+}

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 |         ...
+   |
+
+