diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.py new file mode 100644 index 0000000000..75dbf08dd6 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.py @@ -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") diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi new file mode 100644 index 0000000000..75dbf08dd6 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI018.pyi @@ -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") diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 0d6d096114..306d954788 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -10,6 +10,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::InvalidAllObject, Rule::UnaliasedCollectionsAbcSetImport, Rule::UnconventionalImportAlias, + Rule::UnusedPrivateTypeVar, Rule::UnusedVariable, ]) { return; @@ -63,6 +64,13 @@ pub(crate) fn bindings(checker: &mut Checker) { 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); + } + } } } } diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 97ff354925..06b0b3d47c 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1474,6 +1474,11 @@ impl<'a> Checker<'a> { // Create the `Binding`. 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 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, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c08c86733a..f8fd6321ed 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -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, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember), (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, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub), (Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index c43a023ae0..b39ae31186 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -93,6 +93,8 @@ mod tests { #[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))] #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))] #[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<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 4b8d229df9..6ee3f89e8a 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -29,6 +29,7 @@ pub(crate) use unnecessary_literal_union::*; pub(crate) use unrecognized_platform::*; pub(crate) use unrecognized_version_info::*; pub(crate) use unsupported_method_call_on_all::*; +pub(crate) use unused_private_type_definition::*; mod any_eq_ne_annotation; mod bad_version_info_comparison; @@ -61,3 +62,4 @@ mod unnecessary_literal_union; mod unrecognized_platform; mod unrecognized_version_info; mod unsupported_method_call_on_all; +mod unused_private_type_definition; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs new file mode 100644 index 0000000000..5f0fff3923 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -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 { + 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, + )) +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap new file mode 100644 index 0000000000..b06ccc51bd --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI018_PYI018.pyi.snap @@ -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 + | + + diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 42ccb1962f..783dd9ca36 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -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. pub fn redefines(&self, existing: &'a Binding) -> bool { match &self.kind { @@ -264,6 +270,14 @@ bitflags! { /// __all__ = [1] /// ``` 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; } } diff --git a/ruff.schema.json b/ruff.schema.json index d7d8d0e7b3..8506373e5b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2367,6 +2367,7 @@ "PYI015", "PYI016", "PYI017", + "PYI018", "PYI02", "PYI020", "PYI021",