From 75f22072be6443a8d2969f4f527f0d0be58539ff Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 6 Aug 2024 16:02:34 +0000 Subject: [PATCH] refactor(traverse)!: replace `find_scope` with `ancestor_scopes` returning iterator (#4693) Same as #4692. Replace `find_scope` and `find_scope_by_flags` with `ancestor_scopes ` method which returns an iterator. This does the same thing, but is more idiomatic. --- crates/oxc_transformer/src/react/jsx_self.rs | 13 +++--- crates/oxc_traverse/src/context/mod.rs | 47 +++---------------- crates/oxc_traverse/src/context/scoping.rs | 49 ++------------------ crates/oxc_traverse/src/lib.rs | 2 +- 4 files changed, 17 insertions(+), 94 deletions(-) diff --git a/crates/oxc_transformer/src/react/jsx_self.rs b/crates/oxc_transformer/src/react/jsx_self.rs index b4a59f56602db..8616bfd5a4860 100644 --- a/crates/oxc_transformer/src/react/jsx_self.rs +++ b/crates/oxc_transformer/src/react/jsx_self.rs @@ -1,7 +1,7 @@ use oxc_ast::ast::*; use oxc_diagnostics::OxcDiagnostic; use oxc_span::{Span, SPAN}; -use oxc_traverse::{Ancestor, FinderRet, TraverseCtx}; +use oxc_traverse::{Ancestor, TraverseCtx}; use crate::context::Ctx; @@ -44,13 +44,14 @@ impl<'a> ReactJsxSelf<'a> { #[allow(clippy::unused_self)] fn is_inside_constructor(&self, ctx: &TraverseCtx<'a>) -> bool { - ctx.find_scope_by_flags(|flags| { + for scope_id in ctx.ancestor_scopes() { + let flags = ctx.scopes().get_flags(scope_id); if flags.is_block() || flags.is_arrow() { - return FinderRet::Continue; + continue; } - FinderRet::Found(flags.is_constructor()) - }) - .unwrap_or(false) + return flags.is_constructor(); + } + unreachable!(); // Always hit `Program` and exit before loop ends } fn has_no_super_class(ctx: &TraverseCtx<'a>) -> bool { diff --git a/crates/oxc_traverse/src/context/mod.rs b/crates/oxc_traverse/src/context/mod.rs index 7fbdec45faf85..38ec44824a32d 100644 --- a/crates/oxc_traverse/src/context/mod.rs +++ b/crates/oxc_traverse/src/context/mod.rs @@ -25,7 +25,7 @@ pub use scoping::TraverseScoping; /// Provides ability to: /// * Query parent/ancestor of current node via [`parent`], [`ancestor`], [`ancestors`]. /// * Get scopes tree and symbols table via [`scopes`], [`symbols`], [`scopes_mut`], [`symbols_mut`], -/// [`find_scope`], [`find_scope_by_flags`]. +/// [`ancestor_scopes`]. /// * Create AST nodes via AST builder [`ast`]. /// * Allocate into arena via [`alloc`]. /// @@ -100,8 +100,7 @@ pub use scoping::TraverseScoping; /// [`symbols`]: `TraverseCtx::symbols` /// [`scopes_mut`]: `TraverseCtx::scopes_mut` /// [`symbols_mut`]: `TraverseCtx::symbols_mut` -/// [`find_scope`]: `TraverseCtx::find_scope` -/// [`find_scope_by_flags`]: `TraverseCtx::find_scope_by_flags` +/// [`ancestor_scopes`]: `TraverseCtx::ancestor_scopes` /// [`ast`]: `TraverseCtx::ast` /// [`alloc`]: `TraverseCtx::alloc` pub struct TraverseCtx<'a> { @@ -110,13 +109,6 @@ pub struct TraverseCtx<'a> { pub ast: AstBuilder<'a>, } -/// Return value of closure when using [`TraverseCtx::find_scope`]. -pub enum FinderRet { - Found(T), - Stop, - Continue, -} - // Public methods impl<'a> TraverseCtx<'a> { /// Create new traversal context. @@ -223,38 +215,11 @@ impl<'a> TraverseCtx<'a> { self.scoping.symbols_mut() } - /// Walk up trail of scopes to find a scope. - /// - /// `finder` is called with `ScopeId`. - /// - /// `finder` should return: - /// * `FinderRet::Found(value)` to stop walking and return `Some(value)`. - /// * `FinderRet::Stop` to stop walking and return `None`. - /// * `FinderRet::Continue` to continue walking up. - /// - /// This is a shortcut for `ctx.scoping.find_scope`. - pub fn find_scope(&self, finder: F) -> Option - where - F: Fn(ScopeId) -> FinderRet, - { - self.scoping.find_scope(finder) - } - - /// Walk up trail of scopes to find a scope by checking `ScopeFlags`. - /// - /// `finder` is called with `ScopeFlags`. - /// - /// `finder` should return: - /// * `FinderRet::Found(value)` to stop walking and return `Some(value)`. - /// * `FinderRet::Stop` to stop walking and return `None`. - /// * `FinderRet::Continue` to continue walking up. + /// Get iterator over scopes, starting with current scope and working up. /// - /// This is a shortcut for `ctx.scoping.find_scope_by_flags`. - pub fn find_scope_by_flags(&self, finder: F) -> Option - where - F: Fn(ScopeFlags) -> FinderRet, - { - self.scoping.find_scope_by_flags(finder) + /// This is a shortcut for `ctx.scoping.parent_scopes`. + pub fn ancestor_scopes(&self) -> impl Iterator + '_ { + self.scoping.ancestor_scopes() } /// Create new scope as child of provided scope. diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index b0990e6e2f123..4a4d2ef80d715 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -14,8 +14,6 @@ use oxc_syntax::{ symbol::{SymbolFlags, SymbolId}, }; -use super::FinderRet; - /// Traverse scope context. /// /// Contains the scope tree and symbols table, and provides methods to access them. @@ -70,50 +68,9 @@ impl TraverseScoping { &mut self.symbols } - /// Walk up trail of scopes to find a scope. - /// - /// `finder` is called with `ScopeId`. - /// - /// `finder` should return: - /// * `FinderRet::Found(value)` to stop walking and return `Some(value)`. - /// * `FinderRet::Stop` to stop walking and return `None`. - /// * `FinderRet::Continue` to continue walking up. - pub fn find_scope(&self, finder: F) -> Option - where - F: Fn(ScopeId) -> FinderRet, - { - let mut scope_id = self.current_scope_id; - loop { - match finder(scope_id) { - FinderRet::Found(res) => return Some(res), - FinderRet::Stop => return None, - FinderRet::Continue => {} - } - - if let Some(parent_scope_id) = self.scopes.get_parent_id(scope_id) { - scope_id = parent_scope_id; - } else { - return None; - } - } - } - - /// Walk up trail of scopes to find a scope by checking `ScopeFlags`. - /// - /// `finder` is called with `ScopeFlags`. - /// - /// `finder` should return: - /// * `FinderRet::Found(value)` to stop walking and return `Some(value)`. - /// * `FinderRet::Stop` to stop walking and return `None`. - /// * `FinderRet::Continue` to continue walking up. - pub fn find_scope_by_flags(&self, finder: F) -> Option - where - F: Fn(ScopeFlags) -> FinderRet, - { - self.find_scope(|scope_id| { - let flags = self.scopes.get_flags(scope_id); - finder(flags) - }) + /// Get iterator over scopes, starting with current scope and working up + pub fn ancestor_scopes(&self) -> impl Iterator + '_ { + self.scopes.ancestors(self.current_scope_id) } /// Create new scope as child of provided scope. diff --git a/crates/oxc_traverse/src/lib.rs b/crates/oxc_traverse/src/lib.rs index 743847964e833..5dbd1d12f62d4 100644 --- a/crates/oxc_traverse/src/lib.rs +++ b/crates/oxc_traverse/src/lib.rs @@ -67,7 +67,7 @@ use oxc_semantic::{ScopeTree, SymbolTable}; pub mod ancestor; pub use ancestor::Ancestor; mod context; -pub use context::{FinderRet, TraverseAncestry, TraverseCtx, TraverseScoping}; +pub use context::{TraverseAncestry, TraverseCtx, TraverseScoping}; #[allow(clippy::module_inception)] mod traverse; pub use traverse::Traverse;