diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB189.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB189.py index 9ac8e83b38..4df913feff 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB189.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB189.py @@ -8,7 +8,7 @@ class SetOnceMappingMixin: if key in self: raise KeyError(str(key) + ' already set') return super().__setitem__(key, value) - + class CaseInsensitiveEnumMeta(EnumMeta): pass @@ -23,6 +23,12 @@ class L(list): class S(str): pass +class SubscriptDict(dict[str, str]): + pass + +class SubscriptList(list[str]): + pass + # currently not detected class SetOnceDict(SetOnceMappingMixin, dict): pass diff --git a/crates/ruff_linter/src/rules/refurb/rules/subclass_builtin.rs b/crates/ruff_linter/src/rules/refurb/rules/subclass_builtin.rs index 234003ee73..3ffd1a254c 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/subclass_builtin.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/subclass_builtin.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{Arguments, StmtClassDef}; +use ruff_python_ast::{helpers::map_subscript, Arguments, StmtClassDef}; use ruff_text_size::Ranged; use crate::{checkers::ast::Checker, importer::ImportRequest}; @@ -70,11 +70,16 @@ pub(crate) fn subclass_builtin(checker: &Checker, class: &StmtClassDef) { return; }; + // Expect only one base class else return let [base] = &**bases else { return; }; - let Some(symbol) = checker.semantic().resolve_builtin_symbol(base) else { + // Check if the base class is a subscript expression so that only the name expr + // is checked and modified. + let base_expr = map_subscript(base); + + let Some(symbol) = checker.semantic().resolve_builtin_symbol(base_expr) else { return; }; @@ -89,7 +94,7 @@ pub(crate) fn subclass_builtin(checker: &Checker, class: &StmtClassDef) { subclass: symbol.to_string(), replacement: user_symbol.to_string(), }, - base.range(), + base_expr.range(), ); diagnostic.try_set_fix(|| { let (import_edit, binding) = checker.importer().get_or_import_symbol( @@ -97,7 +102,7 @@ pub(crate) fn subclass_builtin(checker: &Checker, class: &StmtClassDef) { base.start(), checker.semantic(), )?; - let other_edit = Edit::range_replacement(binding, base.range()); + let other_edit = Edit::range_replacement(binding, base_expr.range()); Ok(Fix::unsafe_edits(import_edit, [other_edit])) }); checker.report_diagnostic(diagnostic); diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB189_FURB189.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB189_FURB189.py.snap index 43855d425b..9fc4626b5a 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB189_FURB189.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB189_FURB189.py.snap @@ -74,4 +74,52 @@ FURB189.py:23:9: FURB189 [*] Subclassing `str` can be error prone, use `collecti 23 |+class S(UserString): 24 24 | pass 25 25 | -26 26 | # currently not detected +26 26 | class SubscriptDict(dict[str, str]): + +FURB189.py:26:21: FURB189 [*] Subclassing `dict` can be error prone, use `collections.UserDict` instead + | +24 | pass +25 | +26 | class SubscriptDict(dict[str, str]): + | ^^^^ FURB189 +27 | pass + | + = help: Replace with `collections.UserDict` + +ℹ Unsafe fix +1 1 | # setup +2 2 | from enum import Enum, EnumMeta +3 |-from collections import UserList as UL + 3 |+from collections import UserList as UL, UserDict +4 4 | +5 5 | class SetOnceMappingMixin: +6 6 | __slots__ = () +-------------------------------------------------------------------------------- +23 23 | class S(str): +24 24 | pass +25 25 | +26 |-class SubscriptDict(dict[str, str]): + 26 |+class SubscriptDict(UserDict[str, str]): +27 27 | pass +28 28 | +29 29 | class SubscriptList(list[str]): + +FURB189.py:29:21: FURB189 [*] Subclassing `list` can be error prone, use `collections.UserList` instead + | +27 | pass +28 | +29 | class SubscriptList(list[str]): + | ^^^^ FURB189 +30 | pass + | + = help: Replace with `collections.UserList` + +ℹ Unsafe fix +26 26 | class SubscriptDict(dict[str, str]): +27 27 | pass +28 28 | +29 |-class SubscriptList(list[str]): + 29 |+class SubscriptList(UL[str]): +30 30 | pass +31 31 | +32 32 | # currently not detected