[refurb] Check for subclasses includes subscript expressions (FURB189) (#16155)

## Summary

Added checks for subscript expressions on builtin classes as in FURB189.
The object is changed to use the collections objects and the types from
the subscript are kept.

Resolves #16130 

> Note: Added some comments in the code explaining why
## Test Plan


- Added a subscript dict and list class to the test file.
- Tested locally to check that the symbols are changed and the types are
kept.
- No modifications changed on optional `str` values.

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

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);

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