diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py index 33576061fc23a7..5a4902ebc724a6 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py @@ -32,6 +32,15 @@ class A3(B0, before_metaclass=1, metaclass=abc.ABCMeta): pass +# Fix should be unsafe when a comment would be deleted +class A4_comment( + other_kwarg=1, + # comment + metaclass=abc.ABCMeta, +): + pass + + # OK class Meta(type): 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 7e5a432968334e..a01bb3f302763d 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs @@ -43,6 +43,8 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// 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. /// +/// The fix is also marked unsafe if it would delete comments. +/// /// ## 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) @@ -82,11 +84,6 @@ 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(|| { @@ -98,12 +95,20 @@ 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. + let deletion_range = TextRange::new( + class_def.keywords()[position - 1].end(), + keyword.end(), + ); + // The fix is unsafe if the class has base classes or if comments would be deleted. + let applicability = + if class_def.bases().is_empty() && !checker.comment_ranges().intersects(deletion_range) { + Applicability::Safe + } else { + Applicability::Unsafe + }; 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(), - keyword.end(), - )), + Edit::range_deletion(deletion_range), // Insert `abc.ABC` before the first keyword. [ Edit::insertion(format!("{binding}, "), class_def.keywords()[0].start()), @@ -112,6 +117,11 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) { applicability, ) } else { + let applicability = if class_def.bases().is_empty() { + Applicability::Safe + } else { + Applicability::Unsafe + }; Fix::applicable_edits( Edit::range_replacement(binding, keyword.range), [import_edit], 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 be994f1cc5d438..7e4c40cc835747 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 @@ -75,3 +75,26 @@ help: Replace with `abc.ABC` 33 | 34 | note: This is an unsafe fix and may change runtime behavior + +FURB180 [*] Use of `metaclass=abc.ABCMeta` to define abstract base class + --> FURB180.py:39:5 + | +37 | other_kwarg=1, +38 | # comment +39 | metaclass=abc.ABCMeta, + | ^^^^^^^^^^^^^^^^^^^^^ +40 | ): +41 | pass + | +help: Replace with `abc.ABC` +34 | +35 | # Fix should be unsafe when a comment would be deleted +36 | class A4_comment( + - other_kwarg=1, + - # comment + - metaclass=abc.ABCMeta, +37 + abc.ABC, other_kwarg=1, +38 | ): +39 | pass +40 | +note: This is an unsafe fix and may change runtime behavior