diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py new file mode 100644 index 0000000000000..33576061fc23a --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py @@ -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 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 481e52a008340..fae6b0b527b99 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -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) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 6f802c79bd1e4..ded0fe7eeff7e 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index fde53bda5f2c5..26ba58041933b 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -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()); diff --git a/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs new file mode 100644 index 0000000000000..c5700635c9f76 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs @@ -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); +} diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index ea9fbceef645f..532003ee143df 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB180_FURB180.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB180_FURB180.py.snap new file mode 100644 index 0000000000000..0d683880efd9d --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB180_FURB180.py.snap @@ -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 | + + diff --git a/ruff.schema.json b/ruff.schema.json index b347dcd00af5e..dbebae5df0613 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2956,6 +2956,7 @@ "FURB171", "FURB177", "FURB18", + "FURB180", "FURB181", "G", "G0",