[refurb] Implement subclass-builtin (FURB189)

[sbrugman]

Summary

Implementation for one of the rules in #1348
Refurb only deals only with classes with a single base, however the rule is valid for any base.
(str, Enum is common prior to StrEnum)

Test Plan

cargo test

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

apache/airflow (+6 -0 violations, +0 -0 fixes)

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

+ airflow/decorators/setup_teardown.py:58:22: FURB189 Subclassing `list` can be error prone, use `collections.UserList` instead
+ airflow/io/utils/stat.py:22:19: FURB189 Subclassing `dict` can be error prone, use `collections.UserDict` instead
+ kubernetes_tests/test_base.py:45:26: FURB189 Subclassing `str` can be error prone, use `collections.UserStr` instead
+ providers/src/airflow/providers/openlineage/utils/utils.py:170:25: FURB189 Subclassing `dict` can be error prone, use `collections.UserDict` instead
+ providers/tests/openlineage/plugins/test_utils.py:67:19: FURB189 Subclassing `dict` can be error prone, use `collections.UserDict` instead
+ tests/utils/log/test_secrets_masker.py:317:54: FURB189 Subclassing `str` can be error prone, use `collections.UserStr` instead

bokeh/bokeh (+1 -0 violations, +0 -0 fixes)

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

+ src/bokeh/models/plots.py:903:24: FURB189 Subclassing `list` can be error prone, use `collections.UserList` instead

langchain-ai/langchain (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ libs/core/tests/unit_tests/stubs.py:7:14: FURB189 Subclassing `str` can be error prone, use `collections.UserStr` instead

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

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/scikit_build_core/settings/skbuild_model.py:31:27: FURB189 Subclassing `str` can be error prone, use `collections.UserStr` instead
+ src/scikit_build_core/setuptools/build_cmake.py:214:24: FURB189 Subclassing `list` can be error prone, use `collections.UserList` instead

indico/indico (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ indico/legacy/pdfinterface/latex.py:169:16: FURB189 Subclassing `str` can be error prone, use `collections.UserStr` instead
+ indico/modules/events/abstracts/util.py:141:24: FURB189 Subclassing `dict` can be error prone, use `collections.UserDict` instead

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FURB189	12	12	0	0	0

[sbrugman]

Thanks for the helpful pointers @dhruvmanila!

[dhruvmanila]

Thanks! I made a couple of minor changes:

• Moved the utilities below the rule logic function; we do this for all rules
• Avoid the eager .to_string call for user_symbol method and only allocate it when creating the Diagnostic

[dhruvmanila]

Looking at the ecosystem changes, we might want to special case (str, Enum).

Refurb only deals only with classes with a single base, however the rule is valid for any base.

Sorry, I missed this earlier but can you expand on why do we want to diverge from this behavior.

Edit: I think we should keep the same behavior as refurb and check only when there's one base class.

[sbrugman]

Fair point. After looking at the ecosystem results, I think we indeed should stick with refurb's behaviour (at least until type inference is available).

Only considering classes with a single base is the least error prone.

It also has some false negatives which can be detected by considering multiple bases, e.g. https://github.com/bokeh/bokeh/blob/829b2a75c402d0d0abd7e37ff201fbdfd949d857/src/bokeh/core/property/wrappers.py#L306.

However there are false positives for:

• str, Enum is a special case already handled by another rule: https://docs.astral.sh/ruff/rules/replace-str-enum.
• At the moment we cannot detect cases such as str, SubclassOfEnum: https://github.com/indico/indico/blob/5b70ccf74a5125d22325406c52ce43c82b364170/indico/modules/events/timetable/reschedule.py#L27

[dhruvmanila]

It also has some false negatives which can be detected by considering multiple bases, e.g. bokeh/bokeh@829b2a7/src/bokeh/core/property/wrappers.py#L306.

However there are false positives for:

• str, Enum is a special case already handled by another rule: docs.astral.sh/ruff/rules/replace-str-enum.
• At the moment we cannot detect cases such as str, SubclassOfEnum: indico/indico@5b70ccf/indico/modules/events/timetable/reschedule.py#L27

I think the listed limitations are a good trade-off.