mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-11 14:18:28 +00:00
[flake8-pyi
] Implement PYI018 (#6018)
## Summary
Check for unused private `TypeVar`. See [original
implementation](2a86db8271/pyi.py (L1958)
).
```
$ flake8 --select Y018 crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi:4:1: Y018 TypeVar "_T" is not used
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi:5:1: Y018 TypeVar "_P" is not used
```
```
$ ./target/debug/ruff --select PYI018 crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi --no-cache
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi:4:1: PYI018 TypeVar `_T` is never used
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi:5:1: PYI018 TypeVar `_P` is never used
Found 2 errors.
```
In the file `unused_private_type_declaration.rs`, I'm planning to add
other rules that are similar to `PYI018` like the `PYI046`, `PYI047` and
`PYI049`.
ref #848
## Test Plan
Snapshots and manual runs of flake8.
This commit is contained in:
parent
788643f718
commit
c0dbcb3434
12 changed files with 149 additions and 0 deletions
12
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.py
vendored
Normal file
12
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.py
vendored
Normal file
|
@ -0,0 +1,12 @@
|
||||||
|
import typing
|
||||||
|
from typing import TypeVar
|
||||||
|
|
||||||
|
_T = typing.TypeVar("_T")
|
||||||
|
_P = TypeVar("_P")
|
||||||
|
|
||||||
|
# OK
|
||||||
|
_UsedTypeVar = TypeVar("_UsedTypeVar")
|
||||||
|
def func(arg: _UsedTypeVar) -> _UsedTypeVar: ...
|
||||||
|
|
||||||
|
_A, _B = TypeVar("_A"), TypeVar("_B")
|
||||||
|
_C = _D = TypeVar("_C")
|
12
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi
vendored
Normal file
12
crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi
vendored
Normal file
|
@ -0,0 +1,12 @@
|
||||||
|
import typing
|
||||||
|
from typing import TypeVar
|
||||||
|
|
||||||
|
_T = typing.TypeVar("_T")
|
||||||
|
_P = TypeVar("_P")
|
||||||
|
|
||||||
|
# OK
|
||||||
|
_UsedTypeVar = TypeVar("_UsedTypeVar")
|
||||||
|
def func(arg: _UsedTypeVar) -> _UsedTypeVar: ...
|
||||||
|
|
||||||
|
_A, _B = TypeVar("_A"), TypeVar("_B")
|
||||||
|
_C = _D = TypeVar("_C")
|
|
@ -10,6 +10,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
|
||||||
Rule::InvalidAllObject,
|
Rule::InvalidAllObject,
|
||||||
Rule::UnaliasedCollectionsAbcSetImport,
|
Rule::UnaliasedCollectionsAbcSetImport,
|
||||||
Rule::UnconventionalImportAlias,
|
Rule::UnconventionalImportAlias,
|
||||||
|
Rule::UnusedPrivateTypeVar,
|
||||||
Rule::UnusedVariable,
|
Rule::UnusedVariable,
|
||||||
]) {
|
]) {
|
||||||
return;
|
return;
|
||||||
|
@ -63,6 +64,13 @@ pub(crate) fn bindings(checker: &mut Checker) {
|
||||||
checker.diagnostics.push(diagnostic);
|
checker.diagnostics.push(diagnostic);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::UnusedPrivateTypeVar) {
|
||||||
|
if let Some(diagnostic) =
|
||||||
|
flake8_pyi::rules::unused_private_type_var(checker, binding)
|
||||||
|
{
|
||||||
|
checker.diagnostics.push(diagnostic);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1474,6 +1474,11 @@ impl<'a> Checker<'a> {
|
||||||
// Create the `Binding`.
|
// Create the `Binding`.
|
||||||
let binding_id = self.semantic.push_binding(range, kind, flags);
|
let binding_id = self.semantic.push_binding(range, kind, flags);
|
||||||
|
|
||||||
|
// If the name is private, mark is as such.
|
||||||
|
if name.starts_with('_') {
|
||||||
|
self.semantic.bindings[binding_id].flags |= BindingFlags::PRIVATE_DECLARATION;
|
||||||
|
}
|
||||||
|
|
||||||
// If there's an existing binding in this scope, copy its references.
|
// If there's an existing binding in this scope, copy its references.
|
||||||
if let Some(shadowed_id) = self.semantic.scopes[scope_id].get(name) {
|
if let Some(shadowed_id) = self.semantic.scopes[scope_id].get(name) {
|
||||||
// If this is an annotation, and we already have an existing value in the same scope,
|
// If this is an annotation, and we already have an existing value in the same scope,
|
||||||
|
|
|
@ -632,6 +632,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Flake8Pyi, "015") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AssignmentDefaultInStub),
|
(Flake8Pyi, "015") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AssignmentDefaultInStub),
|
||||||
(Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember),
|
(Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember),
|
||||||
(Flake8Pyi, "017") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ComplexAssignmentInStub),
|
(Flake8Pyi, "017") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ComplexAssignmentInStub),
|
||||||
|
(Flake8Pyi, "018") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateTypeVar),
|
||||||
(Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub),
|
(Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub),
|
||||||
(Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub),
|
(Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub),
|
||||||
(Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple),
|
(Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple),
|
||||||
|
|
|
@ -93,6 +93,8 @@ mod tests {
|
||||||
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))]
|
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))]
|
||||||
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))]
|
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))]
|
||||||
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))]
|
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))]
|
||||||
|
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))]
|
||||||
|
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.pyi"))]
|
||||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
@ -29,6 +29,7 @@ pub(crate) use unnecessary_literal_union::*;
|
||||||
pub(crate) use unrecognized_platform::*;
|
pub(crate) use unrecognized_platform::*;
|
||||||
pub(crate) use unrecognized_version_info::*;
|
pub(crate) use unrecognized_version_info::*;
|
||||||
pub(crate) use unsupported_method_call_on_all::*;
|
pub(crate) use unsupported_method_call_on_all::*;
|
||||||
|
pub(crate) use unused_private_type_definition::*;
|
||||||
|
|
||||||
mod any_eq_ne_annotation;
|
mod any_eq_ne_annotation;
|
||||||
mod bad_version_info_comparison;
|
mod bad_version_info_comparison;
|
||||||
|
@ -61,3 +62,4 @@ mod unnecessary_literal_union;
|
||||||
mod unrecognized_platform;
|
mod unrecognized_platform;
|
||||||
mod unrecognized_version_info;
|
mod unrecognized_version_info;
|
||||||
mod unsupported_method_call_on_all;
|
mod unsupported_method_call_on_all;
|
||||||
|
mod unused_private_type_definition;
|
||||||
|
|
|
@ -0,0 +1,66 @@
|
||||||
|
use ruff_diagnostics::{Diagnostic, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_semantic::Binding;
|
||||||
|
use rustpython_parser::ast::{self, Expr, Stmt};
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for the presence of unused private `TypeVar` declarations.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// A private `TypeVar` that is defined but not used is likely a mistake, and
|
||||||
|
/// should be removed to avoid confusion.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// import typing
|
||||||
|
///
|
||||||
|
/// _T = typing.TypeVar("_T")
|
||||||
|
/// ```
|
||||||
|
#[violation]
|
||||||
|
pub struct UnusedPrivateTypeVar {
|
||||||
|
name: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Violation for UnusedPrivateTypeVar {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
let UnusedPrivateTypeVar { name } = self;
|
||||||
|
format!("Private TypeVar `{name}` is never used")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// PYI018
|
||||||
|
pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> Option<Diagnostic> {
|
||||||
|
if !(binding.kind.is_assignment() && binding.is_private_variable()) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
if binding.is_used() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
let Some(source) = binding.source else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let Stmt::Assign(ast::StmtAssign { targets, value, .. }) = checker.semantic().stmts[source]
|
||||||
|
else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let [Expr::Name(ast::ExprName { id, .. })] = &targets[..] else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
if !checker.semantic().match_typing_expr(func, "TypeVar") {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
Some(Diagnostic::new(
|
||||||
|
UnusedPrivateTypeVar {
|
||||||
|
name: id.to_string(),
|
||||||
|
},
|
||||||
|
binding.range,
|
||||||
|
))
|
||||||
|
}
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
---
|
||||||
|
|
|
@ -0,0 +1,22 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
---
|
||||||
|
PYI018.pyi:4:1: PYI018 Private TypeVar `_T` is never used
|
||||||
|
|
|
||||||
|
2 | from typing import TypeVar
|
||||||
|
3 |
|
||||||
|
4 | _T = typing.TypeVar("_T")
|
||||||
|
| ^^ PYI018
|
||||||
|
5 | _P = TypeVar("_P")
|
||||||
|
|
|
||||||
|
|
||||||
|
PYI018.pyi:5:1: PYI018 Private TypeVar `_P` is never used
|
||||||
|
|
|
||||||
|
4 | _T = typing.TypeVar("_T")
|
||||||
|
5 | _P = TypeVar("_P")
|
||||||
|
| ^^ PYI018
|
||||||
|
6 |
|
||||||
|
7 | # OK
|
||||||
|
|
|
||||||
|
|
||||||
|
|
|
@ -94,6 +94,12 @@ impl<'a> Binding<'a> {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Return `true` if this [`Binding`] represents an private variable
|
||||||
|
/// (e.g., `_x` in `_x = "private variable"`)
|
||||||
|
pub const fn is_private_variable(&self) -> bool {
|
||||||
|
self.flags.contains(BindingFlags::PRIVATE_DECLARATION)
|
||||||
|
}
|
||||||
|
|
||||||
/// Return `true` if this binding redefines the given binding.
|
/// Return `true` if this binding redefines the given binding.
|
||||||
pub fn redefines(&self, existing: &'a Binding) -> bool {
|
pub fn redefines(&self, existing: &'a Binding) -> bool {
|
||||||
match &self.kind {
|
match &self.kind {
|
||||||
|
@ -264,6 +270,14 @@ bitflags! {
|
||||||
/// __all__ = [1]
|
/// __all__ = [1]
|
||||||
/// ```
|
/// ```
|
||||||
const INVALID_ALL_OBJECT = 1 << 6;
|
const INVALID_ALL_OBJECT = 1 << 6;
|
||||||
|
|
||||||
|
/// The binding represents a private declaration.
|
||||||
|
///
|
||||||
|
/// For example, the binding could be `_T` in:
|
||||||
|
/// ```python
|
||||||
|
/// _T = "This is a private variable"
|
||||||
|
/// ```
|
||||||
|
const PRIVATE_DECLARATION = 1 << 7;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -2367,6 +2367,7 @@
|
||||||
"PYI015",
|
"PYI015",
|
||||||
"PYI016",
|
"PYI016",
|
||||||
"PYI017",
|
"PYI017",
|
||||||
|
"PYI018",
|
||||||
"PYI02",
|
"PYI02",
|
||||||
"PYI020",
|
"PYI020",
|
||||||
"PYI021",
|
"PYI021",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue