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

This commit is contained in:
Dunqing 2024-02-20 13:12:32 +08:00 committed by GitHub
parent 950298d960
commit 2a2bb2b7dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 31 additions and 85 deletions

View file

@ -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<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))
)
});
}
}

View file

@ -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