refactor(semantic): improve check private identifier implementation (#1794)

The regression case in typescript is as expected since we don't
currently store `AccessorProperty` in `ClassTable`.

The following code is one of those cases

```ts
class C1 {
    accessor #a: any;
    accessor #b = 1;
    static accessor #c: any;
    static accessor #d = 2;

    constructor() {
        this.#a = 3;
        this.#b = 4;
    }

    static {
        this.#c = 5;
        this.#d = 6;
    }
}
```

But there was an error

```shell
Expect to Parse: "conformance/classes/propertyMemberDeclarations/autoAccessor2.ts"
  × Private field 'a' must be declared in an enclosing class
    ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor2.ts:9:1]
  9 │     constructor() {
 10 │         this.#a = 3;
    ·              ──
 11 │         this.#b = 4;
    ╰────
```

Let's leave it alone for now. Because of the `AccessorProperty` I
haven't seen it in any repo.

crates/oxc_semantic/src/builder.rs

@@ -85,7 +85,7 @@ pub struct SemanticBuilder<'a> {
     check_syntax_error: bool,
 
     redeclare_variables: RedeclareVariables,
-    class_table_builder: ClassTableBuilder,
+    pub class_table_builder: ClassTableBuilder,
 }
 
 pub struct SemanticBuilderReturn<'a> {

crates/oxc_semantic/src/checker/javascript.rs

@@ -36,8 +36,7 @@ impl EarlyErrorJavaScript {
                 check_identifier_reference(ident, node, ctx);
             }
             AstKind::LabelIdentifier(ident) => check_identifier(&ident.name, ident.span, node, ctx),
-            AstKind::PrivateIdentifier(ident) => check_private_identifier(ident, node, ctx),
-
+            AstKind::PrivateIdentifier(ident) => check_private_identifier_outside_class(ident, ctx),
             AstKind::NumberLiteral(lit) => check_number_literal(lit, ctx),
             AstKind::StringLiteral(lit) => check_string_literal(lit, ctx),
             AstKind::RegExpLiteral(lit) => check_regexp_literal(lit, ctx),
@@ -273,61 +272,34 @@ fn check_identifier_reference<'a>(
     }
 }
 
-fn check_private_identifier<'a>(
-    ident: &PrivateIdentifier,
-    node: &AstNode<'a>,
-    ctx: &SemanticBuilder<'a>,
-) {
-    // Ignore private identifier declaration inside class
-    if matches!(ctx.nodes.parent_kind(node.id()), Some(AstKind::PropertyKey(_))) {
-        return;
-    }
-
-    // Find enclosing classes
-    let mut classes = vec![];
-    for node_id in ctx.nodes.ancestors(node.id()).skip(1) {
-        let kind = ctx.nodes.kind(node_id);
-        if let AstKind::Class(class) = kind {
-            classes.push(class);
-        }
-        // stop lookup when the class is a heritage, e.g.
-        // `class C extends class extends class { x = this.#foo; } {} { #foo }`
-        // `class C extends function() { x = this.#foo; } { #foo }`
-        if matches!(kind, AstKind::ClassHeritage(_)) {
-            break;
-        }
-    }
-
-    if classes.is_empty() {
+fn check_private_identifier_outside_class(ident: &PrivateIdentifier, ctx: &SemanticBuilder<'_>) {
+    if ctx.class_table_builder.current_class_id.is_none() {
         #[derive(Debug, Error, Diagnostic)]
         #[error("Private identifier '#{0}' is not allowed outside class bodies")]
         #[diagnostic()]
         struct PrivateNotInClass(Atom, #[label] Span);
-        return ctx.error(PrivateNotInClass(ident.name.clone(), ident.span));
-    };
+        ctx.error(PrivateNotInClass(ident.name.clone(), ident.span));
+    }
+}
 
-    // Check private identifier declarations in class.
-    // This implementations does a simple lookup for private identifier declarations inside a class.
-    // Performance can be improved by storing private identifiers for each class inside a lookup table,
-    // but there are not many private identifiers in the wild so we should be good fow now.
-    let found_private_ident = classes.iter().any(|class| {
-        class.body.body.iter().any(|def| {
-            // let key = match def {
-            // ClassElement::PropertyDefinition(def) => &def.key,
-            // ClassElement::MethodDefinition(def) => &def.key,
-            // _ => return false,
-            // };
-            matches!(def.property_key(), Some(PropertyKey::PrivateIdentifier(prop_ident))
-                if prop_ident.name == ident.name)
-        })
-    });
-
-    if !found_private_ident {
-        #[derive(Debug, Error, Diagnostic)]
-        #[error("Private field '{0}' must be declared in an enclosing class")]
-        #[diagnostic()]
-        struct PrivateFieldUndeclared(Atom, #[label] Span);
-        ctx.error(PrivateFieldUndeclared(ident.name.clone(), ident.span));
+fn check_private_identifier(ctx: &SemanticBuilder<'_>) {
+    if let Some(class_id) = ctx.class_table_builder.current_class_id {
+        ctx.class_table_builder.classes.iter_private_identifiers(class_id).for_each(|reference| {
+            if reference.property_id.is_none()
+                && reference.method_id.is_none()
+                && !ctx.class_table_builder.classes.ancestors(class_id).skip(1).any(|class_id| {
+                    ctx.class_table_builder
+                        .classes
+                        .has_private_definition(class_id, &reference.name)
+                })
+            {
+                #[derive(Debug, Error, Diagnostic)]
+                #[error("Private field '{0}' must be declared in an enclosing class")]
+                #[diagnostic()]
+                struct PrivateFieldUndeclared(Atom, #[label] Span);
+                ctx.error(PrivateFieldUndeclared(reference.name.clone(), reference.span));
+            }
+        });
     }
 }
 
@@ -817,6 +789,8 @@ fn check_class(class: &Class, ctx: &SemanticBuilder<'_>) {
         #[label("it cannot be redeclared here")] Span,
     );
 
+    check_private_identifier(ctx);
+
     // ClassBody : ClassElementList
     // It is a Syntax Error if PrototypePropertyNameList of ClassElementList contains more than one occurrence of "constructor".
     let mut prev_constructor: Option<Span> = None;

crates/oxc_semantic/src/class/builder.rs

@@ -15,7 +15,7 @@ use super::{
 #[derive(Debug, Default)]
 pub struct ClassTableBuilder {
     pub current_class_id: Option<ClassId>,
-    classes: ClassTable,
+    pub classes: ClassTable,
 }
 
 impl ClassTableBuilder {

crates/oxc_semantic/src/class/table.rs

@@ -78,6 +78,13 @@ impl ClassTable {
         self.declarations.iter_enumerated()
     }
 
+    pub fn iter_private_identifiers(
+        &self,
+        class_id: ClassId,
+    ) -> impl Iterator<Item = &PrivateIdentifierReference> + '_ {
+        self.private_identifiers[class_id].iter()
+    }
+
     pub fn get_property_id(&self, class_id: ClassId, name: &Atom) -> Option<PropertyId> {
         self.properties[class_id].iter_enumerated().find_map(|(property_id, property)| {
             if property.name == *name {

tasks/coverage/parser_typescript.snap

@@ -1,6 +1,6 @@
 parser_typescript Summary:
 AST Parsed     : 5201/5205 (99.92%)
-Positive Passed: 5194/5205 (99.79%)
+Positive Passed: 5192/5205 (99.75%)
 Negative Passed: 1023/4859 (21.05%)
 Expect Syntax Error: "compiler/ClassDeclaration10.ts"
 Expect Syntax Error: "compiler/ClassDeclaration11.ts"
@@ -3880,6 +3880,39 @@ Expect to Parse: "compiler/withStatementInternalComments.ts"
    ·       ────
    ╰────
 
+Expect to Parse: "conformance/classes/propertyMemberDeclarations/autoAccessor2.ts"
+  × Private field 'a' must be declared in an enclosing class
+    ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor2.ts:9:1]
+  9 │     constructor() {
+ 10 │         this.#a = 3;
+    ·              ──
+ 11 │         this.#b = 4;
+    ╰────
+
+  × Private field 'b' must be declared in an enclosing class
+    ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor2.ts:10:1]
+ 10 │         this.#a = 3;
+ 11 │         this.#b = 4;
+    ·              ──
+ 12 │     }
+    ╰────
+
+  × Private field 'c' must be declared in an enclosing class
+    ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor2.ts:14:1]
+ 14 │     static {
+ 15 │         this.#c = 5;
+    ·              ──
+ 16 │         this.#d = 6;
+    ╰────
+
+  × Private field 'd' must be declared in an enclosing class
+    ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor2.ts:15:1]
+ 15 │         this.#c = 5;
+ 16 │         this.#d = 6;
+    ·              ──
+ 17 │     }
+    ╰────
+
 Expect to Parse: "conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts"
   × Classes may not have a static property named prototype
     ╭─[conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts:55:1]
@@ -3938,6 +3971,23 @@ Expect to Parse: "conformance/es6/moduleExportsSystem/topLevelVarHoistingCommonJ
  69 │     var y = _;
     ╰────
 
+Expect to Parse: "conformance/esDecorators/classDeclaration/fields/esDecorators-classDeclaration-fields-staticPrivateAccessor.ts"
+  × Private field 'field1' must be declared in an enclosing class
+    ╭─[conformance/esDecorators/classDeclaration/fields/esDecorators-classDeclaration-fields-staticPrivateAccessor.ts:14:1]
+ 14 │     static {
+ 15 │         this.#field1;
+    ·              ───────
+ 16 │         this.#field1 = 1;
+    ╰────
+
+  × Private field 'field1' must be declared in an enclosing class
+    ╭─[conformance/esDecorators/classDeclaration/fields/esDecorators-classDeclaration-fields-staticPrivateAccessor.ts:15:1]
+ 15 │         this.#field1;
+ 16 │         this.#field1 = 1;
+    ·              ───────
+ 17 │     }
+    ╰────
+
 Expect to Parse: "conformance/esDecorators/esDecorators-preservesThis.ts"
   × Unexpected token
     ╭─[conformance/esDecorators/esDecorators-preservesThis.ts:26:1]