Skip to content

Commit

Permalink
refactor(ast)!: remove AstKind::FinallyClause (#6744)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Oct 21, 2024
1 parent 85d5220 commit 1248557
Show file tree
Hide file tree
Showing 15 changed files with 47 additions and 110 deletions.
1 change: 0 additions & 1 deletion crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,6 @@ pub struct TryStatement<'a> {
/// The `catch` clause, including the parameter and the block statement
pub handler: Option<Box<'a, CatchClause<'a>>>,
/// The `finally` clause
#[visit(as(FinallyClause))]
pub finalizer: Option<Box<'a, BlockStatement<'a>>>,
}

Expand Down
1 change: 0 additions & 1 deletion crates/oxc_ast/src/ast_kind_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ impl<'a> AstKind<'a> {

Self::SwitchCase(_) => "SwitchCase".into(),
Self::CatchClause(_) => "CatchClause".into(),
Self::FinallyClause(_) => "FinallyClause".into(),

Self::VariableDeclaration(_) => "VariableDeclaration".into(),
Self::VariableDeclarator(v) => format!(
Expand Down
12 changes: 0 additions & 12 deletions crates/oxc_ast/src/generated/ast_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ pub enum AstType {
LabeledStatement,
ThrowStatement,
TryStatement,
FinallyClause,
CatchClause,
CatchParameter,
DebuggerStatement,
Expand Down Expand Up @@ -246,7 +245,6 @@ pub enum AstKind<'a> {
LabeledStatement(&'a LabeledStatement<'a>),
ThrowStatement(&'a ThrowStatement<'a>),
TryStatement(&'a TryStatement<'a>),
FinallyClause(&'a BlockStatement<'a>),
CatchClause(&'a CatchClause<'a>),
CatchParameter(&'a CatchParameter<'a>),
DebuggerStatement(&'a DebuggerStatement),
Expand Down Expand Up @@ -418,7 +416,6 @@ impl<'a> GetSpan for AstKind<'a> {
Self::LabeledStatement(it) => it.span(),
Self::ThrowStatement(it) => it.span(),
Self::TryStatement(it) => it.span(),
Self::FinallyClause(it) => it.span(),
Self::CatchClause(it) => it.span(),
Self::CatchParameter(it) => it.span(),
Self::DebuggerStatement(it) => it.span(),
Expand Down Expand Up @@ -1125,15 +1122,6 @@ impl<'a> AstKind<'a> {
}
}

#[inline]
pub fn as_finally_clause(&self) -> Option<&'a BlockStatement<'a>> {
if let Self::FinallyClause(v) = self {
Some(*v)
} else {
None
}
}

#[inline]
pub fn as_catch_clause(&self) -> Option<&'a CatchClause<'a>> {
if let Self::CatchClause(v) = self {
Expand Down
17 changes: 1 addition & 16 deletions crates/oxc_ast/src/generated/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1131,11 +1131,6 @@ pub trait Visit<'a>: Sized {
walk_catch_parameter(self, it);
}

#[inline]
fn visit_finally_clause(&mut self, it: &BlockStatement<'a>) {
walk_finally_clause(self, it);
}

#[inline]
fn visit_while_statement(&mut self, it: &WhileStatement<'a>) {
walk_while_statement(self, it);
Expand Down Expand Up @@ -3654,7 +3649,7 @@ pub mod walk {
visitor.visit_catch_clause(handler);
}
if let Some(finalizer) = &it.finalizer {
visitor.visit_finally_clause(finalizer);
visitor.visit_block_statement(finalizer);
}
visitor.leave_node(kind);
}
Expand All @@ -3680,16 +3675,6 @@ pub mod walk {
visitor.leave_node(kind);
}

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

#[inline]
pub fn walk_while_statement<'a, V: Visit<'a>>(visitor: &mut V, it: &WhileStatement<'a>) {
let kind = AstKind::WhileStatement(visitor.alloc(it));
Expand Down
17 changes: 1 addition & 16 deletions crates/oxc_ast/src/generated/visit_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1126,11 +1126,6 @@ pub trait VisitMut<'a>: Sized {
walk_catch_parameter(self, it);
}

#[inline]
fn visit_finally_clause(&mut self, it: &mut BlockStatement<'a>) {
walk_finally_clause(self, it);
}

#[inline]
fn visit_while_statement(&mut self, it: &mut WhileStatement<'a>) {
walk_while_statement(self, it);
Expand Down Expand Up @@ -3860,7 +3855,7 @@ pub mod walk_mut {
visitor.visit_catch_clause(handler);
}
if let Some(finalizer) = &mut it.finalizer {
visitor.visit_finally_clause(finalizer);
visitor.visit_block_statement(finalizer);
}
visitor.leave_node(kind);
}
Expand All @@ -3886,16 +3881,6 @@ pub mod walk_mut {
visitor.leave_node(kind);
}

#[inline]
pub fn walk_finally_clause<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut BlockStatement<'a>) {
let kind = AstType::FinallyClause;
visitor.enter_node(kind);
visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
visitor.visit_statements(&mut it.body);
visitor.leave_scope();
visitor.leave_node(kind);
}

#[inline]
pub fn walk_while_statement<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut WhileStatement<'a>) {
let kind = AstType::WhileStatement;
Expand Down
52 changes: 18 additions & 34 deletions crates/oxc_linter/src/rules/eslint/no_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,47 +59,31 @@ impl Rule for NoEmpty {
}
ctx.diagnostic_with_suggestion(no_empty_diagnostic("block", block.span), |fixer| {
if let Some(parent) = parent {
if let AstKind::TryStatement(try_stmt) = parent {
if let Some(try_block_stmt) = &try_stmt.finalizer {
if try_block_stmt.span == block.span {
return if let Some(finally_kw_start) =
find_finally_start(ctx, block)
{
fixer.delete_range(Span::new(
finally_kw_start,
block.span.end,
))
} else {
fixer.noop()
};
}
}
}
if matches!(parent, AstKind::CatchClause(_)) {
fixer.noop()
} else {
fixer.delete(&parent)
return fixer.noop();
}
fixer.delete(&parent)
} else {
fixer.noop()
}
});
}
// 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().has_comments_between(finally_clause.span) {
return;
}
ctx.diagnostic_with_suggestion(
no_empty_diagnostic("block", finally_clause.span),
|fixer| {
let parent = ctx
.nodes()
.parent_kind(node.id())
.expect("finally clauses must have a parent node");

let AstKind::TryStatement(parent) = parent else {
unreachable!("finally clauses must be children of a try statement");
};

// if there's no `catch`, we can't remove the `finally` block
if parent.handler.is_none() {
return fixer.noop();
}

if let Some(finally_kw_start) = find_finally_start(ctx, finally_clause) {
fixer.delete_range(Span::new(finally_kw_start, finally_clause.span.end))
} else {
fixer.noop()
}
},
);
}
AstKind::SwitchStatement(switch) if switch.cases.is_empty() => {
ctx.diagnostic_with_suggestion(
no_empty_diagnostic("switch", switch.span),
Expand Down
28 changes: 19 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_unsafe_finally.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,38 @@ impl Rule for NoUnsafeFinally {
_ => None,
};

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

if sentinel_node_type.test(ast_kind) {
break;
}

let parent_kind = ctx.nodes().parent_kind(node_id);
let Some(parent_node_id) = nodes.parent_id(node_id) else { break };
let parent_kind = nodes.kind(parent_node_id);

if let Some(AstKind::LabeledStatement(labeled_stmt)) = parent_kind {
if let AstKind::LabeledStatement(labeled_stmt) = parent_kind {
if label_name == Some(&labeled_stmt.label.name) {
label_inside = true;
}
}

if let Some(AstKind::FinallyClause(_)) = parent_kind {
if label_name.is_some() && label_inside {
break;
// Finally Block
let parent_parent_kind = nodes.parent_kind(node_id);
if let Some(AstKind::TryStatement(try_stmt)) = parent_parent_kind {
if let Some(try_block_stmt) = &try_stmt.finalizer {
if let AstKind::BlockStatement(block_stmt) = ast_kind {
if try_block_stmt.span == block_stmt.span {
if label_name.is_some() && label_inside {
break;
}
ctx.diagnostic(no_unsafe_finally_diagnostic(node.kind().span()));
return;
}
}
}
ctx.diagnostic(no_unsafe_finally_diagnostic(node.kind().span()));
return;
}
}
}
Expand Down
7 changes: 0 additions & 7 deletions crates/oxc_linter/src/rules/unicorn/empty_brace_spaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,6 @@ impl Rule for EmptyBraceSpaces {
AstKind::BlockStatement(block_stmt) => {
remove_empty_braces_spaces(ctx, block_stmt.body.is_empty(), block_stmt.span);
}
AstKind::FinallyClause(finally_clause) => {
remove_empty_braces_spaces(
ctx,
finally_clause.body.is_empty(),
finally_clause.span,
);
}
_ => (),
};
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg */

self.visit_finally_clause(finalizer);
self.visit_block_statement(finalizer);

/* cfg */
control_flow!(self, |cfg| {
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_semantic/src/jsdoc/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ fn should_attach_jsdoc(kind: &AstKind) -> bool {

| AstKind::SwitchCase(_)
| AstKind::CatchClause(_)
| AstKind::FinallyClause(_)

| AstKind::VariableDeclaration(_)
| AstKind::VariableDeclarator(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ bb5: {

bb6: {
statement
statement
}

bb7: {
Expand Down Expand Up @@ -67,6 +68,7 @@ TryStatement" shape = box]
5 [ label = "bb5
BlockStatement" shape = box]
6 [ label = "bb6
BlockStatement
DoWhileStatement" shape = box]
7 [ label = "bb7
BlockStatement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ bb3: {
}

bb4: {

statement
}

bb5: {
Expand All @@ -48,7 +48,8 @@ TryStatement" shape = box]
3 [ label = "bb3
BlockStatement
ExpressionStatement" shape = box]
4 [ label = "bb4" shape = box]
4 [ label = "bb4
BlockStatement" shape = box]
5 [ label = "bb5" shape = box]
6 [ label = "bb6" shape = box]
7 [ label = "bb7
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ bb6: {
}

bb7: {
statement
break <label>
}

Expand Down Expand Up @@ -72,6 +73,7 @@ return <value>" shape = box]
6 [ label = "bb6
unreachable" shape = box]
7 [ label = "bb7
BlockStatement
break <label>" shape = box]
8 [ label = "bb8
unreachable" shape = box]
Expand Down
5 changes: 0 additions & 5 deletions crates/oxc_traverse/scripts/lib/scopes_collector.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ export default function generateScopesCollectorCode(types) {
impl<'a> Visit<'a> for ChildScopeCollector {
${methods}
#[inline]
fn visit_finally_clause(&mut self, it: &BlockStatement<'a>) {
self.add_scope(&it.scope_id);
}
}
`;
}
5 changes: 0 additions & 5 deletions crates/oxc_traverse/src/generated/scopes_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,4 @@ impl<'a> Visit<'a> for ChildScopeCollector {
fn visit_ts_mapped_type(&mut self, it: &TSMappedType<'a>) {
self.add_scope(&it.scope_id);
}

#[inline]
fn visit_finally_clause(&mut self, it: &BlockStatement<'a>) {
self.add_scope(&it.scope_id);
}
}

0 comments on commit 1248557

Please sign in to comment.