diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py index 33576061fc23a7..84e309c1d554fc 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py @@ -56,3 +56,11 @@ def foo(self): pass class A7(B0, abc.ABC, B1): @abstractmethod def foo(self): pass + +# Issue 22631 reproduction +class C( + other_kwarg=1, + # comment + metaclass=abc.ABCMeta, +): + pass \ No newline at end of file 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..17152b0b66599e 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs @@ -39,9 +39,11 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// ``` /// /// ## 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. +/// The rule's fix is unsafe if the class has base classes, or if the fix would result in the +/// deletion of comments. +/// +/// If the class has base classes, inheriting from `abc.ABC` might alter the runtime behavior if +/// those base classes validate the class's other base classes (e.g., `typing.Protocol`). /// /// ## References /// - [Python documentation: `abc.ABC`](https://docs.python.org/3/library/abc.html#abc.ABC) @@ -82,11 +84,25 @@ pub(crate) fn metaclass_abcmeta(checker: &Checker, class_def: &StmtClassDef) { return; } - let applicability = if class_def.bases().is_empty() { + let mut applicability = if class_def.bases().is_empty() { Applicability::Safe } else { Applicability::Unsafe }; + + // If the fix involves a range deletion or replacement that contains a comment, the fix is + // unsafe. + if applicability == Applicability::Safe { + let range = if position > 0 { + TextRange::new(class_def.keywords()[position - 1].end(), keyword.end()) + } else { + keyword.range() + }; + if checker.comment_ranges().intersects(range) { + applicability = Applicability::Unsafe; + } + } + let mut diagnostic = checker.report_diagnostic(MetaClassABCMeta, keyword.range); diagnostic.try_set_fix(|| { 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..444068ac397f0c 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,25 @@ 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:64:5 + | +62 | other_kwarg=1, +63 | # comment +64 | metaclass=abc.ABCMeta, + | ^^^^^^^^^^^^^^^^^^^^^ +65 | ): +66 | pass + | +help: Replace with `abc.ABC` +59 | +60 | # Issue 22631 reproduction +61 | class C( + - other_kwarg=1, + - # comment + - metaclass=abc.ABCMeta, +62 + abc.ABC, other_kwarg=1, +63 | ): +64 | pass +note: This is an unsafe fix and may change runtime behavior