Skip to content

Commit

Permalink
fix(ast): FinallyClause won't get visited as BlockStatement anymo…
Browse files Browse the repository at this point in the history
…re. (#2881)

This would fix the issue found in #2877,

As it is right now, the `FinallyClause` gets visited 2 times, once as
the Finally and once as a block.

This PR addresses this issue by making the `visit_finally_clause` method
visit the body statements instead of visiting the block itself. Now it
works similar to the `CatchClause`.
  • Loading branch information
rzvxa authored Apr 1, 2024
1 parent 619fc61 commit 5f8f7f8
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 3 deletions.
4 changes: 3 additions & 1 deletion crates/oxc_ast/src/visit/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,9 +1099,11 @@ pub mod walk {

pub fn walk_finally_clause<'a, V: Visit<'a>>(visitor: &mut V, clause: &BlockStatement<'a>) {
let kind = AstKind::FinallyClause(visitor.alloc(clause));
visitor.enter_scope(ScopeFlags::empty());
visitor.enter_node(kind);
visitor.visit_block_statement(clause);
visitor.visit_statements(&clause.body);
visitor.leave_node(kind);
visitor.leave_scope();
}

pub fn walk_while_statement<'a, V: Visit<'a>>(visitor: &mut V, stmt: &WhileStatement<'a>) {
Expand Down
4 changes: 3 additions & 1 deletion crates/oxc_ast/src/visit/visit_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,9 +1159,11 @@ pub mod walk_mut {
clause: &mut BlockStatement<'a>,
) {
let kind = AstType::FinallyClause;
visitor.enter_scope(ScopeFlags::empty());
visitor.enter_node(kind);
visitor.visit_block_statement(clause);
visitor.visit_statements(&mut clause.body);
visitor.leave_node(kind);
visitor.leave_scope();
}

pub fn walk_while_statement_mut<'a, V: VisitMut<'a>>(
Expand Down
8 changes: 8 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ impl Rule for NoEmpty {
}
ctx.diagnostic(NoEmptyDiagnostic("block", catch_clause.body.span));
}
// The visitor does not visit the `BlockStatement` inside the `FinallyClause`.
// See `Visit::visit_finally_clause`.
AstKind::FinallyClause(finally_clause) if finally_clause.body.is_empty() => {
if ctx.semantic().trivias().has_comments_between(finally_clause.span) {
return;
}
ctx.diagnostic(NoEmptyDiagnostic("block", finally_clause.span));
}
AstKind::SwitchStatement(switch) if switch.cases.is_empty() => {
ctx.diagnostic(NoEmptyDiagnostic("switch", switch.span));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_unsafe_finally.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl Rule for NoUnsafeFinally {
};

let mut label_inside = false;
for node_id in ctx.nodes().ancestors(node.id()).skip(1) {
for node_id in ctx.nodes().ancestors(node.id()) {
let ast_kind = ctx.nodes().kind(node_id);

if sentinel_node_type.test(ast_kind) {
Expand Down
7 changes: 7 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/empty_brace_spaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ impl Rule for EmptyBraceSpaces {
catch_clause.body.span,
);
}
AstKind::FinallyClause(finally_clause) => {
remove_empty_braces_spaces(
ctx,
finally_clause.body.is_empty(),
finally_clause.span,
);
}
_ => (),
};
}
Expand Down

0 comments on commit 5f8f7f8

Please sign in to comment.