diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/singledispatch_method.py b/crates/ruff_linter/resources/test/fixtures/pylint/singledispatch_method.py new file mode 100644 index 0000000000000..72e813c2df80e --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/singledispatch_method.py @@ -0,0 +1,39 @@ +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 + + @singledispatch + @staticmethod + def do(position): + pass + + # False negative (flagged by Pylint). + @convert_position.register + @classmethod + def _(cls, position: str) -> tuple: + position_a, position_b = position.split(",") + return (int(position_a), int(position_b)) + + # False negative (flagged by Pylint). + @convert_position.register + @classmethod + def _(cls, position: tuple) -> str: + return f"{position[0]},{position[1]}" 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..c2d2dc1492357 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::singledispatch_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 d313c6816b800..ae9a1b29ccd47 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/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index 21a4aecd83ef9..24d71fddd7598 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -143,6 +143,7 @@ pub(crate) fn is_dataclass_meta_annotation(annotation: &Expr, semantic: &Semanti /// ```python /// from functools import singledispatch /// +/// /// @singledispatch /// def fun(arg, verbose=False): /// ... @@ -167,6 +168,7 @@ pub(crate) fn is_singledispatch_interface( /// ```python /// from functools import singledispatch /// +/// /// @singledispatch /// def fun(arg, verbose=False): /// ... diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index e9599b963aa70..8b746c8018db4 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("singledispatch_method.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..4049b2dca65fd 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -54,6 +54,7 @@ pub(crate) use repeated_keyword_argument::*; pub(crate) use return_in_init::*; pub(crate) use self_assigning_variable::*; pub(crate) use single_string_slots::*; +pub(crate) use singledispatch_method::*; pub(crate) use subprocess_popen_preexec_fn::*; pub(crate) use subprocess_run_without_check::*; pub(crate) use super_without_brackets::*; @@ -140,6 +141,7 @@ mod repeated_keyword_argument; mod return_in_init; mod self_assigning_variable; mod single_string_slots; +mod singledispatch_method; mod subprocess_popen_preexec_fn; mod subprocess_run_without_check; mod super_without_brackets; diff --git a/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs b/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs new file mode 100644 index 0000000000000..bdc1f015205fd --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs @@ -0,0 +1,119 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_semantic::analyze::function_type; +use ruff_python_semantic::Scope; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; + +/// ## What it does +/// Checks for `@singledispatch` decorators on class and instance methods. +/// +/// ## Why is this bad? +/// The `@singledispatch` decorator is intended for use with functions, not methods. +/// +/// Instead, use the `@singledispatchmethod` decorator, or migrate the method to a +/// standalone function or `@staticmethod`. +/// +/// ## Example +/// ```python +/// from functools import singledispatch +/// +/// +/// class Class: +/// @singledispatch +/// def method(self, arg): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from functools import singledispatchmethod +/// +/// +/// class Class: +/// @singledispatchmethod +/// def method(self, arg): +/// ... +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as migrating from `@singledispatch` to +/// `@singledispatchmethod` may change the behavior of the code. +#[violation] +pub struct SingledispatchMethod; + +impl Violation for SingledispatchMethod { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("`@singledispatch` decorator should not be used on methods") + } + + fn fix_title(&self) -> Option { + Some("Replace with `@singledispatchmethod`".to_string()) + } +} + +/// E1519 +pub(crate) fn singledispatch_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; + }; + + let type_ = function_type::classify( + name, + decorator_list, + parent, + checker.semantic(), + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ); + if !matches!( + type_, + 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"]) + }) + { + let mut diagnostic = Diagnostic::new(SingledispatchMethod, decorator.range()); + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("functools", "singledispatchmethod"), + decorator.start(), + checker.semantic(), + )?; + Ok(Fix::unsafe_edits( + Edit::range_replacement(binding, decorator.expression.range()), + [import_edit], + )) + }); + diagnostics.push(diagnostic); + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_singledispatch_method.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_singledispatch_method.py.snap new file mode 100644 index 0000000000000..fc28e7a514995 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1519_singledispatch_method.py.snap @@ -0,0 +1,43 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +singledispatch_method.py:10:5: PLE1519 [*] `@singledispatch` decorator should not be used on methods + | + 9 | class Board: +10 | @singledispatch # [singledispatch-method] + | ^^^^^^^^^^^^^^^ PLE1519 +11 | @classmethod +12 | def convert_position(cls, position): + | + = help: Replace with `@singledispatchmethod` + +ℹ Unsafe fix +7 7 | +8 8 | +9 9 | class Board: +10 |- @singledispatch # [singledispatch-method] + 10 |+ @singledispatchmethod # [singledispatch-method] +11 11 | @classmethod +12 12 | def convert_position(cls, position): +13 13 | pass + +singledispatch_method.py:15:5: PLE1519 [*] `@singledispatch` decorator should not be used on methods + | +13 | pass +14 | +15 | @singledispatch # [singledispatch-method] + | ^^^^^^^^^^^^^^^ PLE1519 +16 | def move(self, position): +17 | pass + | + = help: Replace with `@singledispatchmethod` + +ℹ Unsafe fix +12 12 | def convert_position(cls, position): +13 13 | pass +14 14 | +15 |- @singledispatch # [singledispatch-method] + 15 |+ @singledispatchmethod # [singledispatch-method] +16 16 | def move(self, position): +17 17 | pass +18 18 | diff --git a/ruff.schema.json b/ruff.schema.json index 8e17f813722a0..d6f734fe60851 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3231,6 +3231,8 @@ "PLE15", "PLE150", "PLE1507", + "PLE151", + "PLE1519", "PLE17", "PLE170", "PLE1700",