From 92d709bf212bdb35a4b1f4e58ce29df3c474604d Mon Sep 17 00:00:00 2001 From: Boshen Date: Sun, 21 Apr 2024 23:43:39 +0800 Subject: [PATCH] feat(ast): add `CatchParameter` node (#3049) --- crates/oxc_ast/src/ast/js.rs | 11 ++++- crates/oxc_ast/src/ast_builder.rs | 6 ++- crates/oxc_ast/src/ast_kind.rs | 3 ++ crates/oxc_ast/src/visit/visit.rs | 13 ++++- crates/oxc_ast/src/visit/visit_mut.rs | 16 ++++++- crates/oxc_codegen/src/gen.rs | 2 +- .../src/rules/eslint/no_useless_catch.rs | 2 +- .../src/rules/unicorn/catch_error_name.rs | 48 +++++++++---------- .../unicorn/prefer_optional_catch_binding.rs | 13 ++--- .../prefer_optional_catch_binding.snap | 2 +- crates/oxc_parser/src/js/statement.rs | 5 +- crates/oxc_prettier/src/format/mod.rs | 2 +- crates/oxc_semantic/src/binder.rs | 4 +- 13 files changed, 81 insertions(+), 46 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index fc33fbdbc..a51797c4f 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -1641,10 +1641,19 @@ pub struct TryStatement<'a> { pub struct CatchClause<'a> { #[cfg_attr(feature = "serialize", serde(flatten))] pub span: Span, - pub param: Option>, + pub param: Option>, pub body: Box<'a, BlockStatement<'a>>, } +#[derive(Debug, Hash)] +#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))] +#[cfg_attr(feature = "serialize", serde(tag = "type"))] +pub struct CatchParameter<'a> { + #[cfg_attr(feature = "serialize", serde(flatten))] + pub span: Span, + pub pattern: BindingPattern<'a>, +} + /// Debugger Statement #[derive(Debug, Hash)] #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))] diff --git a/crates/oxc_ast/src/ast_builder.rs b/crates/oxc_ast/src/ast_builder.rs index 5b7fbc0e3..2fae7012e 100644 --- a/crates/oxc_ast/src/ast_builder.rs +++ b/crates/oxc_ast/src/ast_builder.rs @@ -395,12 +395,16 @@ impl<'a> AstBuilder<'a> { pub fn catch_clause( &self, span: Span, - param: Option>, + param: Option>, body: Box<'a, BlockStatement<'a>>, ) -> Box<'a, CatchClause<'a>> { self.alloc(CatchClause { span, param, body }) } + pub fn catch_parameter(&self, span: Span, pattern: BindingPattern<'a>) -> CatchParameter<'a> { + CatchParameter { span, pattern } + } + pub fn while_statement( &self, span: Span, diff --git a/crates/oxc_ast/src/ast_kind.rs b/crates/oxc_ast/src/ast_kind.rs index 3732dda3e..28099348b 100644 --- a/crates/oxc_ast/src/ast_kind.rs +++ b/crates/oxc_ast/src/ast_kind.rs @@ -107,6 +107,7 @@ ast_kinds! { FunctionBody(&'a FunctionBody<'a>), FormalParameters(&'a FormalParameters<'a>), FormalParameter(&'a FormalParameter<'a>), + CatchParameter(&'a CatchParameter<'a>), Class(&'a Class<'a>), ClassBody(&'a ClassBody<'a>), @@ -443,6 +444,7 @@ impl<'a> GetSpan for AstKind<'a> { Self::FunctionBody(x) => x.span, Self::FormalParameters(x) => x.span, Self::FormalParameter(x) => x.span, + Self::CatchParameter(x) => x.span, Self::Class(x) => x.span, Self::ClassBody(x) => x.span, @@ -640,6 +642,7 @@ impl<'a> AstKind<'a> { Self::FunctionBody(_) => "FunctionBody".into(), Self::FormalParameters(_) => "FormalParameters".into(), Self::FormalParameter(_) => "FormalParameter".into(), + Self::CatchParameter(_) => "CatchParameter".into(), Self::Class(c) => format!( "Class({})", diff --git a/crates/oxc_ast/src/visit/visit.rs b/crates/oxc_ast/src/visit/visit.rs index d40e65d48..8e7fc4164 100644 --- a/crates/oxc_ast/src/visit/visit.rs +++ b/crates/oxc_ast/src/visit/visit.rs @@ -124,6 +124,10 @@ pub trait Visit<'a>: Sized { walk_catch_clause(self, clause); } + fn visit_catch_parameter(&mut self, param: &CatchParameter<'a>) { + walk_catch_parameter(self, param); + } + fn visit_finally_clause(&mut self, clause: &BlockStatement<'a>) { walk_finally_clause(self, clause); } @@ -1145,13 +1149,20 @@ pub mod walk { visitor.enter_scope(ScopeFlags::empty()); visitor.enter_node(kind); if let Some(param) = &clause.param { - visitor.visit_binding_pattern(param); + visitor.visit_catch_parameter(param); } visitor.visit_statements(&clause.body.body); visitor.leave_node(kind); visitor.leave_scope(); } + pub fn walk_catch_parameter<'a, V: Visit<'a>>(visitor: &mut V, param: &CatchParameter<'a>) { + let kind = AstKind::CatchParameter(visitor.alloc(param)); + visitor.enter_node(kind); + visitor.visit_binding_pattern(¶m.pattern); + visitor.leave_node(kind); + } + 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()); diff --git a/crates/oxc_ast/src/visit/visit_mut.rs b/crates/oxc_ast/src/visit/visit_mut.rs index 363470129..30052622d 100644 --- a/crates/oxc_ast/src/visit/visit_mut.rs +++ b/crates/oxc_ast/src/visit/visit_mut.rs @@ -111,6 +111,10 @@ pub trait VisitMut<'a>: Sized { walk_catch_clause_mut(self, clause); } + fn visit_catch_parameter(&mut self, param: &mut CatchParameter<'a>) { + walk_catch_parameter_mut(self, param); + } + fn visit_finally_clause(&mut self, clause: &mut BlockStatement<'a>) { walk_finally_clause_mut(self, clause); } @@ -1147,13 +1151,23 @@ pub mod walk_mut { visitor.enter_scope(ScopeFlags::empty()); visitor.enter_node(kind); if let Some(param) = &mut clause.param { - visitor.visit_binding_pattern(param); + visitor.visit_catch_parameter(param); } visitor.visit_statements(&mut clause.body.body); visitor.leave_node(kind); visitor.leave_scope(); } + pub fn walk_catch_parameter_mut<'a, V: VisitMut<'a>>( + visitor: &mut V, + param: &mut CatchParameter<'a>, + ) { + let kind = AstType::CatchParameter; + visitor.enter_node(kind); + visitor.visit_binding_pattern(&mut param.pattern); + visitor.leave_node(kind); + } + pub fn walk_finally_clause_mut<'a, V: VisitMut<'a>>( visitor: &mut V, clause: &mut BlockStatement<'a>, diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs index 9763b8549..ff037fcaf 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -455,7 +455,7 @@ impl<'a, const MINIFY: bool> Gen for TryStatement<'a> { p.print_str(b"catch"); if let Some(param) = &handler.param { p.print_str(b"("); - param.gen(p, ctx); + param.pattern.gen(p, ctx); p.print_str(b")"); } p.print_block1(&handler.body, ctx); diff --git a/crates/oxc_linter/src/rules/eslint/no_useless_catch.rs b/crates/oxc_linter/src/rules/eslint/no_useless_catch.rs index 58572162c..f7837ccbd 100644 --- a/crates/oxc_linter/src/rules/eslint/no_useless_catch.rs +++ b/crates/oxc_linter/src/rules/eslint/no_useless_catch.rs @@ -59,7 +59,7 @@ impl Rule for NoUselessCatch { let AstKind::TryStatement(try_stmt) = node.kind() else { return }; let Some(catch_clause) = &try_stmt.handler else { return }; let Some(BindingPatternKind::BindingIdentifier(binding_ident)) = - catch_clause.param.as_ref().map(|pattern| &pattern.kind) + catch_clause.param.as_ref().map(|param| ¶m.pattern.kind) else { return; }; diff --git a/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs b/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs index 372683d53..17230c302 100644 --- a/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs +++ b/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs @@ -84,32 +84,30 @@ impl Rule for CatchErrorName { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if let AstKind::CatchClause(catch_node) = node.kind() { - if let Some(catch_param) = &catch_node.param { - if let oxc_ast::ast::BindingPatternKind::BindingIdentifier(binding_ident) = - &catch_param.kind - { - if self.is_name_allowed(&binding_ident.name) { - return; - } - - if binding_ident.name.starts_with('_') { - if symbol_has_references(binding_ident.symbol_id.get(), ctx) { - ctx.diagnostic(CatchErrorNameDiagnostic( - binding_ident.name.to_compact_str(), - self.name.clone(), - binding_ident.span, - )); - } - return; - } - - ctx.diagnostic(CatchErrorNameDiagnostic( - binding_ident.name.to_compact_str(), - self.name.clone(), - binding_ident.span, - )); + if let AstKind::CatchParameter(catch_param) = node.kind() { + if let oxc_ast::ast::BindingPatternKind::BindingIdentifier(binding_ident) = + &catch_param.pattern.kind + { + if self.is_name_allowed(&binding_ident.name) { + return; } + + if binding_ident.name.starts_with('_') { + if symbol_has_references(binding_ident.symbol_id.get(), ctx) { + ctx.diagnostic(CatchErrorNameDiagnostic( + binding_ident.name.to_compact_str(), + self.name.clone(), + binding_ident.span, + )); + } + return; + } + + ctx.diagnostic(CatchErrorNameDiagnostic( + binding_ident.name.to_compact_str(), + self.name.clone(), + binding_ident.span, + )); } } diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_optional_catch_binding.rs b/crates/oxc_linter/src/rules/unicorn/prefer_optional_catch_binding.rs index 52a2a2bb4..dee733b77 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_optional_catch_binding.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_optional_catch_binding.rs @@ -46,17 +46,12 @@ declare_oxc_lint!( impl Rule for PreferOptionalCatchBinding { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - let AstKind::CatchClause(catch_clause) = node.kind() else { return }; - - let Some(catch_param) = &catch_clause.param else { return }; - - let references_count = get_param_references_count(catch_param, ctx); - + let AstKind::CatchParameter(catch_param) = node.kind() else { return }; + let references_count = get_param_references_count(&catch_param.pattern, ctx); if references_count != 0 { return; } - - ctx.diagnostic(PreferOptionalCatchBindingDiagnostic(catch_param.span())); + ctx.diagnostic(PreferOptionalCatchBindingDiagnostic(catch_param.pattern.span())); } } @@ -105,7 +100,7 @@ fn test() { let fail = vec![ r"try {} catch (_) {}", r"try {} catch (theRealErrorName) {}", - r"try { } catch (e) + r"try { } catch (e) { }", r"try {} catch(e) {}", r"try {} catch (e){}", diff --git a/crates/oxc_linter/src/snapshots/prefer_optional_catch_binding.snap b/crates/oxc_linter/src/snapshots/prefer_optional_catch_binding.snap index 4d13346f8..68e9eb21a 100644 --- a/crates/oxc_linter/src/snapshots/prefer_optional_catch_binding.snap +++ b/crates/oxc_linter/src/snapshots/prefer_optional_catch_binding.snap @@ -16,7 +16,7 @@ expression: prefer_optional_catch_binding ⚠ eslint-plugin-unicorn(prefer-optional-catch-binding): Prefer omitting the catch binding parameter if it is unused ╭─[prefer_optional_catch_binding.tsx:1:25] - 1 │ try { } catch (e) + 1 │ try { } catch (e) · ─ 2 │ { } ╰──── diff --git a/crates/oxc_parser/src/js/statement.rs b/crates/oxc_parser/src/js/statement.rs index 27b9f459e..ebc60100d 100644 --- a/crates/oxc_parser/src/js/statement.rs +++ b/crates/oxc_parser/src/js/statement.rs @@ -1,7 +1,7 @@ use oxc_allocator::{Box, Vec}; use oxc_ast::ast::*; use oxc_diagnostics::Result; -use oxc_span::{Atom, Span}; +use oxc_span::{Atom, GetSpan, Span}; use super::{ declaration::{VariableDeclarationContext, VariableDeclarationParent}, @@ -530,7 +530,7 @@ impl<'a> ParserImpl<'a> { fn parse_catch_clause(&mut self) -> Result>> { let span = self.start_span(); self.bump_any(); // advance `catch` - let param = if self.eat(Kind::LParen) { + let pattern = if self.eat(Kind::LParen) { let pattern = self.parse_binding_pattern(false)?; self.expect(Kind::RParen)?; Some(pattern) @@ -538,6 +538,7 @@ impl<'a> ParserImpl<'a> { None }; let body = self.parse_block()?; + let param = pattern.map(|pattern| self.ast.catch_parameter(pattern.kind.span(), pattern)); Ok(self.ast.catch_clause(self.end_span(span), param, body)) } diff --git a/crates/oxc_prettier/src/format/mod.rs b/crates/oxc_prettier/src/format/mod.rs index dd5426eb1..bd1147598 100644 --- a/crates/oxc_prettier/src/format/mod.rs +++ b/crates/oxc_prettier/src/format/mod.rs @@ -501,7 +501,7 @@ impl<'a> Format<'a> for CatchClause<'a> { parts.push(ss!("catch ")); if let Some(param) = &self.param { parts.push(ss!("(")); - parts.push(format!(p, param)); + parts.push(format!(p, param.pattern)); parts.push(ss!(") ")); } parts.push(format!(p, self.body)); diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs index 17b972466..3b628a03a 100644 --- a/crates/oxc_semantic/src/binder.rs +++ b/crates/oxc_semantic/src/binder.rs @@ -248,7 +248,7 @@ impl<'a> Binder for CatchClause<'a> { // https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks // It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the VarDeclaredNames of Block // unless CatchParameter is CatchParameter : BindingIdentifier - if let BindingPatternKind::BindingIdentifier(ident) = ¶m.kind { + if let BindingPatternKind::BindingIdentifier(ident) = ¶m.pattern.kind { let includes = SymbolFlags::FunctionScopedVariable | SymbolFlags::CatchVariable; let symbol_id = builder.declare_shadow_symbol( &ident.name, @@ -258,7 +258,7 @@ impl<'a> Binder for CatchClause<'a> { ); ident.symbol_id.set(Some(symbol_id)); } else { - param.bound_names(&mut |ident| { + param.pattern.bound_names(&mut |ident| { let symbol_id = builder.declare_symbol( ident.span, &ident.name,