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

Commit

Permalink
refactor(rome_js_analyze): handle string literal names in useNamingCo…
Browse files Browse the repository at this point in the history 
…nvention
  • Loading branch information
@Conaclos
Conaclos committed Jul 14, 2023
1 parent 3c251f7 commit 146f5d5
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 75 deletions.
14 changes: 1 addition & 13 deletions crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs
View file
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
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
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
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
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
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
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

0 comments on commit 146f5d5

Please sign in to comment.