diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/E1519.py b/crates/ruff_linter/resources/test/fixtures/pylint/E1519.py new file mode 100644 index 0000000000000..6bb8fa34e23da --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/E1519.py @@ -0,0 +1,20 @@ +from functools import singledispatch, singledispatchmethod + + +@singledispatch +def convert_position(position): + pass + +class Board: + @singledispatch # [singledispatch-method] + @classmethod + def convert_position(cls, position): + pass + + @singledispatch # [singledispatch-method] + def move(self, position): + pass + + @singledispatchmethod + def place(self, position): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index e070ae5adbc0b..92fe0adc54ea1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -40,6 +40,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::UnusedPrivateTypedDict, Rule::UnusedStaticMethodArgument, Rule::UnusedVariable, + Rule::SingleDispatchMethod, ]) { return; } @@ -389,6 +390,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if checker.enabled(Rule::TooManyLocals) { pylint::rules::too_many_locals(checker, scope, &mut diagnostics); } + + if checker.enabled(Rule::SingleDispatchMethod) { + pylint::rules::single_dispatch_method(checker, scope, &mut diagnostics); + } } } checker.diagnostics.extend(diagnostics); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5cf0293d1292e..e55fe0812a938 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -252,6 +252,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E1307") => (RuleGroup::Stable, rules::pylint::rules::BadStringFormatType), (Pylint, "E1310") => (RuleGroup::Stable, rules::pylint::rules::BadStrStripCall), (Pylint, "E1507") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarValue), + (Pylint, "E1519") => (RuleGroup::Preview, rules::pylint::rules::SingleDispatchMethod), (Pylint, "E1700") => (RuleGroup::Stable, rules::pylint::rules::YieldFromInAsyncFunction), (Pylint, "E2502") => (RuleGroup::Stable, rules::pylint::rules::BidirectionalUnicode), (Pylint, "E2510") => (RuleGroup::Stable, rules::pylint::rules::InvalidCharacterBackspace), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index e9599b963aa70..c93dd1544c1a3 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -20,6 +20,7 @@ mod tests { use crate::settings::LinterSettings; use crate::test::test_path; + #[test_case(Rule::SingleDispatchMethod, Path::new("E1519.py"))] #[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"))] #[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async.py"))] #[test_case(Rule::BadOpenMode, Path::new("bad_open_mode.py"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 9963339844a84..806633d58aa1e 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -53,6 +53,7 @@ pub(crate) use repeated_isinstance_calls::*; pub(crate) use repeated_keyword_argument::*; pub(crate) use return_in_init::*; pub(crate) use self_assigning_variable::*; +pub(crate) use single_dispatch_method::*; pub(crate) use single_string_slots::*; pub(crate) use subprocess_popen_preexec_fn::*; pub(crate) use subprocess_run_without_check::*; @@ -139,6 +140,7 @@ mod repeated_isinstance_calls; mod repeated_keyword_argument; mod return_in_init; mod self_assigning_variable; +mod single_dispatch_method; mod single_string_slots; mod subprocess_popen_preexec_fn; mod subprocess_run_without_check; diff --git a/crates/ruff_linter/src/rules/pylint/rules/single_dispatch_method.rs b/crates/ruff_linter/src/rules/pylint/rules/single_dispatch_method.rs new file mode 100644 index 0000000000000..e10241f24b047 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/single_dispatch_method.rs @@ -0,0 +1,71 @@ +use ruff_diagnostics::{Diagnostic, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast}; +use ruff_python_semantic::{analyze::function_type, Scope}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for singledispatch decorators on class methods. +/// +/// ## Why is this bad? +/// Single dispatch must happen on the type of first non self argument +#[violation] +pub struct SingleDispatchMethod; + +impl Violation for SingleDispatchMethod { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + format!("singledispatch decorator should not be used with methods, use singledispatchmethod instead.") + } +} + +/// E1519 +pub(crate) fn single_dispatch_method( + checker: &Checker, + scope: &Scope, + diagnostics: &mut Vec, +) { + let Some(func) = scope.kind.as_function() else { + return; + }; + + let ast::StmtFunctionDef { + name, + decorator_list, + .. + } = func; + + let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + return; + }; + + if !matches!( + function_type::classify( + name, + decorator_list, + parent, + checker.semantic(), + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ), + function_type::FunctionType::Method | function_type::FunctionType::ClassMethod + ) { + return; + } + + for decorator in decorator_list { + if checker + .semantic() + .resolve_call_path(&decorator.expression) + .is_some_and(|call_path| { + matches!(call_path.as_slice(), ["functools", "singledispatch"]) + }) + { + diagnostics.push(Diagnostic::new(SingleDispatchMethod {}, decorator.range())); + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_E1519.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_E1519.py.snap new file mode 100644 index 0000000000000..8b40ebbe5e21d --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_E1519.py.snap @@ -0,0 +1,21 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +E1519.py:9:5: PLE1519 singledispatch decorator should not be used with methods, use singledispatchmethod instead. + | + 8 | class Board: + 9 | @singledispatch # [singledispatch-method] + | ^^^^^^^^^^^^^^^ PLE1519 +10 | @classmethod +11 | def convert_position(cls, position): + | + +E1519.py:14:5: PLE1519 singledispatch decorator should not be used with methods, use singledispatchmethod instead. + | +12 | pass +13 | +14 | @singledispatch # [singledispatch-method] + | ^^^^^^^^^^^^^^^ PLE1519 +15 | def move(self, position): +16 | pass + | diff --git a/ruff.schema.json b/ruff.schema.json index 674003ed05df2..0743fea9dcc5b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3231,6 +3231,8 @@ "PLE15", "PLE150", "PLE1507", + "PLE151", + "PLE1519", "PLE17", "PLE170", "PLE1700",