Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[flake8-pyi] Expand PYI018 to cover ParamSpecs and TypeVarTuples #9198

Merged
merged 4 commits into from
Dec 20, 2023
Merged

[flake8-pyi] Expand PYI018 to cover ParamSpecs and TypeVarTuples #9198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e1ff13
Working version
AlexWaygood Dec 19, 2023
46757dd
Expand PYI018 to cover ParamSpecs and TypeVarTuples
AlexWaygood Dec 19, 2023
76c36c7
Tweak name casing
charliermarsh Dec 20, 2023
0015170
Use a single lookup
charliermarsh Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")
Expand Down
7 changes: 6 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI018.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")
Expand Down
36 changes: 28 additions & 8 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/unused_private_type_definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,35 @@ 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,
type_var_like_name: String,
type_var_like_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 {
type_var_like_name,
type_var_like_kind,
} = self;
format!("Private {type_var_like_kind} `{type_var_like_name}` is never used")
}
}

Expand Down Expand Up @@ -185,13 +192,26 @@ 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") {

let semantic = checker.semantic();
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") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWaygood - I made one change here per your comment in the summary. Each call to semantic.match_typing_expr does a lookup internally to map the expression to a "call path", like ["typing", "TypeVar"]. However, we can just do that lookup once via semantic.resolve_call_path, then pass the call path to semantic.match_typing_call_path. (semantic.match_typing_expr is just a wrapper around this process, so if we want to do multiple checks, it's more efficient to do the lookup outside of the semantic.match_typing_expr.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh nice, thanks! Yep, that sounds like it's exactly what I was looking for :)

Some("TypeVarTuple")
} else {
None
}
}) else {
continue;
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might write this like:

let typevarlike_kind = if semantic.match_typing_expr(f, "TypeVar")  {
   "TypeVar"
} else if ...

But I don't know that what I'm proposing is actually better, I just hadn't seen this pattern before :)


diagnostics.push(Diagnostic::new(
UnusedPrivateTypeVar {
name: id.to_string(),
type_var_like_name: id.to_string(),
type_var_like_kind: type_var_like_kind.to_string(),
},
binding.range(),
));
Expand Down
52 changes: 41 additions & 11 deletions ...c/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
|


52 changes: 41 additions & 11 deletions .../rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
|


Loading