From 3d8096986190f43f83ff378e61c84530eafaa634 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 27 Jul 2023 17:58:40 -0300 Subject: [PATCH 01/11] Implement PYI047 /// ## What it does /// Checks for the presence of unused private `typing.TypeAlias` definitions. /// /// ## Why is this bad? /// A private `typing.TypeAlias` that is defined but not used is likely a /// mistake, and should either be used, made public, or removed to avoid /// confusion. /// /// ## Example /// ```python /// import typing /// /// _UnusedTypeAlias: typing.TypeAlias = int /// ``` /// /// Use instead: /// ```python /// import typing /// /// _UsedTypeAlias: typing.TypeAlias = int /// /// def func(arg: _UsedTypeAlias) -> _UsedTypeAlias: /// ... /// ``` --- .../test/fixtures/flake8_pyi/PYI047.py | 12 +++ .../test/fixtures/flake8_pyi/PYI047.pyi | 12 +++ .../ruff/src/checkers/ast/analyze/bindings.rs | 8 ++ crates/ruff/src/codes.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + .../rules/unused_private_type_definition.rs | 75 +++++++++++++++++++ ...__flake8_pyi__tests__PYI047_PYI047.py.snap | 4 + ..._flake8_pyi__tests__PYI047_PYI047.pyi.snap | 20 +++++ ruff.schema.json | 1 + 9 files changed, 135 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py new file mode 100644 index 0000000000000..36cf1dc19c74f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py @@ -0,0 +1,12 @@ +import typing +from typing import TypeAlias + + +_UnusedPrivateTypeAlias: TypeAlias = int | None +_T: typing.TypeAlias = str + +# OK +_UsedPrivateTypeAlias: TypeAlias = int | None + +def func(arg: _UsedPrivateTypeAlias) -> _UsedPrivateTypeAlias: + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi new file mode 100644 index 0000000000000..36cf1dc19c74f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi @@ -0,0 +1,12 @@ +import typing +from typing import TypeAlias + + +_UnusedPrivateTypeAlias: TypeAlias = int | None +_T: typing.TypeAlias = str + +# OK +_UsedPrivateTypeAlias: TypeAlias = int | None + +def func(arg: _UsedPrivateTypeAlias) -> _UsedPrivateTypeAlias: + ... diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 1c8804272ebef..9be39508fbb5c 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -13,6 +13,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::UnusedPrivateTypeVar, Rule::UnusedVariable, Rule::UnusedPrivateProtocol, + Rule::UnusedPrivateTypeAlias, ]) { return; } @@ -79,6 +80,13 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::UnusedPrivateTypeAlias) { + if let Some(diagnostic) = + flake8_pyi::rules::unused_private_type_alias(checker, binding) + { + checker.diagnostics.push(diagnostic); + } + } } } } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c00f576a38e88..47a02947be97b 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -654,6 +654,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub), (Flake8Pyi, "045") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::IterMethodReturnIterable), (Flake8Pyi, "046") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateProtocol), + (Flake8Pyi, "047") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateTypeAlias), (Flake8Pyi, "048") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StubBodyMultipleStatements), (Flake8Pyi, "050") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NoReturnArgumentAnnotationInStub), (Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 2cbb90c3e0d81..b3f601d7be900 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -97,6 +97,8 @@ mod tests { #[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.pyi"))] #[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.py"))] #[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.pyi"))] + #[test_case(Rule::UnusedPrivateTypeAlias, Path::new("PYI047.py"))] + #[test_case(Rule::UnusedPrivateTypeAlias, Path::new("PYI047.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/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index db0f35a83e62a..89ba60a4b8004 100644 --- 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 @@ -73,6 +73,43 @@ impl Violation for UnusedPrivateProtocol { } } +/// ## What it does +/// Checks for the presence of unused private `typing.TypeAlias` definitions. +/// +/// ## Why is this bad? +/// A private `typing.TypeAlias` that is defined but not used is likely a +/// mistake, and should either be used, made public, or removed to avoid +/// confusion. +/// +/// ## Example +/// ```python +/// import typing +/// +/// _UnusedTypeAlias: typing.TypeAlias = int +/// ``` +/// +/// Use instead: +/// ```python +/// import typing +/// +/// _UsedTypeAlias: typing.TypeAlias = int +/// +/// def func(arg: _UsedTypeAlias) -> _UsedTypeAlias: +/// ... +/// ``` +#[violation] +pub struct UnusedPrivateTypeAlias { + name: String, +} + +impl Violation for UnusedPrivateTypeAlias { + #[derive_message_formats] + fn message(&self) -> String { + let UnusedPrivateTypeAlias { name } = self; + format!("Private TypeAlias `{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_declaration()) { @@ -138,3 +175,41 @@ pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> O binding.range, )) } + +/// PYI047 +pub(crate) fn unused_private_type_alias( + checker: &Checker, + binding: &Binding, +) -> Option { + if !(binding.kind.is_assignment() && binding.is_private_declaration()) { + return None; + } + if binding.is_used() { + return None; + } + + let Some(source) = binding.source else { + return None; + }; + let Stmt::AnnAssign(ast::StmtAnnAssign { target, annotation, .. }) = checker.semantic().stmts[source] + else { + return None + }; + let Some(ast::ExprName { id, .. }) = target.as_name_expr() else { + return None; + }; + + if !checker + .semantic() + .match_typing_expr(annotation, "TypeAlias") + { + return None; + } + + Some(Diagnostic::new( + UnusedPrivateTypeAlias { + name: id.to_string(), + }, + binding.range, + )) +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.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__PYI047_PYI047.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap new file mode 100644 index 0000000000000..db805d3b4c942 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI047.pyi:5:1: PYI047 Private TypeAlias `_UnusedPrivateTypeAlias` is never used + | +5 | _UnusedPrivateTypeAlias: TypeAlias = int | None + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI047 +6 | _T: typing.TypeAlias = str + | + +PYI047.pyi:6:1: PYI047 Private TypeAlias `_T` is never used + | +5 | _UnusedPrivateTypeAlias: TypeAlias = int | None +6 | _T: typing.TypeAlias = str + | ^^ PYI047 +7 | +8 | # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index f56257eb82373..225ac06c6d663 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2393,6 +2393,7 @@ "PYI044", "PYI045", "PYI046", + "PYI047", "PYI048", "PYI05", "PYI050", From b99a560f175f264f89a2a5063c887a73308adb82 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 27 Jul 2023 18:06:16 -0300 Subject: [PATCH 02/11] fix doc formatting --- .../rules/unused_private_type_definition.rs | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) 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 index 89ba60a4b8004..03da2d067bf65 100644 --- 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 @@ -94,6 +94,7 @@ impl Violation for UnusedPrivateProtocol { /// /// _UsedTypeAlias: typing.TypeAlias = int /// +/// /// def func(arg: _UsedTypeAlias) -> _UsedTypeAlias: /// ... /// ``` @@ -137,9 +138,7 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateTypeVar { - name: id.to_string(), - }, + UnusedPrivateTypeVar { name: id.to_string() }, binding.range, )) } @@ -169,18 +168,13 @@ pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateProtocol { - name: name.to_string(), - }, + UnusedPrivateProtocol { name: name.to_string() }, binding.range, )) } /// PYI047 -pub(crate) fn unused_private_type_alias( - checker: &Checker, - binding: &Binding, -) -> Option { +pub(crate) fn unused_private_type_alias(checker: &Checker, binding: &Binding) -> Option { if !(binding.kind.is_assignment() && binding.is_private_declaration()) { return None; } @@ -199,17 +193,12 @@ pub(crate) fn unused_private_type_alias( return None; }; - if !checker - .semantic() - .match_typing_expr(annotation, "TypeAlias") - { + if !checker.semantic().match_typing_expr(annotation, "TypeAlias") { return None; } Some(Diagnostic::new( - UnusedPrivateTypeAlias { - name: id.to_string(), - }, + UnusedPrivateTypeAlias { name: id.to_string() }, binding.range, )) } From 1fe05f651d5774d5a8e71e608b6608cf4b44a713 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 27 Jul 2023 18:27:30 -0300 Subject: [PATCH 03/11] fix formatting --- .../rules/unused_private_type_definition.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) 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 index 03da2d067bf65..42c322546b570 100644 --- 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 @@ -138,7 +138,9 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateTypeVar { name: id.to_string() }, + UnusedPrivateTypeVar { + name: id.to_string(), + }, binding.range, )) } @@ -168,13 +170,18 @@ pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateProtocol { name: name.to_string() }, + UnusedPrivateProtocol { + name: name.to_string(), + }, binding.range, )) } /// PYI047 -pub(crate) fn unused_private_type_alias(checker: &Checker, binding: &Binding) -> Option { +pub(crate) fn unused_private_type_alias( + checker: &Checker, + binding: &Binding, +) -> Option { if !(binding.kind.is_assignment() && binding.is_private_declaration()) { return None; } @@ -193,12 +200,17 @@ pub(crate) fn unused_private_type_alias(checker: &Checker, binding: &Binding) -> return None; }; - if !checker.semantic().match_typing_expr(annotation, "TypeAlias") { + if !checker + .semantic() + .match_typing_expr(annotation, "TypeAlias") + { return None; } Some(Diagnostic::new( - UnusedPrivateTypeAlias { name: id.to_string() }, + UnusedPrivateTypeAlias { + name: id.to_string(), + }, binding.range, )) } From 0a61d87293f38377fbcdd1ede7748d2ca1178508 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 27 Jul 2023 20:01:42 -0400 Subject: [PATCH 04/11] Add documentation and test cases for redefinition (#6135) --- crates/ruff/src/rules/pyflakes/mod.rs | 39 ++++++++++++ crates/ruff_python_semantic/src/binding.rs | 70 ++++++++++++++-------- 2 files changed, 84 insertions(+), 25 deletions(-) diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index e2dae95b695c9..08170bb5191d7 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/crates/ruff/src/rules/pyflakes/mod.rs @@ -2414,6 +2414,45 @@ mod tests { ); } + #[test] + fn aliased_submodule_import() { + flakes( + r#" + import fu.bar as baz + import fu.bar as baz + baz + "#, + &[Rule::RedefinedWhileUnused], + ); + + flakes( + r#" + import fu.bar as baz + import baz + baz + "#, + &[Rule::RedefinedWhileUnused], + ); + + flakes( + r#" + import fu.bar as baz + import fu.bar as bop + baz, bop + "#, + &[], + ); + + flakes( + r#" + import foo.baz + import foo.baz as foo + foo + "#, + &[Rule::RedefinedWhileUnused], + ); + } + #[test] fn used_package_with_submodule_import() { // Usage of package marks submodule imports as used. diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 622ae324359c9..6a6e712f5864f 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -100,43 +100,62 @@ impl<'a> Binding<'a> { self.flags.contains(BindingFlags::PRIVATE_DECLARATION) } - /// Return `true` if this binding redefines the given binding. + /// Return `true` if this binding "redefines" the given binding, as per Pyflake's definition of + /// redefinition. pub fn redefines(&self, existing: &'a Binding) -> bool { match &self.kind { - BindingKind::Import(Import { qualified_name }) => { + // Submodule imports are only considered redefinitions if they import the same + // submodule. For example, this is a redefinition: + // ```python + // import foo.bar + // import foo.bar + // ``` + // + // This, however, is not: + // ```python + // import foo.bar + // import foo.baz + // ``` + BindingKind::Import(Import { + qualified_name: redefinition, + }) => { if let BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: existing, + qualified_name: definition, }) = &existing.kind { - return qualified_name == existing; + return redefinition == definition; } } - BindingKind::FromImport(FromImport { qualified_name }) => { + BindingKind::FromImport(FromImport { + qualified_name: redefinition, + }) => { if let BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: existing, + qualified_name: definition, }) = &existing.kind { - return qualified_name == existing; + return redefinition == definition; } } - BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => { - match &existing.kind { - BindingKind::Import(Import { - qualified_name: existing, - }) - | BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: existing, - }) => { - return qualified_name == existing; - } - BindingKind::FromImport(FromImport { - qualified_name: existing, - }) => { - return qualified_name == existing; - } - _ => {} + BindingKind::SubmoduleImport(SubmoduleImport { + qualified_name: redefinition, + }) => match &existing.kind { + BindingKind::Import(Import { + qualified_name: definition, + }) + | BindingKind::SubmoduleImport(SubmoduleImport { + qualified_name: definition, + }) => { + return redefinition == definition; } - } + BindingKind::FromImport(FromImport { + qualified_name: definition, + }) => { + return redefinition == definition; + } + _ => {} + }, + // Deletions, annotations, `__future__` imports, and builtins are never considered + // redefinitions. BindingKind::Deletion | BindingKind::Annotation | BindingKind::FutureImport @@ -145,13 +164,14 @@ impl<'a> Binding<'a> { } _ => {} } + // Otherwise, the shadowed binding must be a class definition, function definition, or + // import to be considered a redefinition. matches!( existing.kind, BindingKind::ClassDefinition(_) | BindingKind::FunctionDefinition(_) | BindingKind::Import(_) | BindingKind::FromImport(_) - | BindingKind::SubmoduleImport(_) ) } From ad3081d65b969c55aa23146649d00876206c25ee Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 27 Jul 2023 22:16:09 -0400 Subject: [PATCH 05/11] Only run unused private type rules over finalized bindings (#6142) In #6134 and #6136, we see some false positives for "shadowed" class definitions. For example, here, the first definition is flagged as unused, since from the perspective of the semantic model (which doesn't understand branching), it appears to be immediately shadowed in the `else`, and thus never used: ```python if sys.version_info >= (3, 11): class _RootLoggerConfiguration(TypedDict, total=False): level: _Level filters: Sequence[str | _FilterType] handlers: Sequence[str] else: class _RootLoggerConfiguration(TypedDict, total=False): level: _Level filters: Sequence[str] handlers: Sequence[str] ``` Instead of looking at _all_ bindings, we should instead look at the "live" bindings, which is similar to how other rules (like unused variables detection) is structured. We thus move the rule from `bindings.rs` (which iterates over _all_ bindings, regardless of whether they're shadowed) to `deferred_scopes.rs`, which iterates over all "live" bindings once a scope has been fully analyzed. `cargo test` --- .../ruff/src/checkers/ast/analyze/bindings.rs | 27 +--- .../checkers/ast/analyze/deferred_scopes.rs | 13 +- .../rules/unused_private_type_definition.rs | 135 ++++++++++-------- 3 files changed, 91 insertions(+), 84 deletions(-) diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 9be39508fbb5c..80c632308fbea 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -1,7 +1,8 @@ +use ruff_diagnostics::{Diagnostic, Fix}; + use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint}; -use ruff_diagnostics::{Diagnostic, Fix}; /// Run lint rules over the [`Binding`]s. pub(crate) fn bindings(checker: &mut Checker) { @@ -10,10 +11,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::InvalidAllObject, Rule::UnaliasedCollectionsAbcSetImport, Rule::UnconventionalImportAlias, - Rule::UnusedPrivateTypeVar, Rule::UnusedVariable, - Rule::UnusedPrivateProtocol, - Rule::UnusedPrivateTypeAlias, ]) { return; } @@ -66,27 +64,6 @@ 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); - } - } - if checker.enabled(Rule::UnusedPrivateProtocol) { - if let Some(diagnostic) = - flake8_pyi::rules::unused_private_protocol(checker, binding) - { - checker.diagnostics.push(diagnostic); - } - } - if checker.enabled(Rule::UnusedPrivateTypeAlias) { - if let Some(diagnostic) = - flake8_pyi::rules::unused_private_type_alias(checker, binding) - { - checker.diagnostics.push(diagnostic); - } - } } } } diff --git a/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs index 1d0f2a5ba0bb9..8fb6363c19bcd 100644 --- a/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs @@ -5,7 +5,7 @@ use ruff_python_semantic::{Binding, BindingKind, ScopeKind}; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::{flake8_type_checking, flake8_unused_arguments, pyflakes, pylint}; +use crate::rules::{flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint}; /// Run lint rules over all deferred scopes in the [`SemanticModel`]. pub(crate) fn deferred_scopes(checker: &mut Checker) { @@ -24,6 +24,8 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::UnusedImport, Rule::UnusedLambdaArgument, Rule::UnusedMethodArgument, + Rule::UnusedPrivateProtocol, + Rule::UnusedPrivateTypeVar, Rule::UnusedStaticMethodArgument, Rule::UnusedVariable, ]) { @@ -214,6 +216,15 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { } } + if checker.is_stub { + if checker.enabled(Rule::UnusedPrivateTypeVar) { + flake8_pyi::rules::unused_private_type_var(checker, scope, &mut diagnostics); + } + if checker.enabled(Rule::UnusedPrivateProtocol) { + flake8_pyi::rules::unused_private_protocol(checker, scope, &mut diagnostics); + } + } + if matches!( scope.kind, ScopeKind::Function(_) | ScopeKind::AsyncFunction(_) | ScopeKind::Lambda(_) 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 index 42c322546b570..eca55fa14cacb 100644 --- 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 @@ -1,7 +1,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr, Stmt}; -use ruff_python_semantic::Binding; +use ruff_python_semantic::Scope; use crate::checkers::ast::Checker; @@ -112,69 +112,88 @@ impl Violation for UnusedPrivateTypeAlias { } /// PYI018 -pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> Option { - if !(binding.kind.is_assignment() && binding.is_private_declaration()) { - 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; +pub(crate) fn unused_private_type_var( + checker: &Checker, + scope: &Scope, + diagnostics: &mut Vec, +) { + for binding in scope + .binding_ids() + .map(|binding_id| checker.semantic().binding(binding_id)) + { + if !(binding.kind.is_assignment() && binding.is_private_declaration()) { + continue; + } + if binding.is_used() { + continue; + } + + let Some(source) = binding.source else { + continue; + }; + let Stmt::Assign(ast::StmtAssign { targets, value, .. }) = checker.semantic().stmts[source] + else { + continue; + }; + let [Expr::Name(ast::ExprName { id, .. })] = &targets[..] else { + continue; + }; + let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else { + continue; + }; + if !checker.semantic().match_typing_expr(func, "TypeVar") { + continue; + } + + diagnostics.push(Diagnostic::new( + UnusedPrivateTypeVar { + name: id.to_string(), + }, + binding.range, + )); } - - Some(Diagnostic::new( - UnusedPrivateTypeVar { - name: id.to_string(), - }, - binding.range, - )) } /// PYI046 -pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> Option { - if !(binding.kind.is_class_definition() && binding.is_private_declaration()) { - return None; - } - if binding.is_used() { - return None; - } - - let Some(source) = binding.source else { - return None; - }; - let Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) = checker.semantic().stmts[source] - else { - return None; - }; - - if !bases - .iter() - .any(|base| checker.semantic().match_typing_expr(base, "Protocol")) +pub(crate) fn unused_private_protocol( + checker: &Checker, + scope: &Scope, + diagnostics: &mut Vec, +) { + for binding in scope + .binding_ids() + .map(|binding_id| checker.semantic().binding(binding_id)) { - return None; + if !(binding.kind.is_class_definition() && binding.is_private_declaration()) { + continue; + } + if binding.is_used() { + continue; + } + + let Some(source) = binding.source else { + continue; + }; + let Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) = + checker.semantic().stmts[source] + else { + continue; + }; + + if !bases + .iter() + .any(|base| checker.semantic().match_typing_expr(base, "Protocol")) + { + continue; + } + + diagnostics.push(Diagnostic::new( + UnusedPrivateProtocol { + name: name.to_string(), + }, + binding.range, + )); } - - Some(Diagnostic::new( - UnusedPrivateProtocol { - name: name.to_string(), - }, - binding.range, - )) } /// PYI047 From 74cdfdb8e689b5ad7f3967387228f0a89d305f7d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 28 Jul 2023 00:55:55 -0400 Subject: [PATCH 06/11] Skip `PERF203` violations for multi-statement loops (#6145) Closes https://github.com/astral-sh/ruff/issues/5858. --- .../test/fixtures/perflint/PERF203.py | 15 ++++------- .../perflint/rules/try_except_in_loop.rs | 21 ++++++++------- ...__perflint__tests__PERF203_PERF203.py.snap | 26 +++++-------------- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF203.py b/crates/ruff/resources/test/fixtures/perflint/PERF203.py index ec3ca5feeefaf..14b3fa38a3800 100644 --- a/crates/ruff/resources/test/fixtures/perflint/PERF203.py +++ b/crates/ruff/resources/test/fixtures/perflint/PERF203.py @@ -1,28 +1,23 @@ +# PERF203 for i in range(10): - try: # PERF203 + try: print(f"{i}") except: print("error") +# OK try: for i in range(10): print(f"{i}") except: print("error") +# OK i = 0 -while i < 10: # PERF203 +while i < 10: try: print(f"{i}") except: print("error") i += 1 - -try: - i = 0 - while i < 10: - print(f"{i}") - i += 1 -except: - print("error") diff --git a/crates/ruff/src/rules/perflint/rules/try_except_in_loop.rs b/crates/ruff/src/rules/perflint/rules/try_except_in_loop.rs index fe57321d8ca3a..ffa8a428faa3e 100644 --- a/crates/ruff/src/rules/perflint/rules/try_except_in_loop.rs +++ b/crates/ruff/src/rules/perflint/rules/try_except_in_loop.rs @@ -67,14 +67,15 @@ pub(crate) fn try_except_in_loop(checker: &mut Checker, body: &[Stmt]) { return; } - checker.diagnostics.extend(body.iter().filter_map(|stmt| { - if let Stmt::Try(ast::StmtTry { handlers, .. }) = stmt { - handlers - .iter() - .next() - .map(|handler| Diagnostic::new(TryExceptInLoop, handler.range())) - } else { - None - } - })); + let [Stmt::Try(ast::StmtTry { handlers, .. })] = body else { + return; + }; + + let Some(handler) = handlers.first() else { + return; + }; + + checker + .diagnostics + .push(Diagnostic::new(TryExceptInLoop, handler.range())); } diff --git a/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF203_PERF203.py.snap b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF203_PERF203.py.snap index 08a287819e7ab..d74621c223552 100644 --- a/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF203_PERF203.py.snap +++ b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF203_PERF203.py.snap @@ -1,28 +1,16 @@ --- source: crates/ruff/src/rules/perflint/mod.rs --- -PERF203.py:4:5: PERF203 `try`-`except` within a loop incurs performance overhead +PERF203.py:5:5: PERF203 `try`-`except` within a loop incurs performance overhead | -2 | try: # PERF203 -3 | print(f"{i}") -4 | except: +3 | try: +4 | print(f"{i}") +5 | except: | _____^ -5 | | print("error") +6 | | print("error") | |______________________^ PERF203 -6 | -7 | try: +7 | +8 | # OK | -PERF203.py:17:5: PERF203 `try`-`except` within a loop incurs performance overhead - | -15 | try: -16 | print(f"{i}") -17 | except: - | _____^ -18 | | print("error") - | |______________________^ PERF203 -19 | -20 | i += 1 - | - From c93bbbf622279dbd97cf7b7361412535bd3a7524 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 28 Jul 2023 09:38:13 -0400 Subject: [PATCH 07/11] Avoid refactoring `x[:1]`-like slices in RUF015 (#6150) ## Summary Right now, `RUF015` will try to rewrite `x[:1]` as `[next(x)]`. This isn't equivalent if `x`, for example, is empty, where slicing like `x[:1]` is forgiving, but `next` raises `StopIteration`. For me this is a little too much of a deviation to be comfortable with, and most of the value in this rule is the `x[0]` to `next(x)` conversion anyway. Closes https://github.com/astral-sh/ruff/issues/6148. --- .../resources/test/fixtures/ruff/RUF015.py | 15 +- ...y_iterable_allocation_for_first_element.rs | 121 +--- ..._rules__ruff__tests__RUF015_RUF015.py.snap | 564 +++++------------- 3 files changed, 183 insertions(+), 517 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF015.py b/crates/ruff/resources/test/fixtures/ruff/RUF015.py index 393694d89b3f1..2aa92f467cfe8 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF015.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF015.py @@ -2,21 +2,9 @@ # RUF015 list(x)[0] -list(x)[:1] -list(x)[:1:1] -list(x)[:1:2] tuple(x)[0] -tuple(x)[:1] -tuple(x)[:1:1] -tuple(x)[:1:2] list(i for i in x)[0] -list(i for i in x)[:1] -list(i for i in x)[:1:1] -list(i for i in x)[:1:2] [i for i in x][0] -[i for i in x][:1] -[i for i in x][:1:1] -[i for i in x][:1:2] # OK (not indexing (solely) the first element) list(x) @@ -29,6 +17,9 @@ [i for i in x] [i for i in x][1] [i for i in x][-1] +[i for i in x][:1] +[i for i in x][:1:1] +[i for i in x][:1:2] [i for i in x][1:] [i for i in x][:3:2] [i for i in x][::2] diff --git a/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs b/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs index 47c789f65e3ca..85ae2bb9dfedd 100644 --- a/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs +++ b/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs @@ -1,14 +1,13 @@ use std::borrow::Cow; -use num_bigint::BigInt; -use num_traits::{One, Zero}; -use ruff_python_ast::{self as ast, Comprehension, Constant, Expr, Ranged}; -use ruff_text_size::{TextRange, TextSize}; +use num_traits::Zero; use unicode_width::UnicodeWidthStr; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Comprehension, Constant, Expr, Ranged}; use ruff_python_semantic::SemanticModel; +use ruff_text_size::{TextRange, TextSize}; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -49,37 +48,20 @@ use crate::registry::AsRule; #[violation] pub(crate) struct UnnecessaryIterableAllocationForFirstElement { iterable: String, - subscript_kind: HeadSubscriptKind, } impl AlwaysAutofixableViolation for UnnecessaryIterableAllocationForFirstElement { #[derive_message_formats] fn message(&self) -> String { - let UnnecessaryIterableAllocationForFirstElement { - iterable, - subscript_kind, - } = self; + let UnnecessaryIterableAllocationForFirstElement { iterable } = self; let iterable = Self::truncate(iterable); - match subscript_kind { - HeadSubscriptKind::Index => { - format!("Prefer `next({iterable})` over single element slice") - } - HeadSubscriptKind::Slice => { - format!("Prefer `[next({iterable})]` over single element slice") - } - } + format!("Prefer `next({iterable})` over single element slice") } fn autofix_title(&self) -> String { - let UnnecessaryIterableAllocationForFirstElement { - iterable, - subscript_kind, - } = self; + let UnnecessaryIterableAllocationForFirstElement { iterable } = self; let iterable = Self::truncate(iterable); - match subscript_kind { - HeadSubscriptKind::Index => format!("Replace with `next({iterable})`"), - HeadSubscriptKind::Slice => format!("Replace with `[next({iterable})]"), - } + format!("Replace with `next({iterable})`") } } @@ -106,9 +88,9 @@ pub(crate) fn unnecessary_iterable_allocation_for_first_element( .. } = subscript; - let Some(subscript_kind) = classify_subscript(slice) else { + if !is_head_slice(slice) { return; - }; + } let Some(target) = match_iteration_target(value, checker.semantic()) else { return; @@ -123,72 +105,31 @@ pub(crate) fn unnecessary_iterable_allocation_for_first_element( let mut diagnostic = Diagnostic::new( UnnecessaryIterableAllocationForFirstElement { iterable: iterable.to_string(), - subscript_kind, }, *range, ); if checker.patch(diagnostic.kind.rule()) { - let replacement = match subscript_kind { - HeadSubscriptKind::Index => format!("next({iterable})"), - HeadSubscriptKind::Slice => format!("[next({iterable})]"), - }; - diagnostic.set_fix(Fix::suggested(Edit::range_replacement(replacement, *range))); + diagnostic.set_fix(Fix::suggested(Edit::range_replacement( + format!("next({iterable})"), + *range, + ))); } checker.diagnostics.push(diagnostic); } -/// A subscript slice that represents the first element of a list. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum HeadSubscriptKind { - /// The subscript is an index (e.g., `[0]`). - Index, - /// The subscript is a slice (e.g., `[:1]`). - Slice, -} - -/// Check that the slice [`Expr`] is functionally equivalent to slicing into the first element. The -/// first `bool` checks that the element is in fact first, the second checks if it's a slice or an -/// index. -fn classify_subscript(expr: &Expr) -> Option { - let result = match expr { - Expr::Constant(ast::ExprConstant { - value: Constant::Int(value), - .. - }) if value.is_zero() => HeadSubscriptKind::Index, - Expr::Slice(ast::ExprSlice { - step, lower, upper, .. - }) => { - // Avoid, e.g., `list(...)[:2]` - let upper = upper.as_ref()?; - let upper = as_int(upper)?; - if !upper.is_one() { - return None; - } - - // Avoid, e.g., `list(...)[2:]`. - if let Some(lower) = lower.as_ref() { - let lower = as_int(lower)?; - if !lower.is_zero() { - return None; - } - } - - // Avoid, e.g., `list(...)[::-1]` - if let Some(step) = step.as_ref() { - let step = as_int(step)?; - if step < upper { - return None; - } - } - - HeadSubscriptKind::Slice - } - _ => return None, - }; - - Some(result) +/// Check that the slice [`Expr`] is a slice of the first element (e.g., `x[0]`). +fn is_head_slice(expr: &Expr) -> bool { + if let Expr::Constant(ast::ExprConstant { + value: Constant::Int(value), + .. + }) = expr + { + value.is_zero() + } else { + false + } } #[derive(Debug)] @@ -310,17 +251,3 @@ fn match_simple_comprehension(elt: &Expr, generators: &[Comprehension]) -> Optio Some(generator.iter.range()) } - -/// If an expression is a constant integer, returns the value of that integer; otherwise, -/// returns `None`. -fn as_int(expr: &Expr) -> Option<&BigInt> { - if let Expr::Constant(ast::ExprConstant { - value: Constant::Int(value), - .. - }) = expr - { - Some(value) - } else { - None - } -} diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF015_RUF015.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF015_RUF015.py.snap index c55e737c6d0b3..e8f01fe1a2c47 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF015_RUF015.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF015_RUF015.py.snap @@ -6,8 +6,8 @@ RUF015.py:4:1: RUF015 [*] Prefer `next(iter(x))` over single element slice 3 | # RUF015 4 | list(x)[0] | ^^^^^^^^^^ RUF015 -5 | list(x)[:1] -6 | list(x)[:1:1] +5 | tuple(x)[0] +6 | list(i for i in x)[0] | = help: Replace with `next(iter(x))` @@ -17,490 +17,238 @@ RUF015.py:4:1: RUF015 [*] Prefer `next(iter(x))` over single element slice 3 3 | # RUF015 4 |-list(x)[0] 4 |+next(iter(x)) -5 5 | list(x)[:1] -6 6 | list(x)[:1:1] -7 7 | list(x)[:1:2] +5 5 | tuple(x)[0] +6 6 | list(i for i in x)[0] +7 7 | [i for i in x][0] -RUF015.py:5:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice +RUF015.py:5:1: RUF015 [*] Prefer `next(iter(x))` over single element slice | 3 | # RUF015 4 | list(x)[0] -5 | list(x)[:1] +5 | tuple(x)[0] | ^^^^^^^^^^^ RUF015 -6 | list(x)[:1:1] -7 | list(x)[:1:2] +6 | list(i for i in x)[0] +7 | [i for i in x][0] | - = help: Replace with `[next(iter(x))] + = help: Replace with `next(iter(x))` ℹ Suggested fix 2 2 | 3 3 | # RUF015 4 4 | list(x)[0] -5 |-list(x)[:1] - 5 |+[next(iter(x))] -6 6 | list(x)[:1:1] -7 7 | list(x)[:1:2] -8 8 | tuple(x)[0] +5 |-tuple(x)[0] + 5 |+next(iter(x)) +6 6 | list(i for i in x)[0] +7 7 | [i for i in x][0] +8 8 | -RUF015.py:6:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice +RUF015.py:6:1: RUF015 [*] Prefer `next(iter(x))` over single element slice | 4 | list(x)[0] -5 | list(x)[:1] -6 | list(x)[:1:1] - | ^^^^^^^^^^^^^ RUF015 -7 | list(x)[:1:2] -8 | tuple(x)[0] +5 | tuple(x)[0] +6 | list(i for i in x)[0] + | ^^^^^^^^^^^^^^^^^^^^^ RUF015 +7 | [i for i in x][0] | - = help: Replace with `[next(iter(x))] + = help: Replace with `next(iter(x))` ℹ Suggested fix 3 3 | # RUF015 4 4 | list(x)[0] -5 5 | list(x)[:1] -6 |-list(x)[:1:1] - 6 |+[next(iter(x))] -7 7 | list(x)[:1:2] -8 8 | tuple(x)[0] -9 9 | tuple(x)[:1] - -RUF015.py:7:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice +5 5 | tuple(x)[0] +6 |-list(i for i in x)[0] + 6 |+next(iter(x)) +7 7 | [i for i in x][0] +8 8 | +9 9 | # OK (not indexing (solely) the first element) + +RUF015.py:7:1: RUF015 [*] Prefer `next(iter(x))` over single element slice | -5 | list(x)[:1] -6 | list(x)[:1:1] -7 | list(x)[:1:2] - | ^^^^^^^^^^^^^ RUF015 -8 | tuple(x)[0] -9 | tuple(x)[:1] +5 | tuple(x)[0] +6 | list(i for i in x)[0] +7 | [i for i in x][0] + | ^^^^^^^^^^^^^^^^^ RUF015 +8 | +9 | # OK (not indexing (solely) the first element) | - = help: Replace with `[next(iter(x))] + = help: Replace with `next(iter(x))` ℹ Suggested fix 4 4 | list(x)[0] -5 5 | list(x)[:1] -6 6 | list(x)[:1:1] -7 |-list(x)[:1:2] - 7 |+[next(iter(x))] -8 8 | tuple(x)[0] -9 9 | tuple(x)[:1] -10 10 | tuple(x)[:1:1] - -RUF015.py:8:1: RUF015 [*] Prefer `next(iter(x))` over single element slice - | - 6 | list(x)[:1:1] - 7 | list(x)[:1:2] - 8 | tuple(x)[0] - | ^^^^^^^^^^^ RUF015 - 9 | tuple(x)[:1] -10 | tuple(x)[:1:1] - | - = help: Replace with `next(iter(x))` - -ℹ Suggested fix -5 5 | list(x)[:1] -6 6 | list(x)[:1:1] -7 7 | list(x)[:1:2] -8 |-tuple(x)[0] - 8 |+next(iter(x)) -9 9 | tuple(x)[:1] -10 10 | tuple(x)[:1:1] -11 11 | tuple(x)[:1:2] - -RUF015.py:9:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | - 7 | list(x)[:1:2] - 8 | tuple(x)[0] - 9 | tuple(x)[:1] - | ^^^^^^^^^^^^ RUF015 -10 | tuple(x)[:1:1] -11 | tuple(x)[:1:2] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -6 6 | list(x)[:1:1] -7 7 | list(x)[:1:2] -8 8 | tuple(x)[0] -9 |-tuple(x)[:1] - 9 |+[next(iter(x))] -10 10 | tuple(x)[:1:1] -11 11 | tuple(x)[:1:2] -12 12 | list(i for i in x)[0] - -RUF015.py:10:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | - 8 | tuple(x)[0] - 9 | tuple(x)[:1] -10 | tuple(x)[:1:1] - | ^^^^^^^^^^^^^^ RUF015 -11 | tuple(x)[:1:2] -12 | list(i for i in x)[0] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -7 7 | list(x)[:1:2] -8 8 | tuple(x)[0] -9 9 | tuple(x)[:1] -10 |-tuple(x)[:1:1] - 10 |+[next(iter(x))] -11 11 | tuple(x)[:1:2] -12 12 | list(i for i in x)[0] -13 13 | list(i for i in x)[:1] - -RUF015.py:11:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | - 9 | tuple(x)[:1] -10 | tuple(x)[:1:1] -11 | tuple(x)[:1:2] - | ^^^^^^^^^^^^^^ RUF015 -12 | list(i for i in x)[0] -13 | list(i for i in x)[:1] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -8 8 | tuple(x)[0] -9 9 | tuple(x)[:1] -10 10 | tuple(x)[:1:1] -11 |-tuple(x)[:1:2] - 11 |+[next(iter(x))] -12 12 | list(i for i in x)[0] -13 13 | list(i for i in x)[:1] -14 14 | list(i for i in x)[:1:1] - -RUF015.py:12:1: RUF015 [*] Prefer `next(iter(x))` over single element slice - | -10 | tuple(x)[:1:1] -11 | tuple(x)[:1:2] -12 | list(i for i in x)[0] - | ^^^^^^^^^^^^^^^^^^^^^ RUF015 -13 | list(i for i in x)[:1] -14 | list(i for i in x)[:1:1] - | - = help: Replace with `next(iter(x))` - -ℹ Suggested fix -9 9 | tuple(x)[:1] -10 10 | tuple(x)[:1:1] -11 11 | tuple(x)[:1:2] -12 |-list(i for i in x)[0] - 12 |+next(iter(x)) -13 13 | list(i for i in x)[:1] -14 14 | list(i for i in x)[:1:1] -15 15 | list(i for i in x)[:1:2] - -RUF015.py:13:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -11 | tuple(x)[:1:2] -12 | list(i for i in x)[0] -13 | list(i for i in x)[:1] - | ^^^^^^^^^^^^^^^^^^^^^^ RUF015 -14 | list(i for i in x)[:1:1] -15 | list(i for i in x)[:1:2] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -10 10 | tuple(x)[:1:1] -11 11 | tuple(x)[:1:2] -12 12 | list(i for i in x)[0] -13 |-list(i for i in x)[:1] - 13 |+[next(iter(x))] -14 14 | list(i for i in x)[:1:1] -15 15 | list(i for i in x)[:1:2] -16 16 | [i for i in x][0] - -RUF015.py:14:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -12 | list(i for i in x)[0] -13 | list(i for i in x)[:1] -14 | list(i for i in x)[:1:1] - | ^^^^^^^^^^^^^^^^^^^^^^^^ RUF015 -15 | list(i for i in x)[:1:2] -16 | [i for i in x][0] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -11 11 | tuple(x)[:1:2] -12 12 | list(i for i in x)[0] -13 13 | list(i for i in x)[:1] -14 |-list(i for i in x)[:1:1] - 14 |+[next(iter(x))] -15 15 | list(i for i in x)[:1:2] -16 16 | [i for i in x][0] -17 17 | [i for i in x][:1] - -RUF015.py:15:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -13 | list(i for i in x)[:1] -14 | list(i for i in x)[:1:1] -15 | list(i for i in x)[:1:2] - | ^^^^^^^^^^^^^^^^^^^^^^^^ RUF015 -16 | [i for i in x][0] -17 | [i for i in x][:1] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -12 12 | list(i for i in x)[0] -13 13 | list(i for i in x)[:1] -14 14 | list(i for i in x)[:1:1] -15 |-list(i for i in x)[:1:2] - 15 |+[next(iter(x))] -16 16 | [i for i in x][0] -17 17 | [i for i in x][:1] -18 18 | [i for i in x][:1:1] - -RUF015.py:16:1: RUF015 [*] Prefer `next(iter(x))` over single element slice - | -14 | list(i for i in x)[:1:1] -15 | list(i for i in x)[:1:2] -16 | [i for i in x][0] - | ^^^^^^^^^^^^^^^^^ RUF015 -17 | [i for i in x][:1] -18 | [i for i in x][:1:1] - | - = help: Replace with `next(iter(x))` - -ℹ Suggested fix -13 13 | list(i for i in x)[:1] -14 14 | list(i for i in x)[:1:1] -15 15 | list(i for i in x)[:1:2] -16 |-[i for i in x][0] - 16 |+next(iter(x)) -17 17 | [i for i in x][:1] -18 18 | [i for i in x][:1:1] -19 19 | [i for i in x][:1:2] - -RUF015.py:17:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -15 | list(i for i in x)[:1:2] -16 | [i for i in x][0] -17 | [i for i in x][:1] - | ^^^^^^^^^^^^^^^^^^ RUF015 -18 | [i for i in x][:1:1] -19 | [i for i in x][:1:2] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -14 14 | list(i for i in x)[:1:1] -15 15 | list(i for i in x)[:1:2] -16 16 | [i for i in x][0] -17 |-[i for i in x][:1] - 17 |+[next(iter(x))] -18 18 | [i for i in x][:1:1] -19 19 | [i for i in x][:1:2] -20 20 | - -RUF015.py:18:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -16 | [i for i in x][0] -17 | [i for i in x][:1] -18 | [i for i in x][:1:1] - | ^^^^^^^^^^^^^^^^^^^^ RUF015 -19 | [i for i in x][:1:2] - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -15 15 | list(i for i in x)[:1:2] -16 16 | [i for i in x][0] -17 17 | [i for i in x][:1] -18 |-[i for i in x][:1:1] - 18 |+[next(iter(x))] -19 19 | [i for i in x][:1:2] -20 20 | -21 21 | # OK (not indexing (solely) the first element) - -RUF015.py:19:1: RUF015 [*] Prefer `[next(iter(x))]` over single element slice - | -17 | [i for i in x][:1] -18 | [i for i in x][:1:1] -19 | [i for i in x][:1:2] - | ^^^^^^^^^^^^^^^^^^^^ RUF015 -20 | -21 | # OK (not indexing (solely) the first element) - | - = help: Replace with `[next(iter(x))] - -ℹ Suggested fix -16 16 | [i for i in x][0] -17 17 | [i for i in x][:1] -18 18 | [i for i in x][:1:1] -19 |-[i for i in x][:1:2] - 19 |+[next(iter(x))] -20 20 | -21 21 | # OK (not indexing (solely) the first element) -22 22 | list(x) - -RUF015.py:38:1: RUF015 [*] Prefer `next(i + 1 for i in x)` over single element slice - | -37 | # RUF015 (doesn't mirror the underlying list) -38 | [i + 1 for i in x][0] +5 5 | tuple(x)[0] +6 6 | list(i for i in x)[0] +7 |-[i for i in x][0] + 7 |+next(iter(x)) +8 8 | +9 9 | # OK (not indexing (solely) the first element) +10 10 | list(x) + +RUF015.py:29:1: RUF015 [*] Prefer `next(i + 1 for i in x)` over single element slice + | +28 | # RUF015 (doesn't mirror the underlying list) +29 | [i + 1 for i in x][0] | ^^^^^^^^^^^^^^^^^^^^^ RUF015 -39 | [i for i in x if i > 5][0] -40 | [(i, i + 1) for i in x][0] +30 | [i for i in x if i > 5][0] +31 | [(i, i + 1) for i in x][0] | = help: Replace with `next(i + 1 for i in x)` ℹ Suggested fix -35 35 | [i for i in x][::] -36 36 | -37 37 | # RUF015 (doesn't mirror the underlying list) -38 |-[i + 1 for i in x][0] - 38 |+next(i + 1 for i in x) -39 39 | [i for i in x if i > 5][0] -40 40 | [(i, i + 1) for i in x][0] -41 41 | +26 26 | [i for i in x][::] +27 27 | +28 28 | # RUF015 (doesn't mirror the underlying list) +29 |-[i + 1 for i in x][0] + 29 |+next(i + 1 for i in x) +30 30 | [i for i in x if i > 5][0] +31 31 | [(i, i + 1) for i in x][0] +32 32 | -RUF015.py:39:1: RUF015 [*] Prefer `next(i for i in x if i > 5)` over single element slice +RUF015.py:30:1: RUF015 [*] Prefer `next(i for i in x if i > 5)` over single element slice | -37 | # RUF015 (doesn't mirror the underlying list) -38 | [i + 1 for i in x][0] -39 | [i for i in x if i > 5][0] +28 | # RUF015 (doesn't mirror the underlying list) +29 | [i + 1 for i in x][0] +30 | [i for i in x if i > 5][0] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015 -40 | [(i, i + 1) for i in x][0] +31 | [(i, i + 1) for i in x][0] | = help: Replace with `next(i for i in x if i > 5)` ℹ Suggested fix -36 36 | -37 37 | # RUF015 (doesn't mirror the underlying list) -38 38 | [i + 1 for i in x][0] -39 |-[i for i in x if i > 5][0] - 39 |+next(i for i in x if i > 5) -40 40 | [(i, i + 1) for i in x][0] -41 41 | -42 42 | # RUF015 (multiple generators) +27 27 | +28 28 | # RUF015 (doesn't mirror the underlying list) +29 29 | [i + 1 for i in x][0] +30 |-[i for i in x if i > 5][0] + 30 |+next(i for i in x if i > 5) +31 31 | [(i, i + 1) for i in x][0] +32 32 | +33 33 | # RUF015 (multiple generators) -RUF015.py:40:1: RUF015 [*] Prefer `next((i, i + 1) for i in x)` over single element slice +RUF015.py:31:1: RUF015 [*] Prefer `next((i, i + 1) for i in x)` over single element slice | -38 | [i + 1 for i in x][0] -39 | [i for i in x if i > 5][0] -40 | [(i, i + 1) for i in x][0] +29 | [i + 1 for i in x][0] +30 | [i for i in x if i > 5][0] +31 | [(i, i + 1) for i in x][0] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015 -41 | -42 | # RUF015 (multiple generators) +32 | +33 | # RUF015 (multiple generators) | = help: Replace with `next((i, i + 1) for i in x)` ℹ Suggested fix -37 37 | # RUF015 (doesn't mirror the underlying list) -38 38 | [i + 1 for i in x][0] -39 39 | [i for i in x if i > 5][0] -40 |-[(i, i + 1) for i in x][0] - 40 |+next((i, i + 1) for i in x) -41 41 | -42 42 | # RUF015 (multiple generators) -43 43 | y = range(10) +28 28 | # RUF015 (doesn't mirror the underlying list) +29 29 | [i + 1 for i in x][0] +30 30 | [i for i in x if i > 5][0] +31 |-[(i, i + 1) for i in x][0] + 31 |+next((i, i + 1) for i in x) +32 32 | +33 33 | # RUF015 (multiple generators) +34 34 | y = range(10) -RUF015.py:44:1: RUF015 [*] Prefer `next(i + j for i in x for j in y)` over single element slice +RUF015.py:35:1: RUF015 [*] Prefer `next(i + j for i in x for j in y)` over single element slice | -42 | # RUF015 (multiple generators) -43 | y = range(10) -44 | [i + j for i in x for j in y][0] +33 | # RUF015 (multiple generators) +34 | y = range(10) +35 | [i + j for i in x for j in y][0] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015 -45 | -46 | # RUF015 +36 | +37 | # RUF015 | = help: Replace with `next(i + j for i in x for j in y)` ℹ Suggested fix -41 41 | -42 42 | # RUF015 (multiple generators) -43 43 | y = range(10) -44 |-[i + j for i in x for j in y][0] - 44 |+next(i + j for i in x for j in y) -45 45 | -46 46 | # RUF015 -47 47 | list(range(10))[0] +32 32 | +33 33 | # RUF015 (multiple generators) +34 34 | y = range(10) +35 |-[i + j for i in x for j in y][0] + 35 |+next(i + j for i in x for j in y) +36 36 | +37 37 | # RUF015 +38 38 | list(range(10))[0] -RUF015.py:47:1: RUF015 [*] Prefer `next(iter(range(10)))` over single element slice +RUF015.py:38:1: RUF015 [*] Prefer `next(iter(range(10)))` over single element slice | -46 | # RUF015 -47 | list(range(10))[0] +37 | # RUF015 +38 | list(range(10))[0] | ^^^^^^^^^^^^^^^^^^ RUF015 -48 | list(x.y)[0] -49 | list(x["y"])[0] +39 | list(x.y)[0] +40 | list(x["y"])[0] | = help: Replace with `next(iter(range(10)))` ℹ Suggested fix -44 44 | [i + j for i in x for j in y][0] -45 45 | -46 46 | # RUF015 -47 |-list(range(10))[0] - 47 |+next(iter(range(10))) -48 48 | list(x.y)[0] -49 49 | list(x["y"])[0] -50 50 | +35 35 | [i + j for i in x for j in y][0] +36 36 | +37 37 | # RUF015 +38 |-list(range(10))[0] + 38 |+next(iter(range(10))) +39 39 | list(x.y)[0] +40 40 | list(x["y"])[0] +41 41 | -RUF015.py:48:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice +RUF015.py:39:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice | -46 | # RUF015 -47 | list(range(10))[0] -48 | list(x.y)[0] +37 | # RUF015 +38 | list(range(10))[0] +39 | list(x.y)[0] | ^^^^^^^^^^^^ RUF015 -49 | list(x["y"])[0] +40 | list(x["y"])[0] | = help: Replace with `next(iter(x.y))` ℹ Suggested fix -45 45 | -46 46 | # RUF015 -47 47 | list(range(10))[0] -48 |-list(x.y)[0] - 48 |+next(iter(x.y)) -49 49 | list(x["y"])[0] -50 50 | -51 51 | # RUF015 (multi-line) +36 36 | +37 37 | # RUF015 +38 38 | list(range(10))[0] +39 |-list(x.y)[0] + 39 |+next(iter(x.y)) +40 40 | list(x["y"])[0] +41 41 | +42 42 | # RUF015 (multi-line) -RUF015.py:49:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice +RUF015.py:40:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice | -47 | list(range(10))[0] -48 | list(x.y)[0] -49 | list(x["y"])[0] +38 | list(range(10))[0] +39 | list(x.y)[0] +40 | list(x["y"])[0] | ^^^^^^^^^^^^^^^ RUF015 -50 | -51 | # RUF015 (multi-line) +41 | +42 | # RUF015 (multi-line) | = help: Replace with `next(iter(x["y"]))` ℹ Suggested fix -46 46 | # RUF015 -47 47 | list(range(10))[0] -48 48 | list(x.y)[0] -49 |-list(x["y"])[0] - 49 |+next(iter(x["y"])) -50 50 | -51 51 | # RUF015 (multi-line) -52 52 | revision_heads_map_ast = [ +37 37 | # RUF015 +38 38 | list(range(10))[0] +39 39 | list(x.y)[0] +40 |-list(x["y"])[0] + 40 |+next(iter(x["y"])) +41 41 | +42 42 | # RUF015 (multi-line) +43 43 | revision_heads_map_ast = [ -RUF015.py:52:26: RUF015 [*] Prefer `next(...)` over single element slice +RUF015.py:43:26: RUF015 [*] Prefer `next(...)` over single element slice | -51 | # RUF015 (multi-line) -52 | revision_heads_map_ast = [ +42 | # RUF015 (multi-line) +43 | revision_heads_map_ast = [ | __________________________^ -53 | | a -54 | | for a in revision_heads_map_ast_obj.body -55 | | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP" -56 | | ][0] +44 | | a +45 | | for a in revision_heads_map_ast_obj.body +46 | | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP" +47 | | ][0] | |____^ RUF015 | = help: Replace with `next(...)` ℹ Suggested fix -49 49 | list(x["y"])[0] -50 50 | -51 51 | # RUF015 (multi-line) -52 |-revision_heads_map_ast = [ - 52 |+revision_heads_map_ast = next( -53 53 | a -54 54 | for a in revision_heads_map_ast_obj.body -55 55 | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP" -56 |-][0] - 56 |+) +40 40 | list(x["y"])[0] +41 41 | +42 42 | # RUF015 (multi-line) +43 |-revision_heads_map_ast = [ + 43 |+revision_heads_map_ast = next( +44 44 | a +45 45 | for a in revision_heads_map_ast_obj.body +46 46 | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP" +47 |-][0] + 47 |+) From 926562ac5f8f5898776bee305f33b78fc9018b4a Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 27 Jul 2023 17:58:40 -0300 Subject: [PATCH 08/11] Implement PYI047 /// ## What it does /// Checks for the presence of unused private `typing.TypeAlias` definitions. /// /// ## Why is this bad? /// A private `typing.TypeAlias` that is defined but not used is likely a /// mistake, and should either be used, made public, or removed to avoid /// confusion. /// /// ## Example /// ```python /// import typing /// /// _UnusedTypeAlias: typing.TypeAlias = int /// ``` /// /// Use instead: /// ```python /// import typing /// /// _UsedTypeAlias: typing.TypeAlias = int /// /// def func(arg: _UsedTypeAlias) -> _UsedTypeAlias: /// ... /// ``` --- crates/ruff/src/checkers/ast/analyze/bindings.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 80c632308fbea..80d7102404778 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -1,5 +1,3 @@ -use ruff_diagnostics::{Diagnostic, Fix}; - use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint}; From b5a3e85d8be610dc65a5713554649e9573ac2895 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Thu, 27 Jul 2023 18:06:16 -0300 Subject: [PATCH 09/11] fix doc formatting --- .../rules/flake8_pyi/rules/unused_private_type_definition.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index eca55fa14cacb..22e785c4f9a2b 100644 --- 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 @@ -199,7 +199,7 @@ pub(crate) fn unused_private_protocol( /// PYI047 pub(crate) fn unused_private_type_alias( checker: &Checker, - binding: &Binding, + binding: &Scope, ) -> Option { if !(binding.kind.is_assignment() && binding.is_private_declaration()) { return None; From 99c0347d8ce0245a40637f39234afe3ac18c1ac5 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Fri, 28 Jul 2023 16:55:46 -0300 Subject: [PATCH 10/11] Avoid false positives These changes are based on #6142 --- .../test/fixtures/flake8_pyi/PYI047.py | 10 +++ .../test/fixtures/flake8_pyi/PYI047.pyi | 10 +++ .../ruff/src/checkers/ast/analyze/bindings.rs | 2 + .../checkers/ast/analyze/deferred_scopes.rs | 4 ++ .../rules/unused_private_type_definition.rs | 68 ++++++++++--------- ..._flake8_pyi__tests__PYI047_PYI047.pyi.snap | 16 ++--- 6 files changed, 71 insertions(+), 39 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py index 36cf1dc19c74f..91bc2cd413c1b 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.py @@ -1,4 +1,5 @@ import typing +import sys from typing import TypeAlias @@ -10,3 +11,12 @@ def func(arg: _UsedPrivateTypeAlias) -> _UsedPrivateTypeAlias: ... + + +if sys.version_info > (3, 9): + _PrivateTypeAlias: TypeAlias = str | None +else: + _PrivateTypeAlias: TypeAlias = float | None + + +def func2(arg: _PrivateTypeAlias) -> None: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi index 36cf1dc19c74f..91bc2cd413c1b 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI047.pyi @@ -1,4 +1,5 @@ import typing +import sys from typing import TypeAlias @@ -10,3 +11,12 @@ _UsedPrivateTypeAlias: TypeAlias = int | None def func(arg: _UsedPrivateTypeAlias) -> _UsedPrivateTypeAlias: ... + + +if sys.version_info > (3, 9): + _PrivateTypeAlias: TypeAlias = str | None +else: + _PrivateTypeAlias: TypeAlias = float | None + + +def func2(arg: _PrivateTypeAlias) -> None: ... diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 80d7102404778..80c632308fbea 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -1,3 +1,5 @@ +use ruff_diagnostics::{Diagnostic, Fix}; + use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint}; diff --git a/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs index 8fb6363c19bcd..ce1308e186217 100644 --- a/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs @@ -25,6 +25,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::UnusedLambdaArgument, Rule::UnusedMethodArgument, Rule::UnusedPrivateProtocol, + Rule::UnusedPrivateTypeAlias, Rule::UnusedPrivateTypeVar, Rule::UnusedStaticMethodArgument, Rule::UnusedVariable, @@ -223,6 +224,9 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if checker.enabled(Rule::UnusedPrivateProtocol) { flake8_pyi::rules::unused_private_protocol(checker, scope, &mut diagnostics); } + if checker.enabled(Rule::UnusedPrivateTypeAlias) { + flake8_pyi::rules::unused_private_type_alias(checker, scope, &mut diagnostics); + } } if matches!( 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 index 22e785c4f9a2b..79d009bd27e9e 100644 --- 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 @@ -199,37 +199,43 @@ pub(crate) fn unused_private_protocol( /// PYI047 pub(crate) fn unused_private_type_alias( checker: &Checker, - binding: &Scope, -) -> Option { - if !(binding.kind.is_assignment() && binding.is_private_declaration()) { - return None; - } - if binding.is_used() { - return None; - } - - let Some(source) = binding.source else { - return None; - }; - let Stmt::AnnAssign(ast::StmtAnnAssign { target, annotation, .. }) = checker.semantic().stmts[source] - else { - return None - }; - let Some(ast::ExprName { id, .. }) = target.as_name_expr() else { - return None; - }; - - if !checker - .semantic() - .match_typing_expr(annotation, "TypeAlias") + scope: &Scope, + diagnostics: &mut Vec, +) { + for binding in scope + .binding_ids() + .map(|binding_id| checker.semantic().binding(binding_id)) { - return None; - } + if !(binding.kind.is_assignment() && binding.is_private_declaration()) { + continue; + } + if binding.is_used() { + continue; + } - Some(Diagnostic::new( - UnusedPrivateTypeAlias { - name: id.to_string(), - }, - binding.range, - )) + let Some(source) = binding.source else { + continue; + }; + let Stmt::AnnAssign(ast::StmtAnnAssign { target, annotation, .. }) = checker.semantic().stmts[source] + else { + continue + }; + let Some(ast::ExprName { id, .. }) = target.as_name_expr() else { + continue; + }; + + if !checker + .semantic() + .match_typing_expr(annotation, "TypeAlias") + { + continue; + } + + diagnostics.push(Diagnostic::new( + UnusedPrivateTypeAlias { + name: id.to_string(), + }, + binding.range, + )); + } } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap index db805d3b4c942..2d1f0275d7a5e 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI047_PYI047.pyi.snap @@ -1,20 +1,20 @@ --- source: crates/ruff/src/rules/flake8_pyi/mod.rs --- -PYI047.pyi:5:1: PYI047 Private TypeAlias `_UnusedPrivateTypeAlias` is never used +PYI047.pyi:6:1: PYI047 Private TypeAlias `_UnusedPrivateTypeAlias` is never used | -5 | _UnusedPrivateTypeAlias: TypeAlias = int | None +6 | _UnusedPrivateTypeAlias: TypeAlias = int | None | ^^^^^^^^^^^^^^^^^^^^^^^ PYI047 -6 | _T: typing.TypeAlias = str +7 | _T: typing.TypeAlias = str | -PYI047.pyi:6:1: PYI047 Private TypeAlias `_T` is never used +PYI047.pyi:7:1: PYI047 Private TypeAlias `_T` is never used | -5 | _UnusedPrivateTypeAlias: TypeAlias = int | None -6 | _T: typing.TypeAlias = str +6 | _UnusedPrivateTypeAlias: TypeAlias = int | None +7 | _T: typing.TypeAlias = str | ^^ PYI047 -7 | -8 | # OK +8 | +9 | # OK | From b8943f919605439cde0394ecfc3b5d02a517ac4f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 28 Jul 2023 20:07:17 -0400 Subject: [PATCH 11/11] Format, etc. --- .../flake8_pyi/rules/unused_private_type_definition.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 index 79d009bd27e9e..70b6c83127f99 100644 --- 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 @@ -214,11 +214,13 @@ pub(crate) fn unused_private_type_alias( } let Some(source) = binding.source else { - continue; + continue; }; - let Stmt::AnnAssign(ast::StmtAnnAssign { target, annotation, .. }) = checker.semantic().stmts[source] + let Stmt::AnnAssign(ast::StmtAnnAssign { + target, annotation, .. + }) = checker.semantic().stmts[source] else { - continue + continue; }; let Some(ast::ExprName { id, .. }) = target.as_name_expr() else { continue;