From 14c42a8ddf377c8c79e43970a87b80af01258b5a Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Tue, 3 Jun 2025 00:51:09 +0000 Subject: [PATCH] [`refurb`] Mark `FURB180` fix unsafe when class has bases (#18149) ## 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 --- .../src/rules/refurb/rules/metaclass_abcmeta.rs | 17 +++++++++++++++-- ...ules__refurb__tests__FURB180_FURB180.py.snap | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs index e079bd93d8..96016b0351 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs @@ -1,5 +1,6 @@ use itertools::Itertools; +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::StmtClassDef; use ruff_text_size::{Ranged, TextRange}; @@ -31,6 +32,11 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// 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 /// - [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) @@ -69,6 +75,11 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) { return; } + let applicability = if class_def.bases().is_empty() { + Applicability::Safe + } else { + Applicability::Unsafe + }; let mut diagnostic = checker.report_diagnostic(MetaClassABCMeta, keyword.range); diagnostic.try_set_fix(|| { @@ -80,7 +91,7 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) { Ok(if position > 0 { // When the `abc.ABCMeta` is not the first keyword, put `abc.ABC` before the first // keyword. - Fix::safe_edits( + Fix::applicable_edits( // Delete from the previous argument, to the end of the `metaclass` argument. Edit::range_deletion(TextRange::new( 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()), import_edit, ], + applicability, ) } else { - Fix::safe_edits( + Fix::applicable_edits( Edit::range_replacement(binding, keyword.range), [import_edit], + applicability, ) }) }); 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 index 45df8c8e53..d351d92f93 100644 --- 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 @@ -50,7 +50,7 @@ FURB180.py:26:18: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract | = help: Replace with `abc.ABC` -ℹ Safe fix +ℹ Unsafe fix 23 23 | pass 24 24 | 25 25 | @@ -68,7 +68,7 @@ FURB180.py:31:34: FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract | = help: Replace with `abc.ABC` -ℹ Safe fix +ℹ Unsafe fix 28 28 | def foo(self): pass 29 29 | 30 30 |