feat(semantic): add check for duplicate class elements in checker (#2455)

1. Remove the check implementation of the parser
2. Implement it to semantic checker
3. Support typescript's check for duplicate class elements

Support checking for duplicate class elements in semantic checker is
easier to support typescript checking rules.

crates/oxc_parser/src/js/list.rs

@@ -1,5 +1,5 @@
 use oxc_allocator::Vec;
-use oxc_ast::{ast::*, syntax_directed_operations::PrivateBoundIdentifiers};
+use oxc_ast::ast::*;
 use oxc_diagnostics::{
     miette::{self, Diagnostic},
     thiserror::{self, Error},
@@ -382,65 +382,13 @@ impl<'a> SeparatedList<'a> for ExportNamedSpecifiers<'a> {
     }
 }
 
-pub struct PrivateBoundIdentifierMeta {
-    span: Span,
-    r#static: bool,
-    kind: Option<MethodDefinitionKind>,
-}
-
 pub struct ClassElements<'a> {
     pub elements: Vec<'a, ClassElement<'a>>,
-
-    /// <https://tc39.es/ecma262/#sec-static-semantics-privateboundidentifiers>
-    pub private_bound_identifiers: FxHashMap<Atom, PrivateBoundIdentifierMeta>,
 }
 
 impl<'a> ClassElements<'a> {
     pub(crate) fn new(p: &ParserImpl<'a>) -> Self {
-        Self { elements: p.ast.new_vec(), private_bound_identifiers: FxHashMap::default() }
-    }
-
-    fn detect_private_name_conflict(
-        &self,
-        p: &mut ParserImpl,
-        private_ident: &PrivateIdentifier,
-        r#static: bool,
-        kind: Option<MethodDefinitionKind>,
-    ) {
-        if let Some(existed) = self.private_bound_identifiers.get(&private_ident.name) {
-            if !(r#static == existed.r#static
-                && match existed.kind {
-                    Some(MethodDefinitionKind::Get) => {
-                        kind.as_ref().map_or(false, |kind| *kind == MethodDefinitionKind::Set)
-                    }
-                    Some(MethodDefinitionKind::Set) => {
-                        kind.as_ref().map_or(false, |kind| *kind == MethodDefinitionKind::Get)
-                    }
-                    _ => false,
-                })
-            {
-                p.error(Redeclaration(
-                    private_ident.name.clone(),
-                    existed.span,
-                    private_ident.span,
-                ));
-            }
-        }
-    }
-
-    fn on_declare_private_property(
-        &mut self,
-        p: &mut ParserImpl,
-        private_ident: &PrivateIdentifier,
-        r#static: bool,
-        kind: Option<MethodDefinitionKind>,
-    ) {
-        self.detect_private_name_conflict(p, private_ident, r#static, kind);
-
-        self.private_bound_identifiers.insert(
-            private_ident.name.clone(),
-            PrivateBoundIdentifierMeta { r#static, kind, span: private_ident.span },
-        );
+        Self { elements: p.ast.new_vec() }
     }
 }
 
@@ -463,15 +411,6 @@ impl<'a> NormalList<'a> for ClassElements<'a> {
         }
         let element = p.parse_class_element()?;
 
-        if let Some(private_ident) = element.private_bound_identifiers() {
-            self.on_declare_private_property(
-                p,
-                &private_ident,
-                element.r#static(),
-                element.method_definition_kind(),
-            );
-        }
-
         self.elements.push(element);
         Ok(())
     }

crates/oxc_semantic/src/checker/javascript.rs

@@ -26,7 +26,10 @@ impl EarlyErrorJavaScript {
         let kind = node.kind();
 
         match kind {
-            AstKind::Program(_) => check_labeled_statement(ctx),
+            AstKind::Program(_) => {
+                check_labeled_statement(ctx);
+                check_duplicate_class_elements(ctx);
+            }
             AstKind::BindingIdentifier(ident) => {
                 check_identifier(&ident.name, ident.span, node, ctx);
                 check_binding_identifier(ident, node, ctx);
@@ -102,6 +105,42 @@ impl EarlyErrorJavaScript {
     }
 }
 
+fn check_duplicate_class_elements(ctx: &SemanticBuilder<'_>) {
+    let classes = &ctx.class_table_builder.classes;
+    classes.iter_enumerated().for_each(|(class_id, _)| {
+        let mut defined_elements = FxHashMap::default();
+        let elements = &classes.elements[class_id];
+        for (element_id, element) in elements.iter_enumerated() {
+            if let Some(prev_element_id) = defined_elements.insert(&element.name, element_id) {
+                let prev_element = &elements[prev_element_id];
+
+                let mut is_duplicate = element.is_private == prev_element.is_private
+                    && if element.kind.is_setter_or_getter()
+                        && prev_element.kind.is_setter_or_getter()
+                    {
+                        element.kind == prev_element.kind
+                            || element.r#static != prev_element.r#static
+                    } else {
+                        true
+                    };
+
+                is_duplicate = if ctx.source_type.is_typescript() {
+                    element.r#static == prev_element.r#static && is_duplicate
+                } else {
+                    // * It is a Syntax Error if PrivateBoundIdentifiers of ClassElementList contains any duplicate entries,
+                    // unless the name is used once for a getter and once for a setter and in no other entries,
+                    // and the getter and setter are either both static or both non-static.
+                    element.is_private && is_duplicate
+                };
+
+                if is_duplicate {
+                    ctx.error(Redeclaration(element.name.clone(), prev_element.span, element.span));
+                }
+            }
+        }
+    });
+}
+
 fn check_module_record(ctx: &SemanticBuilder<'_>) {
     #[derive(Debug, Error, Diagnostic)]
     #[error("Export '{0}' is not defined")]

tasks/coverage/parser_babel.snap

@@ -1,6 +1,6 @@
 parser_babel Summary:
 AST Parsed     : 2090/2096 (99.71%)
-Positive Passed: 2086/2096 (99.52%)
+Positive Passed: 2083/2096 (99.38%)
 Negative Passed: 1361/1500 (90.73%)
 Expect Syntax Error: "annex-b/disabled/1.1-html-comments-close/input.js"
 Expect Syntax Error: "annex-b/disabled/3.1-sloppy-labeled-functions/input.js"
@@ -177,6 +177,44 @@ Expect to Parse: "typescript/class/constructor-with-modifier-names/input.ts"
    ·        ╰── it cannot be redeclared here
  4 │ }
    ╰────
+Expect to Parse: "typescript/class/declare/input.ts"
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/declare/input.ts:3:5]
+ 2 │     [x: string]: any;
+ 3 │     x;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 4 │     x: number;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 5 │     f();
+   ╰────
+Expect to Parse: "typescript/class/modifiers-override/input.ts"
+
+  × Identifier `show` has already been declared
+   ╭─[typescript/class/modifiers-override/input.ts:2:12]
+ 1 │ class MyClass extends BaseClass {
+ 2 │   override show() {}
+   ·            ──┬─
+   ·              ╰── `show` has already been declared here
+ 3 │   public override show() {}
+   ·                   ──┬─
+   ·                     ╰── It can not be redeclared here
+ 4 │   override size = 5;
+   ╰────
+
+  × Identifier `size` has already been declared
+   ╭─[typescript/class/modifiers-override/input.ts:4:12]
+ 3 │   public override show() {}
+ 4 │   override size = 5;
+   ·            ──┬─
+   ·              ╰── `size` has already been declared here
+ 5 │   override readonly size = 5;
+   ·                     ──┬─
+   ·                       ╰── It can not be redeclared here
+ 6 │ 
+   ╰────
 Expect to Parse: "typescript/class/parameter-properties/input.ts"
 
   × A required parameter cannot follow an optional parameter.
@@ -186,6 +224,67 @@ Expect to Parse: "typescript/class/parameter-properties/input.ts"
    ·         ───────────────────
  8 │         // Also works on AssignmentPattern
    ╰────
+Expect to Parse: "typescript/class/properties/input.ts"
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/properties/input.ts:2:5]
+ 1 │ class C {
+ 2 │     x;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 3 │     x?;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 4 │     x: number;
+   ╰────
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/properties/input.ts:3:5]
+ 2 │     x;
+ 3 │     x?;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 4 │     x: number;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 5 │     x: number = 1;
+   ╰────
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/properties/input.ts:4:5]
+ 3 │     x?;
+ 4 │     x: number;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 5 │     x: number = 1;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 6 │     x!;
+   ╰────
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/properties/input.ts:5:5]
+ 4 │     x: number;
+ 5 │     x: number = 1;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 6 │     x!;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 7 │     x!: number;
+   ╰────
+
+  × Identifier `x` has already been declared
+   ╭─[typescript/class/properties/input.ts:6:5]
+ 5 │     x: number = 1;
+ 6 │     x!;
+   ·     ┬
+   ·     ╰── `x` has already been declared here
+ 7 │     x!: number;
+   ·     ┬
+   ·     ╰── It can not be redeclared here
+ 8 │ }
+   ╰────
 Expect to Parse: "typescript/function/declare-pattern-parameters/input.ts"
 
   × A required parameter cannot follow an optional parameter.