From 7e1ff1320e3afb8e7a137ace406f03a60131f1a9 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Tue, 19 Dec 2023 11:06:57 +0000 Subject: [PATCH 1/4] Working version --- .../rules/unused_private_type_definition.rs | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 094d1697c9f06..3c5f64ffa64bf 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -7,28 +7,32 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for the presence of unused private `TypeVar` declarations. +/// Checks for the presence of unused private `TypeVar`, `ParamSpec` or +/// `TypeVarTuple` declarations. /// /// ## Why is this bad? -/// A private `TypeVar` that is defined but not used is likely a mistake, and +/// A private `TypeVar` that is defined but not used is likely a mistake. It /// should either be used, made public, or removed to avoid confusion. /// /// ## Example /// ```python /// import typing +/// import typing_extensions /// /// _T = typing.TypeVar("_T") +/// _Ts = typing_extensions.TypeVarTuple("_Ts") /// ``` #[violation] pub struct UnusedPrivateTypeVar { - name: String, + typevarlike_name: String, + typevarlike_kind: String, } impl Violation for UnusedPrivateTypeVar { #[derive_message_formats] fn message(&self) -> String { - let UnusedPrivateTypeVar { name } = self; - format!("Private TypeVar `{name}` is never used") + let UnusedPrivateTypeVar{typevarlike_name, typevarlike_kind} = self; + format!("Private {typevarlike_kind} `{typevarlike_name}` is never used") } } @@ -185,13 +189,23 @@ pub(crate) fn unused_private_type_var( let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else { continue; }; - if !checker.semantic().match_typing_expr(func, "TypeVar") { - continue; - } + + let semantic = checker.semantic(); + + let typevarlike_kind = { + if semantic.match_typing_expr(func, "TypeVar") {"TypeVar"} else { + if semantic.match_typing_expr(func, "ParamSpec") {"ParamSpec"} else { + if semantic.match_typing_expr(func, "TypeVarTuple") {"TypeVarTuple"} else { + continue; + } + } + } + }; diagnostics.push(Diagnostic::new( UnusedPrivateTypeVar { - name: id.to_string(), + typevarlike_name: id.to_string(), + typevarlike_kind: typevarlike_kind.to_string() }, binding.range(), )); From 46757dd42517b8fa7bca25c1d616ca1294566e7c Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Tue, 19 Dec 2023 12:00:13 +0000 Subject: [PATCH 2/4] Expand PYI018 to cover ParamSpecs and TypeVarTuples --- .../test/fixtures/flake8_pyi/PYI018.py | 7 ++- .../test/fixtures/flake8_pyi/PYI018.pyi | 7 ++- .../rules/unused_private_type_definition.rs | 20 +++---- ...__flake8_pyi__tests__PYI018_PYI018.py.snap | 52 +++++++++++++++---- ..._flake8_pyi__tests__PYI018_PYI018.pyi.snap | 52 +++++++++++++++---- 5 files changed, 105 insertions(+), 33 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.py index 75dbf08dd698b..402c17d7ba734 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.py @@ -1,8 +1,13 @@ import typing +import typing_extensions from typing import TypeVar +from typing_extensions import ParamSpec, TypeVarTuple _T = typing.TypeVar("_T") -_P = TypeVar("_P") +_Ts = typing_extensions.TypeVarTuple("_Ts") +_P = ParamSpec("_P") +_P2 = typing.ParamSpec("_P2") +_Ts2 = TypeVarTuple("_Ts2") # OK _UsedTypeVar = TypeVar("_UsedTypeVar") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.pyi index 75dbf08dd698b..402c17d7ba734 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.pyi @@ -1,8 +1,13 @@ import typing +import typing_extensions from typing import TypeVar +from typing_extensions import ParamSpec, TypeVarTuple _T = typing.TypeVar("_T") -_P = TypeVar("_P") +_Ts = typing_extensions.TypeVarTuple("_Ts") +_P = ParamSpec("_P") +_P2 = typing.ParamSpec("_P2") +_Ts2 = TypeVarTuple("_Ts2") # OK _UsedTypeVar = TypeVar("_UsedTypeVar") diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 3c5f64ffa64bf..48bcb5e6d355d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -31,7 +31,10 @@ pub struct UnusedPrivateTypeVar { impl Violation for UnusedPrivateTypeVar { #[derive_message_formats] fn message(&self) -> String { - let UnusedPrivateTypeVar{typevarlike_name, typevarlike_kind} = self; + let UnusedPrivateTypeVar { + typevarlike_name, + typevarlike_kind, + } = self; format!("Private {typevarlike_kind} `{typevarlike_name}` is never used") } } @@ -192,20 +195,19 @@ pub(crate) fn unused_private_type_var( let semantic = checker.semantic(); - let typevarlike_kind = { - if semantic.match_typing_expr(func, "TypeVar") {"TypeVar"} else { - if semantic.match_typing_expr(func, "ParamSpec") {"ParamSpec"} else { - if semantic.match_typing_expr(func, "TypeVarTuple") {"TypeVarTuple"} else { - continue; - } - } + let typevarlike_kind = match func { + f if semantic.match_typing_expr(f, "TypeVar") => "TypeVar", + f if semantic.match_typing_expr(f, "ParamSpec") => "ParamSpec", + f if semantic.match_typing_expr(f, "TypeVarTuple") => "TypeVarTuple", + _ => { + continue; } }; diagnostics.push(Diagnostic::new( UnusedPrivateTypeVar { typevarlike_name: id.to_string(), - typevarlike_kind: typevarlike_kind.to_string() + typevarlike_kind: typevarlike_kind.to_string(), }, binding.range(), )); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.py.snap index 878d32f43d633..9ca737a9209ca 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.py.snap @@ -1,22 +1,52 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs --- -PYI018.py:4:1: PYI018 Private TypeVar `_T` is never used +PYI018.py:6:1: PYI018 Private TypeVar `_T` is never used | -2 | from typing import TypeVar -3 | -4 | _T = typing.TypeVar("_T") +4 | from typing_extensions import ParamSpec, TypeVarTuple +5 | +6 | _T = typing.TypeVar("_T") | ^^ PYI018 -5 | _P = TypeVar("_P") +7 | _Ts = typing_extensions.TypeVarTuple("_Ts") +8 | _P = ParamSpec("_P") | -PYI018.py:5:1: PYI018 Private TypeVar `_P` is never used +PYI018.py:7:1: PYI018 Private TypeVarTuple `_Ts` is never used | -4 | _T = typing.TypeVar("_T") -5 | _P = TypeVar("_P") - | ^^ PYI018 -6 | -7 | # OK +6 | _T = typing.TypeVar("_T") +7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + | ^^^ PYI018 +8 | _P = ParamSpec("_P") +9 | _P2 = typing.ParamSpec("_P2") | +PYI018.py:8:1: PYI018 Private ParamSpec `_P` is never used + | + 6 | _T = typing.TypeVar("_T") + 7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + 8 | _P = ParamSpec("_P") + | ^^ PYI018 + 9 | _P2 = typing.ParamSpec("_P2") +10 | _Ts2 = TypeVarTuple("_Ts2") + | + +PYI018.py:9:1: PYI018 Private ParamSpec `_P2` is never used + | + 7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + 8 | _P = ParamSpec("_P") + 9 | _P2 = typing.ParamSpec("_P2") + | ^^^ PYI018 +10 | _Ts2 = TypeVarTuple("_Ts2") + | + +PYI018.py:10:1: PYI018 Private TypeVarTuple `_Ts2` is never used + | + 8 | _P = ParamSpec("_P") + 9 | _P2 = typing.ParamSpec("_P2") +10 | _Ts2 = TypeVarTuple("_Ts2") + | ^^^^ PYI018 +11 | +12 | # OK + | + diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap index d82b93d9b1c24..4585ae25babeb 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap @@ -1,22 +1,52 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs --- -PYI018.pyi:4:1: PYI018 Private TypeVar `_T` is never used +PYI018.pyi:6:1: PYI018 Private TypeVar `_T` is never used | -2 | from typing import TypeVar -3 | -4 | _T = typing.TypeVar("_T") +4 | from typing_extensions import ParamSpec, TypeVarTuple +5 | +6 | _T = typing.TypeVar("_T") | ^^ PYI018 -5 | _P = TypeVar("_P") +7 | _Ts = typing_extensions.TypeVarTuple("_Ts") +8 | _P = ParamSpec("_P") | -PYI018.pyi:5:1: PYI018 Private TypeVar `_P` is never used +PYI018.pyi:7:1: PYI018 Private TypeVarTuple `_Ts` is never used | -4 | _T = typing.TypeVar("_T") -5 | _P = TypeVar("_P") - | ^^ PYI018 -6 | -7 | # OK +6 | _T = typing.TypeVar("_T") +7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + | ^^^ PYI018 +8 | _P = ParamSpec("_P") +9 | _P2 = typing.ParamSpec("_P2") | +PYI018.pyi:8:1: PYI018 Private ParamSpec `_P` is never used + | + 6 | _T = typing.TypeVar("_T") + 7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + 8 | _P = ParamSpec("_P") + | ^^ PYI018 + 9 | _P2 = typing.ParamSpec("_P2") +10 | _Ts2 = TypeVarTuple("_Ts2") + | + +PYI018.pyi:9:1: PYI018 Private ParamSpec `_P2` is never used + | + 7 | _Ts = typing_extensions.TypeVarTuple("_Ts") + 8 | _P = ParamSpec("_P") + 9 | _P2 = typing.ParamSpec("_P2") + | ^^^ PYI018 +10 | _Ts2 = TypeVarTuple("_Ts2") + | + +PYI018.pyi:10:1: PYI018 Private TypeVarTuple `_Ts2` is never used + | + 8 | _P = ParamSpec("_P") + 9 | _P2 = typing.ParamSpec("_P2") +10 | _Ts2 = TypeVarTuple("_Ts2") + | ^^^^ PYI018 +11 | +12 | # OK + | + From 76c36c73f43a9a22e8efaa4a2626fa0cc2b3c45e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 19 Dec 2023 22:02:09 -0500 Subject: [PATCH 3/4] Tweak name casing --- .../rules/unused_private_type_definition.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 48bcb5e6d355d..aeff1792d5b08 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -24,18 +24,18 @@ use crate::checkers::ast::Checker; /// ``` #[violation] pub struct UnusedPrivateTypeVar { - typevarlike_name: String, - typevarlike_kind: String, + type_var_like_name: String, + type_var_like_kind: String, } impl Violation for UnusedPrivateTypeVar { #[derive_message_formats] fn message(&self) -> String { let UnusedPrivateTypeVar { - typevarlike_name, - typevarlike_kind, + type_var_like_name, + type_var_like_kind, } = self; - format!("Private {typevarlike_kind} `{typevarlike_name}` is never used") + format!("Private {type_var_like_kind} `{type_var_like_name}` is never used") } } @@ -194,11 +194,10 @@ pub(crate) fn unused_private_type_var( }; let semantic = checker.semantic(); - - let typevarlike_kind = match func { - f if semantic.match_typing_expr(f, "TypeVar") => "TypeVar", - f if semantic.match_typing_expr(f, "ParamSpec") => "ParamSpec", - f if semantic.match_typing_expr(f, "TypeVarTuple") => "TypeVarTuple", + let type_var_like_kind = match func { + func if semantic.match_typing_expr(func, "TypeVar") => "TypeVar", + func if semantic.match_typing_expr(func, "ParamSpec") => "ParamSpec", + func if semantic.match_typing_expr(func, "TypeVarTuple") => "TypeVarTuple", _ => { continue; } @@ -206,8 +205,8 @@ pub(crate) fn unused_private_type_var( diagnostics.push(Diagnostic::new( UnusedPrivateTypeVar { - typevarlike_name: id.to_string(), - typevarlike_kind: typevarlike_kind.to_string(), + type_var_like_name: id.to_string(), + type_var_like_kind: type_var_like_kind.to_string(), }, binding.range(), )); From 00151705e03e1620ed7004c92b6da7c4932b2179 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 19 Dec 2023 22:04:40 -0500 Subject: [PATCH 4/4] Use a single lookup --- .../rules/unused_private_type_definition.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index aeff1792d5b08..5da722a904394 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -194,13 +194,18 @@ pub(crate) fn unused_private_type_var( }; let semantic = checker.semantic(); - let type_var_like_kind = match func { - func if semantic.match_typing_expr(func, "TypeVar") => "TypeVar", - func if semantic.match_typing_expr(func, "ParamSpec") => "ParamSpec", - func if semantic.match_typing_expr(func, "TypeVarTuple") => "TypeVarTuple", - _ => { - continue; + let Some(type_var_like_kind) = semantic.resolve_call_path(func).and_then(|call_path| { + if semantic.match_typing_call_path(&call_path, "TypeVar") { + Some("TypeVar") + } else if semantic.match_typing_call_path(&call_path, "ParamSpec") { + Some("ParamSpec") + } else if semantic.match_typing_call_path(&call_path, "TypeVarTuple") { + Some("TypeVarTuple") + } else { + None } + }) else { + continue; }; diagnostics.push(Diagnostic::new(