Only run unused private type rules over finalized bindings

astral-sh · Jul 28, 2023 · 507e8a0 · 507e8a0
1 parent 0bc3edf
commit 507e8a0
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 74 deletions.
diff --git 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
@@ -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,17 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
             }
         }
 
+        if checker.is_stub {
+            if scope.kind.is_module() {
+                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
@@ -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<Diagnostic> {
-    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<Diagnostic>,
+) {
+    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<Diagnostic> {
-    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<Diagnostic>,
+) {
+    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,
+        ));
+    }
 }