Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(traverse)!: replace find_scope with ancestor_scopes returning iterator #4693

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions crates/oxc_transformer/src/react/jsx_self.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
47 changes: 6 additions & 41 deletions crates/oxc_traverse/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
///
Expand Down Expand Up @@ -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> {
Expand All @@ -110,13 +109,6 @@ pub struct TraverseCtx<'a> {
pub ast: AstBuilder<'a>,
}

/// Return value of closure when using [`TraverseCtx::find_scope`].
pub enum FinderRet<T> {
Found(T),
Stop,
Continue,
}

// Public methods
impl<'a> TraverseCtx<'a> {
/// Create new traversal context.
Expand Down Expand Up @@ -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<F, O>(&self, finder: F) -> Option<O>
where
F: Fn(ScopeId) -> FinderRet<O>,
{
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<F, O>(&self, finder: F) -> Option<O>
where
F: Fn(ScopeFlags) -> FinderRet<O>,
{
self.scoping.find_scope_by_flags(finder)
/// This is a shortcut for `ctx.scoping.parent_scopes`.
pub fn ancestor_scopes(&self) -> impl Iterator<Item = ScopeId> + '_ {
self.scoping.ancestor_scopes()
}

/// Create new scope as child of provided scope.
Expand Down
49 changes: 3 additions & 46 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<F, O>(&self, finder: F) -> Option<O>
where
F: Fn(ScopeId) -> FinderRet<O>,
{
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<F, O>(&self, finder: F) -> Option<O>
where
F: Fn(ScopeFlags) -> FinderRet<O>,
{
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<Item = ScopeId> + '_ {
self.scopes.ancestors(self.current_scope_id)
}

/// Create new scope as child of provided scope.
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_traverse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down