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

refactor(rome_js_analyze): handle string literal names in useNamingCo… #4698

Merged
merged 1 commit into from
Jul 14, 2023
Merged

refactor(rome_js_analyze): handle string literal names in useNamingCo… #4698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rome_js_syntax::{
AnyJsComputedMember, AnyJsExpression, AnyJsLiteralExpression, AnyJsName, JsComputedMemberName,
JsLiteralMemberName, JsSyntaxKind, T,
};
use rome_js_unicode_table::{is_id_continue, is_id_start};
use rome_js_unicode_table::is_js_ident;
use rome_rowan::{declare_node_union, AstNode, BatchMutationExt, TextRange};

declare_rule! {
Expand Down Expand Up @@ -153,15 +153,3 @@ impl Rule for UseLiteralKeys {
declare_node_union! {
pub(crate) AnyJsMember = AnyJsComputedMember | JsLiteralMemberName | JsComputedMemberName
}

// This function check if the key is valid JavaScript identifier.
// Currently, it doesn't check escaped unicode chars.
fn is_js_ident(key: &str) -> bool {
key.chars().enumerate().all(|(index, c)| {
if index == 0 {
is_id_start(c)
} else {
is_id_continue(c)
}
})
}
19 changes: 13 additions & 6 deletions crates/rome_js_analyze/src/semantic_analyzers/nursery/use_naming_convention.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use rome_js_syntax::{
JsLiteralMemberName, JsPrivateClassMemberName, JsSyntaxKind, JsSyntaxToken,
JsVariableDeclarator, JsVariableKind, TsEnumMember, TsIdentifierBinding, TsTypeParameterName,
};
use rome_js_unicode_table::is_js_ident;
use rome_json_syntax::JsonLanguage;
use rome_rowan::{
declare_node_union, AstNode, AstNodeList, BatchMutationExt, SyntaxNode, SyntaxResult,
Expand Down Expand Up @@ -289,7 +290,14 @@ impl Rule for UseNamingConvention {
return None;
}
let name_token = node.name_token().ok()?;
let name = name_token.text_trimmed();
let mut name = name_token.text_trimmed();
if name_token.kind() == JsSyntaxKind::JS_STRING_LITERAL {
name = &name[1..name.len() - 1];
}
if !is_js_ident(name) {
// ignore non-identifier strings
return None;
}
let trimmed_name = trim_underscore_dollar(name);
let actual_case = Case::identify(trimmed_name, options.strict_case);
if trimmed_name.is_empty()
Expand All @@ -315,7 +323,10 @@ impl Rule for UseNamingConvention {
suggested_name,
} = state;
let name_token = ctx.query().name_token().ok()?;
let name = name_token.text_trimmed();
let mut name = name_token.text_trimmed();
if name_token.kind() == JsSyntaxKind::JS_STRING_LITERAL {
name = &name[1..name.len() - 1];
}
let trimmed_name = trim_underscore_dollar(name);
let allowed_cases = element.allowed_cases(ctx.options());
let allowed_case_names = allowed_cases
Expand Down Expand Up @@ -622,10 +633,6 @@ impl Named {
Named::from_binding_declaration(&binding.declaration()?)
}
AnyName::JsLiteralMemberName(member_name) => {
// ignore quoted names
if member_name.value().ok()?.kind() == JsSyntaxKind::JS_STRING_LITERAL {
return None;
}
if let Some(member) = member_name.parent::<AnyJsClassMember>() {
Named::from_class_member(&member)
} else if let Some(member) = member_name.parent::<AnyTsTypeMember>() {
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useNamingConvention/invalidClassProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
export default class {
X

"Y" = 0

#X

Initialized = 0
Expand Down
131 changes: 75 additions & 56 deletions crates/rome_js_analyze/tests/specs/nursery/useNamingConvention/invalidClassProperty.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ expression: invalidClassProperty.js
export default class {
X

"Y" = 0

#X

Initialized = 0
Expand Down Expand Up @@ -45,7 +47,7 @@ invalidClassProperty.js:2:5 lint/nursery/useNamingConvention ━━━━━━
> 2 │ X
│ ^
3 │
4 │ #X
4 │ "Y" = 0

i The name could be renamed to `x`.

Expand All @@ -59,12 +61,12 @@ invalidClassProperty.js:4:5 lint/nursery/useNamingConvention ━━━━━━

2 │ X
3 │
> 4 │ #X
│ ^^
> 4 │ "Y" = 0
│ ^^^
5 │
6 │ Initialized = 0
6 │ #X

i The name could be renamed to `x`.
i The name could be renamed to `y`.


```
Expand All @@ -74,14 +76,14 @@ invalidClassProperty.js:6:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

4 │ #X
4 │ "Y" = 0
5 │
> 6 │ Initialized = 0
│ ^^^^^^^^^^^
> 6 │ #X
│ ^^
7 │
8 │ #Initialized = 0
8 │ Initialized = 0

i The name could be renamed to `initialized`.
i The name could be renamed to `x`.


```
Expand All @@ -91,12 +93,12 @@ invalidClassProperty.js:8:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

6 │ Initialized = 0
6 │ #X
7 │
> 8 │ #Initialized = 0
│ ^^^^^^^^^^^^
> 8 │ Initialized = 0
│ ^^^^^^^^^^^
9 │
10 │ PROPERTY
10 │ #Initialized = 0

i The name could be renamed to `initialized`.

Expand All @@ -108,14 +110,14 @@ invalidClassProperty.js:10:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

8 │ #Initialized = 0
8 │ Initialized = 0
9 │
> 10 │ PROPERTY
│ ^^^^^^^^
> 10 │ #Initialized = 0
│ ^^^^^^^^^^^^
11 │
12 │ #PROPERTY
12 │ PROPERTY

i The name could be renamed to `property`.
i The name could be renamed to `initialized`.


```
Expand All @@ -125,12 +127,12 @@ invalidClassProperty.js:12:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

10 │ PROPERTY
10 │ #Initialized = 0
11 │
> 12 │ #PROPERTY
│ ^^^^^^^^^
> 12 │ PROPERTY
│ ^^^^^^^^
13 │
14 │ SpecialProperty
14 │ #PROPERTY

i The name could be renamed to `property`.

Expand All @@ -142,14 +144,14 @@ invalidClassProperty.js:14:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

12 │ #PROPERTY
12 │ PROPERTY
13 │
> 14 │ SpecialProperty
│ ^^^^^^^^^^^^^^^
> 14 │ #PROPERTY
│ ^^^^^^^^^
15 │
16 │ #SpecialProperty
16 │ SpecialProperty

i The name could be renamed to `specialProperty`.
i The name could be renamed to `property`.


```
Expand All @@ -159,12 +161,12 @@ invalidClassProperty.js:16:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

14 │ SpecialProperty
14 │ #PROPERTY
15 │
> 16 │ #SpecialProperty
│ ^^^^^^^^^^^^^^^^
> 16 │ SpecialProperty
│ ^^^^^^^^^^^^^^^
17 │
18 │ special_property
18 │ #SpecialProperty

i The name could be renamed to `specialProperty`.

Expand All @@ -176,12 +178,12 @@ invalidClassProperty.js:18:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

16 │ #SpecialProperty
16 │ SpecialProperty
17 │
> 18 │ special_property
> 18 │ #SpecialProperty
│ ^^^^^^^^^^^^^^^^
19 │
20 │ #special_property
20 │ special_property

i The name could be renamed to `specialProperty`.

Expand All @@ -193,12 +195,12 @@ invalidClassProperty.js:20:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

18 │ special_property
18 │ #SpecialProperty
19 │
> 20 │ #special_property
│ ^^^^^^^^^^^^^^^^^
> 20 │ special_property
│ ^^^^^^^^^^^^^^^^
21 │
22 │ Unknown_Style
22 │ #special_property

i The name could be renamed to `specialProperty`.

Expand All @@ -210,14 +212,14 @@ invalidClassProperty.js:22:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

20 │ #special_property
20 │ special_property
21 │
> 22 │ Unknown_Style
│ ^^^^^^^^^^^^^
> 22 │ #special_property
│ ^^^^^^^^^^^^^^^^^
23 │
24 │ #Unknown_Style
24 │ Unknown_Style

i The name could be renamed to `unknownStyle`.
i The name could be renamed to `specialProperty`.


```
Expand All @@ -227,12 +229,12 @@ invalidClassProperty.js:24:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

22 │ Unknown_Style
22 │ #special_property
23 │
> 24 │ #Unknown_Style
│ ^^^^^^^^^^^^^^
> 24 │ Unknown_Style
│ ^^^^^^^^^^^^^
25 │
26 │ Unknown_Init_Style = 0
26 │ #Unknown_Style

i The name could be renamed to `unknownStyle`.

Expand All @@ -244,14 +246,14 @@ invalidClassProperty.js:26:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

24 │ #Unknown_Style
24 │ Unknown_Style
25 │
> 26 │ Unknown_Init_Style = 0
│ ^^^^^^^^^^^^^^^^^^
> 26 │ #Unknown_Style
│ ^^^^^^^^^^^^^^
27 │
28 │ #Unknown_Init_Style = 0
28 │ Unknown_Init_Style = 0

i The name could be renamed to `unknownInitStyle`.
i The name could be renamed to `unknownStyle`.


```
Expand All @@ -261,11 +263,28 @@ invalidClassProperty.js:28:5 lint/nursery/useNamingConvention ━━━━━━

! This class property name should be in camelCase.

26 │ Unknown_Init_Style = 0
26 │ #Unknown_Style
27 │
> 28 │ #Unknown_Init_Style = 0
> 28 │ Unknown_Init_Style = 0
│ ^^^^^^^^^^^^^^^^^^
29 │
30 │ #Unknown_Init_Style = 0

i The name could be renamed to `unknownInitStyle`.


```

```
invalidClassProperty.js:30:5 lint/nursery/useNamingConvention ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This class property name should be in camelCase.

28 │ Unknown_Init_Style = 0
29 │
> 30 │ #Unknown_Init_Style = 0
│ ^^^^^^^^^^^^^^^^^^^
29 │ }
31 │ }

i The name could be renamed to `unknownInitStyle`.

Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useNamingConvention/validClassProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
export default class {
p

"q" = 0

#p

initialized = 0
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useNamingConvention/validClassProperty.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ expression: validClassProperty.js
export default class {
p

"q" = 0

#p

initialized = 0
Expand Down
24 changes: 24 additions & 0 deletions crates/rome_js_unicode_table/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ pub fn is_id_continue(c: char) -> bool {
c == '$' || c == '\u{200d}' || c == '\u{200c}' || ID_Continue(c)
}

/// Check if `s` is a valid _JavaScript_ identifier.
/// Currently, it doesn't check escaped unicode chars.
///
/// ```
/// use rome_js_unicode_table::is_js_ident;
///
/// assert!(is_js_ident("id0"));
/// assert!(is_js_ident("$id$"));
/// assert!(is_js_ident("_id_"));
///
/// assert!(!is_js_ident("@"));
/// assert!(!is_js_ident("custom-id"));
/// assert!(!is_js_ident("0"));
/// ```
pub fn is_js_ident(s: &str) -> bool {
s.chars().enumerate().all(|(index, c)| {
if index == 0 {
is_id_start(c)
} else {
is_id_continue(c)
}
})
}

/// Looks up a byte in the lookup table.
#[inline]
pub fn lookup_byte(byte: u8) -> Dispatch {
Expand Down