[`refurb`] Do not add `abc.ABC` if already present (`FURB180`) by akawd · Pull Request #22234 · astral-sh/ruff

akawd · 2025-12-28T11:38:46Z

Summary

According to the FURB180 rule, abc.ABC should be used instead of metaclass=abc.ABCMeta. This issue can be fixed automatically, but if the class being checked already contains abc.ABC, it would be added a second time.

Fixes: #17162

Test Plan

I ran cargo test refurb.
For checking the "fix" I used this python file:

Python file (input)

import abc
from abc import abstractmethod, ABCMeta, ABC
from abc import ABC as ABCAnotherName

class ParentClass:
    ...

from abc import ABC, ABCMeta

class Foo(metaclass=ABCMeta):
    ...
    
class Foo2(abc.ABC, ParentClass, metaclass=ABCMeta):
    ...
    
class Foo3(ParentClass, ABC, metaclass=ABCMeta):
    ...
    
class Foo4(ABCAnotherName, metaclass=ABCMeta):
    ...

The output after applying the fix is so:

Python file (output)

import abc
from abc import abstractmethod, ABCMeta, ABC
from abc import ABC as ABCAnotherName

class ParentClass:
    ...

from abc import ABC, ABCMeta

class Foo(ABC):
    ...
    
class Foo2(abc.ABC, ParentClass, ):
    ...
    
class Foo3(ParentClass, ABC, ):
    ...
    
class Foo4(ABCAnotherName, ):
    ...

Note: Trailing commas may remain after the automatic fix. It is unclear whether this is an issue.

Summary According to the FURB180 rule, abc.ABC should be used instead of metaclass=abc.ABCMeta. This issue can be fixed automatically, but if the class being checked already contains abc.ABC, it would be added a second time.

ntBre

Thanks! This looks good to me overall, I just had a couple of small suggestions. We should also add your manual tests from the PR summary as regression tests. At the bottom of this file would be a good place for them:

ruff/crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py

Line 58 in b2b9d91

def foo(self): pass

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

astral-sh-bot · 2026-01-01T14:35:56Z

`ruff-ecosystem` results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+0 -12 violations, +10 -0 fixes in 5 projects; 50 projects unchanged)

apache/superset (+0 -1 violations, +10 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --no-preview --select ALL

- superset/charts/client_processing.py:285:17: UP045 Use `X | None` for type annotations
+ superset/charts/client_processing.py:285:17: UP045 [*] Use `X | None` for type annotations
- superset/db_engine_specs/snowflake.py:348:26: UP045 Use `X | None` for type annotations
+ superset/db_engine_specs/snowflake.py:348:26: UP045 [*] Use `X | None` for type annotations
- superset/db_engine_specs/snowflake.py:370:26: UP045 Use `X | None` for type annotations
+ superset/db_engine_specs/snowflake.py:370:26: UP045 [*] Use `X | None` for type annotations
- superset/mcp_service/utils/retry_utils.py:229:9: PYI034 `__enter__` methods in classes like `RetryableOperation` usually return `self` at runtime
- superset/models/helpers.py:2634:17: UP045 Use `X | None` for type annotations
+ superset/models/helpers.py:2634:17: UP045 [*] Use `X | None` for type annotations
- superset/models/helpers.py:897:29: UP045 Use `X | None` for type annotations
+ superset/models/helpers.py:897:29: UP045 [*] Use `X | None` for type annotations

freedomofpress/securedrop (+0 -1 violations, +0 -0 fixes)

- devops/scripts/verify-mo.py:54:9: PYI034 `__enter__` methods in classes like `CatalogVerifier` usually return `self` at runtime

reflex-dev/reflex (+0 -2 violations, +0 -0 fixes)

- reflex/components/recharts/cartesian.py:245:5: PIE794 Class field `fill` is defined multiple times
- reflex/components/recharts/cartesian.py:248:5: PIE794 Class field `stroke` is defined multiple times

rotki/rotki (+0 -7 violations, +0 -0 fixes)

- rotkehlchen/assets/resolver.py:38:9: PYI034 `__new__` methods in classes like `AssetResolver` usually return `self` at runtime
- rotkehlchen/db/drivers/gevent.py:72:9: PYI034 `__enter__` methods in classes like `DBCursor` usually return `self` at runtime
- rotkehlchen/db/settings.py:512:9: PYI034 `__new__` methods in classes like `CachedSettings` usually return `self` at runtime
- rotkehlchen/globaldb/handler.py:118:9: PYI034 `__new__` methods in classes like `GlobalDBHandler` usually return `self` at runtime
- rotkehlchen/history/price.py:82:9: PYI034 `__new__` methods in classes like `PriceHistorian` usually return `self` at runtime
- rotkehlchen/inquirer.py:355:9: PYI034 `__new__` methods in classes like `Inquirer` usually return `self` at runtime
- rotkehlchen/utils/hexbytes.py:46:9: PYI034 `__new__` methods in classes like `HexBytes` usually return `self` at runtime

scikit-build/scikit-build-core (+0 -1 violations, +0 -0 fixes)

- src/scikit_build_core/settings/skbuild_model.py:50:9: PYI034 `__new__` methods in classes like `CMakeSettingsDefine` usually return `self` at runtime

Changes by rule (3 rules affected)

code	total	- violation	+ fix
PYI034	10	10	0
UP045	10	0	10
PIE794	2	2	0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -12 violations, +10 -0 fixes in 5 projects; 50 projects unchanged)

apache/superset (+0 -1 violations, +10 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL

- superset/charts/client_processing.py:285:17: UP045 Use `X | None` for type annotations
+ superset/charts/client_processing.py:285:17: UP045 [*] Use `X | None` for type annotations
- superset/db_engine_specs/snowflake.py:348:26: UP045 Use `X | None` for type annotations
+ superset/db_engine_specs/snowflake.py:348:26: UP045 [*] Use `X | None` for type annotations
- superset/db_engine_specs/snowflake.py:370:26: UP045 Use `X | None` for type annotations
+ superset/db_engine_specs/snowflake.py:370:26: UP045 [*] Use `X | None` for type annotations
- superset/mcp_service/utils/retry_utils.py:229:9: PYI034 `__enter__` methods in classes like `RetryableOperation` usually return `self` at runtime
- superset/models/helpers.py:2634:17: UP045 Use `X | None` for type annotations
+ superset/models/helpers.py:2634:17: UP045 [*] Use `X | None` for type annotations
- superset/models/helpers.py:897:29: UP045 Use `X | None` for type annotations
+ superset/models/helpers.py:897:29: UP045 [*] Use `X | None` for type annotations

freedomofpress/securedrop (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

- devops/scripts/verify-mo.py:54:9: PYI034 `__enter__` methods in classes like `CatalogVerifier` usually return `self` at runtime

reflex-dev/reflex (+0 -2 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

- reflex/components/recharts/cartesian.py:245:5: PIE794 Class field `fill` is defined multiple times
- reflex/components/recharts/cartesian.py:248:5: PIE794 Class field `stroke` is defined multiple times

rotki/rotki (+0 -7 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

- rotkehlchen/assets/resolver.py:38:9: PYI034 `__new__` methods in classes like `AssetResolver` usually return `self` at runtime
- rotkehlchen/db/drivers/gevent.py:72:9: PYI034 `__enter__` methods in classes like `DBCursor` usually return `self` at runtime
- rotkehlchen/db/settings.py:512:9: PYI034 `__new__` methods in classes like `CachedSettings` usually return `self` at runtime
- rotkehlchen/globaldb/handler.py:118:9: PYI034 `__new__` methods in classes like `GlobalDBHandler` usually return `self` at runtime
- rotkehlchen/history/price.py:82:9: PYI034 `__new__` methods in classes like `PriceHistorian` usually return `self` at runtime
- rotkehlchen/inquirer.py:355:9: PYI034 `__new__` methods in classes like `Inquirer` usually return `self` at runtime
- rotkehlchen/utils/hexbytes.py:46:9: PYI034 `__new__` methods in classes like `HexBytes` usually return `self` at runtime

scikit-build/scikit-build-core (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

- src/scikit_build_core/settings/skbuild_model.py:50:9: PYI034 `__new__` methods in classes like `CMakeSettingsDefine` usually return `self` at runtime

Changes by rule (3 rules affected)

code	total	- violation	+ fix
PYI034	10	10	0
UP045	10	0	10
PIE794	2	2	0

- use of `analyze::class::any_qualified_base_class`, - use of a stack slice instead of a heap-allocated `Vec`, - `range_deletion` instead of `deletion`, - the appropriate `py.snap` file was updated.

akawd · 2026-01-02T09:00:05Z

According to @"... add your manual tests from the PR summary as regression tests." - I added the following changes to FURB180.py:

from abc import abstractmethod, ABCMeta, ABC
from abc import ABC as ABCAlternativeName

...
class A7(ABC):
    @abstractmethod
    def foo(self): pass

class A8(ABCAlternativeName):
    @abstractmethod
    def foo(self): pass  
...

I hope this is what was expected.

ntBre · 2026-01-02T14:58:13Z

Thank you! That was close to what I had in mind, but I think your tests in the summary were a bit more helpful than the ones added to FURB180.py. I just pushed two commits updating the tests. A couple of things I tried to fix:

I avoided changing any lines earlier in the file, this is nice for review because it doesn't shift the line numbers in the existing snapshots
I added metaclass=ABCMeta to the new test cases so that FURB180 triggers
I added class A11 showing off the parent class traversal that any_qualified_base_class gets us

After making these changes, it showed that there's an issue with the fix:

- class A11(MyMetaClass, metaclass=ABCMeta):  # FURB180
+ class A11(MyMetaClass, ):  # FURB180

We're leaving a trailing comma after removing the metaclass argument. I think we might want to try the remove_argument helper function here. Sorry, I should have recommended that sooner!

akawd · 2026-01-02T19:26:16Z

According to: "We're leaving a trailing comma after removing the metaclass argument".
Marked as draft to cover this issue.

akawd · 2026-01-03T07:23:22Z

We're leaving a trailing comma after removing the metaclass argument. I think we might want to try the remove_argument helper function here.

Fixed the issue above here: 740da9e

Example.

Before fix:

import abc
from abc import abstractmethod, ABCMeta, ABC
from abc import ABC as ABCAnotherName

class ParentClass:
    ...

class Foo(metaclass=ABCMeta):
    ...
    
class Foo2(abc.ABC, ParentClass, metaclass=ABCMeta):
    ...
    
class Foo3(ParentClass, ABC, metaclass=ABCMeta):
    ...
    
class Foo4(ABCAnotherName, metaclass=ABCMeta):
    ...

After fix:

import abc
from abc import abstractmethod, ABCMeta, ABC
from abc import ABC as ABCAnotherName

class ParentClass:
    ...

class Foo(ABCAnotherName):
    ...
    
class Foo2(abc.ABC, ParentClass):
    ...
    
class Foo3(ParentClass, ABC):
    ...
    
class Foo4(ABCAnotherName):
    ...

akawd · 2026-01-03T07:27:43Z

And since there are several commits here, do I need to squash them?

akawd · 2026-01-13T18:43:05Z

Hello, @ntBre . Just a friendly reminder and request to take a look at this if possible.

ntBre · 2026-01-13T20:08:46Z

Thank you! I was on PTO last week, but I'm back now :) I'll take a look soon.

And since there are several commits here, do I need to squash them?

Don't worry about squashing, we'll squash-merge at the end, so it's easier to keep all the commits for review in the meantime.

ntBre

Thank you! This looks great, I just had a couple of minor suggestions. I also noticed a potential issue with the fix safety, but I can turn that into a separate issue if you'd rather not update it here.

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

Better comment. Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>

akawd · 2026-01-16T20:14:34Z

@ntBre , sorry for delay with the answer.
Thank you for your suggestions, I added them.

Could you please clarify what docs you meant here?

We'd also need to update the docs.

ntBre

Thank you! I had one suggestion about narrowing the comment range check and a test for that. Then we just need to update the ## Fix safety section of the rule docs.

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py

akawd · 2026-01-17T05:05:06Z

Thank you! I had one suggestion about narrowing the comment range check and a test for that. Then we just need to update the ## Fix safety section of the rule docs.

Yep, updated the doc too: 0292c87

ntBre

Thank you!

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

* main: (62 commits) [`refurb`] Do not add `abc.ABC` if already present (`FURB180`) (#22234) [ty] Add a new `assert-type-unspellable-subtype` diagnostic (#22815) [ty] Avoid duplicate syntax errors for `await` outside functions (#22826) [ty] Fix unary operator false-positive for constrained TypeVars (#22783) [ty] Fix binary operator false-positive for constrained TypeVars (#22782) [ty] Fix false-positive `unsupported-operator` for "symmetric" TypeVars (#22756) [`pydocstyle`] Clarify which quote styles are allowed (`D300`) (#22825) [ty] Use distributed versions of AND and OR on constraint sets (#22614) [ty] Add support for dict literals and dict() calls as default values for parameters with TypedDict types (#22161) Document `-` stdin convention in CLI help text (#22817) [ty] Make `infer_subscript_expression_types` a method on `Type` (#22731) [ty] Simplify `OverloadLiteral::spans` and `OverloadLiteral::parameter_span` (#22823) [ty] Require both `*args` and `**kwargs` when calling a `ParamSpec` callable (#22820) [ty] Handle tagged errors in conformance (#22746) Add `--color` cli option to force colored output (#22806) Identify notebooks by LSP didOpen instead of `.ipynb` file extension (#22810) [ty] Fix docstring rendering for literal blocks after doctests (#22676) [ty] Update salsa to fix out-of-order query validation (#22498) [ty] Inline cycle initial and recovery functions (#22814) [ty] Pass the generic context through the decorator (#22544) ...

[refurb] Do not add abc.ABC if already present

50a2f2e

Summary According to the FURB180 rule, abc.ABC should be used instead of metaclass=abc.ABCMeta. This issue can be fixed automatically, but if the class being checked already contains abc.ABC, it would be added a second time.

ntBre self-requested a review December 29, 2025 14:18

ntBre added the fixes Related to suggested fixes for violations label Jan 1, 2026

ntBre requested changes Jan 1, 2026

View reviewed changes

ntBre changed the title ~~[refurb] Do not add abc.ABC if already present~~ [refurb] Do not add abc.ABC if already present (FURB180) Jan 1, 2026

Review comments were applied.

bbd48bf

- use of `analyze::class::any_qualified_base_class`, - use of a stack slice instead of a heap-allocated `Vec`, - `range_deletion` instead of `deletion`, - the appropriate `py.snap` file was updated.

akawd requested a review from ntBre January 2, 2026 09:06

ntBre added 2 commits January 2, 2026 09:39

restore old test line numbers, copy more tests from summary

cd6e448

add a parent class case using any_qualified_base_class

3cbe1e6

Use of a heap-allocated Vec instead a stack slice.

aa524c8

akawd marked this pull request as draft January 2, 2026 19:24

Remove redundant commas.

740da9e

akawd marked this pull request as ready for review January 3, 2026 07:24

ntBre reviewed Jan 13, 2026

View reviewed changes

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs Show resolved Hide resolved

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs Outdated Show resolved Hide resolved

akawd and others added 3 commits January 14, 2026 14:42

Update crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

f4d30a6

Better comment. Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>

Some refactor of the deletion action.

f52d83e

Comment makes the fix unsafe.

a209175

akawd requested a review from ntBre January 16, 2026 20:14

ntBre linked an issue Jan 16, 2026 that may be closed by this pull request

Safe fix for FURB180 can delete comments #22631

Closed

ntBre reviewed Jan 16, 2026

View reviewed changes

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs Show resolved Hide resolved

crates/ruff_linter/resources/test/fixtures/refurb/FURB180.py Show resolved Hide resolved

Rule doc safaty updated.

0292c87

The test case added for a rule.

9c42918

akawd requested a review from ntBre January 17, 2026 05:05

This was referenced Jan 18, 2026

Safe fix for FURB180 can delete comments #22631

Closed

[refurb] Mark FURB180 fix as unsafe when comments would be deleted #22685

Closed

refactor(refurb): mark MetaClassABCMeta fix as unsafe if comments would be deleted #22824

Closed

ntBre approved these changes Jan 23, 2026

View reviewed changes

crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs Outdated Show resolved Hide resolved

Update crates/ruff_linter/src/rules/refurb/rules/metaclass_abcmeta.rs

df931e0

ntBre enabled auto-merge (squash) January 23, 2026 21:31

ntBre merged commit 0255200 into astral-sh:main Jan 23, 2026
40 checks passed

BrewTestBot mentioned this pull request Feb 3, 2026

ruff 0.15.0 Homebrew/homebrew-core#265732

Merged

This was referenced Feb 9, 2026

[pre-commit.ci] pre-commit autoupdate kraken-tech/django-subatomic#125

Merged

[pre-commit.ci] pre-commit autoupdate kraken-tech/timezone-tools#235

Merged

Conversation

akawd commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

astral-sh-bot bot commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

akawd commented Jan 2, 2026

Uh oh!

ntBre commented Jan 2, 2026

Uh oh!

akawd commented Jan 2, 2026

Uh oh!

akawd commented Jan 3, 2026

Uh oh!

akawd commented Jan 3, 2026

Uh oh!

akawd commented Jan 13, 2026

Uh oh!

ntBre commented Jan 13, 2026

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akawd commented Jan 16, 2026

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akawd commented Jan 17, 2026

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

akawd commented Dec 28, 2025 •

edited

Loading

astral-sh-bot bot commented Jan 1, 2026 •

edited

Loading

`ruff-ecosystem` results