refactor(linter/block-scoped-var): avoid iter_all_scope_child_ids by walking references/redeclarations scope ancestors

Author: Dunqing

crates/oxc_linter/src/rules/eslint/block_scoped_var.rs

@@ -5,6 +5,7 @@ use oxc_ast::{
 };
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
+use oxc_semantic::Scoping;
 use oxc_span::{GetSpan, Span};
 use oxc_syntax::{scope::ScopeId, symbol::SymbolId};
 
@@ -135,27 +136,32 @@ impl Rule for BlockScopedVar {
         if !ctx.scoping().scope_flags(scope_id).is_strict_mode() {
             return;
         }
-        // `scope_ids` contains all the scopes that are children of the current scope
-        // we should eliminate all of them
-        let scope_ids = ctx.scoping().iter_all_scope_child_ids(node.scope_id()).collect::<Vec<_>>();
 
         for item in &decl.declarations {
-            run_for_declaration(&item.id, &scope_ids, node, ctx);
+            run_for_declaration(&item.id, node, ctx);
         }
     }
 }
 
 fn run_for_all_references(
     (pattern, name, symbol): (&BindingPattern, &str, &SymbolId),
-    scope_ids: &[ScopeId],
     node: &AstNode,
     ctx: &LintContext,
 ) {
-    ctx.scoping()
-        .get_resolved_references(*symbol)
+    let mut references = ctx.scoping().get_resolved_references(*symbol).peekable();
+    if references.peek().is_none() {
+        return;
+    }
+
+    let declare_scope_id = node.scope_id();
+    references
         .filter(|reference| {
             let reference_scope_id = reference.scope_id();
-            reference_scope_id != node.scope_id() && !scope_ids.contains(&reference_scope_id)
+            check_if_has_reference_outside_scope(
+                declare_scope_id,
+                reference_scope_id,
+                ctx.scoping(),
+            )
         })
         .for_each(|reference| {
             ctx.diagnostic(use_outside_scope_diagnostic(
@@ -168,29 +174,32 @@ fn run_for_all_references(
 
 fn run_for_all_redeclarations(
     (pattern, name, symbol): (&BindingPattern, &str, &SymbolId),
-    scope_ids: &[ScopeId],
     node: &AstNode,
     ctx: &LintContext,
 ) {
-    ctx.scoping()
-        .symbol_redeclarations(*symbol)
+    let redeclarations = ctx.scoping().symbol_redeclarations(*symbol);
+    if redeclarations.is_empty() {
+        return;
+    }
+
+    let declare_scope_id = node.scope_id();
+
+    redeclarations
         .iter()
         .filter(|redeclaration| {
             let redeclare_scope_id = ctx.nodes().get_node(redeclaration.declaration).scope_id();
-
-            redeclare_scope_id != node.scope_id() && !scope_ids.contains(&redeclare_scope_id)
+            check_if_has_reference_outside_scope(
+                declare_scope_id,
+                redeclare_scope_id,
+                ctx.scoping(),
+            )
         })
         .for_each(|redeclaration| {
             ctx.diagnostic(redeclaration_diagnostic(pattern.span(), redeclaration.span, name));
         });
 }
 
-fn run_for_declaration(
-    pattern: &BindingPattern,
-    scope_ids: &[ScopeId],
-    node: &AstNode,
-    ctx: &LintContext,
-) {
+fn run_for_declaration(pattern: &BindingPattern, node: &AstNode, ctx: &LintContext) {
     // e.g. "var [a, b] = [1, 2]"
     for ident in pattern.get_binding_identifiers() {
         let name = ident.name.as_str();
@@ -203,13 +212,31 @@ fn run_for_declaration(
         // e.g. "if (true) { var a = 4; } else { var a = 4; }"
         // in this case we can't find the reference of 'a' by call `get_resolved_references`
         // so I use `symbol_redeclarations` to find all the redeclarations
-        run_for_all_redeclarations(binding, scope_ids, node, ctx);
+        run_for_all_redeclarations(binding, node, ctx);
 
         // e.g. "var a = 4; console.log(a);"
-        run_for_all_references(binding, scope_ids, node, ctx);
+        run_for_all_references(binding, node, ctx);
     }
 }
 
+/// Returns true if the reference is outside the declaration scope
+fn check_if_has_reference_outside_scope(
+    declare_scope_id: ScopeId,
+    reference_scope_id: ScopeId,
+    scoping: &Scoping,
+) -> bool {
+    // Walk up the scope chain from the reference scope to see if we reach the declaration scope,
+    // if we do, then the reference is inside the scope, otherwise it's outside
+    for ancestor_scope_id in scoping.scope_ancestors(reference_scope_id) {
+        // Already reached the declaration scope, so the reference is inside the scope
+        if ancestor_scope_id == declare_scope_id {
+            return false;
+        }
+    }
+
+    true
+}
+
 #[test]
 fn test() {
     use crate::tester::Tester;