From a771c18cf6e9bb11f278211b06a5280ed9c9a9a5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 27 Jul 2023 21:47:07 -0400 Subject: [PATCH] Only run unused private type rules over finalized bindings --- .../ruff/src/checkers/ast/analyze/bindings.rs | 19 +-- .../checkers/ast/analyze/deferred_scopes.rs | 13 +- .../rules/unused_private_type_definition.rs | 131 ++++++++++-------- 3 files changed, 89 insertions(+), 74 deletions(-) diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 1c8804272ebef..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,9 +11,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::InvalidAllObject, Rule::UnaliasedCollectionsAbcSetImport, Rule::UnconventionalImportAlias, - Rule::UnusedPrivateTypeVar, Rule::UnusedVariable, - Rule::UnusedPrivateProtocol, ]) { return; } @@ -65,20 +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); - } - } } } } 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 db0f35a83e62a..2d5e0d3082b37 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; @@ -74,67 +74,86 @@ impl Violation for UnusedPrivateProtocol { } /// 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; - } +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 { - 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; - } + 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; + } - Some(Diagnostic::new( - UnusedPrivateTypeVar { - name: id.to_string(), - }, - binding.range, - )) + diagnostics.push(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; - } +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)) + { + if !(binding.kind.is_class_definition() && binding.is_private_declaration()) { + continue; + } + if binding.is_used() { + continue; + } - let Some(source) = binding.source else { - return None; - }; - let Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) = checker.semantic().stmts[source] - else { - return None; - }; + 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")) - { - return None; - } + if !bases + .iter() + .any(|base| checker.semantic().match_typing_expr(base, "Protocol")) + { + continue; + } - Some(Diagnostic::new( - UnusedPrivateProtocol { - name: name.to_string(), - }, - binding.range, - )) + diagnostics.push(Diagnostic::new( + UnusedPrivateProtocol { + name: name.to_string(), + }, + binding.range, + )); + } }