diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/singledispatchmethod_function.py b/crates/ruff_linter/resources/test/fixtures/pylint/singledispatchmethod_function.py new file mode 100644 index 00000000000000..cf249f184fc835 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/singledispatchmethod_function.py @@ -0,0 +1,23 @@ +from functools import singledispatchmethod + + +@singledispatchmethod # [singledispatchmethod-function] +def convert_position(position): + pass + + +class Board: + + @singledispatchmethod # Ok + @classmethod + def convert_position(cls, position): + pass + + @singledispatchmethod # Ok + def move(self, position): + pass + + @singledispatchmethod # [singledispatchmethod-function] + @staticmethod + def do(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 c35cf53a77a3a1..67543aa6c7d5be 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -43,6 +43,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::UnusedStaticMethodArgument, Rule::UnusedVariable, Rule::SingledispatchMethod, + Rule::SingledispatchmethodFunction, ]) { return; } @@ -403,6 +404,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { pylint::rules::singledispatch_method(checker, scope, &mut diagnostics); } + if checker.enabled(Rule::SingledispatchmethodFunction) { + pylint::rules::singledispatchmethod_function(checker, scope, &mut diagnostics); + } + if checker.any_enabled(&[ Rule::InvalidFirstArgumentNameForClassMethod, Rule::InvalidFirstArgumentNameForMethod, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c471c890a0f1a7..10f13057305a42 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -256,6 +256,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (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, "E1520") => (RuleGroup::Preview, rules::pylint::rules::SingledispatchmethodFunction), (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 0cbd284ddbc8f0..f08b920a28c723 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -21,6 +21,10 @@ mod tests { use crate::test::test_path; #[test_case(Rule::SingledispatchMethod, Path::new("singledispatch_method.py"))] + #[test_case( + Rule::SingledispatchmethodFunction, + Path::new("singledispatchmethod_function.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 8f6d40554d2ad8..447156f63f7a31 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -56,6 +56,7 @@ 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 singledispatchmethod_function::*; pub(crate) use subprocess_popen_preexec_fn::*; pub(crate) use subprocess_run_without_check::*; pub(crate) use super_without_brackets::*; @@ -145,6 +146,7 @@ mod return_in_init; mod self_assigning_variable; mod single_string_slots; mod singledispatch_method; +mod singledispatchmethod_function; mod subprocess_popen_preexec_fn; mod subprocess_run_without_check; mod super_without_brackets; diff --git a/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs b/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs new file mode 100644 index 00000000000000..f3b10bf18f5c22 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.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 `@singledispatchmethod` decorators on functions or static methods. +/// +/// ## Why is this bad? +/// The `@singledispatchmethod` decorator is intended for use with class and instance methods, not functions. +/// +/// Instead, use the `@singledispatch` decorator. +/// +/// ## Example +/// ```python +/// from functools import singledispatchmethod +/// +/// +/// @singledispatchmethod +/// def foo(arg): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from functools import singledispatchmethod +/// +/// +/// @singledispatch +/// def foo(arg): +/// ... +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as migrating from `@singledispatchmethod` to +/// `@singledispatch` may change the behavior of the code. +#[violation] +pub struct SingledispatchmethodFunction; + +impl Violation for SingledispatchmethodFunction { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("`@singledispatchmethod` decorator should not be used on functions") + } + + fn fix_title(&self) -> Option { + Some("Replace with `@singledispatch`".to_string()) + } +} + +/// E1520 +pub(crate) fn singledispatchmethod_function( + 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::Function | function_type::FunctionType::StaticMethod + ) { + return; + } + + for decorator in decorator_list { + if checker + .semantic() + .resolve_qualified_name(&decorator.expression) + .is_some_and(|qualified_name| { + matches!( + qualified_name.segments(), + ["functools", "singledispatchmethod"] + ) + }) + { + let mut diagnostic = Diagnostic::new(SingledispatchmethodFunction, decorator.range()); + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("functools", "singledispatch"), + 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__PLE1520_singledispatchmethod_function.py.snap.new b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1520_singledispatchmethod_function.py.snap.new new file mode 100644 index 00000000000000..834a175b100c80 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1520_singledispatchmethod_function.py.snap.new @@ -0,0 +1,50 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +assertion_line: 200 +--- +singledispatchmethod_function.py:4:1: PLE1520 [*] `@singledispatchmethod` decorator should not be used on functions + | +4 | @singledispatchmethod # [singledispatchmethod-function] + | ^^^^^^^^^^^^^^^^^^^^^ PLE1520 +5 | def convert_position(position): +6 | pass + | + = help: Replace with `@singledispatch` + +ℹ Unsafe fix +1 |-from functools import singledispatchmethod + 1 |+from functools import singledispatchmethod, singledispatch +2 2 | +3 3 | +4 |-@singledispatchmethod # [singledispatchmethod-function] + 4 |+@singledispatch # [singledispatchmethod-function] +5 5 | def convert_position(position): +6 6 | pass +7 7 | + +singledispatchmethod_function.py:20:5: PLE1520 [*] `@singledispatchmethod` decorator should not be used on functions + | +18 | pass +19 | +20 | @singledispatchmethod # [singledispatchmethod-function] + | ^^^^^^^^^^^^^^^^^^^^^ PLE1520 +21 | @staticmethod +22 | def do(position): + | + = help: Replace with `@singledispatch` + +ℹ Unsafe fix +1 |-from functools import singledispatchmethod + 1 |+from functools import singledispatchmethod, singledispatch +2 2 | +3 3 | +4 4 | @singledispatchmethod # [singledispatchmethod-function] +-------------------------------------------------------------------------------- +17 17 | def move(self, position): +18 18 | pass +19 19 | +20 |- @singledispatchmethod # [singledispatchmethod-function] + 20 |+ @singledispatch # [singledispatchmethod-function] +21 21 | @staticmethod +22 22 | def do(position): +23 23 | pass diff --git a/ruff.schema.json b/ruff.schema.json index 7b65b8f158596f..5bc5edf0c3833c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3252,6 +3252,8 @@ "PLE1507", "PLE151", "PLE1519", + "PLE152", + "PLE1520", "PLE17", "PLE170", "PLE1700",