diff --git a/crates/oxc_linter/src/rules/eslint/no_dupe_class_members.rs b/crates/oxc_linter/src/rules/eslint/no_dupe_class_members.rs index adae4c07f..e4504ccf4 100644 --- a/crates/oxc_linter/src/rules/eslint/no_dupe_class_members.rs +++ b/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>) { - let AstKind::Class(class) = node.kind() else { - return; - }; - - let num_element = class.body.body.len(); - let mut property_table = PropertyTable::with_capacity(num_element); - for element in &class.body.body { - if ctx.source_type().is_typescript() && element.is_ts_empty_body_function() { - // Skip functions with no function bodies, which are Typescript's overload signatures - continue; + fn run_once(&self, ctx: &LintContext) { + ctx.semantic().classes().iter_enumerated().for_each(|(class_id, _)| { + let mut defined_elements = FxHashMap::default(); + let elements = &ctx.semantic().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]; + if element.r#static == prev_element.r#static + && element.is_private == prev_element.is_private + && (!(element.kind.is_setter_or_getter() + && 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, 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>); - -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 { - // 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, other: Option) -> bool { - // getter and setter can share the same name - !matches!( - (kind, other), - (Some(MethodDefinitionKind::Get), Some(MethodDefinitionKind::Set)) - | (Some(MethodDefinitionKind::Set), Some(MethodDefinitionKind::Get)) - ) + }); } } diff --git a/crates/oxc_linter/src/snapshots/no_dupe_class_members.snap b/crates/oxc_linter/src/snapshots/no_dupe_class_members.snap index f6bde6ce7..e8e2c77c3 100644 --- a/crates/oxc_linter/src/snapshots/no_dupe_class_members.snap +++ b/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 previously declared here + · ─┬─ ─┬─ + · │ ╰── "foo" is re-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 previously declared here + · ─┬─ ─┬─ + · │ ╰── "foo" is re-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