Skip to content

Commit

Permalink
[refurb] Implement metaclass_abcmeta (FURB180) (#9658)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
alex-700 authored Jan 31, 2024
1 parent ad83944 commit 2cc8acb
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 0 deletions.
58 changes: 58 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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 @@ -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
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
104 changes: 104 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 change: 1 addition & 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 2cc8acb

Please sign in to comment.