Skip to content

Commit

Permalink
Tweak message and docs
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jan 31, 2024
1 parent a5998a1 commit 4b64a17
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 28 deletions.
57 changes: 37 additions & 20 deletions crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use itertools::Itertools;

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::map_callable;
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).
/// Checks for uses of `metaclass=abc.ABCMeta` to define abstract base classes
/// (ABCs).
///
/// ## Why is this bad?
/// Inheritance from the ABC wrapper class is semantically the same thing, but more succinct.
///
/// 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
Expand All @@ -34,52 +40,63 @@ 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")
format!("Use of `metaclass=abc.ABCMeta` to define abstract base class")
}

fn fix_title(&self) -> String {
"Replace with abc.ABC".into()
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")
&& checker
.semantic()
.resolve_call_path(map_callable(&keyword.value))
.is_some_and(|call_path| matches!(call_path.as_slice(), ["abc", "ABCMeta"]))
}) 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 {
Fix::safe_edits(
Edit::range_replacement(binding, keyword.range),
[import_edit],
)
} else {
// When the `abc.ABCMeta` is not the first keyword, put `abc.ABC` before the first keyword
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].range.end(),
keyword.range.end(),
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],
)
})
});

Expand Down
16 changes: 8 additions & 8 deletions ...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB180_FURB180.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB180.py:7:10: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Base Class
FURB180.py:7:10: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class
|
5 | # Errors
6 |
Expand All @@ -10,7 +10,7 @@ FURB180.py:7:10: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Bas
8 | @abstractmethod
9 | def foo(self): pass
|
= help: Replace with abc.ABC
= help: Replace with `abc.ABC`

ℹ Safe fix
4 4 |
Expand All @@ -22,14 +22,14 @@ FURB180.py:7:10: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Bas
9 9 | def foo(self): pass
10 10 |

FURB180.py:12:10: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Base Class
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
= help: Replace with `abc.ABC`

ℹ Safe fix
9 9 | def foo(self): pass
Expand All @@ -41,14 +41,14 @@ FURB180.py:12:10: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Ba
14 14 | def foo(self): pass
15 15 |

FURB180.py:26:18: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Base Class
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
= help: Replace with `abc.ABC`

ℹ Safe fix
23 23 | pass
Expand All @@ -60,13 +60,13 @@ FURB180.py:26:18: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Ba
28 28 | def foo(self): pass
29 29 |

FURB180.py:31:34: FURB180 [*] Use of metaclass=abc.ABCMeta to define Abstract Base Class
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
= help: Replace with `abc.ABC`

ℹ Safe fix
28 28 | def foo(self): pass
Expand Down

0 comments on commit 4b64a17

Please sign in to comment.