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)!: TraverseCtx::ancestor return Ancestor::None if out of bounds #5286

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
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ impl<'a> Traverse<'a> for NullishCoalescingOperator<'a> {
// ctx.ancestor(1) is AssignmentPattern
// ctx.ancestor(2) is BindingPattern;
// ctx.ancestor(3) is FormalParameter
let is_parent_formal_parameter = ctx
.ancestor(3)
.is_some_and(|ancestor| matches!(ancestor, Ancestor::FormalParameterPattern(_)));
let is_parent_formal_parameter =
matches!(ctx.ancestor(3), Ancestor::FormalParameterPattern(_));

let current_scope_id = if is_parent_formal_parameter {
ctx.create_child_scope_of_current(ScopeFlags::Arrow | ScopeFlags::Function)
Expand Down
14 changes: 11 additions & 3 deletions crates/oxc_traverse/src/context/ancestry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,22 @@ impl<'a> TraverseAncestry<'a> {
///
/// `level` is number of levels above.
/// `ancestor(1).unwrap()` is equivalent to `parent()`.
///
/// If `level` is out of bounds (above `Program`), returns `Ancestor::None`.
#[inline]
pub fn ancestor<'t>(&'t self, level: usize) -> Option<Ancestor<'a, 't>> {
self.stack.get(self.stack.len() - level).map(|&ancestor| {
pub fn ancestor<'t>(&'t self, level: usize) -> Ancestor<'a, 't> {
if level < self.stack.len() {
// SAFETY: We just checked that `level < self.stack.len()` so `self.stack.len() - level`
// cannot wrap around or be out of bounds
let ancestor = unsafe { *self.stack.get_unchecked(self.stack.len() - level) };

// Shrink `Ancestor`'s `'t` lifetime to lifetime of `&'t self`.
// SAFETY: The `Ancestor` is guaranteed valid for `'t`. It is not possible to obtain
// a `&mut` ref to any AST node which this `Ancestor` gives access to during `'t`.
unsafe { transmute::<Ancestor<'a, '_>, Ancestor<'a, 't>>(ancestor) }
})
} else {
Ancestor::None
}
}

/// Get iterator over ancestors, starting with closest ancestor
Expand Down
4 changes: 3 additions & 1 deletion crates/oxc_traverse/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ impl<'a> TraverseCtx<'a> {
/// `level` is number of levels above.
/// `ancestor(1).unwrap()` is equivalent to `parent()`.
///
/// If `level` is out of bounds (above `Program`), returns `Ancestor::None`.
///
/// Shortcut for `ctx.ancestry.ancestor`.
#[inline]
pub fn ancestor<'t>(&'t self, level: usize) -> Option<Ancestor<'a, 't>> {
pub fn ancestor<'t>(&'t self, level: usize) -> Ancestor<'a, 't> {
self.ancestry.ancestor(level)
}

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 @@ -134,7 +134,7 @@ mod compile_fail_tests;
/// }
///
/// // Read grandparent
/// if let Some(Ancestor::ExpressionStatementExpression(stmt_ref)) = ctx.ancestor(2) {
/// if let Ancestor::ExpressionStatementExpression(stmt_ref) = ctx.ancestor(2) {
/// // This is legal
/// println!("expression stmt's span: {:?}", stmt_ref.span());
///
Expand Down