Skip to content

Commit

Permalink
[pylint] Implement singledispatchmethod-function (PLE5120) (#10428
Browse files Browse the repository at this point in the history
)

## Summary

Implement `singledispatchmethod-function` from pylint, part of #970.

This is essentially a copy paste of #8934 for `@singledispatchmethod`
decorator.

## Test Plan

Text fixture added.
  • Loading branch information
augustelalande authored Mar 18, 2024
1 parent 8619986 commit 229a50a
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
Rule::UnusedStaticMethodArgument,
Rule::UnusedVariable,
Rule::SingledispatchMethod,
Rule::SingledispatchmethodFunction,
]) {
return;
}
Expand Down Expand Up @@ -419,6 +420,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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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::*;
Expand Down Expand Up @@ -147,6 +148,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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
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 func(arg):
/// ...
/// ```
///
/// Use instead:
/// ```python
/// from functools import singledispatchmethod
///
///
/// @singledispatch
/// def func(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 non-method functions")
}

fn fix_title(&self) -> Option<String> {
Some("Replace with `@singledispatch`".to_string())
}
}

/// E1520
pub(crate) fn singledispatchmethod_function(
checker: &Checker,
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
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);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
singledispatchmethod_function.py:4:1: PLE1520 [*] `@singledispatchmethod` decorator should not be used on non-method 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 non-method 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
2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 229a50a

Please sign in to comment.