Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_analyze): fix a false positive for noDuplicateCase (#4709)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored Jul 19, 2023
1 parent 4c8cf32 commit 9360776
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
16 changes: 2 additions & 14 deletions crates/rome_js_analyze/src/utils.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
Expand All @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,9 @@ switch (a) {
case toString:
break;
}
switch (a) {
case "'":
return ''';
case '"':
return '"';
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,12 @@ switch (a) {
case toString:
break;
}

switch (a) {
case "'":
return ''';
case '"':
return '"';
}
```


27 changes: 27 additions & 0 deletions crates/rome_js_syntax/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 9360776

Please sign in to comment.