From 9360776bbf397189d3330d976131750e55358615 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Wed, 19 Jul 2023 13:31:49 +0200 Subject: [PATCH] fix(rome_js_analyze): fix a false positive for `noDuplicateCase` (#4709) --- CHANGELOG.md | 2 ++ .../analyzers/style/use_enum_initializers.rs | 7 ++--- .../nursery/use_naming_convention.rs | 12 +++------ crates/rome_js_analyze/src/utils.rs | 16 ++--------- .../specs/suspicious/noDuplicateCase/valid.js | 6 +++++ .../suspicious/noDuplicateCase/valid.js.snap | 7 ++++- crates/rome_js_syntax/src/lib.rs | 27 +++++++++++++++++++ 7 files changed, 50 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdb0eeefef4..6d68d5278fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,8 @@ if no error diagnostics are emitted. - Fix [`noInvalidConstructorSuper`](https://docs.rome.tools/lint/rules/noinvalidconstructorsuper/) rule that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624). +- Fix [`noDuplicateCase`](https://docs.rome.tools/lint/rules/noDuplicateCase/) rule that erroneously reported as equals the strings literals `"'"` and `'"'` [#4706](https://github.com/rome/tools/issues/4706). + - Relax [`noBannedTypes`](https://docs.rome.tools/lint/rules/nobannedtypes/) and improve documentation The rule no longer reports a user type that reuses a banned type name. diff --git a/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs b/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs index 2ac375f91ce..dcd2f48e02d 100644 --- a/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs +++ b/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs @@ -4,7 +4,9 @@ use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic}; use rome_console::markup; use rome_diagnostics::Applicability; use rome_js_factory::make; -use rome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumMember}; +use rome_js_syntax::{ + inner_text, AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumMember, +}; use rome_rowan::{AstNode, BatchMutationExt, Direction}; declare_rule! { @@ -118,8 +120,7 @@ impl Rule for UseEnumInitializers { } AnyJsLiteralExpression::JsStringLiteralExpression(expr) => { let prev_enum_delim_val = expr.value_token().ok()?; - let prev_enum_delim_val = prev_enum_delim_val.text(); - let prev_enum_val = &prev_enum_delim_val[1..(prev_enum_delim_val.len() - 1)]; + let prev_enum_val = inner_text(&prev_enum_delim_val); if prev_name.text() == prev_enum_val { let enum_name = enum_member.name().ok()?.text(); Some(AnyJsLiteralExpression::JsStringLiteralExpression( diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_naming_convention.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_naming_convention.rs index 48aba09f8dc..1e85b9d0e26 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_naming_convention.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_naming_convention.rs @@ -17,7 +17,7 @@ use rome_deserialize::{ use rome_diagnostics::Applicability; use rome_js_semantic::CanBeImportedExported; use rome_js_syntax::{ - binding_ext::AnyJsBindingDeclaration, AnyJsClassMember, AnyJsObjectMember, + binding_ext::AnyJsBindingDeclaration, inner_text, AnyJsClassMember, AnyJsObjectMember, AnyJsVariableDeclaration, AnyTsTypeMember, JsIdentifierBinding, JsLiteralExportName, JsLiteralMemberName, JsPrivateClassMemberName, JsSyntaxKind, JsSyntaxToken, JsVariableDeclarator, JsVariableKind, TsEnumMember, TsIdentifierBinding, TsTypeParameterName, @@ -290,10 +290,7 @@ impl Rule for UseNamingConvention { return None; } let name_token = node.name_token().ok()?; - let mut name = name_token.text_trimmed(); - if name_token.kind() == JsSyntaxKind::JS_STRING_LITERAL { - name = &name[1..name.len() - 1]; - } + let name = inner_text(&name_token); if !is_js_ident(name) { // ignore non-identifier strings return None; @@ -323,10 +320,7 @@ impl Rule for UseNamingConvention { suggested_name, } = state; let name_token = ctx.query().name_token().ok()?; - let mut name = name_token.text_trimmed(); - if name_token.kind() == JsSyntaxKind::JS_STRING_LITERAL { - name = &name[1..name.len() - 1]; - } + let name = inner_text(&name_token); let trimmed_name = trim_underscore_dollar(name); let allowed_cases = element.allowed_cases(ctx.options()); let allowed_case_names = allowed_cases diff --git a/crates/rome_js_analyze/src/utils.rs b/crates/rome_js_analyze/src/utils.rs index 4020a68aec8..b1d67aca1dc 100644 --- a/crates/rome_js_analyze/src/utils.rs +++ b/crates/rome_js_analyze/src/utils.rs @@ -1,6 +1,6 @@ use rome_js_factory::make; use rome_js_syntax::{ - AnyJsStatement, JsLanguage, JsModuleItemList, JsStatementList, JsSyntaxNode, JsSyntaxToken, + inner_text, AnyJsStatement, JsLanguage, JsModuleItemList, JsStatementList, JsSyntaxNode, JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList, JsVariableStatement, T, }; use rome_rowan::{AstNode, AstSeparatedList, BatchMutation, Direction, WalkEvent}; @@ -225,7 +225,7 @@ pub(crate) fn is_node_equal(a_node: &JsSyntaxNode, b_node: &JsSyntaxNode) -> boo (None, Some(_)) | (Some(_), None) => return false, // both are tokens (Some(a), Some(b)) => { - if !is_token_text_equal(a, b) { + if inner_text(a) != inner_text(b) { return false; } continue; @@ -236,18 +236,6 @@ pub(crate) fn is_node_equal(a_node: &JsSyntaxNode, b_node: &JsSyntaxNode) -> boo true } -/// Verify that tokens' inner text are equal -fn is_token_text_equal(a: &JsSyntaxToken, b: &JsSyntaxToken) -> bool { - static QUOTES: [char; 2] = ['"', '\'']; - - a.token_text_trimmed() - .trim_start_matches(QUOTES) - .trim_end_matches(QUOTES) - == b.token_text_trimmed() - .trim_start_matches(QUOTES) - .trim_end_matches(QUOTES) -} - #[test] fn ok_to_camel_case() { assert_eq!(to_camel_case("camelCase"), Cow::Borrowed("camelCase")); diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js index 6a6ae6f0fef..e7a4357fdee 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js @@ -113,3 +113,9 @@ switch (a) { case toString: break; } +switch (a) { + case "'": + return '''; + case '"': + return '"'; +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js.snap b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js.snap index eaaf04e1d3f..b663c3a3ef6 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateCase/valid.js.snap @@ -119,7 +119,12 @@ switch (a) { case toString: break; } - +switch (a) { + case "'": + return '''; + case '"': + return '"'; +} ``` diff --git a/crates/rome_js_syntax/src/lib.rs b/crates/rome_js_syntax/src/lib.rs index 1ee6942ef45..3d1b14de746 100644 --- a/crates/rome_js_syntax/src/lib.rs +++ b/crates/rome_js_syntax/src/lib.rs @@ -272,3 +272,30 @@ impl OperatorPrecedence { matches!(self, OperatorPrecedence::Exponential) } } + +/// Similar to `JsSyntaxToken::text_trimmed` with the difference that delimiters of string literals are trimmed. +/// +/// ## Examples +/// +/// ``` +/// use rome_js_syntax::{JsSyntaxKind, JsSyntaxToken, inner_text}; +/// +/// let a = JsSyntaxToken::new_detached(JsSyntaxKind::JS_STRING_LITERAL, "'inner_text'", [], []); +/// let b = JsSyntaxToken::new_detached(JsSyntaxKind::JS_STRING_LITERAL, "\"inner_text\"", [], []); +/// assert_eq!(inner_text(&a), inner_text(&b)); +/// +/// let a = JsSyntaxToken::new_detached(JsSyntaxKind::LET_KW, "let", [], []); +/// let b = JsSyntaxToken::new_detached(JsSyntaxKind::LET_KW, "let", [], []); +/// assert_eq!(inner_text(&a), inner_text(&b)); +/// +/// let a = JsSyntaxToken::new_detached(JsSyntaxKind::LET_KW, "let", [], []); +/// let b = JsSyntaxToken::new_detached(JsSyntaxKind::CONST_KW, "const", [], []); +/// assert!(inner_text(&a) != inner_text(&b)); +/// ``` +pub fn inner_text(token: &JsSyntaxToken) -> &str { + let mut result = token.text_trimmed(); + if token.kind() == JsSyntaxKind::JS_STRING_LITERAL { + result = &result[1..result.len() - 1]; + } + result +}