refactor(linter): improve implementation of no_dupe_class_members based on ClassTable (#2446)

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

@@ -1,18 +1,12 @@
-use std::hash::BuildHasherDefault;
-
-use oxc_ast::{
-    ast::{ClassElement, MethodDefinitionKind},
-    AstKind,
-};
 use oxc_diagnostics::{
     miette::{self, Diagnostic},
     thiserror::Error,
 };
 use oxc_macros::declare_oxc_lint;
-use oxc_span::{Atom, GetSpan, Span};
+use oxc_span::{Atom, Span};
 use rustc_hash::FxHashMap;
 
-use crate::{context::LintContext, rule::Rule, AstNode};
+use crate::{context::LintContext, rule::Rule};
 
 #[derive(Debug, Error, Diagnostic)]
 #[error("eslint(no-dupe-class-members): Duplicate class member: {0:?}")]
@@ -53,76 +47,28 @@ declare_oxc_lint!(
 );
 
 impl Rule for NoDupeClassMembers {
-    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+    fn run_once(&self, ctx: &LintContext) {
-        let AstKind::Class(class) = node.kind() else {
+        ctx.semantic().classes().iter_enumerated().for_each(|(class_id, _)| {
-            return;
+            let mut defined_elements = FxHashMap::default();
-        };
+            let elements = &ctx.semantic().classes().elements[class_id];
-
+            for (element_id, element) in elements.iter_enumerated() {
-        let num_element = class.body.body.len();
+                if let Some(prev_element_id) = defined_elements.insert(&element.name, element_id) {
-        let mut property_table = PropertyTable::with_capacity(num_element);
+                    let prev_element = &elements[prev_element_id];
-        for element in &class.body.body {
+                    if element.r#static == prev_element.r#static
-            if ctx.source_type().is_typescript() && element.is_ts_empty_body_function() {
+                        && element.is_private == prev_element.is_private
-                // Skip functions with no function bodies, which are Typescript's overload signatures
+                        && (!(element.kind.is_setter_or_getter()
-                continue;
+                            && prev_element.kind.is_setter_or_getter())
+                            || element.kind == prev_element.kind)
+                    {
+                        ctx.diagnostic(NoDupeClassMembersDiagnostic(
+                            element.name.clone(),
+                            prev_element.span,
+                            element.span,
+                        ));
+                    }
+                }
             }
-
+        });
-            if let Some(dup_span) = property_table.insert(element) {
-                let property_name = element.property_key().unwrap().static_name().unwrap();
-                let cur_span = element.property_key().unwrap().span();
-                ctx.diagnostic(NoDupeClassMembersDiagnostic(property_name, dup_span, cur_span));
-            }
-        }
-    }
-}
-
-/// (static, name)
-type PropertyTableKey = (bool, Atom);
-/// (Definition kind, span of last declaration)
-type PropertyTableEntry = (Option<MethodDefinitionKind>, Span);
-/// Table to track whether a name is defined in static/non-static context as a getter/setter/normal class members
-/// Maps (static, name) -> (kind -> span of last declaration)
-#[derive(Debug, Clone, Default)]
-struct PropertyTable(FxHashMap<PropertyTableKey, Vec<PropertyTableEntry>>);
-
-impl PropertyTable {
-    /// Return the last duplicate span if element's property key is duplicate,
-    /// otherwise return None and insert the property key into the table.
-    pub fn insert(&mut self, element: &ClassElement) -> Option<Span> {
-        // It is valid to have a normal method named 'constructor'
-        if element.method_definition_kind() == Some(MethodDefinitionKind::Constructor) {
-            return None;
-        }
-
-        let Some((property_name, property_span)) =
-            element.property_key().and_then(|key| key.static_name().map(|name| (name, key.span())))
-        else {
-            return None;
-        };
-        let property_kind = element.method_definition_kind();
-
-        let key = (element.r#static(), property_name);
-        let entry = self.0.entry(key).or_default();
-        for (kind, span) in &*entry {
-            if Self::conflict(*kind, property_kind) {
-                return Some(*span);
-            }
-        }
-
-        entry.push((property_kind, property_span));
-        None
-    }
-
-    pub fn with_capacity(capacity: usize) -> Self {
-        Self(FxHashMap::with_capacity_and_hasher(capacity, BuildHasherDefault::default()))
-    }
-
-    fn conflict(kind: Option<MethodDefinitionKind>, other: Option<MethodDefinitionKind>) -> bool {
-        // getter and setter can share the same name
-        !matches!(
-            (kind, other),
-            (Some(MethodDefinitionKind::Get), Some(MethodDefinitionKind::Set))
-                | (Some(MethodDefinitionKind::Set), Some(MethodDefinitionKind::Get))
-        )
     }
 }
 

crates/oxc_linter/src/snapshots/no_dupe_class_members.snap

@@ -211,11 +211,11 @@ expression: no_dupe_class_members
   help: The last declaration overwrites previous ones, remove one of them or rename if both should be retained
 
   ⚠ eslint(no-dupe-class-members): Duplicate class member: "foo"
-   ╭─[no_dupe_class_members.tsx:1:11]
+   ╭─[no_dupe_class_members.tsx:1:20]
  1 │ class A { foo() {} foo() {} foo() {} }
-   ·           ─┬─               ─┬─
+   ·                    ─┬─      ─┬─
-   ·            │                 ╰── "foo" is re-declared here
+   ·                     │        ╰── "foo" is re-declared here
-   ·            ╰── "foo" is previously declared here
+   ·                     ╰── "foo" is previously declared here
    ╰────
   help: The last declaration overwrites previous ones, remove one of them or rename if both should be retained
 
@@ -301,11 +301,11 @@ expression: no_dupe_class_members
   help: The last declaration overwrites previous ones, remove one of them or rename if both should be retained
 
   ⚠ eslint(no-dupe-class-members): Duplicate class member: "foo"
-   ╭─[no_dupe_class_members.tsx:1:11]
+   ╭─[no_dupe_class_members.tsx:1:21]
  1 │ class A { foo() {}  foo() {}  foo() {}}
-   ·           ─┬─                 ─┬─
+   ·                     ─┬─       ─┬─
-   ·            │                   ╰── "foo" is re-declared here
+   ·                      │         ╰── "foo" is re-declared here
-   ·            ╰── "foo" is previously declared here
+   ·                      ╰── "foo" is previously declared here
    ╰────
   help: The last declaration overwrites previous ones, remove one of them or rename if both should be retained