Only run unused private type rules over finalized bindings (astral-sh…

…#6142)

In astral-sh#6134 and astral-sh#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`

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);
-                }
-            }
         }
     }
 }

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(_)

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<Diagnostic> {
-    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<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 {
+            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<Diagnostic> {
-    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<Diagnostic>,
+) {
+    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