Skip to content

Commit

Permalink
fix(linter): fix no_dupe_keys false postive on similar key names
Browse files Browse the repository at this point in the history
closes #2287
  • Loading branch information
Boshen committed Feb 4, 2024
1 parent 2f97b33 commit bb8b2c6
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 86 deletions.
1 change: 0 additions & 1 deletion crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ pub enum PropertyKey<'a> {
}

impl<'a> PropertyKey<'a> {
// FIXME: this would ideally return Option<&'a Atom> or a Cow
pub fn static_name(&self) -> Option<Atom> {
match self {
Self::Identifier(ident) => Some(ident.name.clone()),
Expand Down
24 changes: 1 addition & 23 deletions crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::hash::{Hash, Hasher};

use oxc_ast::AstKind;
use oxc_semantic::AstNode;
use oxc_span::{Atom, GetSpan, Span};
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator};
use rustc_hash::FxHasher;

Expand Down Expand Up @@ -264,28 +264,6 @@ pub fn outermost_paren_parent<'a, 'b>(
ctx.nodes().parent_node(node.id())
}

pub fn get_name_from_property_key(key: &PropertyKey<'_>) -> Option<Atom> {
match key {
PropertyKey::Identifier(ident) => Some(ident.name.clone()),
PropertyKey::PrivateIdentifier(ident) => {
let name = ident.name.clone();

Some(Atom::from(format!("#{name}")))
}
PropertyKey::Expression(expr) => match expr {
Expression::StringLiteral(lit) => Some(lit.value.clone()),
Expression::RegExpLiteral(lit) => Some(Atom::from(format!("{0}", lit.regex))),
Expression::NumberLiteral(lit) => Some(Atom::from(lit.raw)),
Expression::BigintLiteral(lit) => Some(Atom::from(format!("{0}", lit.value))),
Expression::NullLiteral(_) => Some("null".into()),
Expression::TemplateLiteral(lit) => {
lit.expressions.is_empty().then(|| lit.quasi()).flatten().cloned()
}
_ => None,
},
}
}

pub fn get_declaration_of_variable<'a, 'b>(
ident: &IdentifierReference,
ctx: &'b LintContext<'a>,
Expand Down
36 changes: 4 additions & 32 deletions crates/oxc_linter/src/rules/eslint/no_dupe_keys.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use lazy_static::lazy_static;
use oxc_ast::{
ast::{Expression, ObjectPropertyKind, PropertyKey, PropertyKind},
ast::{ObjectPropertyKind, PropertyKind},
AstKind,
};
use oxc_diagnostics::{
Expand All @@ -11,7 +10,7 @@ use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use rustc_hash::FxHashMap;

use crate::{ast_util::calculate_hash, context::LintContext, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint(no-dupe-keys): Disallow duplicate keys in object literals")]
Expand Down Expand Up @@ -47,8 +46,8 @@ impl Rule for NoDupeKeys {
let mut map = FxHashMap::default();
for prop in &obj_expr.properties {
let ObjectPropertyKind::ObjectProperty(prop) = prop else { continue };
let Some(hash) = calculate_property_kind_hash(&prop.key) else { continue };
if let Some((prev_kind, prev_span)) = map.insert(hash, (prop.kind, prop.key.span())) {
let Some(name) = prop.key.static_name() else { return };
if let Some((prev_kind, prev_span)) = map.insert(name, (prop.kind, prop.key.span())) {
if prev_kind == PropertyKind::Init
|| prop.kind == PropertyKind::Init
|| prev_kind == prop.kind
Expand All @@ -60,33 +59,6 @@ impl Rule for NoDupeKeys {
}
}

// todo: should this be located within oxc_ast?
fn calculate_property_kind_hash(key: &PropertyKey) -> Option<u64> {
lazy_static! {
static ref NULL_HASH: u64 = calculate_hash(&"null");
}

match key {
PropertyKey::Identifier(ident) => Some(calculate_hash(&ident)),
PropertyKey::PrivateIdentifier(_) => None,
PropertyKey::Expression(expr) => match expr {
Expression::StringLiteral(lit) => Some(calculate_hash(&lit.value)),
// note: hashes won't work as expected if these aren't strings. Save
// NumberLiteral I don't think this should be too much of a problem
// b/c most people don't use `null`, regexes, etc. as object
// property keys when writing real code.
Expression::RegExpLiteral(lit) => Some(calculate_hash(&lit.regex.to_string())),
Expression::NumberLiteral(lit) => Some(calculate_hash(&lit.value.to_string())),
Expression::BigintLiteral(lit) => Some(calculate_hash(&lit.value.to_string())),
Expression::NullLiteral(_) => Some(*NULL_HASH),
Expression::TemplateLiteral(lit) => {
lit.expressions.is_empty().then(|| lit.quasi()).flatten().map(calculate_hash)
}
_ => None,
},
}
}

#[test]
fn test() {
use crate::tester::Tester;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use oxc_diagnostics::{
use oxc_macros::declare_oxc_lint;
use oxc_span::{Atom, GetSpan, Span};

use crate::{ast_util::get_name_from_property_key, context::LintContext, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error(
Expand Down Expand Up @@ -125,15 +125,13 @@ trait GetMethod {
impl GetMethod for ClassElement<'_> {
fn get_method(&self) -> Option<Method> {
match self {
ClassElement::MethodDefinition(def) => {
get_name_from_property_key(&def.key).map(|name| Method {
name,
r#static: def.r#static,
call_signature: false,
kind: get_kind_from_key(&def.key),
span: Span::new(def.span.start, def.key.span().end),
})
}
ClassElement::MethodDefinition(def) => def.key.static_name().map(|name| Method {
name,
r#static: def.r#static,
call_signature: false,
kind: get_kind_from_key(&def.key),
span: Span::new(def.span.start, def.key.span().end),
}),
_ => None,
}
}
Expand All @@ -142,15 +140,13 @@ impl GetMethod for ClassElement<'_> {
impl GetMethod for TSSignature<'_> {
fn get_method(&self) -> Option<Method> {
match self {
TSSignature::TSMethodSignature(sig) => {
get_name_from_property_key(&sig.key).map(|name| Method {
name,
r#static: false,
call_signature: false,
kind: get_kind_from_key(&sig.key),
span: sig.key.span(),
})
}
TSSignature::TSMethodSignature(sig) => sig.key.static_name().map(|name| Method {
name,
r#static: false,
call_signature: false,
kind: get_kind_from_key(&sig.key),
span: sig.key.span(),
}),
TSSignature::TSCallSignatureDeclaration(sig) => Some(Method {
name: Atom::from("call"),
r#static: false,
Expand Down
11 changes: 0 additions & 11 deletions crates/oxc_linter/src/snapshots/adjacent_overload_signatures.snap
Original file line number Diff line number Diff line change
Expand Up @@ -359,17 +359,6 @@ expression: adjacent_overload_signatures
5static foo(sn: string | number): void {}
╰────

typescript-eslint(adjacent-overload-signatures): All "#private" signatures should be adjacent.
╭─[adjacent_overload_signatures.tsx:2:9]
1 │ class Test {
2 │ #private(): void;
· ────────
3'#private'(): void;
4 │ #private(arg: number): void {}
· ────────
5'#private'(arg: number): void {}
╰────

typescript-eslint(adjacent-overload-signatures): All "#private" signatures should be adjacent.
╭─[adjacent_overload_signatures.tsx:3:9]
2 │ #private(): void;
Expand Down

0 comments on commit bb8b2c6

Please sign in to comment.