mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 18:28:24 +00:00
[refurb
] Implement metaclass_abcmeta
(FURB180
) (#9658)
## Summary Implement [use-abc-shorthand (FURB180)](https://github.com/dosisod/refurb/blob/master/refurb/checks/readability/use_abc_shorthand.py) lint. I changed the name to be more conformant with ruff rule-naming rules. ## Test Plan cargo test
This commit is contained in:
parent
ad83944ded
commit
2cc8acb0b7
8 changed files with 251 additions and 0 deletions
58
crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py
vendored
Normal file
58
crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py
vendored
Normal file
|
@ -0,0 +1,58 @@
|
|||
import abc
|
||||
from abc import abstractmethod, ABCMeta
|
||||
|
||||
|
||||
# Errors
|
||||
|
||||
class A0(metaclass=abc.ABCMeta):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
||||
|
||||
|
||||
class A1(metaclass=ABCMeta):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
||||
|
||||
|
||||
class B0:
|
||||
def __init_subclass__(cls, **kwargs):
|
||||
super().__init_subclass__()
|
||||
|
||||
|
||||
class B1:
|
||||
pass
|
||||
|
||||
|
||||
class A2(B0, B1, metaclass=ABCMeta):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
||||
|
||||
|
||||
class A3(B0, before_metaclass=1, metaclass=abc.ABCMeta):
|
||||
pass
|
||||
|
||||
|
||||
# OK
|
||||
|
||||
class Meta(type):
|
||||
def __new__(cls, *args, **kwargs):
|
||||
return super().__new__(cls, *args)
|
||||
|
||||
|
||||
class A4(metaclass=Meta, no_metaclass=ABCMeta):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
||||
|
||||
|
||||
class A5(metaclass=Meta):
|
||||
pass
|
||||
|
||||
|
||||
class A6(abc.ABC):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
||||
|
||||
|
||||
class A7(B0, abc.ABC, B1):
|
||||
@abstractmethod
|
||||
def foo(self): pass
|
|
@ -516,6 +516,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
if checker.enabled(Rule::BadDunderMethodName) {
|
||||
pylint::rules::bad_dunder_method_name(checker, body);
|
||||
}
|
||||
if checker.enabled(Rule::MetaClassABCMeta) {
|
||||
refurb::rules::metaclass_abcmeta(checker, class_def);
|
||||
}
|
||||
}
|
||||
Stmt::Import(ast::StmtImport { names, range: _ }) => {
|
||||
if checker.enabled(Rule::MultipleImportsOnOneLine) {
|
||||
|
|
|
@ -1001,6 +1001,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
|
||||
(Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),
|
||||
(Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd),
|
||||
(Refurb, "180") => (RuleGroup::Preview, rules::refurb::rules::MetaClassABCMeta),
|
||||
(Refurb, "181") => (RuleGroup::Preview, rules::refurb::rules::HashlibDigestHex),
|
||||
|
||||
// flake8-logging
|
||||
|
|
|
@ -32,6 +32,7 @@ mod tests {
|
|||
#[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
|
||||
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
|
||||
#[test_case(Rule::RedundantLogBase, Path::new("FURB163.py"))]
|
||||
#[test_case(Rule::MetaClassABCMeta, Path::new("FURB180.py"))]
|
||||
#[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||
|
|
104
crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs
Normal file
104
crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs
Normal file
|
@ -0,0 +1,104 @@
|
|||
use itertools::Itertools;
|
||||
|
||||
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::StmtClassDef;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::importer::ImportRequest;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for uses of `metaclass=abc.ABCMeta` to define abstract base classes
|
||||
/// (ABCs).
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
///
|
||||
/// Instead of `class C(metaclass=abc.ABCMeta): ...`, use `class C(ABC): ...`
|
||||
/// to define an abstract base class. Inheriting from the `ABC` wrapper class
|
||||
/// is semantically identical to setting `metaclass=abc.ABCMeta`, but more
|
||||
/// succinct.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// class C(metaclass=ABCMeta):
|
||||
/// pass
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// class C(ABC):
|
||||
/// pass
|
||||
/// ```
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: `abc.ABC`](https://docs.python.org/3/library/abc.html#abc.ABC)
|
||||
/// - [Python documentation: `abc.ABCMeta`](https://docs.python.org/3/library/abc.html#abc.ABCMeta)
|
||||
#[violation]
|
||||
pub struct MetaClassABCMeta;
|
||||
|
||||
impl AlwaysFixableViolation for MetaClassABCMeta {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use of `metaclass=abc.ABCMeta` to define abstract base class")
|
||||
}
|
||||
|
||||
fn fix_title(&self) -> String {
|
||||
format!("Replace with `abc.ABC`")
|
||||
}
|
||||
}
|
||||
|
||||
/// FURB180
|
||||
pub(crate) fn metaclass_abcmeta(checker: &mut Checker, class_def: &StmtClassDef) {
|
||||
// Identify the `metaclass` keyword.
|
||||
let Some((position, keyword)) = class_def.keywords().iter().find_position(|&keyword| {
|
||||
keyword
|
||||
.arg
|
||||
.as_ref()
|
||||
.is_some_and(|arg| arg.as_str() == "metaclass")
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Determine whether it's assigned to `abc.ABCMeta`.
|
||||
if !checker
|
||||
.semantic()
|
||||
.resolve_call_path(&keyword.value)
|
||||
.is_some_and(|call_path| matches!(call_path.as_slice(), ["abc", "ABCMeta"]))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let mut diagnostic = Diagnostic::new(MetaClassABCMeta, keyword.range);
|
||||
|
||||
diagnostic.try_set_fix(|| {
|
||||
let (import_edit, binding) = checker.importer().get_or_import_symbol(
|
||||
&ImportRequest::import("abc", "ABC"),
|
||||
keyword.range.start(),
|
||||
checker.semantic(),
|
||||
)?;
|
||||
Ok(if position > 0 {
|
||||
// When the `abc.ABCMeta` is not the first keyword, put `abc.ABC` before the first
|
||||
// keyword.
|
||||
Fix::safe_edits(
|
||||
// Delete from the previous argument, to the end of the `metaclass` argument.
|
||||
Edit::range_deletion(TextRange::new(
|
||||
class_def.keywords()[position - 1].end(),
|
||||
keyword.end(),
|
||||
)),
|
||||
// Insert `abc.ABC` before the first keyword.
|
||||
[
|
||||
Edit::insertion(format!("{binding}, "), class_def.keywords()[0].start()),
|
||||
import_edit,
|
||||
],
|
||||
)
|
||||
} else {
|
||||
Fix::safe_edits(
|
||||
Edit::range_replacement(binding, keyword.range),
|
||||
[import_edit],
|
||||
)
|
||||
})
|
||||
});
|
||||
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
|
@ -6,6 +6,7 @@ pub(crate) use if_expr_min_max::*;
|
|||
pub(crate) use implicit_cwd::*;
|
||||
pub(crate) use isinstance_type_none::*;
|
||||
pub(crate) use math_constant::*;
|
||||
pub(crate) use metaclass_abcmeta::*;
|
||||
pub(crate) use print_empty_string::*;
|
||||
pub(crate) use read_whole_file::*;
|
||||
pub(crate) use redundant_log_base::*;
|
||||
|
@ -26,6 +27,7 @@ mod if_expr_min_max;
|
|||
mod implicit_cwd;
|
||||
mod isinstance_type_none;
|
||||
mod math_constant;
|
||||
mod metaclass_abcmeta;
|
||||
mod print_empty_string;
|
||||
mod read_whole_file;
|
||||
mod redundant_log_base;
|
||||
|
|
|
@ -0,0 +1,81 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/refurb/mod.rs
|
||||
---
|
||||
FURB180.py:7:10: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class
|
||||
|
|
||||
5 | # Errors
|
||||
6 |
|
||||
7 | class A0(metaclass=abc.ABCMeta):
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ FURB180
|
||||
8 | @abstractmethod
|
||||
9 | def foo(self): pass
|
||||
|
|
||||
= help: Replace with `abc.ABC`
|
||||
|
||||
ℹ Safe fix
|
||||
4 4 |
|
||||
5 5 | # Errors
|
||||
6 6 |
|
||||
7 |-class A0(metaclass=abc.ABCMeta):
|
||||
7 |+class A0(abc.ABC):
|
||||
8 8 | @abstractmethod
|
||||
9 9 | def foo(self): pass
|
||||
10 10 |
|
||||
|
||||
FURB180.py:12:10: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class
|
||||
|
|
||||
12 | class A1(metaclass=ABCMeta):
|
||||
| ^^^^^^^^^^^^^^^^^ FURB180
|
||||
13 | @abstractmethod
|
||||
14 | def foo(self): pass
|
||||
|
|
||||
= help: Replace with `abc.ABC`
|
||||
|
||||
ℹ Safe fix
|
||||
9 9 | def foo(self): pass
|
||||
10 10 |
|
||||
11 11 |
|
||||
12 |-class A1(metaclass=ABCMeta):
|
||||
12 |+class A1(abc.ABC):
|
||||
13 13 | @abstractmethod
|
||||
14 14 | def foo(self): pass
|
||||
15 15 |
|
||||
|
||||
FURB180.py:26:18: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class
|
||||
|
|
||||
26 | class A2(B0, B1, metaclass=ABCMeta):
|
||||
| ^^^^^^^^^^^^^^^^^ FURB180
|
||||
27 | @abstractmethod
|
||||
28 | def foo(self): pass
|
||||
|
|
||||
= help: Replace with `abc.ABC`
|
||||
|
||||
ℹ Safe fix
|
||||
23 23 | pass
|
||||
24 24 |
|
||||
25 25 |
|
||||
26 |-class A2(B0, B1, metaclass=ABCMeta):
|
||||
26 |+class A2(B0, B1, abc.ABC):
|
||||
27 27 | @abstractmethod
|
||||
28 28 | def foo(self): pass
|
||||
29 29 |
|
||||
|
||||
FURB180.py:31:34: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class
|
||||
|
|
||||
31 | class A3(B0, before_metaclass=1, metaclass=abc.ABCMeta):
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ FURB180
|
||||
32 | pass
|
||||
|
|
||||
= help: Replace with `abc.ABC`
|
||||
|
||||
ℹ Safe fix
|
||||
28 28 | def foo(self): pass
|
||||
29 29 |
|
||||
30 30 |
|
||||
31 |-class A3(B0, before_metaclass=1, metaclass=abc.ABCMeta):
|
||||
31 |+class A3(B0, abc.ABC, before_metaclass=1):
|
||||
32 32 | pass
|
||||
33 33 |
|
||||
34 34 |
|
||||
|
||||
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -2956,6 +2956,7 @@
|
|||
"FURB171",
|
||||
"FURB177",
|
||||
"FURB18",
|
||||
"FURB180",
|
||||
"FURB181",
|
||||
"G",
|
||||
"G0",
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue