From 7082c830d26de192e6e2b6d20d794d5636831ec3 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Mon, 14 Nov 2022 16:38:48 +0100 Subject: [PATCH] feat(rome_js_analyze): lint/correctness/noDupeKeys (#3562) Co-authored-by: Micha Reiser Resolves https://github.com/rome/tools/issues/3364 --- .../src/categories.rs | 9 +- .../rome_js_analyze/src/analyzers/nursery.rs | 3 +- .../src/analyzers/nursery/no_dupe_keys.rs | 311 +++++++++++ crates/rome_js_analyze/src/utils/batch.rs | 67 ++- crates/rome_js_analyze/src/utils/tests.rs | 76 ++- .../tests/specs/nursery/noDupeKeys.js | 43 ++ .../tests/specs/nursery/noDupeKeys.js.snap | 500 ++++++++++++++++++ .../src/configuration/linter/rules.rs | 4 +- editors/vscode/configuration_schema.json | 10 + npm/backend-jsonrpc/src/workspace.ts | 10 +- website/src/pages/lint/rules/index.mdx | 7 + website/src/pages/lint/rules/noDupeKeys.md | 108 ++++ 12 files changed, 1105 insertions(+), 43 deletions(-) create mode 100644 crates/rome_js_analyze/src/analyzers/nursery/no_dupe_keys.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js.snap create mode 100644 website/src/pages/lint/rules/noDupeKeys.md diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 05da9f00e40..88ec7330441 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -75,15 +75,16 @@ define_dategories! { // nursery - "lint/nursery/useFlatMap": "https://docs.rome.tools/lint/rules/useFlatMap", + "lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes", "lint/nursery/noConstAssign": "https://docs.rome.tools/lint/rules/noConstAssign", + "lint/nursery/noDupeKeys":"https://docs.rome.tools/lint/rules/noDupeKeys", "lint/nursery/noExplicitAny": "https://docs.rome.tools/lint/rules/noExplicitAny", - "lint/nursery/useValidForDirection": "https://docs.rome.tools/lint/rules/useValidForDirection", "lint/nursery/noInvalidConstructorSuper": "https://docs.rome.tools/lint/rules/noInvalidConstructorSuper", - "lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies", "lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase", - "lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes", + "lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies", + "lint/nursery/useFlatMap": "https://docs.rome.tools/lint/rules/useFlatMap", "lint/nursery/useNumericLiterals": "https://docs.rome.tools/lint/rules/useNumericLiterals", + "lint/nursery/useValidForDirection": "https://docs.rome.tools/lint/rules/useValidForDirection", ; diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs index 962f8f260f5..12989c2ac42 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery.rs @@ -2,9 +2,10 @@ use rome_analyze::declare_group; mod no_banned_types; +mod no_dupe_keys; mod no_explicit_any; mod no_invalid_constructor_super; mod use_flat_map; mod use_numeric_literals; mod use_valid_for_direction; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_banned_types :: NoBannedTypes , self :: no_explicit_any :: NoExplicitAny , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_banned_types :: NoBannedTypes , self :: no_dupe_keys :: NoDupeKeys , self :: no_explicit_any :: NoExplicitAny , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_dupe_keys.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_dupe_keys.rs new file mode 100644 index 00000000000..181e5dc0998 --- /dev/null +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_dupe_keys.rs @@ -0,0 +1,311 @@ +use crate::utils::batch::JsBatchMutation; +use rome_analyze::context::RuleContext; +use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_js_syntax::{ + JsAnyObjectMember, JsGetterObjectMember, JsObjectExpression, JsSetterObjectMember, +}; +use rome_js_syntax::{ + JsMethodObjectMember, JsPropertyObjectMember, JsShorthandPropertyObjectMember, TextRange, +}; +use rome_rowan::{AstNode, BatchMutationExt}; +use std::cmp::Ordering; +use std::collections::HashMap; +use std::fmt::Display; + +use crate::JsRuleAction; + +declare_rule! { + /// Prevents object literals having more than one property declaration for the same name. + /// If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored, which is likely a mistake. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// const obj = { + /// a: 1, + /// a: 2, + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// const obj = { + /// set a(v) {}, + /// a: 2, + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// const obj = { + /// a: 1, + /// b: 2, + /// } + /// ``` + /// + /// ```js + /// const obj = { + /// get a() { return 1; }, + /// set a(v) {}, + /// } + /// ``` + /// + pub(crate) NoDupeKeys { + version: "11.0.0", + name: "noDupeKeys", + recommended: false, // should be once out of nursery + } +} + +/// An object member defining a single object property. +enum MemberDefinition { + Getter(JsGetterObjectMember), + Setter(JsSetterObjectMember), + Method(JsMethodObjectMember), + Property(JsPropertyObjectMember), + ShorthandProperty(JsShorthandPropertyObjectMember), +} +impl MemberDefinition { + fn name(&self) -> Option { + match self { + MemberDefinition::Getter(getter) => { + getter.name().ok()?.as_js_literal_member_name()?.name().ok() + } + MemberDefinition::Setter(setter) => { + setter.name().ok()?.as_js_literal_member_name()?.name().ok() + } + MemberDefinition::Method(method) => { + method.name().ok()?.as_js_literal_member_name()?.name().ok() + } + MemberDefinition::Property(property) => property + .name() + .ok()? + .as_js_literal_member_name()? + .name() + .ok(), + MemberDefinition::ShorthandProperty(shorthand_property) => { + Some(shorthand_property.name().ok()?.text()) + } + } + } + + fn range(&self) -> TextRange { + match self { + MemberDefinition::Getter(getter) => getter.range(), + MemberDefinition::Setter(setter) => setter.range(), + MemberDefinition::Method(method) => method.range(), + MemberDefinition::Property(property) => property.range(), + MemberDefinition::ShorthandProperty(shorthand_property) => shorthand_property.range(), + } + } + + fn node(&self) -> JsAnyObjectMember { + match self { + MemberDefinition::Getter(getter) => JsAnyObjectMember::from(getter.clone()), + MemberDefinition::Setter(setter) => JsAnyObjectMember::from(setter.clone()), + MemberDefinition::Method(method) => JsAnyObjectMember::from(method.clone()), + MemberDefinition::Property(property) => JsAnyObjectMember::from(property.clone()), + MemberDefinition::ShorthandProperty(shorthand_property) => { + JsAnyObjectMember::from(shorthand_property.clone()) + } + } + } +} +impl Display for MemberDefinition { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Getter(_) => "getter", + Self::Setter(_) => "setter", + Self::Method(_) => "method", + Self::Property(_) => "property value", + Self::ShorthandProperty(_) => "shorthand property", + })?; + if let Some(name) = self.name() { + f.write_str(" named ")?; + f.write_str(&name)?; + } + Ok(()) + } +} +enum MemberDefinitionError { + NotASinglePropertyMember, + UnknownMemberType, +} +impl TryFrom for MemberDefinition { + type Error = MemberDefinitionError; + + fn try_from(member: JsAnyObjectMember) -> Result { + match member { + JsAnyObjectMember::JsGetterObjectMember(member) => Ok(MemberDefinition::Getter(member)), + JsAnyObjectMember::JsSetterObjectMember(member) => Ok(MemberDefinition::Setter(member)), + JsAnyObjectMember::JsMethodObjectMember(member) => Ok(MemberDefinition::Method(member)), + JsAnyObjectMember::JsPropertyObjectMember(member) => { + Ok(MemberDefinition::Property(member)) + } + JsAnyObjectMember::JsShorthandPropertyObjectMember(member) => { + Ok(MemberDefinition::ShorthandProperty(member)) + } + JsAnyObjectMember::JsSpread(_) => Err(MemberDefinitionError::NotASinglePropertyMember), + JsAnyObjectMember::JsUnknownMember(_) => Err(MemberDefinitionError::UnknownMemberType), + } + } +} + +/// A descriptor for a property that is, as far as we can tell from statically analyzing the object expression, +/// not overwritten by another object member and will make it into the object. +#[derive(Clone)] +enum DefinedProperty { + Get(TextRange), + Set(TextRange), + GetSet(TextRange, TextRange), + Value(TextRange), +} +impl From for DefinedProperty { + fn from(definition: MemberDefinition) -> Self { + match definition { + MemberDefinition::Getter(getter) => DefinedProperty::Get(getter.range()), + MemberDefinition::Setter(setter) => DefinedProperty::Set(setter.range()), + MemberDefinition::Method(method) => DefinedProperty::Value(method.range()), + MemberDefinition::Property(property) => DefinedProperty::Value(property.range()), + MemberDefinition::ShorthandProperty(shorthand_property) => { + DefinedProperty::Value(shorthand_property.range()) + } + } + } +} + +pub(crate) struct PropertyConflict(DefinedProperty, MemberDefinition); +impl DefinedProperty { + fn extend_with( + &self, + member_definition: MemberDefinition, + ) -> Result { + match (self, member_definition) { + // Add missing get/set counterpart + (DefinedProperty::Set(set_range), MemberDefinition::Getter(getter)) => { + Ok(DefinedProperty::GetSet(getter.range(), *set_range)) + } + + (DefinedProperty::Get(get_range), MemberDefinition::Setter(setter)) => { + Ok(DefinedProperty::GetSet(*get_range, setter.range())) + } + // Else conflict + (defined_property, member_definition) => Err(PropertyConflict( + defined_property.clone(), + member_definition, + )), + } + } +} + +impl Rule for NoDupeKeys { + type Query = Ast; + type State = PropertyConflict; + type Signals = Vec; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let node = ctx.query(); + + let mut defined_properties: HashMap = HashMap::new(); + let mut signals: Self::Signals = Vec::new(); + + for member_definition in node + .members() + .into_iter() + .flatten() + .filter_map(|member| MemberDefinition::try_from(member).ok()) + // Note that we iterate from last to first property, so that we highlight properties being overwritten as problems and not those that take effect. + .rev() + { + if let Some(member_name) = member_definition.name() { + match defined_properties.remove(&member_name) { + None => { + defined_properties + .insert(member_name, DefinedProperty::from(member_definition)); + } + Some(defined_property) => { + match defined_property.extend_with(member_definition) { + Ok(new_defined_property) => { + defined_properties.insert(member_name, new_defined_property); + } + Err(conflict) => { + signals.push(conflict); + defined_properties.insert(member_name, defined_property); + } + } + } + } + } + } + + signals + } + + fn diagnostic( + _ctx: &RuleContext, + PropertyConflict(defined_property, member_definition): &Self::State, + ) -> Option { + let mut diagnostic = RuleDiagnostic::new( + rule_category!(), + member_definition.range(), + format!( + "This {} is later overwritten by an object member with the same name.", + member_definition + ), + ); + diagnostic = match defined_property { + DefinedProperty::Get(range) => { + diagnostic.detail(range, "Overwritten with this getter.") + } + DefinedProperty::Set(range) => { + diagnostic.detail(range, "Overwritten with this setter.") + } + DefinedProperty::Value(range) => { + diagnostic.detail(range, "Overwritten with this value.") + } + DefinedProperty::GetSet(get_range, set_range) => match member_definition { + MemberDefinition::Getter(_) => { + diagnostic.detail(get_range, "Overwritten with this getter.") + } + MemberDefinition::Setter(_) => { + diagnostic.detail(set_range, "Overwritten with this setter.") + } + MemberDefinition::Method(_) + | MemberDefinition::Property(_) + | MemberDefinition::ShorthandProperty(_) => match get_range.ordering(*set_range) { + Ordering::Less => diagnostic.detail(set_range, "Overwritten with this setter."), + Ordering::Greater => { + diagnostic.detail(get_range, "Overwritten with this getter.") + } + Ordering::Equal => { + panic!( + "The ranges of the property getter and property setter cannot overlap." + ) + } + }, + }, + }; + diagnostic = diagnostic.note("If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored."); + + Some(diagnostic) + } + + fn action( + ctx: &RuleContext, + PropertyConflict(_, member_definition): &Self::State, + ) -> Option { + let mut batch = ctx.root().begin(); + batch.remove_js_object_member(&member_definition.node()); + Some(JsRuleAction { + category: rome_analyze::ActionCategory::QuickFix, + // The property initialization could contain side effects + applicability: rome_diagnostics::Applicability::MaybeIncorrect, + message: markup!("Remove this " {member_definition.to_string()}).to_owned(), + mutation: batch, + }) + } +} diff --git a/crates/rome_js_analyze/src/utils/batch.rs b/crates/rome_js_analyze/src/utils/batch.rs index 1476d507bdc..fc787c3c2a3 100644 --- a/crates/rome_js_analyze/src/utils/batch.rs +++ b/crates/rome_js_analyze/src/utils/batch.rs @@ -1,7 +1,8 @@ use rome_js_syntax::{ - JsAnyConstructorParameter, JsAnyFormalParameter, JsAnyParameter, JsConstructorParameterList, - JsFormalParameter, JsLanguage, JsParameterList, JsSyntaxKind, JsSyntaxNode, - JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList, JsVariableStatement, + JsAnyConstructorParameter, JsAnyFormalParameter, JsAnyObjectMember, JsAnyParameter, + JsConstructorParameterList, JsFormalParameter, JsLanguage, JsObjectMemberList, JsParameterList, + JsSyntaxKind, JsSyntaxNode, JsVariableDeclaration, JsVariableDeclarator, + JsVariableDeclaratorList, JsVariableStatement, }; use rome_rowan::{AstNode, AstSeparatedList, BatchMutation}; @@ -14,6 +15,10 @@ pub trait JsBatchMutation { /// Removes the parameter, and: /// 1 - removes commas around the parameter to keep the list valid. fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter) -> bool; + + /// Removes the object member, and: + /// 1 - removes commas around the member to keep the list valid. + fn remove_js_object_member(&mut self, parameter: &JsAnyObjectMember) -> bool; } fn remove_js_formal_parameter_from_js_parameter_list( @@ -34,7 +39,7 @@ fn remove_js_formal_parameter_from_js_parameter_list( node, )) if node == parameter => { batch.remove_node(node.clone()); - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { batch.remove_token(comma.clone()); } break; @@ -49,7 +54,7 @@ fn remove_js_formal_parameter_from_js_parameter_list( // removes the comma before this element if elements.next().is_none() { if let Some(element) = previous_element { - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { batch.remove_token(comma.clone()); } } @@ -76,7 +81,7 @@ fn remove_js_formal_parameter_from_js_constructor_parameter_list( JsAnyFormalParameter::JsFormalParameter(node), ) if node == parameter => { batch.remove_node(node.clone()); - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { batch.remove_token(comma.clone()); } break; @@ -91,7 +96,7 @@ fn remove_js_formal_parameter_from_js_constructor_parameter_list( // removes the comma before this element if elements.next().is_none() { if let Some(element) = previous_element { - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { batch.remove_token(comma.clone()); } } @@ -120,7 +125,7 @@ impl JsBatchMutation for BatchMutation { if let Ok(node) = element.node() { if node == declarator { self.remove_node(node.clone()); - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { self.remove_token(comma.clone()); } break; @@ -139,7 +144,7 @@ impl JsBatchMutation for BatchMutation { if remove_previous_element_comma { if let Some(element) = previous_element { - if let Some(comma) = element.trailing_separator().ok().flatten() { + if let Ok(Some(comma)) = element.trailing_separator() { self.remove_token(comma.clone()); } } @@ -168,14 +173,36 @@ impl JsBatchMutation for BatchMutation { }) .unwrap_or(false) } + + fn remove_js_object_member(&mut self, member: &JsAnyObjectMember) -> bool { + member + .syntax() + .parent() + .and_then(|parent| { + let parent = JsObjectMemberList::cast(parent)?; + for element in parent.elements() { + if element.node() == Ok(member) { + self.remove_node(member.clone()); + if let Ok(Some(comma)) = element.trailing_separator() { + self.remove_token(comma.clone()); + } + } + } + + Some(true) + }) + .unwrap_or(false) + } } #[cfg(test)] mod tests { use crate::assert_remove_ok; + use rome_js_syntax::{JsAnyObjectMember, JsFormalParameter, JsVariableDeclarator}; // Remove JsVariableDeclarator assert_remove_ok! { + JsVariableDeclarator, ok_remove_variable_declarator_single, "let a;", "", @@ -195,6 +222,7 @@ mod tests { // Remove JsFormalParameter from functions assert_remove_ok! { + JsFormalParameter, ok_remove_formal_parameter_single, "function f(a) {}", "function f() {}", @@ -214,6 +242,7 @@ mod tests { // Remove JsFormalParameter from class constructors assert_remove_ok! { + JsFormalParameter, ok_remove_formal_parameter_from_class_constructor_single, "class A { constructor(a) {} }", "class A { constructor() {} }", @@ -230,4 +259,24 @@ mod tests { "class A { constructor(b, a, c) {} }", "class A { constructor(b, c) {} }", } + + // Remove JsAnyObjectMember from object expression + assert_remove_ok! { + JsAnyObjectMember, + ok_remove_first_member, + "({ a: 1, b: 2 })", + "({ b: 2 })", + ok_remove_middle_member, + "({ z: 1, a: 2, b: 3 })", + "({ z: 1, b: 3 })", + ok_remove_last_member, + "({ z: 1, a: 2 })", + "({ z: 1, })", + ok_remove_last_member_trailing_comma, + "({ z: 1, a: 2, })", + "({ z: 1, })", + ok_remove_only_member, + "({ a: 2 })", + "({ })", + } } diff --git a/crates/rome_js_analyze/src/utils/tests.rs b/crates/rome_js_analyze/src/utils/tests.rs index 33011443fc6..921353d7c05 100644 --- a/crates/rome_js_analyze/src/utils/tests.rs +++ b/crates/rome_js_analyze/src/utils/tests.rs @@ -1,13 +1,19 @@ +use rome_js_syntax::JsSyntaxNode; +use std::{any::type_name, fmt::Debug}; + use super::rename::*; use crate::utils::batch::JsBatchMutation; use rome_diagnostics::file::FileId; use rome_js_semantic::{semantic_model, SemanticModelOptions}; -use rome_js_syntax::{JsFormalParameter, JsIdentifierBinding, JsVariableDeclarator, SourceType}; +use rome_js_syntax::{ + JsAnyObjectMember, JsFormalParameter, JsIdentifierBinding, JsLanguage, JsVariableDeclarator, + SourceType, +}; use rome_rowan::{AstNode, BatchMutationExt, SyntaxNodeCast}; /// Search and renames a binding named "a" to "b". /// Asserts the renaming worked. -pub fn assert_rename_ok(before: &str, expected: &str) { +pub fn assert_rename_binding_a_to_b_ok(before: &str, expected: &str) { let r = rome_js_parser::parse(before, FileId::zero(), SourceType::js_module()); let model = semantic_model(&r.tree(), SemanticModelOptions::default()); @@ -28,7 +34,7 @@ pub fn assert_rename_ok(before: &str, expected: &str) { /// Search and renames a binding named "a" to "b". /// Asserts the renaming to fail. -pub fn assert_rename_nok(before: &str) { +pub fn assert_rename_binding_a_to_b_nok(before: &str) { let r = rome_js_parser::parse(before, FileId::zero(), SourceType::js_module()); let model = semantic_model(&r.tree(), SemanticModelOptions::default()); @@ -43,28 +49,51 @@ pub fn assert_rename_nok(before: &str) { assert!(!batch.rename_node_declaration(&model, binding_a, "b")); } -/// Search a binding named "a" and remove it. +/// Search an identifier named "a" and remove the entire node of type Anc around it. /// Asserts the removal worked. -pub fn assert_remove_ok(before: &str, expected: &str) { +pub fn assert_remove_identifier_a_ok + Debug>( + before: &str, + expected: &str, +) { let r = rome_js_parser::parse(before, FileId::zero(), SourceType::js_module()); - let binding_a = r + let identifiers_a: Vec = r .syntax() .descendants() - .filter_map(|x| x.cast::()) - .find(|x| x.text() == "a") - .unwrap(); + .filter(|x| x.tokens().any(|token| token.text_trimmed() == "a")) + .collect(); + let node_to_remove = match identifiers_a.as_slice() { + [identifier_a] => identifier_a + .ancestors() + .find_map(|ancestor| ancestor.cast::()) + .unwrap_or_else(|| { + panic!( + "Trying to remove the {} ancestor of identifier a, but it has no such ancestor", + type_name::() + ) + }), + _ => panic!( + "Expected exactly one identifier named a, but got {:?}", + identifiers_a + ), + }; let mut batch = r.tree().begin(); - - let r = if let Some(parameter) = binding_a.parent::() { - batch.remove_js_formal_parameter(¶meter) - } else if let Some(declarator) = binding_a.parent::() { - batch.remove_js_variable_declarator(&declarator) - } else { - panic!("Don't know how to remove this node: {:?}", binding_a); - }; - assert!(r); + let batch_result = + if let Some(parameter) = node_to_remove.syntax().clone().cast::() { + batch.remove_js_formal_parameter(¶meter) + } else if let Some(declarator) = node_to_remove + .syntax() + .clone() + .cast::() + { + batch.remove_js_variable_declarator(&declarator) + } else if let Some(member) = node_to_remove.syntax().clone().cast::() { + batch.remove_js_object_member(&member) + } else { + panic!("Don't know how to remove this node: {:?}", node_to_remove); + }; + assert!(batch_result); let root = batch.commit(); let after = root.to_string(); @@ -77,7 +106,7 @@ macro_rules! assert_rename_ok { $( #[test] pub fn $name() { - $crate::utils::tests::assert_rename_ok($before, $expected); + $crate::utils::tests::assert_rename_binding_a_to_b_ok($before, $expected); } )* }; @@ -89,7 +118,7 @@ macro_rules! assert_rename_nok { $( #[test] pub fn $name() { - $crate::utils::tests::assert_rename_nok($before); + $crate::utils::tests::assert_rename_binding_a_to_b_nok($before); } )* }; @@ -97,11 +126,11 @@ macro_rules! assert_rename_nok { #[macro_export] macro_rules! assert_remove_ok { - ($(#[$attr:meta])* $($name:ident, $before:expr, $expected:expr,)*) => { + ($(#[$attr:meta])* $ancestor:ty, $($name:ident, $before:expr, $expected:expr,)*) => { $( #[test] pub fn $name() { - $crate::utils::tests::assert_remove_ok($before, $expected); + $crate::utils::tests::assert_remove_identifier_a_ok::<$ancestor>($before, $expected); } )* }; @@ -113,8 +142,7 @@ pub fn ok_find_attributes_by_name() { let list = r .syntax() .descendants() - .filter_map(rome_js_syntax::JsxAttributeList::cast) - .next() + .find_map(rome_js_syntax::JsxAttributeList::cast) .unwrap(); let [a, c, d] = list.find_by_names(["a", "c", "d"]); assert_eq!( diff --git a/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js b/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js new file mode 100644 index 00000000000..2537b483313 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js @@ -0,0 +1,43 @@ +// invalid + +({ a: 1, a: 2 }); +({ a: 1, a: 2, a: 3 }); +({ "": 1, "": 2 }); +({ z: 1, z: 2 }); +({ get a() {}, get a() {} }); +({ set a(v) {}, set a(v) {} }); +({ a: 1, get a() {} }); +({ a: 1, set a(v) {} }); +({ get a() {}, a: 1 }); +({ set a(v) {}, a: 1 }); +({ a: 1, get a() {}, set a(v) {} }); +({ get a() {}, a: 1, set a(v) {} }); +({ get a() {}, set a(v) {}, a: 1 }); + +// valid for now + +// ESLint already catches properties keyed with different-formatted number literals, we haven't implemented it yet. +({ 0x1: 1, 1: 2 }); +({ 012: 1, 10: 2 }); +({ 0b1: 1, 1: 2 }); +({ 0o1: 1, 1: 2 }); +({ 1n: 1, 1: 2 }); +({ 1_0: 1, 10: 2 }); + +// This particular simple computed property case with just a string literal would be easy to catch, +// but we don't want to open Pandora's static analysis box so we have to draw a line somewhere +({ a: 1, ["a"]: 1 }); + +// valid + +({ a: 1, b: 1 }); +({ "": 1, " ": 1 }); +({ 012: 1, 12: 1 }); +({ 1_0: 1, 1: 1 }); +({ a: 1, [a]: 1 }); +({ [a]: 1, [a]: 1 }); +({ get a() {}, set a(v) {} }); +({ a: 1, ...a }); +({ a: 1, b: { a: 1, b: 1 } }); +// Not object keys, so out of scope for this rule +var { a, a } = obj; diff --git a/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js.snap new file mode 100644 index 00000000000..8f9e49c03d0 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noDupeKeys.js.snap @@ -0,0 +1,500 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: noDupeKeys.js +--- +# Input +```js +// invalid + +({ a: 1, a: 2 }); +({ a: 1, a: 2, a: 3 }); +({ "": 1, "": 2 }); +({ z: 1, z: 2 }); +({ get a() {}, get a() {} }); +({ set a(v) {}, set a(v) {} }); +({ a: 1, get a() {} }); +({ a: 1, set a(v) {} }); +({ get a() {}, a: 1 }); +({ set a(v) {}, a: 1 }); +({ a: 1, get a() {}, set a(v) {} }); +({ get a() {}, a: 1, set a(v) {} }); +({ get a() {}, set a(v) {}, a: 1 }); + +// valid for now + +// ESLint already catches properties keyed with different-formatted number literals, we haven't implemented it yet. +({ 0x1: 1, 1: 2 }); +({ 012: 1, 10: 2 }); +({ 0b1: 1, 1: 2 }); +({ 0o1: 1, 1: 2 }); +({ 1n: 1, 1: 2 }); +({ 1_0: 1, 10: 2 }); + +// This particular simple computed property case with just a string literal would be easy to catch, +// but we don't want to open Pandora's static analysis box so we have to draw a line somewhere +({ a: 1, ["a"]: 1 }); + +// valid + +({ a: 1, b: 1 }); +({ "": 1, " ": 1 }); +({ 012: 1, 12: 1 }); +({ 1_0: 1, 1: 1 }); +({ a: 1, [a]: 1 }); +({ [a]: 1, [a]: 1 }); +({ get a() {}, set a(v) {} }); +({ a: 1, ...a }); +({ a: 1, b: { a: 1, b: 1 } }); +// Not object keys, so out of scope for this rule +var { a, a } = obj; + +``` + +# Diagnostics +``` +noDupeKeys.js:3:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 1 │ // invalid + 2 │ + > 3 │ ({ a: 1, a: 2 }); + │ ^^^^ + 4 │ ({ a: 1, a: 2, a: 3 }); + 5 │ ({ "": 1, "": 2 }); + + i Overwritten with this value. + + 1 │ // invalid + 2 │ + > 3 │ ({ a: 1, a: 2 }); + │ ^^^^ + 4 │ ({ a: 1, a: 2, a: 3 }); + 5 │ ({ "": 1, "": 2 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 3 │ ({·a:·1,·a:·2·}); + │ ------ + +``` + +``` +noDupeKeys.js:4:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 3 │ ({ a: 1, a: 2 }); + > 4 │ ({ a: 1, a: 2, a: 3 }); + │ ^^^^ + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + + i Overwritten with this value. + + 3 │ ({ a: 1, a: 2 }); + > 4 │ ({ a: 1, a: 2, a: 3 }); + │ ^^^^ + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 4 │ ({·a:·1,·a:·2,·a:·3·}); + │ ------ + +``` + +``` +noDupeKeys.js:4:10 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 3 │ ({ a: 1, a: 2 }); + > 4 │ ({ a: 1, a: 2, a: 3 }); + │ ^^^^ + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + + i Overwritten with this value. + + 3 │ ({ a: 1, a: 2 }); + > 4 │ ({ a: 1, a: 2, a: 3 }); + │ ^^^^ + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 4 │ ({·a:·1,·a:·2,·a:·3·}); + │ ------ + +``` + +``` +noDupeKeys.js:5:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named is later overwritten by an object member with the same name. + + 3 │ ({ a: 1, a: 2 }); + 4 │ ({ a: 1, a: 2, a: 3 }); + > 5 │ ({ "": 1, "": 2 }); + │ ^^^^^ + 6 │ ({ z: 1, z: 2 }); + 7 │ ({ get a() {}, get a() {} }); + + i Overwritten with this value. + + 3 │ ({ a: 1, a: 2 }); + 4 │ ({ a: 1, a: 2, a: 3 }); + > 5 │ ({ "": 1, "": 2 }); + │ ^^^^^ + 6 │ ({ z: 1, z: 2 }); + 7 │ ({ get a() {}, get a() {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named + + 5 │ ({·"":·1,·"":·2·}); + │ ------- + +``` + +``` +noDupeKeys.js:6:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named z is later overwritten by an object member with the same name. + + 4 │ ({ a: 1, a: 2, a: 3 }); + 5 │ ({ "": 1, "": 2 }); + > 6 │ ({ z: 1, z: 2 }); + │ ^^^^ + 7 │ ({ get a() {}, get a() {} }); + 8 │ ({ set a(v) {}, set a(v) {} }); + + i Overwritten with this value. + + 4 │ ({ a: 1, a: 2, a: 3 }); + 5 │ ({ "": 1, "": 2 }); + > 6 │ ({ z: 1, z: 2 }); + │ ^^^^ + 7 │ ({ get a() {}, get a() {} }); + 8 │ ({ set a(v) {}, set a(v) {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named z + + 6 │ ({·z:·1,·z:·2·}); + │ ------ + +``` + +``` +noDupeKeys.js:7:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This getter named a is later overwritten by an object member with the same name. + + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + > 7 │ ({ get a() {}, get a() {} }); + │ ^^^^^^^^^^ + 8 │ ({ set a(v) {}, set a(v) {} }); + 9 │ ({ a: 1, get a() {} }); + + i Overwritten with this getter. + + 5 │ ({ "": 1, "": 2 }); + 6 │ ({ z: 1, z: 2 }); + > 7 │ ({ get a() {}, get a() {} }); + │ ^^^^^^^^^^ + 8 │ ({ set a(v) {}, set a(v) {} }); + 9 │ ({ a: 1, get a() {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this getter named a + + 7 │ ({·get·a()·{},·get·a()·{}·}); + │ ------------ + +``` + +``` +noDupeKeys.js:8:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This setter named a is later overwritten by an object member with the same name. + + 6 │ ({ z: 1, z: 2 }); + 7 │ ({ get a() {}, get a() {} }); + > 8 │ ({ set a(v) {}, set a(v) {} }); + │ ^^^^^^^^^^^ + 9 │ ({ a: 1, get a() {} }); + 10 │ ({ a: 1, set a(v) {} }); + + i Overwritten with this setter. + + 6 │ ({ z: 1, z: 2 }); + 7 │ ({ get a() {}, get a() {} }); + > 8 │ ({ set a(v) {}, set a(v) {} }); + │ ^^^^^^^^^^^ + 9 │ ({ a: 1, get a() {} }); + 10 │ ({ a: 1, set a(v) {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this setter named a + + 8 │ ({·set·a(v)·{},·set·a(v)·{}·}); + │ ------------- + +``` + +``` +noDupeKeys.js:9:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 7 │ ({ get a() {}, get a() {} }); + 8 │ ({ set a(v) {}, set a(v) {} }); + > 9 │ ({ a: 1, get a() {} }); + │ ^^^^ + 10 │ ({ a: 1, set a(v) {} }); + 11 │ ({ get a() {}, a: 1 }); + + i Overwritten with this getter. + + 7 │ ({ get a() {}, get a() {} }); + 8 │ ({ set a(v) {}, set a(v) {} }); + > 9 │ ({ a: 1, get a() {} }); + │ ^^^^^^^^^^ + 10 │ ({ a: 1, set a(v) {} }); + 11 │ ({ get a() {}, a: 1 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 9 │ ({·a:·1,·get·a()·{}·}); + │ ------ + +``` + +``` +noDupeKeys.js:10:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 8 │ ({ set a(v) {}, set a(v) {} }); + 9 │ ({ a: 1, get a() {} }); + > 10 │ ({ a: 1, set a(v) {} }); + │ ^^^^ + 11 │ ({ get a() {}, a: 1 }); + 12 │ ({ set a(v) {}, a: 1 }); + + i Overwritten with this setter. + + 8 │ ({ set a(v) {}, set a(v) {} }); + 9 │ ({ a: 1, get a() {} }); + > 10 │ ({ a: 1, set a(v) {} }); + │ ^^^^^^^^^^^ + 11 │ ({ get a() {}, a: 1 }); + 12 │ ({ set a(v) {}, a: 1 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 10 │ ({·a:·1,·set·a(v)·{}·}); + │ ------ + +``` + +``` +noDupeKeys.js:11:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This getter named a is later overwritten by an object member with the same name. + + 9 │ ({ a: 1, get a() {} }); + 10 │ ({ a: 1, set a(v) {} }); + > 11 │ ({ get a() {}, a: 1 }); + │ ^^^^^^^^^^ + 12 │ ({ set a(v) {}, a: 1 }); + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + + i Overwritten with this value. + + 9 │ ({ a: 1, get a() {} }); + 10 │ ({ a: 1, set a(v) {} }); + > 11 │ ({ get a() {}, a: 1 }); + │ ^^^^ + 12 │ ({ set a(v) {}, a: 1 }); + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this getter named a + + 11 │ ({·get·a()·{},·a:·1·}); + │ ------------ + +``` + +``` +noDupeKeys.js:12:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This setter named a is later overwritten by an object member with the same name. + + 10 │ ({ a: 1, set a(v) {} }); + 11 │ ({ get a() {}, a: 1 }); + > 12 │ ({ set a(v) {}, a: 1 }); + │ ^^^^^^^^^^^ + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + + i Overwritten with this value. + + 10 │ ({ a: 1, set a(v) {} }); + 11 │ ({ get a() {}, a: 1 }); + > 12 │ ({ set a(v) {}, a: 1 }); + │ ^^^^ + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this setter named a + + 12 │ ({·set·a(v)·{},·a:·1·}); + │ ------------- + +``` + +``` +noDupeKeys.js:13:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 11 │ ({ get a() {}, a: 1 }); + 12 │ ({ set a(v) {}, a: 1 }); + > 13 │ ({ a: 1, get a() {}, set a(v) {} }); + │ ^^^^ + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + + i Overwritten with this setter. + + 11 │ ({ get a() {}, a: 1 }); + 12 │ ({ set a(v) {}, a: 1 }); + > 13 │ ({ a: 1, get a() {}, set a(v) {} }); + │ ^^^^^^^^^^^ + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 13 │ ({·a:·1,·get·a()·{},·set·a(v)·{}·}); + │ ------ + +``` + +``` +noDupeKeys.js:14:16 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This property value named a is later overwritten by an object member with the same name. + + 12 │ ({ set a(v) {}, a: 1 }); + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + > 14 │ ({ get a() {}, a: 1, set a(v) {} }); + │ ^^^^ + 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + 16 │ + + i Overwritten with this setter. + + 12 │ ({ set a(v) {}, a: 1 }); + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + > 14 │ ({ get a() {}, a: 1, set a(v) {} }); + │ ^^^^^^^^^^^ + 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + 16 │ + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this property value named a + + 14 │ ({·get·a()·{},·a:·1,·set·a(v)·{}·}); + │ ------ + +``` + +``` +noDupeKeys.js:15:4 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This getter named a is later overwritten by an object member with the same name. + + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + > 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + │ ^^^^^^^^^^ + 16 │ + 17 │ // valid for now + + i Overwritten with this value. + + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + > 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + │ ^^^^ + 16 │ + 17 │ // valid for now + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this getter named a + + 15 │ ({·get·a()·{},·set·a(v)·{},·a:·1·}); + │ ------------ + +``` + +``` +noDupeKeys.js:15:16 lint/nursery/noDupeKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This setter named a is later overwritten by an object member with the same name. + + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + > 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + │ ^^^^^^^^^^^ + 16 │ + 17 │ // valid for now + + i Overwritten with this value. + + 13 │ ({ a: 1, get a() {}, set a(v) {} }); + 14 │ ({ get a() {}, a: 1, set a(v) {} }); + > 15 │ ({ get a() {}, set a(v) {}, a: 1 }); + │ ^^^^ + 16 │ + 17 │ // valid for now + + i If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored. + + i Suggested fix: Remove this setter named a + + 15 │ ({·get·a()·{},·set·a(v)·{},·a:·1·}); + │ ------------- + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 818b9300e6e..e7cf365c81a 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -689,6 +689,7 @@ pub struct Nursery { struct NurserySchema { no_banned_types: Option, no_const_assign: Option, + no_dupe_keys: Option, no_explicit_any: Option, no_invalid_constructor_super: Option, use_camel_case: Option, @@ -699,9 +700,10 @@ struct NurserySchema { } impl Nursery { const CATEGORY_NAME: &'static str = "nursery"; - pub(crate) const CATEGORY_RULES: [&'static str; 9] = [ + pub(crate) const CATEGORY_RULES: [&'static str; 10] = [ "noBannedTypes", "noConstAssign", + "noDupeKeys", "noExplicitAny", "noInvalidConstructorSuper", "useCamelCase", diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 7095f75c1c6..546b4d5190d 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -704,6 +704,16 @@ } ] }, + "noDupeKeys": { + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "noExplicitAny": { "anyOf": [ { diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 7963f1bc254..b92068329aa 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -210,6 +210,7 @@ export interface Correctness { export interface Nursery { noBannedTypes?: RuleConfiguration; noConstAssign?: RuleConfiguration; + noDupeKeys?: RuleConfiguration; noExplicitAny?: RuleConfiguration; noInvalidConstructorSuper?: RuleConfiguration; /** @@ -386,15 +387,16 @@ export type Category = | "lint/a11y/useAltText" | "lint/security/noDangerouslySetInnerHtml" | "lint/security/noDangerouslySetInnerHtmlWithChildren" - | "lint/nursery/useFlatMap" + | "lint/nursery/noBannedTypes" | "lint/nursery/noConstAssign" + | "lint/nursery/noDupeKeys" | "lint/nursery/noExplicitAny" - | "lint/nursery/useValidForDirection" | "lint/nursery/noInvalidConstructorSuper" - | "lint/nursery/useExhaustiveDependencies" | "lint/nursery/useCamelCase" - | "lint/nursery/noBannedTypes" + | "lint/nursery/useExhaustiveDependencies" + | "lint/nursery/useFlatMap" | "lint/nursery/useNumericLiterals" + | "lint/nursery/useValidForDirection" | "files/missingHandler" | "format" | "internalError/io" diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index 719d4acd0bc..34114113579 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -454,6 +454,13 @@ Disallow certain types. Prevents from having const variables being re-assigned.
+

+ noDupeKeys +

+Prevents object literals having more than one property declaration for the same name. +If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored, which is likely a mistake. +
+

noExplicitAny

diff --git a/website/src/pages/lint/rules/noDupeKeys.md b/website/src/pages/lint/rules/noDupeKeys.md new file mode 100644 index 00000000000..dd9cc24121b --- /dev/null +++ b/website/src/pages/lint/rules/noDupeKeys.md @@ -0,0 +1,108 @@ +--- +title: Lint Rule noDupeKeys +parent: lint/rules/index +--- + +# noDupeKeys (since v11.0.0) + +Prevents object literals having more than one property declaration for the same name. +If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored, which is likely a mistake. + +## Examples + +### Invalid + +```jsx +const obj = { + a: 1, + a: 2, +} +``` + +
nursery/noDupeKeys.js:2:5 lint/nursery/noDupeKeys  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ⚠ This property value named a is later overwritten by an object member with the same name.
+  
+    1 │ const obj = {
+  > 2 │    	a: 1,
+      │    	^^^^
+    3 │    	a: 2,
+    4 │ }
+  
+  ℹ Overwritten with this value.
+  
+    1 │ const obj = {
+    2 │    	a: 1,
+  > 3 │    	a: 2,
+      │    	^^^^
+    4 │ }
+    5 │ 
+  
+  ℹ If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
+  
+  ℹ Suggested fix: Remove this property value named a
+  
+    1 1 │   const obj = {
+    2   │ - ···→ a:·1,
+    3   │ - ···→ a:·2,
+      2 │ + ···→ a:·2,
+    4 3 │   }
+    5 4 │   
+  
+
+ +```jsx +const obj = { + set a(v) {}, + a: 2, +} +``` + +
nursery/noDupeKeys.js:2:5 lint/nursery/noDupeKeys  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ⚠ This setter named a is later overwritten by an object member with the same name.
+  
+    1 │ const obj = {
+  > 2 │    	set a(v) {},
+      │    	^^^^^^^^^^^
+    3 │    	a: 2,
+    4 │ }
+  
+  ℹ Overwritten with this value.
+  
+    1 │ const obj = {
+    2 │    	set a(v) {},
+  > 3 │    	a: 2,
+      │    	^^^^
+    4 │ }
+    5 │ 
+  
+  ℹ If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
+  
+  ℹ Suggested fix: Remove this setter named a
+  
+    1 1 │   const obj = {
+    2   │ - ···→ set·a(v)·{},
+    3   │ - ···→ a:·2,
+      2 │ + ···→ a:·2,
+    4 3 │   }
+    5 4 │   
+  
+
+ +### Valid + +```jsx +const obj = { + a: 1, + b: 2, +} +``` + +```jsx +const obj = { + get a() { return 1; }, + set a(v) {}, +} +``` +