feat(semantic): support scope descendents starting from a certain scope. (#1629)

@Boshen 

The `ScopeTree.descendants` function would return all scopes starting
from the root, and wasn't truly descendants from a specific scope. To
improve this, I've renamed this function to `descendants_from_root` and
have introduced a new `descendants` function that does support from a
specific scope.

Furthermore, I've introduced helper functions to `SymbolTree` to make
reading symbols/scopes easier.

To verify this functionality, I enabled the `function_name` transformer
(and fixed it), and ran some example transforms. Here's the input:

```js
let fn;

fn = function (a, b, c) {
  const d = "";
};

const func = function (arg) {
  {
    const value = "";
  }
};

const f = function (f) {};
```

And the output using _the old implementation_. Note that all function
names are suffixed with a number, this is incorrect, since it was
inheriting far too many scopes.

```js
let fn;
fn = function fn1(a, b, c) {
	const d = '';
};
const func = function func1(arg) {
	{
		const value = '';
	}
};
const f = function f2(f) {
};
```

And here's the output with the new implementation. Note that only `f` is
suffixed with a number. That's because it has a shadowed argument of the
same name.

```js
let fn;
fn = function fn(a, b, c) {
	const d = '';
};
const func = function func(arg) {
	{
		const value = '';
	}
};
const f = function f1(f) {
};
```

crates/oxc_minifier/src/mangler/mod.rs

@@ -86,7 +86,7 @@ impl ManglerBuilder {
         let mut max_slot_for_scope = vec![0; scope_tree.len()];
 
         // Walk the scope tree and compute the slot number for each scope
-        for scope_id in scope_tree.descendants() {
+        for scope_id in scope_tree.descendants_from_root() {
             let bindings = scope_tree.get_bindings(scope_id);
             // The current slot number is continued by the maximum slot from the parent scope
             let parent_max_slot = scope_tree

crates/oxc_semantic/src/scope.rs

@@ -19,7 +19,12 @@ type UnresolvedReferences = FxHashMap<Atom, Vec<ReferenceId>>;
 /// `SoA` (Struct of Arrays) for memory efficiency.
 #[derive(Debug, Default)]
 pub struct ScopeTree {
+    /// Maps a scope to the parent scope it belongs in
     parent_ids: IndexVec<ScopeId, Option<ScopeId>>,
+
+    /// Maps a scope to direct children scopes
+    child_ids: FxHashMap<ScopeId, Vec<ScopeId>>,
+
     flags: IndexVec<ScopeId, ScopeFlags>,
     bindings: IndexVec<ScopeId, Bindings>,
     unresolved_references: IndexVec<ScopeId, UnresolvedReferences>,
@@ -38,7 +43,30 @@ impl ScopeTree {
         std::iter::successors(Some(scope_id), |scope_id| self.parent_ids[*scope_id])
     }
 
-    pub fn descendants(&self) -> impl Iterator<Item = ScopeId> + '_ {
+    pub fn descendants(&self, scope_id: ScopeId) -> impl Iterator<Item = ScopeId> + '_ {
+        // Has to be a `fn` and pass arguments because we can't
+        // have recursive closures
+        fn add_to_list(
+            parent_id: ScopeId,
+            child_ids: &FxHashMap<ScopeId, Vec<ScopeId>>,
+            items: &mut Vec<ScopeId>,
+        ) {
+            if let Some(children) = child_ids.get(&parent_id) {
+                for child_id in children {
+                    items.push(*child_id);
+                    add_to_list(*child_id, child_ids, items);
+                }
+            }
+        }
+
+        let mut list = vec![];
+
+        add_to_list(scope_id, &self.child_ids, &mut list);
+
+        list.into_iter()
+    }
+
+    pub fn descendants_from_root(&self) -> impl Iterator<Item = ScopeId> + '_ {
         self.parent_ids.iter_enumerated().map(|(scope_id, _)| scope_id)
     }
 
@@ -98,6 +126,11 @@ impl ScopeTree {
         _ = self.flags.push(flags);
         _ = self.bindings.push(Bindings::default());
         _ = self.unresolved_references.push(UnresolvedReferences::default());
+
+        if let Some(parent_id) = parent_id {
+            self.child_ids.entry(parent_id).or_default().push(scope_id);
+        }
+
         scope_id
     }
 

crates/oxc_semantic/src/symbol.rs

@@ -39,6 +39,18 @@ impl SymbolTable {
         self.spans.iter_enumerated().map(|(symbol_id, _)| symbol_id)
     }
 
+    pub fn get_symbol_id_from_span(&self, span: &Span) -> Option<SymbolId> {
+        self.spans
+            .iter_enumerated()
+            .find_map(|(symbol, inner_span)| if inner_span == span { Some(symbol) } else { None })
+    }
+
+    pub fn get_symbol_id_from_name(&self, name: &Atom) -> Option<SymbolId> {
+        self.names
+            .iter_enumerated()
+            .find_map(|(symbol, inner_name)| if inner_name == name { Some(symbol) } else { None })
+    }
+
     pub fn get_span(&self, symbol_id: SymbolId) -> Span {
         self.spans[symbol_id]
     }
@@ -63,6 +75,14 @@ impl SymbolTable {
         self.scope_ids[symbol_id]
     }
 
+    pub fn get_scope_id_from_span(&self, span: &Span) -> Option<ScopeId> {
+        self.get_symbol_id_from_span(span).map(|symbol_id| self.get_scope_id(symbol_id))
+    }
+
+    pub fn get_scope_id_from_name(&self, name: &Atom) -> Option<ScopeId> {
+        self.get_symbol_id_from_name(name).map(|symbol_id| self.get_scope_id(symbol_id))
+    }
+
     pub fn get_declaration(&self, symbol_id: SymbolId) -> AstNodeId {
         self.declarations[symbol_id]
     }

crates/oxc_transformer/examples/transformer.rs

@@ -21,6 +21,7 @@ fn main() {
     let source_text = std::fs::read_to_string(path).expect("{name} not found");
     let allocator = Allocator::default();
     let source_type = SourceType::from_path(path).unwrap();
+
     let ret = Parser::new(&allocator, &source_text, source_type).parse();
 
     if !ret.errors.is_empty() {
@@ -31,10 +32,8 @@ fn main() {
         return;
     }
 
-    let codegen_options = CodegenOptions;
-    let printed = Codegen::<false>::new(source_text.len(), codegen_options).build(&ret.program);
     println!("Original:\n");
-    println!("{printed}\n");
+    println!("{source_text}\n");
 
     let semantic = SemanticBuilder::new(&source_text, source_type)
         .with_trivias(ret.trivias)
@@ -43,7 +42,7 @@ fn main() {
 
     let program = allocator.alloc(ret.program);
     let transform_options = TransformOptions {
-        target: TransformTarget::ES2015,
+        target: TransformTarget::ES5,
         react_jsx: Some(ReactJsxOptions {
             runtime: Some(ReactJsxRuntimeOption::Valid(ReactJsxRuntime::Classic)),
             ..ReactJsxOptions::default()
@@ -52,7 +51,7 @@ fn main() {
     };
     Transformer::new(&allocator, source_type, semantic, transform_options).build(program).unwrap();
 
-    let printed = Codegen::<false>::new(source_text.len(), codegen_options).build(program);
+    let printed = Codegen::<false>::new(source_text.len(), CodegenOptions).build(program);
     println!("Transformed:\n");
     println!("{printed}");
 }

crates/oxc_transformer/src/es2015/function_name.rs

@@ -1,7 +1,8 @@
 use std::rc::Rc;
 
 // use lazy_static::lazy_static;
-use oxc_ast::{ast::*, AstBuilder, Visit};
+use oxc_ast::{ast::*, AstBuilder};
+use oxc_semantic::ScopeId;
 use oxc_span::{Atom, Span};
 use oxc_syntax::operator::AssignmentOperator;
 use oxc_syntax::unicode_id_start::is_id_continue;
@@ -10,6 +11,7 @@ use oxc_syntax::unicode_id_start::is_id_continue;
 use crate::context::TransformerCtx;
 use crate::options::TransformOptions;
 use crate::utils::is_valid_identifier;
+use crate::TransformTarget;
 
 /// ES2015: Function Name
 ///
@@ -24,18 +26,16 @@ pub struct FunctionName<'a> {
 
 impl<'a> FunctionName<'a> {
     pub fn new(
-        _ast: &Rc<AstBuilder<'a>>,
+        ast: Rc<AstBuilder<'a>>,
-        _ctx: &TransformerCtx<'a>,
+        ctx: TransformerCtx<'a>,
-        _options: &TransformOptions,
+        options: &TransformOptions,
     ) -> Option<Self> {
-        // Disabled for now
+        (options.target < TransformTarget::ES2015 || options.function_name).then(|| Self {
-        None
+            _ast: ast,
-        // (options.target < TransformTarget::ES2015 || options.function_name).then(|| Self {
+            ctx,
-        //     ast,
+            // TODO hook up the plugin
-        //     ctx,
+            unicode_escapes: true,
-        //     // TODO hook up the plugin
+        })
-        //     unicode_escapes: true,
-        // })
     }
 
     pub fn transform_assignment_expression(&mut self, expr: &mut AssignmentExpression<'a>) {
@@ -47,7 +47,9 @@ impl<'a> FunctionName<'a> {
                 if let Some(id) =
                     create_valid_identifier(target.span, target.name.clone(), self.unicode_escapes)
                 {
-                    self.transform_expression(&mut expr.right, id);
+                    let scope_id = self.ctx.symbols().get_scope_id_from_span(&target.span);
+
+                    self.transform_expression(&mut expr.right, id, scope_id);
                 }
             }
         }
@@ -60,22 +62,28 @@ impl<'a> FunctionName<'a> {
                     continue;
                 }
 
-                let id = match &property.key {
+                let (id, scope_id) = match &property.key {
-                    PropertyKey::Identifier(ident) => create_valid_identifier(
+                    PropertyKey::Identifier(ident) => (
-                        ident.span,
+                        create_valid_identifier(
-                        ident.name.clone(),
+                            ident.span,
-                        self.unicode_escapes,
+                            ident.name.clone(),
+                            self.unicode_escapes,
+                        ),
+                        self.ctx.symbols().get_scope_id_from_span(&ident.span),
                     ),
-                    PropertyKey::PrivateIdentifier(ident) => create_valid_identifier(
+                    PropertyKey::PrivateIdentifier(ident) => (
-                        ident.span,
+                        create_valid_identifier(
-                        ident.name.clone(),
+                            ident.span,
-                        self.unicode_escapes,
+                            ident.name.clone(),
+                            self.unicode_escapes,
+                        ),
+                        self.ctx.symbols().get_scope_id_from_span(&ident.span),
                     ),
                     PropertyKey::Expression(_) => continue,
                 };
 
                 if let Some(id) = id {
-                    self.transform_expression(&mut property.value, id);
+                    self.transform_expression(&mut property.value, id, scope_id);
                 }
             }
         }
@@ -89,26 +97,36 @@ impl<'a> FunctionName<'a> {
             if let Some(id) =
                 create_valid_identifier(ident.span, ident.name.clone(), self.unicode_escapes)
             {
-                self.transform_expression(init, id);
+                let scope_id = match ident.symbol_id.get() {
+                    Some(symbol_id) => Some(self.ctx.symbols().get_scope_id(symbol_id)),
+                    None => self.ctx.symbols().get_scope_id_from_span(&ident.span),
+                };
+
+                self.transform_expression(init, id, scope_id);
             }
         };
     }
 
     // Internal only
-    fn transform_expression(&mut self, expr: &mut Expression<'a>, mut id: BindingIdentifier) {
+    fn transform_expression(
+        &mut self,
+        expr: &mut Expression<'a>,
+        mut id: BindingIdentifier,
+        scope_id: Option<ScopeId>,
+    ) {
         // function () {} -> function name() {}
         if let Expression::FunctionExpression(func) = expr {
-            let scopes = self.ctx.scopes();
             let mut count = 0;
 
-            // let mut finder = IdentFinder { id, found: 0 };
-            // finder.visit_expression(expr);
-
             // Check for nested params/vars of the same name
-            for scope in scopes.descendants() {
+            if let Some(scope_id) = scope_id {
-                for binding in scopes.get_bindings(scope) {
+                let scopes = self.ctx.scopes();
-                    if binding.0 == &id.name {
+
-                        count += 1;
+                for scope in scopes.descendants(scope_id) {
+                    for binding in scopes.get_bindings(scope) {
+                        if binding.0 == &id.name {
+                            count += 1;
+                        }
                     }
                 }
             }
@@ -125,19 +143,6 @@ impl<'a> FunctionName<'a> {
     }
 }
 
-struct IdentFinder {
-    id: BindingIdentifier,
-    found: usize,
-}
-
-impl<'a> Visit<'a> for IdentFinder {
-    fn visit_binding_identifier(&mut self, ident: &BindingIdentifier) {
-        if ident.name == self.id.name {
-            self.found += 1;
-        }
-    }
-}
-
 // https://github.com/babel/babel/blob/main/packages/babel-helper-function-name/src/index.ts
 // https://github.com/babel/babel/blob/main/packages/babel-types/src/converters/toBindingIdentifierName.ts#L3
 // https://github.com/babel/babel/blob/main/packages/babel-types/src/converters/toIdentifier.ts#L4

crates/oxc_transformer/src/lib.rs

@@ -98,7 +98,7 @@ impl<'a> Transformer<'a> {
             // es2016
             es2016_exponentiation_operator: ExponentiationOperator::new(Rc::clone(&ast), ctx.clone(), &options),
             // es2015
-            es2015_function_name: FunctionName::new(&ast, &ctx.clone(), &options),
+            es2015_function_name: FunctionName::new(Rc::clone(&ast), ctx.clone(), &options),
             es2015_shorthand_properties: ShorthandProperties::new(Rc::clone(&ast), &options),
             es2015_template_literals: TemplateLiterals::new(Rc::clone(&ast), &options),
             // other