refactor(semantic): correct scope in CatchClause (#4192)

close: #4186

CatchClause has two scopes. The first one is `CatchClause`, which will add a `CatchParameter` to it. The second one is `Block`, which will add binding that declares in the current block scope.

The spec has a syntax error about `CatchParameter`
- It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the LexicallyDeclaredNames of Block.

crates/oxc_ast/src/ast/js.rs

@@ -1300,7 +1300,7 @@ pub struct TryStatement<'a> {
 }
 
 #[visited_node]
-#[scope(if(self.param.is_some()))]
+#[scope]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]

crates/oxc_ast/src/generated/visit.rs

@@ -3690,20 +3690,15 @@ pub mod walk {
 
     #[inline]
     pub fn walk_catch_clause<'a, V: Visit<'a>>(visitor: &mut V, it: &CatchClause<'a>) {
-        let scope_events_cond = it.param.is_some();
-        if scope_events_cond {
-            visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
-        }
+        visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
         let kind = AstKind::CatchClause(visitor.alloc(it));
         visitor.enter_node(kind);
         if let Some(param) = &it.param {
             visitor.visit_catch_parameter(param);
         }
         visitor.visit_block_statement(&it.body);
         visitor.leave_node(kind);
-        if scope_events_cond {
-            visitor.leave_scope();
-        }
+        visitor.leave_scope();
     }
 
     #[inline]

crates/oxc_ast/src/generated/visit_mut.rs

@@ -3895,20 +3895,15 @@ pub mod walk_mut {
 
     #[inline]
     pub fn walk_catch_clause<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut CatchClause<'a>) {
-        let scope_events_cond = it.param.is_some();
-        if scope_events_cond {
-            visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
-        }
+        visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
         let kind = AstType::CatchClause;
         visitor.enter_node(kind);
         if let Some(param) = &mut it.param {
             visitor.visit_catch_parameter(param);
         }
         visitor.visit_block_statement(&mut it.body);
         visitor.leave_node(kind);
-        if scope_events_cond {
-            visitor.leave_scope();
-        }
+        visitor.leave_scope();
     }
 
     #[inline]

crates/oxc_linter/src/rules/eslint/no_empty.rs

@@ -48,20 +48,16 @@ impl Rule for NoEmpty {
     fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
         match node.kind() {
             AstKind::BlockStatement(block) if block.body.is_empty() => {
-                if ctx.semantic().trivias().has_comments_between(block.span) {
+                if self.allow_empty_catch
+                    && matches!(ctx.nodes().parent_kind(node.id()), Some(AstKind::CatchClause(_)))
+                {
                     return;
                 }
-                ctx.diagnostic(no_empty_diagnostic("block", block.span));
-            }
-            // The visitor does not visit the `BlockStatement` inside the `CatchClause`.
-            // See `Visit::visit_catch_clause`.
-            AstKind::CatchClause(catch_clause)
-                if !self.allow_empty_catch && catch_clause.body.body.is_empty() =>
-            {
-                if ctx.semantic().trivias().has_comments_between(catch_clause.body.span) {
+
+                if ctx.semantic().trivias().has_comments_between(block.span) {
                     return;
                 }
-                ctx.diagnostic(no_empty_diagnostic("block", catch_clause.body.span));
+                ctx.diagnostic(no_empty_diagnostic("block", block.span));
             }
             // The visitor does not visit the `BlockStatement` inside the `FinallyClause`.
             // See `Visit::visit_finally_clause`.

crates/oxc_linter/src/rules/unicorn/empty_brace_spaces.rs

@@ -62,13 +62,6 @@ impl Rule for EmptyBraceSpaces {
             AstKind::BlockStatement(block_stmt) => {
                 remove_empty_braces_spaces(ctx, block_stmt.body.is_empty(), block_stmt.span);
             }
-            AstKind::CatchClause(catch_clause) => {
-                remove_empty_braces_spaces(
-                    ctx,
-                    catch_clause.body.body.is_empty(),
-                    catch_clause.body.span,
-                );
-            }
             AstKind::FinallyClause(finally_clause) => {
                 remove_empty_braces_spaces(
                     ctx,

crates/oxc_semantic/src/builder.rs

@@ -1348,19 +1348,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.leave_node(kind);
     }
 
-    fn visit_catch_clause(&mut self, clause: &CatchClause<'a>) {
-        let kind = AstKind::CatchClause(self.alloc(clause));
-        self.enter_scope(ScopeFlags::empty(), &clause.scope_id);
-        clause.scope_id.set(Some(self.current_scope_id));
-        self.enter_node(kind);
-        if let Some(param) = &clause.param {
-            self.visit_catch_parameter(param);
-        }
-        self.visit_statements(&clause.body.body);
-        self.leave_node(kind);
-        self.leave_scope();
-    }
-
     fn visit_finally_clause(&mut self, clause: &BlockStatement<'a>) {
         let kind = AstKind::FinallyClause(self.alloc(clause));
         self.enter_scope(ScopeFlags::empty(), &clause.scope_id);
@@ -1826,6 +1813,19 @@ impl<'a> SemanticBuilder<'a> {
             AstKind::YieldExpression(_) => {
                 self.set_function_node_flag(NodeFlags::HasYield);
             }
+            AstKind::BlockStatement(_) => {
+                if matches!(
+                    self.nodes.parent_kind(self.current_node_id),
+                    Some(AstKind::CatchClause(_))
+                ) {
+                    // Clone the `CatchClause` bindings and add them to the current scope.
+                    // to make it easier to check redeclare errors.
+                    if let Some(parent_scope_id) = self.scope.get_parent_id(self.current_scope_id) {
+                        let bindings = self.scope.get_bindings(parent_scope_id).clone();
+                        self.scope.get_bindings_mut(self.current_scope_id).extend(bindings);
+                    }
+                }
+            }
             _ => {}
         }
     }

crates/oxc_semantic/tests/integration/snapshots/labeled_block_break.snap

@@ -23,6 +23,7 @@ bb4: {
 	statement
 	statement
 	statement
+	statement
 }
 
 bb5: {
@@ -55,7 +56,7 @@ digraph {
     1 [ label = "TryStatement" ]
     2 [ label = "" ]
     3 [ label = "BlockStatement" ]
-    4 [ label = "LabeledStatement\nBlockStatement\nIfStatement" ]
+    4 [ label = "BlockStatement\nLabeledStatement\nBlockStatement\nIfStatement" ]
     5 [ label = "Condition(IdentifierReference(condition))" ]
     6 [ label = "BlockStatement\nbreak <LABEL>" ]
     7 [ label = "unreachable" ]

tasks/coverage/parser_typescript.snap

@@ -2,7 +2,7 @@ commit: d8086f14
 
 parser_typescript Summary:
 AST Parsed     : 5279/5283 (99.92%)
-Positive Passed: 5272/5283 (99.79%)
+Positive Passed: 5271/5283 (99.77%)
 Negative Passed: 1094/4875 (22.44%)
 Expect Syntax Error: "compiler/ClassDeclaration10.ts"
 Expect Syntax Error: "compiler/ClassDeclaration11.ts"
@@ -3818,6 +3818,19 @@ Expect to Parse: "compiler/withStatementInternalComments.ts"
  2 │ /*1*/ with /*2*/ ( /*3*/ false /*4*/ ) /*5*/ {}
    ·       ────
    ╰────
+Expect to Parse: "conformance/async/es6/asyncWithVarShadowing_es6.ts"
+
+  × Identifier `x` has already been declared
+     ╭─[conformance/async/es6/asyncWithVarShadowing_es6.ts:130:14]
+ 129 │     }
+ 130 │     catch ({ x }) {
+     ·              ┬
+     ·              ╰── `x` has already been declared here
+ 131 │         var x;
+     ·             ┬
+     ·             ╰── It can not be redeclared here
+ 132 │     }
+     ╰────
 Expect to Parse: "conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts"
 
   × Classes may not have a static property named prototype