From 848e473f69d0451bc442b1d2ea63112a260f1f19 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Thu, 18 Jan 2024 03:14:18 +0000 Subject: [PATCH] [flake8-pyi] Fix PYI047 false negatives on PEP-695 type aliases (#9566) ## Summary Fixes one of the issues listed in https://github.com/astral-sh/ruff/issues/8771. Fairly straightforward! ## Test Plan `cargo test` / `cargo insta review` --- .../test/fixtures/flake8_pyi/PYI047.py | 9 ++++ .../test/fixtures/flake8_pyi/PYI047.pyi | 9 ++++ .../rules/unused_private_type_definition.rs | 44 ++++++++++++------- ...__flake8_pyi__tests__PYI047_PYI047.py.snap | 18 ++++++++ ..._flake8_pyi__tests__PYI047_PYI047.pyi.snap | 18 ++++++++ 5 files changed, 81 insertions(+), 17 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.py index 91bc2cd413c1b..62da99f741477 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.py @@ -20,3 +20,12 @@ def func(arg: _UsedPrivateTypeAlias) -> _UsedPrivateTypeAlias: def func2(arg: _PrivateTypeAlias) -> None: ... + +type _UnusedPEP695 = int +type _UnusedGeneric695[T] = list[T] + +type _UsedPEP695 = str +type _UsedGeneric695[T] = tuple[T, ...] + +def func4(arg: _UsedPEP695) -> None: ... +def func5(arg: _UsedGeneric695[bytes]) -> None: ... diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.pyi index 91bc2cd413c1b..62da99f741477 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI047.pyi @@ -20,3 +20,12 @@ else: def func2(arg: _PrivateTypeAlias) -> None: ... + +type _UnusedPEP695 = int +type _UnusedGeneric695[T] = list[T] + +type _UsedPEP695 = str +type _UsedGeneric695[T] = tuple[T, ...] + +def func4(arg: _UsedPEP695) -> None: ... +def func5(arg: _UsedGeneric695[bytes]) -> None: ... 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 545b5f7f656d3..4a4c60b9f39b6 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 @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::map_subscript; use ruff_python_ast::{self as ast, Expr, Stmt}; -use ruff_python_semantic::Scope; +use ruff_python_semantic::{Scope, SemanticModel}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -267,9 +267,11 @@ pub(crate) fn unused_private_type_alias( scope: &Scope, diagnostics: &mut Vec, ) { + let semantic = checker.semantic(); + for binding in scope .binding_ids() - .map(|binding_id| checker.semantic().binding(binding_id)) + .map(|binding_id| semantic.binding(binding_id)) { if !(binding.kind.is_assignment() && binding.is_private_declaration()) { continue; @@ -281,32 +283,40 @@ pub(crate) fn unused_private_type_alias( let Some(source) = binding.source else { continue; }; - let Stmt::AnnAssign(ast::StmtAnnAssign { - target, annotation, .. - }) = checker.semantic().statement(source) - else { - continue; - }; - let Some(ast::ExprName { id, .. }) = target.as_name_expr() else { - continue; - }; - if !checker - .semantic() - .match_typing_expr(annotation, "TypeAlias") - { + let Some(alias_name) = extract_type_alias_name(semantic.statement(source), semantic) else { continue; - } + }; diagnostics.push(Diagnostic::new( UnusedPrivateTypeAlias { - name: id.to_string(), + name: alias_name.to_string(), }, binding.range(), )); } } +fn extract_type_alias_name<'a>(stmt: &'a ast::Stmt, semantic: &SemanticModel) -> Option<&'a str> { + match stmt { + ast::Stmt::AnnAssign(ast::StmtAnnAssign { + target, annotation, .. + }) => { + let ast::ExprName { id, .. } = target.as_name_expr()?; + if semantic.match_typing_expr(annotation, "TypeAlias") { + Some(id) + } else { + None + } + } + ast::Stmt::TypeAlias(ast::StmtTypeAlias { name, .. }) => { + let ast::ExprName { id, .. } = name.as_name_expr()?; + Some(id) + } + _ => None, + } +} + /// PYI049 pub(crate) fn unused_private_typed_dict( checker: &Checker, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.py.snap index f6bf21b1ed7bf..73a6e051d15b5 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.py.snap @@ -17,4 +17,22 @@ PYI047.py:7:1: PYI047 Private TypeAlias `_T` is never used 9 | # OK | +PYI047.py:24:6: PYI047 Private TypeAlias `_UnusedPEP695` is never used + | +22 | def func2(arg: _PrivateTypeAlias) -> None: ... +23 | +24 | type _UnusedPEP695 = int + | ^^^^^^^^^^^^^ PYI047 +25 | type _UnusedGeneric695[T] = list[T] + | + +PYI047.py:25:6: PYI047 Private TypeAlias `_UnusedGeneric695` is never used + | +24 | type _UnusedPEP695 = int +25 | type _UnusedGeneric695[T] = list[T] + | ^^^^^^^^^^^^^^^^^ PYI047 +26 | +27 | type _UsedPEP695 = str + | + diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap index 1eae7ab37cb56..096a1dd558aa6 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap @@ -17,4 +17,22 @@ PYI047.pyi:7:1: PYI047 Private TypeAlias `_T` is never used 9 | # OK | +PYI047.pyi:24:6: PYI047 Private TypeAlias `_UnusedPEP695` is never used + | +22 | def func2(arg: _PrivateTypeAlias) -> None: ... +23 | +24 | type _UnusedPEP695 = int + | ^^^^^^^^^^^^^ PYI047 +25 | type _UnusedGeneric695[T] = list[T] + | + +PYI047.pyi:25:6: PYI047 Private TypeAlias `_UnusedGeneric695` is never used + | +24 | type _UnusedPEP695 = int +25 | type _UnusedGeneric695[T] = list[T] + | ^^^^^^^^^^^^^^^^^ PYI047 +26 | +27 | type _UsedPEP695 = str + | +