diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index c9b136bcf7..4aa40ea1d5 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2174,3 +2174,54 @@ fn flake8_import_convention_unused_aliased_import() { .arg("-") .pass_stdin("1")); } + +/// Test that private, old-style `TypeVar` generics +/// 1. Get replaced with PEP 695 type parameters (UP046, UP047) +/// 2. Get renamed to remove leading underscores (UP049) +/// 3. Emit a warning that the standalone type variable is now unused (PYI018) +/// 4. Remove the now-unused `Generic` import +#[test] +fn pep695_generic_rename() { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(["--select", "F401,PYI018,UP046,UP047,UP049"]) + .args(["--stdin-filename", "test.py"]) + .arg("--unsafe-fixes") + .arg("--fix") + .arg("--preview") + .arg("--target-version=py312") + .arg("-") + .pass_stdin( + r#" +from typing import Generic, TypeVar +_T = TypeVar("_T") + +class OldStyle(Generic[_T]): + var: _T + +def func(t: _T) -> _T: + x: _T + return x +"# + ), + @r#" + success: false + exit_code: 1 + ----- stdout ----- + + from typing import TypeVar + _T = TypeVar("_T") + + class OldStyle[T]: + var: T + + def func[T](t: T) -> T: + x: T + return x + + ----- stderr ----- + test.py:3:1: PYI018 Private TypeVar `_T` is never used + Found 6 errors (5 fixed, 1 remaining). + "# + ); +} diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_0.py new file mode 100644 index 0000000000..217dbbef84 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_0.py @@ -0,0 +1,30 @@ +# simple case, replace _T in signature and body +class Generic[_T]: + buf: list[_T] + + def append(self, t: _T): + self.buf.append(t) + + +# simple case, replace _T in signature and body +def second[_T](var: tuple[_T]) -> _T: + y: _T = var[1] + return y + + +# one diagnostic for each variable, comments are preserved +def many_generics[ + _T, # first generic + _U, # second generic +](args): + return args + + +# neither of these are currently renamed +from typing import Literal, cast + + +def f[_T](v): + cast("_T", v) + cast("Literal['_T']") + cast("list[_T]", v) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_1.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_1.py new file mode 100644 index 0000000000..5df573a1f7 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_1.py @@ -0,0 +1,56 @@ +# bound +class Foo[_T: str]: + var: _T + + +# constraint +class Foo[_T: (str, bytes)]: + var: _T + + +# python 3.13+ default +class Foo[_T = int]: + var: _T + + +# tuple +class Foo[*_Ts]: + var: tuple[*_Ts] + + +# paramspec +class C[**_P]: + var: _P + + +from typing import Callable + + +# each of these will get a separate diagnostic, but at least they'll all get +# fixed +class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + @staticmethod + def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + return None + + +# this should not be fixed because the new name is a keyword, but we still +# offer a diagnostic +class F[_async]: ... + + +# and this should not be fixed because of the conflict with the outer X, but it +# also gets a diagnostic +def f(): + type X = int + + class ScopeConflict[_X]: + var: _X + x: X + + +# these cases should be skipped entirely +def f[_](x: _) -> _: ... +def g[__](x: __) -> __: ... +def h[_T_](x: _T_) -> _T_: ... +def i[__T__](x: __T__) -> __T__: ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index 3815ea21e3..a042f7506b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -5,7 +5,7 @@ use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::rules::{ flake8_import_conventions, flake8_pyi, flake8_pytest_style, flake8_type_checking, pyflakes, - pylint, refurb, ruff, + pylint, pyupgrade, refurb, ruff, }; /// Run lint rules over the [`Binding`]s. @@ -24,6 +24,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::PytestUnittestRaisesAssertion, Rule::ForLoopWrites, Rule::CustomTypeVarForSelf, + Rule::PrivateTypeParameter, ]) { return; } @@ -123,5 +124,10 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::PrivateTypeParameter) { + if let Some(diagnostic) = pyupgrade::rules::private_type_parameter(checker, binding) { + checker.diagnostics.push(diagnostic); + } + } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 09af31938a..b9be03c40a 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -542,6 +542,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyupgrade, "045") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationOptional), (Pyupgrade, "046") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericClass), (Pyupgrade, "047") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP695GenericFunction), + (Pyupgrade, "049") => (RuleGroup::Preview, rules::pyupgrade::rules::PrivateTypeParameter), // pydocstyle (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), diff --git a/crates/ruff_linter/src/renamer.rs b/crates/ruff_linter/src/renamer.rs index a5308d9029..fb87c83ca3 100644 --- a/crates/ruff_linter/src/renamer.rs +++ b/crates/ruff_linter/src/renamer.rs @@ -7,8 +7,11 @@ use ruff_diagnostics::Edit; use ruff_python_ast as ast; use ruff_python_codegen::Stylist; use ruff_python_semantic::{Binding, BindingKind, Scope, ScopeId, SemanticModel}; +use ruff_python_stdlib::{builtins::is_python_builtin, keyword::is_keyword}; use ruff_text_size::Ranged; +use crate::checkers::ast::Checker; + pub(crate) struct Renamer; impl Renamer { @@ -369,3 +372,52 @@ impl Renamer { } } } + +/// Enumeration of various ways in which a binding can shadow other variables +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +pub(crate) enum ShadowedKind { + /// The variable shadows a global, nonlocal or local symbol + Some, + /// The variable shadows a builtin symbol + BuiltIn, + /// The variable shadows a keyword + Keyword, + /// The variable does not shadow any other symbols + None, +} + +impl ShadowedKind { + /// Determines the kind of shadowing or conflict for a given variable name. + /// + /// This function is useful for checking whether or not the `target` of a [`Rename::rename`] + /// will shadow another binding. + pub(crate) fn new(name: &str, checker: &Checker, scope_id: ScopeId) -> ShadowedKind { + // Check the kind in order of precedence + if is_keyword(name) { + return ShadowedKind::Keyword; + } + + if is_python_builtin( + name, + checker.settings.target_version.minor(), + checker.source_type.is_ipynb(), + ) { + return ShadowedKind::BuiltIn; + } + + if !checker.semantic().is_available_in_scope(name, scope_id) { + return ShadowedKind::Some; + } + + // Default to no shadowing + ShadowedKind::None + } + + /// Returns `true` if `self` shadows any global, nonlocal, or local symbol, keyword, or builtin. + pub(crate) const fn shadows_any(self) -> bool { + matches!( + self, + ShadowedKind::Some | ShadowedKind::BuiltIn | ShadowedKind::Keyword + ) + } +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index f10421e4ff..e033cd3fb9 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -106,6 +106,8 @@ mod tests { #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))] #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))] + #[test_case(Rule::PrivateTypeParameter, Path::new("UP049_0.py"))] + #[test_case(Rule::PrivateTypeParameter, Path::new("UP049_1.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = path.to_string_lossy().to_string(); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs index 273c0867d4..68db219871 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs @@ -18,12 +18,14 @@ use ruff_text_size::{Ranged, TextRange}; pub(crate) use non_pep695_generic_class::*; pub(crate) use non_pep695_generic_function::*; pub(crate) use non_pep695_type_alias::*; +pub(crate) use private_type_parameter::*; use crate::checkers::ast::Checker; mod non_pep695_generic_class; mod non_pep695_generic_function; mod non_pep695_type_alias; +mod private_type_parameter; #[derive(Debug)] enum TypeVarRestriction<'a> { diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/private_type_parameter.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/private_type_parameter.rs new file mode 100644 index 0000000000..ad0cd13a70 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/private_type_parameter.rs @@ -0,0 +1,149 @@ +use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::Stmt; +use ruff_python_semantic::Binding; + +use crate::{ + checkers::ast::Checker, + renamer::{Renamer, ShadowedKind}, +}; + +/// ## What it does +/// +/// Checks for use of [PEP 695] type parameters with leading underscores in generic classes and +/// functions. +/// +/// ## Why is this bad? +/// +/// [PEP 695] type parameters are already restricted in scope to the class or function in which they +/// appear, so leading underscores just hurt readability without the usual privacy benefits. +/// +/// However, neither a diagnostic nor a fix will be emitted for "sunder" (`_T_`) or "dunder" +/// (`__T__`) type parameter names as these are not considered private. +/// +/// ## Example +/// +/// ```python +/// class GenericClass[_T]: +/// var: _T +/// +/// +/// def generic_function[_T](var: _T) -> list[_T]: +/// return var[0] +/// ``` +/// +/// Use instead: +/// +/// ```python +/// class GenericClass[T]: +/// var: T +/// +/// +/// def generic_function[T](var: T) -> list[T]: +/// return var[0] +/// ``` +/// +/// ## Fix availability +/// +/// If the name without an underscore would shadow a builtin or another variable, would be a +/// keyword, or would otherwise be an invalid identifier, a fix will not be available. In these +/// situations, you can consider using a trailing underscore or a different name entirely to satisfy +/// the lint rule. +/// +/// ## See also +/// +/// This rule renames private [PEP 695] type parameters but doesn't convert pre-[PEP 695] generics +/// to the new format. See [`non-pep695-generic-function`] and [`non-pep695-generic-class`] for +/// rules that will make this transformation. Those rules do not remove unused type variables after +/// their changes, so you may also want to consider enabling [`unused-private-type-var`] to complete +/// the transition to [PEP 695] generics. +/// +/// [PEP 695]: https://peps.python.org/pep-0695/ +/// [non-pep695-generic-function]: https://docs.astral.sh/ruff/rules/non-pep695-generic-function +/// [non-pep695-generic-class]: https://docs.astral.sh/ruff/rules/non-pep695-generic-class +/// [unused-private-type-var]: https://docs.astral.sh/ruff/rules/unused-private-type-var +#[derive(ViolationMetadata)] +pub(crate) struct PrivateTypeParameter { + kind: ParamKind, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum ParamKind { + Class, + Function, +} + +impl ParamKind { + const fn as_str(self) -> &'static str { + match self { + ParamKind::Class => "class", + ParamKind::Function => "function", + } + } +} + +impl Violation for PrivateTypeParameter { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Generic {} uses private type parameters", + self.kind.as_str() + ) + } + + fn fix_title(&self) -> Option { + Some("Remove the leading underscores".to_string()) + } +} + +/// UP049 +pub(crate) fn private_type_parameter(checker: &Checker, binding: &Binding) -> Option { + let semantic = checker.semantic(); + let stmt = binding.statement(semantic)?; + if !binding.kind.is_type_param() { + return None; + } + + let kind = match stmt { + Stmt::FunctionDef(_) => ParamKind::Function, + Stmt::ClassDef(_) => ParamKind::Class, + _ => return None, + }; + + let old_name = binding.name(checker.source()); + + if !old_name.starts_with('_') { + return None; + } + + // Sunder `_T_`, dunder `__T__`, and all all-under `_` or `__` cases should all be skipped, as + // these are not "private" names + if old_name.ends_with('_') { + return None; + } + + let mut diagnostic = Diagnostic::new(PrivateTypeParameter { kind }, binding.range); + + let new_name = old_name.trim_start_matches('_'); + + // if the new name would shadow another variable, keyword, or builtin, emit a diagnostic without + // a suggested fix + if ShadowedKind::new(new_name, checker, binding.scope).shadows_any() { + return Some(diagnostic); + } + + diagnostic.try_set_fix(|| { + let (first, rest) = Renamer::rename( + old_name, + new_name, + &semantic.scopes[binding.scope], + checker.semantic(), + checker.stylist(), + )?; + + Ok(Fix::safe_edits(first, rest)) + }); + + Some(diagnostic) +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_0.py.snap new file mode 100644 index 0000000000..76c5f7864f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_0.py.snap @@ -0,0 +1,109 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP049_0.py:2:15: UP049 [*] Generic class uses private type parameters + | +1 | # simple case, replace _T in signature and body +2 | class Generic[_T]: + | ^^ UP049 +3 | buf: list[_T] + | + = help: Remove the leading underscores + +ℹ Safe fix +1 1 | # simple case, replace _T in signature and body +2 |-class Generic[_T]: +3 |- buf: list[_T] + 2 |+class Generic[T]: + 3 |+ buf: list[T] +4 4 | +5 |- def append(self, t: _T): + 5 |+ def append(self, t: T): +6 6 | self.buf.append(t) +7 7 | +8 8 | + +UP049_0.py:10:12: UP049 [*] Generic function uses private type parameters + | + 9 | # simple case, replace _T in signature and body +10 | def second[_T](var: tuple[_T]) -> _T: + | ^^ UP049 +11 | y: _T = var[1] +12 | return y + | + = help: Remove the leading underscores + +ℹ Safe fix +7 7 | +8 8 | +9 9 | # simple case, replace _T in signature and body +10 |-def second[_T](var: tuple[_T]) -> _T: +11 |- y: _T = var[1] + 10 |+def second[T](var: tuple[T]) -> T: + 11 |+ y: T = var[1] +12 12 | return y +13 13 | +14 14 | + +UP049_0.py:17:5: UP049 [*] Generic function uses private type parameters + | +15 | # one diagnostic for each variable, comments are preserved +16 | def many_generics[ +17 | _T, # first generic + | ^^ UP049 +18 | _U, # second generic +19 | ](args): + | + = help: Remove the leading underscores + +ℹ Safe fix +14 14 | +15 15 | # one diagnostic for each variable, comments are preserved +16 16 | def many_generics[ +17 |- _T, # first generic + 17 |+ T, # first generic +18 18 | _U, # second generic +19 19 | ](args): +20 20 | return args + +UP049_0.py:18:5: UP049 [*] Generic function uses private type parameters + | +16 | def many_generics[ +17 | _T, # first generic +18 | _U, # second generic + | ^^ UP049 +19 | ](args): +20 | return args + | + = help: Remove the leading underscores + +ℹ Safe fix +15 15 | # one diagnostic for each variable, comments are preserved +16 16 | def many_generics[ +17 17 | _T, # first generic +18 |- _U, # second generic + 18 |+ U, # second generic +19 19 | ](args): +20 20 | return args +21 21 | + +UP049_0.py:27:7: UP049 [*] Generic function uses private type parameters + | +27 | def f[_T](v): + | ^^ UP049 +28 | cast("_T", v) +29 | cast("Literal['_T']") + | + = help: Remove the leading underscores + +ℹ Safe fix +24 24 | from typing import Literal, cast +25 25 | +26 26 | +27 |-def f[_T](v): +28 |- cast("_T", v) + 27 |+def f[T](v): + 28 |+ cast("T", v) +29 29 | cast("Literal['_T']") +30 |- cast("list[_T]", v) + 30 |+ cast("list[T]", v) diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_1.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_1.py.snap new file mode 100644 index 0000000000..96df294002 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_1.py.snap @@ -0,0 +1,245 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP049_1.py:2:11: UP049 [*] Generic class uses private type parameters + | +1 | # bound +2 | class Foo[_T: str]: + | ^^ UP049 +3 | var: _T + | + = help: Remove the leading underscores + +ℹ Safe fix +1 1 | # bound +2 |-class Foo[_T: str]: +3 |- var: _T + 2 |+class Foo[T: str]: + 3 |+ var: T +4 4 | +5 5 | +6 6 | # constraint + +UP049_1.py:7:11: UP049 [*] Generic class uses private type parameters + | +6 | # constraint +7 | class Foo[_T: (str, bytes)]: + | ^^ UP049 +8 | var: _T + | + = help: Remove the leading underscores + +ℹ Safe fix +4 4 | +5 5 | +6 6 | # constraint +7 |-class Foo[_T: (str, bytes)]: +8 |- var: _T + 7 |+class Foo[T: (str, bytes)]: + 8 |+ var: T +9 9 | +10 10 | +11 11 | # python 3.13+ default + +UP049_1.py:12:11: UP049 [*] Generic class uses private type parameters + | +11 | # python 3.13+ default +12 | class Foo[_T = int]: + | ^^ UP049 +13 | var: _T + | + = help: Remove the leading underscores + +ℹ Safe fix +9 9 | +10 10 | +11 11 | # python 3.13+ default +12 |-class Foo[_T = int]: +13 |- var: _T + 12 |+class Foo[T = int]: + 13 |+ var: T +14 14 | +15 15 | +16 16 | # tuple + +UP049_1.py:17:12: UP049 [*] Generic class uses private type parameters + | +16 | # tuple +17 | class Foo[*_Ts]: + | ^^^ UP049 +18 | var: tuple[*_Ts] + | + = help: Remove the leading underscores + +ℹ Safe fix +14 14 | +15 15 | +16 16 | # tuple +17 |-class Foo[*_Ts]: +18 |- var: tuple[*_Ts] + 17 |+class Foo[*Ts]: + 18 |+ var: tuple[*Ts] +19 19 | +20 20 | +21 21 | # paramspec + +UP049_1.py:22:11: UP049 [*] Generic class uses private type parameters + | +21 | # paramspec +22 | class C[**_P]: + | ^^ UP049 +23 | var: _P + | + = help: Remove the leading underscores + +ℹ Safe fix +19 19 | +20 20 | +21 21 | # paramspec +22 |-class C[**_P]: +23 |- var: _P + 22 |+class C[**P]: + 23 |+ var: P +24 24 | +25 25 | +26 26 | from typing import Callable + +UP049_1.py:31:18: UP049 [*] Generic class uses private type parameters + | +29 | # each of these will get a separate diagnostic, but at least they'll all get +30 | # fixed +31 | class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + | ^^ UP049 +32 | @staticmethod +33 | def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + | + = help: Remove the leading underscores + +ℹ Safe fix +28 28 | +29 29 | # each of these will get a separate diagnostic, but at least they'll all get +30 30 | # fixed +31 |-class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + 31 |+class Everything[T, _U: str, _V: (int, float), *_W, **_X]: +32 32 | @staticmethod +33 |- def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + 33 |+ def transform(t: T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, T] | None: +34 34 | return None +35 35 | +36 36 | + +UP049_1.py:31:22: UP049 [*] Generic class uses private type parameters + | +29 | # each of these will get a separate diagnostic, but at least they'll all get +30 | # fixed +31 | class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + | ^^ UP049 +32 | @staticmethod +33 | def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + | + = help: Remove the leading underscores + +ℹ Safe fix +28 28 | +29 29 | # each of these will get a separate diagnostic, but at least they'll all get +30 30 | # fixed +31 |-class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + 31 |+class Everything[_T, U: str, _V: (int, float), *_W, **_X]: +32 32 | @staticmethod +33 |- def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + 33 |+ def transform(t: _T, u: U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: +34 34 | return None +35 35 | +36 36 | + +UP049_1.py:31:31: UP049 [*] Generic class uses private type parameters + | +29 | # each of these will get a separate diagnostic, but at least they'll all get +30 | # fixed +31 | class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + | ^^ UP049 +32 | @staticmethod +33 | def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + | + = help: Remove the leading underscores + +ℹ Safe fix +28 28 | +29 29 | # each of these will get a separate diagnostic, but at least they'll all get +30 30 | # fixed +31 |-class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + 31 |+class Everything[_T, _U: str, V: (int, float), *_W, **_X]: +32 32 | @staticmethod +33 |- def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + 33 |+ def transform(t: _T, u: _U, v: V) -> tuple[*_W] | Callable[_X, _T] | None: +34 34 | return None +35 35 | +36 36 | + +UP049_1.py:31:50: UP049 [*] Generic class uses private type parameters + | +29 | # each of these will get a separate diagnostic, but at least they'll all get +30 | # fixed +31 | class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + | ^^ UP049 +32 | @staticmethod +33 | def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + | + = help: Remove the leading underscores + +ℹ Safe fix +28 28 | +29 29 | # each of these will get a separate diagnostic, but at least they'll all get +30 30 | # fixed +31 |-class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + 31 |+class Everything[_T, _U: str, _V: (int, float), *W, **_X]: +32 32 | @staticmethod +33 |- def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + 33 |+ def transform(t: _T, u: _U, v: _V) -> tuple[*W] | Callable[_X, _T] | None: +34 34 | return None +35 35 | +36 36 | + +UP049_1.py:31:56: UP049 [*] Generic class uses private type parameters + | +29 | # each of these will get a separate diagnostic, but at least they'll all get +30 | # fixed +31 | class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + | ^^ UP049 +32 | @staticmethod +33 | def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + | + = help: Remove the leading underscores + +ℹ Safe fix +28 28 | +29 29 | # each of these will get a separate diagnostic, but at least they'll all get +30 30 | # fixed +31 |-class Everything[_T, _U: str, _V: (int, float), *_W, **_X]: + 31 |+class Everything[_T, _U: str, _V: (int, float), *_W, **X]: +32 32 | @staticmethod +33 |- def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None: + 33 |+ def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[X, _T] | None: +34 34 | return None +35 35 | +36 36 | + +UP049_1.py:39:9: UP049 Generic class uses private type parameters + | +37 | # this should not be fixed because the new name is a keyword, but we still +38 | # offer a diagnostic +39 | class F[_async]: ... + | ^^^^^^ UP049 + | + = help: Remove the leading underscores + +UP049_1.py:47:25: UP049 Generic class uses private type parameters + | +45 | type X = int +46 | +47 | class ScopeConflict[_X]: + | ^^ UP049 +48 | var: _X +49 | x: X + | + = help: Remove the leading underscores diff --git a/crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs b/crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs index 525aba5f26..6687983ad7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs @@ -2,12 +2,13 @@ use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::helpers::is_dunder; use ruff_python_semantic::{Binding, BindingId, ScopeId}; -use ruff_python_stdlib::{ - builtins::is_python_builtin, identifiers::is_identifier, keyword::is_keyword, -}; +use ruff_python_stdlib::identifiers::is_identifier; use ruff_text_size::Ranged; -use crate::{checkers::ast::Checker, renamer::Renamer}; +use crate::{ + checkers::ast::Checker, + renamer::{Renamer, ShadowedKind}, +}; /// ## What it does /// Checks for "dummy variables" (variables that are named as if to indicate they are unused) @@ -164,53 +165,50 @@ pub(crate) fn used_dummy_variable( return None; } - let shadowed_kind = try_shadowed_kind(name, checker, binding.scope); + // If the name doesn't start with an underscore, we don't consider it for a fix + if !name.starts_with('_') { + return Some(Diagnostic::new( + UsedDummyVariable { + name: name.to_string(), + shadowed_kind: None, + }, + binding.range(), + )); + } + + // Trim the leading underscores for further checks + let trimmed_name = name.trim_start_matches('_'); + + let shadowed_kind = ShadowedKind::new(trimmed_name, checker, binding.scope); let mut diagnostic = Diagnostic::new( UsedDummyVariable { name: name.to_string(), - shadowed_kind, + shadowed_kind: Some(shadowed_kind), }, binding.range(), ); - // If fix available - if let Some(shadowed_kind) = shadowed_kind { - // Get the possible fix based on the scope - if let Some(fix) = get_possible_fix(name, shadowed_kind, binding.scope, checker) { - diagnostic.try_set_fix(|| { - Renamer::rename(name, &fix, scope, semantic, checker.stylist()) - .map(|(edit, rest)| Fix::unsafe_edits(edit, rest)) - }); - } + // Get the possible fix based on the scope + if let Some(new_name) = + get_possible_new_name(trimmed_name, shadowed_kind, binding.scope, checker) + { + diagnostic.try_set_fix(|| { + Renamer::rename(name, &new_name, scope, semantic, checker.stylist()) + .map(|(edit, rest)| Fix::unsafe_edits(edit, rest)) + }); } Some(diagnostic) } -/// Enumeration of various ways in which a binding can shadow other variables -#[derive(Debug, PartialEq, Eq, Copy, Clone)] -enum ShadowedKind { - /// The variable shadows a global, nonlocal or local symbol - Some, - /// The variable shadows a builtin symbol - BuiltIn, - /// The variable shadows a keyword - Keyword, - /// The variable does not shadow any other symbols - None, -} - /// Suggests a potential alternative name to resolve a shadowing conflict. -fn get_possible_fix( - name: &str, +fn get_possible_new_name( + trimmed_name: &str, kind: ShadowedKind, scope_id: ScopeId, checker: &Checker, ) -> Option { - // Remove leading underscores for processing - let trimmed_name = name.trim_start_matches('_'); - // Construct the potential fix name based on ShadowedKind let fix_name = match kind { ShadowedKind::Some | ShadowedKind::BuiltIn | ShadowedKind::Keyword => { @@ -235,37 +233,3 @@ fn get_possible_fix( // Check if the fix name is a valid identifier is_identifier(&fix_name).then_some(fix_name) } - -/// Determines the kind of shadowing or conflict for a given variable name. -fn try_shadowed_kind(name: &str, checker: &Checker, scope_id: ScopeId) -> Option { - // If the name starts with an underscore, we don't consider it - if !name.starts_with('_') { - return None; - } - - // Trim the leading underscores for further checks - let trimmed_name = name.trim_start_matches('_'); - - // Check the kind in order of precedence - if is_keyword(trimmed_name) { - return Some(ShadowedKind::Keyword); - } - - if is_python_builtin( - trimmed_name, - checker.settings.target_version.minor(), - checker.source_type.is_ipynb(), - ) { - return Some(ShadowedKind::BuiltIn); - } - - if !checker - .semantic() - .is_available_in_scope(trimmed_name, scope_id) - { - return Some(ShadowedKind::Some); - } - - // Default to no shadowing - Some(ShadowedKind::None) -} diff --git a/ruff.schema.json b/ruff.schema.json index 48042711f2..05cecddeb1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4218,6 +4218,7 @@ "UP045", "UP046", "UP047", + "UP049", "W", "W1", "W19",