Skip to content

refactor(formatter): use AstNode::parent method instead of accessing the parent field directly #18427

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.

Sign up for GitHub

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

Merged
graphite-app merged 1 commit into from
Jan 23, 2026
Merged

refactor(formatter): use AstNode::parent method instead of accessing the parent field directly #18427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 15 additions & 9 deletions crates/oxc_formatter/src/ast_nodes/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use super::AstNodes;

pub struct AstNode<'a, T> {
pub(super) inner: &'a T,
pub parent: &'a AstNodes<'a>,
pub(super) parent: &'a AstNodes<'a>,
pub(super) allocator: &'a Allocator,
/// The start position of the following sibling node, or 0 if none.
pub(super) following_span_start: u32,
Expand All @@ -22,7 +22,7 @@ impl<T: fmt::Debug> fmt::Debug for AstNode<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("AstNode")
.field("inner", &self.inner)
.field("parent", &self.parent.debug_name())
.field("parent", &self.parent().debug_name())
.field("following_span_start", &self.following_span_start)
.finish()
}
Expand Down Expand Up @@ -73,7 +73,7 @@ impl<'a, T> AstNode<'a, T> {
/// The iteration includes the current node and proceeds upward through the tree,
/// terminating after yielding the root `Program` node.
///
/// This is a convenience method that delegates to `self.parent.ancestors()`.
/// This is a convenience method that delegates to `self.parent().ancestors()`.
///
/// # Example
/// ```text
Expand All @@ -98,14 +98,20 @@ impl<'a, T> AstNode<'a, T> {
/// .any(|p| matches!(p, AstNodes::ArrowFunctionExpression(_)));
/// ```
pub fn ancestors(&self) -> impl Iterator<Item = &AstNodes<'a>> {
self.parent.ancestors()
self.parent().ancestors()
}

/// Returns the parent node.
#[inline]
pub fn parent(&self) -> &'a AstNodes<'a> {
self.parent
}

/// Returns the grandparent node (parent's parent).
///
/// This is a convenience method equivalent to `self.parent.parent()`.
/// This is a convenience method equivalent to `self.parent().parent()`.
pub fn grand_parent(&self) -> &AstNodes<'a> {
self.parent.parent()
self.parent().parent()
}
}

Expand All @@ -118,7 +124,7 @@ impl<'a> AstNode<'a, Program<'a>> {
impl<T: GetSpan> AstNode<'_, T> {
/// Check if this node is the callee of a CallExpression or NewExpression
pub fn is_call_like_callee(&self) -> bool {
let callee = match self.parent {
let callee = match self.parent() {
AstNodes::CallExpression(call) => &call.callee,
AstNodes::NewExpression(new) => &new.callee,
_ => return false,
Expand All @@ -129,7 +135,7 @@ impl<T: GetSpan> AstNode<'_, T> {

/// Check if this node is the callee of a NewExpression
pub fn is_new_callee(&self) -> bool {
matches!(self.parent, AstNodes::NewExpression(new) if new.callee.span() == self.span())
matches!(self.parent(), AstNodes::NewExpression(new) if new.callee.span() == self.span())
}
}

Expand All @@ -143,7 +149,7 @@ impl<'a> AstNode<'a, ExpressionStatement<'a>> {
/// `() => { return expression; }`
/// ^^^^^^^^^^^^^^^^^^^^ This ExpressionStatement is NOT the body of an arrow function
pub fn is_arrow_function_body(&self) -> bool {
matches!(self.parent.parent(), AstNodes::ArrowFunctionExpression(arrow) if arrow.expression)
matches!(self.parent().parent(), AstNodes::ArrowFunctionExpression(arrow) if arrow.expression)
}
}

Expand Down
74 changes: 37 additions & 37 deletions crates/oxc_formatter/src/parentheses/expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@ impl NeedsParentheses<'_> for AstNode<'_, IdentifierReference<'_>> {

match self.name.as_str() {
"async" => {
matches!(self.parent, AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span))
matches!(self.parent(), AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span))
}
"let" => {
// `let[a]` at statement start looks like a lexical declaration, needs parens
// Only applies when `let` is the object of a computed member expression
if !matches!(self.parent, AstNodes::ComputedMemberExpression(m) if m.object.span() == self.span())
if !matches!(self.parent(), AstNodes::ComputedMemberExpression(m) if m.object.span() == self.span())
{
// Not `let[...]` - check special cases only
return self.ancestors().any(|parent| match parent {
AstNodes::ForOfStatement(s) => s.left.span().contains_inclusive(self.span),
AstNodes::ForInStatement(s) => {
s.left.span().contains_inclusive(self.span)
&& !matches!(self.parent, AstNodes::StaticMemberExpression(_))
&& !matches!(self.parent(), AstNodes::StaticMemberExpression(_))
}
AstNodes::TSSatisfiesExpression(e) => e.expression.span() == self.span(),
_ => false,
Expand Down Expand Up @@ -142,7 +142,7 @@ impl NeedsParentheses<'_> for AstNode<'_, IdentifierReference<'_>> {
return false;
}

let mut parent = self.parent;
let mut parent = self.parent();
while matches!(
parent,
AstNodes::TSSatisfiesExpression(_) | AstNodes::TSAsExpression(_)
Expand All @@ -151,7 +151,7 @@ impl NeedsParentheses<'_> for AstNode<'_, IdentifierReference<'_>> {
}

// Early return if the parent isn't a `TSSatisfiesExpression` or `TSAsExpression`
if ptr::eq(self.parent, parent) {
if ptr::eq(self.parent(), parent) {
return false;
}

Expand Down Expand Up @@ -219,7 +219,7 @@ impl NeedsParentheses<'_> for AstNode<'_, NumericLiteral<'_>> {
return false;
}

if let AstNodes::StaticMemberExpression(member) = self.parent {
if let AstNodes::StaticMemberExpression(member) = self.parent() {
return member.object.span() == self.span();
}
false
Expand All @@ -232,7 +232,7 @@ impl NeedsParentheses<'_> for AstNode<'_, StringLiteral<'_>> {
return false;
}

if let AstNodes::ExpressionStatement(stmt) = self.parent {
if let AstNodes::ExpressionStatement(stmt) = self.parent() {
// `() => "foo"`
!stmt.is_arrow_function_body()
} else {
Expand Down Expand Up @@ -261,7 +261,7 @@ impl NeedsParentheses<'_> for AstNode<'_, ObjectExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
is_class_extends(self.span, parent)
|| is_first_in_statement(
self.span,
Expand Down Expand Up @@ -314,7 +314,7 @@ impl NeedsParentheses<'_> for AstNode<'_, CallExpression<'_>> {
return false;
}

match self.parent {
match self.parent() {
AstNodes::ExportDefaultDeclaration(_) => {
let callee = &self.callee();
let callee_span = callee.span();
Expand All @@ -338,7 +338,7 @@ impl NeedsParentheses<'_> for AstNode<'_, NewExpression<'_>> {
return false;
}

is_class_extends(self.span, self.parent)
is_class_extends(self.span, self.parent())
}
}

Expand All @@ -348,7 +348,7 @@ impl NeedsParentheses<'_> for AstNode<'_, UpdateExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
if self.prefix()
&& let AstNodes::UnaryExpression(unary) = parent
{
Expand All @@ -369,7 +369,7 @@ impl NeedsParentheses<'_> for AstNode<'_, UnaryExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
match parent {
AstNodes::UnaryExpression(parent_unary) => {
let parent_operator = parent_unary.operator();
Expand Down Expand Up @@ -413,7 +413,7 @@ fn is_in_for_initializer(expr: &AstNode<'_, BinaryExpression<'_>>) -> bool {
continue;
}
// Block body: `() => { expr; }` - check if we're inside a FunctionBody
if matches!(stmt.parent, AstNodes::FunctionBody(_)) {
if matches!(stmt.parent(), AstNodes::FunctionBody(_)) {
continue;
}

Expand Down Expand Up @@ -446,8 +446,8 @@ impl NeedsParentheses<'_> for AstNode<'_, PrivateInExpression<'_>> {
return false;
}

is_class_extends(self.span, self.parent)
|| matches!(self.parent, AstNodes::UnaryExpression(_))
is_class_extends(self.span, self.parent())
|| matches!(self.parent(), AstNodes::UnaryExpression(_))
}
}

Expand All @@ -457,7 +457,7 @@ impl NeedsParentheses<'_> for AstNode<'_, LogicalExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
if let AstNodes::LogicalExpression(parent) = parent {
parent.operator() != self.operator()
} else if self.operator().is_coalesce()
Expand All @@ -476,7 +476,7 @@ impl NeedsParentheses<'_> for AstNode<'_, ConditionalExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
if matches!(
parent,
AstNodes::UnaryExpression(_)
Expand Down Expand Up @@ -509,7 +509,7 @@ impl NeedsParentheses<'_> for AstNode<'_, Function<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
matches!(parent, AstNodes::TaggedTemplateExpression(_))
|| self.is_call_like_callee()
|| is_first_in_statement(
Expand All @@ -526,7 +526,7 @@ impl NeedsParentheses<'_> for AstNode<'_, AssignmentExpression<'_>> {
return false;
}

match self.parent {
match self.parent() {
// Expression statements, only object destructuring needs parens:
// - `a = b` = no parens
// - `{ x } = obj` -> `({ x } = obj)` = needed to prevent parsing as block statement
Expand All @@ -540,7 +540,7 @@ impl NeedsParentheses<'_> for AstNode<'_, AssignmentExpression<'_>> {
matches!(self.left, AssignmentTarget::ObjectAssignmentTarget(_))
&& is_first_in_statement(
self.span,
self.parent,
self.parent(),
FirstInStatementMode::ExpressionStatementOrArrow,
)
}
Expand Down Expand Up @@ -605,7 +605,7 @@ impl NeedsParentheses<'_> for AstNode<'_, SequenceExpression<'_>> {
return false;
}

match self.parent {
match self.parent() {
AstNodes::ReturnStatement(_)
| AstNodes::ThrowStatement(_)
// There's a precedence for writing `x++, y++`
Expand All @@ -623,7 +623,7 @@ impl NeedsParentheses<'_> for AstNode<'_, AwaitExpression<'_>> {
return false;
}

await_or_yield_needs_parens(self.span(), self.parent)
await_or_yield_needs_parens(self.span(), self.parent())
}
}

Expand All @@ -640,7 +640,7 @@ impl NeedsParentheses<'_> for AstNode<'_, ChainExpression<'_>> {
}

// Check if chain expression needs parens based on how it's being accessed
chain_expression_needs_parens(self.span, self.parent)
chain_expression_needs_parens(self.span, self.parent())
}
}

Expand Down Expand Up @@ -681,10 +681,10 @@ impl NeedsParentheses<'_> for AstNode<'_, Class<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
matches!(parent, AstNodes::TaggedTemplateExpression(_))
|| self.is_call_like_callee()
|| (is_class_extends(self.span, self.parent) && !self.decorators.is_empty())
|| (is_class_extends(self.span, self.parent()) && !self.decorators.is_empty())
|| is_first_in_statement(
self.span,
parent,
Expand All @@ -705,7 +705,7 @@ impl NeedsParentheses<'_> for AstNode<'_, ArrowFunctionExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
if matches!(
parent,
AstNodes::TSAsExpression(_)
Expand Down Expand Up @@ -733,7 +733,7 @@ impl NeedsParentheses<'_> for AstNode<'_, YieldExpression<'_>> {
return false;
}

let parent = self.parent;
let parent = self.parent();
matches!(parent, AstNodes::AwaitExpression(_) | AstNodes::TSTypeAssertion(_))
|| await_or_yield_needs_parens(self.span(), parent)
}
Expand Down Expand Up @@ -779,24 +779,24 @@ impl NeedsParentheses<'_> for AstNode<'_, JSXEmptyExpression> {

impl NeedsParentheses<'_> for AstNode<'_, TSAsExpression<'_>> {
fn needs_parentheses(&self, _f: &Formatter<'_, '_>) -> bool {
ts_as_or_satisfies_needs_parens(self.span(), &self.expression, self.parent)
ts_as_or_satisfies_needs_parens(self.span(), &self.expression, self.parent())
}
}

impl NeedsParentheses<'_> for AstNode<'_, TSSatisfiesExpression<'_>> {
fn needs_parentheses(&self, _f: &Formatter<'_, '_>) -> bool {
ts_as_or_satisfies_needs_parens(self.span(), &self.expression, self.parent)
ts_as_or_satisfies_needs_parens(self.span(), &self.expression, self.parent())
}
}

impl NeedsParentheses<'_> for AstNode<'_, TSTypeAssertion<'_>> {
fn needs_parentheses(&self, _f: &Formatter<'_, '_>) -> bool {
match self.parent {
match self.parent() {
AstNodes::TSAsExpression(_) | AstNodes::TSSatisfiesExpression(_) => true,
AstNodes::BinaryExpression(binary) => {
matches!(binary.operator, BinaryOperator::ShiftLeft)
}
_ => type_cast_like_needs_parens(self.span(), self.parent),
_ => type_cast_like_needs_parens(self.span(), self.parent()),
}
}
}
Expand Down Expand Up @@ -830,15 +830,15 @@ fn type_cast_like_needs_parens(span: Span, parent: &AstNodes<'_>) -> bool {

impl NeedsParentheses<'_> for AstNode<'_, TSNonNullExpression<'_>> {
fn needs_parentheses(&self, _f: &Formatter<'_, '_>) -> bool {
let parent = self.parent;
let parent = self.parent();
is_class_extends(self.span, parent)
|| (self.is_new_callee() && member_chain_callee_needs_parens(&self.expression))
}
}

impl NeedsParentheses<'_> for AstNode<'_, TSInstantiationExpression<'_>> {
fn needs_parentheses(&self, _f: &Formatter<'_, '_>) -> bool {
let expr = match self.parent {
let expr = match self.parent() {
AstNodes::StaticMemberExpression(expr) => &expr.object,
AstNodes::ComputedMemberExpression(expr) => &expr.object,
AstNodes::PrivateFieldExpression(expr) => &expr.object,
Expand Down Expand Up @@ -928,8 +928,8 @@ enum UnaryLike<'a, 'b> {
impl UnaryLike<'_, '_> {
fn parent(&self) -> &AstNodes<'_> {
match self {
Self::UpdateExpression(e) => e.parent,
Self::UnaryExpression(e) => e.parent,
Self::UpdateExpression(e) => e.parent(),
Self::UnaryExpression(e) => e.parent(),
}
}
}
Expand Down Expand Up @@ -1142,7 +1142,7 @@ impl NeedsParentheses<'_> for AstNode<'_, JSXElement<'_>> {
return false;
}

jsx_element_or_fragment_needs_paren(self.span, self.parent)
jsx_element_or_fragment_needs_paren(self.span, self.parent())
}
}

Expand All @@ -1152,6 +1152,6 @@ impl NeedsParentheses<'_> for AstNode<'_, JSXFragment<'_>> {
return false;
}

jsx_element_or_fragment_needs_paren(self.span, self.parent)
jsx_element_or_fragment_needs_paren(self.span, self.parent())
}
}
Loading
Loading