mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 13:51:16 +00:00
[refurb
] Mark FURB180
fix unsafe when class has bases (#18149)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary Mark `FURB180`'s fix as unsafe if the class already has base classes. This is because the base classes might validate the other base classes (like `typing.Protocol` does) or otherwise alter runtime behavior if more base classes are added. ## Test Plan The existing snapshot test covers this case already. ## References Partially addresses https://github.com/astral-sh/ruff/issues/13307 (left out way to permit certain exceptions) --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
e677863787
commit
14c42a8ddf
2 changed files with 17 additions and 4 deletions
|
@ -1,5 +1,6 @@
|
||||||
use itertools::Itertools;
|
use itertools::Itertools;
|
||||||
|
|
||||||
|
use ruff_diagnostics::Applicability;
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||||
use ruff_python_ast::StmtClassDef;
|
use ruff_python_ast::StmtClassDef;
|
||||||
use ruff_text_size::{Ranged, TextRange};
|
use ruff_text_size::{Ranged, TextRange};
|
||||||
|
@ -31,6 +32,11 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
|
||||||
/// pass
|
/// pass
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
|
/// ## Fix safety
|
||||||
|
/// The rule's fix is unsafe if the class has base classes. This is because the base classes might
|
||||||
|
/// be validating the class's other base classes (e.g., `typing.Protocol` does this) or otherwise
|
||||||
|
/// alter runtime behavior if more base classes are added.
|
||||||
|
///
|
||||||
/// ## References
|
/// ## References
|
||||||
/// - [Python documentation: `abc.ABC`](https://docs.python.org/3/library/abc.html#abc.ABC)
|
/// - [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)
|
/// - [Python documentation: `abc.ABCMeta`](https://docs.python.org/3/library/abc.html#abc.ABCMeta)
|
||||||
|
@ -69,6 +75,11 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let applicability = if class_def.bases().is_empty() {
|
||||||
|
Applicability::Safe
|
||||||
|
} else {
|
||||||
|
Applicability::Unsafe
|
||||||
|
};
|
||||||
let mut diagnostic = checker.report_diagnostic(MetaClassABCMeta, keyword.range);
|
let mut diagnostic = checker.report_diagnostic(MetaClassABCMeta, keyword.range);
|
||||||
|
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
|
@ -80,7 +91,7 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) {
|
||||||
Ok(if position > 0 {
|
Ok(if position > 0 {
|
||||||
// When the `abc.ABCMeta` is not the first keyword, put `abc.ABC` before the first
|
// When the `abc.ABCMeta` is not the first keyword, put `abc.ABC` before the first
|
||||||
// keyword.
|
// keyword.
|
||||||
Fix::safe_edits(
|
Fix::applicable_edits(
|
||||||
// Delete from the previous argument, to the end of the `metaclass` argument.
|
// Delete from the previous argument, to the end of the `metaclass` argument.
|
||||||
Edit::range_deletion(TextRange::new(
|
Edit::range_deletion(TextRange::new(
|
||||||
class_def.keywords()[position - 1].end(),
|
class_def.keywords()[position - 1].end(),
|
||||||
|
@ -91,11 +102,13 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) {
|
||||||
Edit::insertion(format!("{binding}, "), class_def.keywords()[0].start()),
|
Edit::insertion(format!("{binding}, "), class_def.keywords()[0].start()),
|
||||||
import_edit,
|
import_edit,
|
||||||
],
|
],
|
||||||
|
applicability,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
Fix::safe_edits(
|
Fix::applicable_edits(
|
||||||
Edit::range_replacement(binding, keyword.range),
|
Edit::range_replacement(binding, keyword.range),
|
||||||
[import_edit],
|
[import_edit],
|
||||||
|
applicability,
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
});
|
});
|
||||||
|
|
|
@ -50,7 +50,7 @@ FURB180.py:26:18: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract
|
||||||
|
|
|
|
||||||
= help: Replace with `abc.ABC`
|
= help: Replace with `abc.ABC`
|
||||||
|
|
||||||
ℹ Safe fix
|
ℹ Unsafe fix
|
||||||
23 23 | pass
|
23 23 | pass
|
||||||
24 24 |
|
24 24 |
|
||||||
25 25 |
|
25 25 |
|
||||||
|
@ -68,7 +68,7 @@ FURB180.py:31:34: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract
|
||||||
|
|
|
|
||||||
= help: Replace with `abc.ABC`
|
= help: Replace with `abc.ABC`
|
||||||
|
|
||||||
ℹ Safe fix
|
ℹ Unsafe fix
|
||||||
28 28 | def foo(self): pass
|
28 28 | def foo(self): pass
|
||||||
29 29 |
|
29 29 |
|
||||||
30 30 |
|
30 30 |
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue