diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e66a4563c5..d8a57651d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -207,6 +207,28 @@ if no error diagnostics are emitted. var x = a => 1 ? 2 : 3; ``` +- Improve [useLiteralKeys](https://docs.rome.tools/lint/rules/useLiteralKeys/). + + Now, the rule suggests simplifying computed properties to string literal properties: + + ```diff + { + - ["1+1"]: 2, + + "1+1": 2, + } + ``` + + It also suggests simplifying string literal properties to static properties: + + ```diff + { + - "a": 0, + + a: 0, + } + ``` + + These suggestions are made in object literals, classes, interfaces, and object types. + - The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different shape of options Old configuration diff --git a/crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs b/crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs index 86ba0257553..0aa18d0efc6 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/use_literal_keys.rs @@ -3,14 +3,14 @@ use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule use rome_console::markup; use rome_diagnostics::Applicability; use rome_js_factory::make::{ - ident, js_literal_member_name, js_name, js_static_member_expression, token, + self, ident, js_literal_member_name, js_name, js_static_member_expression, token, }; use rome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsName, JsComputedMemberAssignment, - JsComputedMemberExpression, JsComputedMemberName, T, + AnyJsComputedMember, AnyJsExpression, AnyJsLiteralExpression, AnyJsName, JsComputedMemberName, + JsLiteralMemberName, JsSyntaxKind, T, }; use rome_js_unicode_table::{is_id_continue, is_id_start}; -use rome_rowan::{declare_node_union, AstNode, BatchMutationExt, SyntaxResult, TextRange}; +use rome_rowan::{declare_node_union, AstNode, BatchMutationExt, TextRange}; declare_rule! { /// Enforce the usage of a literal access to properties over computed property access. @@ -51,52 +51,34 @@ declare_rule! { } } -declare_node_union! { - pub(crate) AnyJsComputedMember = AnyJsCompputedExpression | JsComputedMemberName -} - -declare_node_union! { - pub(crate) AnyJsCompputedExpression = JsComputedMemberExpression | JsComputedMemberAssignment -} - -impl AnyJsCompputedExpression { - pub(crate) fn member(&self) -> SyntaxResult { - match self { - AnyJsCompputedExpression::JsComputedMemberExpression(node) => node.member(), - AnyJsCompputedExpression::JsComputedMemberAssignment(node) => node.member(), - } - } - - pub(crate) fn object(&self) -> SyntaxResult { - match self { - AnyJsCompputedExpression::JsComputedMemberExpression(node) => node.object(), - AnyJsCompputedExpression::JsComputedMemberAssignment(node) => node.object(), - } - } -} - impl Rule for UseLiteralKeys { - type Query = Ast; + type Query = Ast; type State = (TextRange, String); type Signals = Option; type Options = (); fn run(ctx: &RuleContext) -> Self::Signals { - let computed_expression = ctx.query(); - - let inner_expression = match computed_expression { - AnyJsComputedMember::AnyJsCompputedExpression(computed_expression) => { - computed_expression.member().ok()? + let node = ctx.query(); + let inner_expression = match node { + AnyJsMember::AnyJsComputedMember(computed_member) => computed_member.member().ok()?, + AnyJsMember::JsLiteralMemberName(member) => { + if member.value().ok()?.kind() == JsSyntaxKind::JS_STRING_LITERAL { + let name = member.name().ok()?; + if is_js_ident(&name) { + return Some((member.range(), name)); + } + } + return None; } - AnyJsComputedMember::JsComputedMemberName(member) => member.expression().ok()?, + AnyJsMember::JsComputedMemberName(member) => member.expression().ok()?, }; - match inner_expression { AnyJsExpression::AnyJsLiteralExpression( AnyJsLiteralExpression::JsStringLiteralExpression(string_literal), ) => { let value = string_literal.inner_string_text().ok()?; - if can_convert_to_static_member(&value) { + // A computed property `["something"]` can always be simplified to a string literal "something". + if matches!(node, AnyJsMember::JsComputedMemberName(_)) || is_js_ident(&value) { return Some((string_literal.range(), value.to_string())); } } @@ -104,17 +86,15 @@ impl Rule for UseLiteralKeys { let mut value = String::new(); for element in template_expression.elements() { let chunk = element.as_js_template_chunk_element()?; - value.push_str(chunk.template_chunk_token().ok()?.text_trimmed()); } - if can_convert_to_static_member(&value) { + // A computed property ``[`something`]`` can always be simplified to a string literal "something". + if matches!(node, AnyJsMember::JsComputedMemberName(_)) || is_js_ident(&value) { return Some((template_expression.range(), value)); } } - _ => {} } - None } @@ -130,11 +110,9 @@ impl Rule for UseLiteralKeys { fn action(ctx: &RuleContext, (_, identifier): &Self::State) -> Option { let node = ctx.query(); - let mut mutation = ctx.root().begin(); - match node { - AnyJsComputedMember::AnyJsCompputedExpression(node) => { + AnyJsMember::AnyJsComputedMember(node) => { let object = node.object().ok()?; let member = js_name(ident(identifier)); let static_expression = @@ -143,39 +121,42 @@ impl Rule for UseLiteralKeys { node.clone().into_syntax().into(), static_expression.into_syntax().into(), ); - Some(JsRuleAction { - mutation, - applicability: Applicability::MaybeIncorrect, - category: ActionCategory::QuickFix, - message: markup! { - "Replace it with a static expression." - } - .to_owned(), - }) } - AnyJsComputedMember::JsComputedMemberName(member) => { - let literal_member_name = js_literal_member_name(ident(identifier)); + AnyJsMember::JsLiteralMemberName(node) => { + mutation.replace_token(node.value().ok()?, make::ident(identifier)); + } + AnyJsMember::JsComputedMemberName(member) => { + let name_token = if is_js_ident(identifier) { + make::ident(identifier) + } else { + make::js_string_literal(identifier) + }; + let literal_member_name = js_literal_member_name(name_token); mutation.replace_element( member.clone().into_syntax().into(), literal_member_name.into_syntax().into(), ); - Some(JsRuleAction { - mutation, - applicability: Applicability::MaybeIncorrect, - category: ActionCategory::QuickFix, - message: markup! { - "Replace it with a static expression." - } - .to_owned(), - }) } } + Some(JsRuleAction { + mutation, + applicability: Applicability::MaybeIncorrect, + category: ActionCategory::QuickFix, + message: markup! { + "Use a literal key instead." + } + .to_owned(), + }) } } +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 can_convert_to_static_member(key: &str) -> bool { +fn is_js_ident(key: &str) -> bool { key.chars().enumerate().all(|(index, c)| { if index == 0 { is_id_start(c) diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js index 13dc6732b47..b6988492ff6 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js @@ -14,5 +14,22 @@ a = { a = { [`b`]: d }; +a = { + "b": d +}; a.b[`$c`]; a.b["_d"]; +class C { ["a"] = 0 } +class C { "a" = 0 } +class C { ["a"](){} } +class C { "a"(){} } +class C { get ["a"](){} } +class C { get "a"(){} } +class C { set ["a"](x){} } +class C { set "a"(x){} } +a = { + ["1+1"]: 2 +} +a = { + [`1+1`]: 2 +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js.snap index a6e5ffda9c2..51d74210997 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 103 expression: invalid.js --- # Input @@ -20,9 +21,25 @@ a = { a = { [`b`]: d }; +a = { + "b": d +}; a.b[`$c`]; a.b["_d"]; - +class C { ["a"] = 0 } +class C { "a" = 0 } +class C { ["a"](){} } +class C { "a"(){} } +class C { get ["a"](){} } +class C { get "a"(){} } +class C { set ["a"](x){} } +class C { set "a"(x){} } +a = { + ["1+1"]: 2 +} +a = { + [`1+1`]: 2 +} ``` # Diagnostics @@ -36,7 +53,7 @@ invalid.js:1:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 2 │ a.b["c"]; 3 │ a.b["c"].d.e["f"]; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 1 │ - a["b"]; 1 │ + a.b; @@ -57,7 +74,7 @@ invalid.js:2:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 3 │ a.b["c"].d.e["f"]; 4 │ a.b[`c`]; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 1 1 │ a["b"]; 2 │ - a.b["c"]; @@ -80,7 +97,7 @@ invalid.js:3:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 4 │ a.b[`c`]; 5 │ a.b[c["d"]]; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 1 1 │ a["b"]; 2 2 │ a.b["c"]; @@ -104,7 +121,7 @@ invalid.js:3:14 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ 4 │ a.b[`c`]; 5 │ a.b[c["d"]]; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 1 1 │ a["b"]; 2 2 │ a.b["c"]; @@ -128,7 +145,7 @@ invalid.js:4:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 5 │ a.b[c["d"]]; 6 │ a["b"] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 2 2 │ a.b["c"]; 3 3 │ a.b["c"].d.e["f"]; @@ -152,7 +169,7 @@ invalid.js:5:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 6 │ a["b"] = "something"; 7 │ a.b["c"] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 3 3 │ a.b["c"].d.e["f"]; 4 4 │ a.b[`c`]; @@ -176,7 +193,7 @@ invalid.js:6:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 7 │ a.b["c"] = "something"; 8 │ a.b["c"].d.e["f"] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 4 4 │ a.b[`c`]; 5 5 │ a.b[c["d"]]; @@ -200,7 +217,7 @@ invalid.js:7:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 8 │ a.b["c"].d.e["f"] = "something"; 9 │ a.b[`c`] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 5 5 │ a.b[c["d"]]; 6 6 │ a["b"] = "something"; @@ -224,7 +241,7 @@ invalid.js:8:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 9 │ a.b[`c`] = "something"; 10 │ a.b[c["d"]] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 6 6 │ a["b"] = "something"; 7 7 │ a.b["c"] = "something"; @@ -248,7 +265,7 @@ invalid.js:8:14 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ 9 │ a.b[`c`] = "something"; 10 │ a.b[c["d"]] = "something"; - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 6 6 │ a["b"] = "something"; 7 7 │ a.b["c"] = "something"; @@ -272,7 +289,7 @@ invalid.js:9:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━ 10 │ a.b[c["d"]] = "something"; 11 │ a = { - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 7 7 │ a.b["c"] = "something"; 8 8 │ a.b["c"].d.e["f"] = "something"; @@ -296,7 +313,7 @@ invalid.js:10:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ 11 │ a = { 12 │ ['b']: d - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 8 8 │ a.b["c"].d.e["f"] = "something"; 9 9 │ a.b[`c`] = "something"; @@ -320,7 +337,7 @@ invalid.js:12:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ 13 │ }; 14 │ a = { - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 12 │ → ['b']:·d │ -- -- @@ -337,9 +354,9 @@ invalid.js:15:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ > 15 │ [`b`]: d │ ^^^ 16 │ }; - 17 │ a.b[`$c`]; + 17 │ a = { - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. 15 │ → [`b`]:·d │ -- -- @@ -347,47 +364,261 @@ invalid.js:15:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━ ``` ``` -invalid.js:17:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.js:18:2 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! The computed expression can be simplified without the use of a string literal. - 15 │ [`b`]: d 16 │ }; - > 17 │ a.b[`$c`]; + 17 │ a = { + > 18 │ "b": d + │ ^^^ + 19 │ }; + 20 │ a.b[`$c`]; + + i Suggested fix: Use a literal key instead. + + 18 │ → "b":·d + │ - - + +``` + +``` +invalid.js:20:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 18 │ "b": d + 19 │ }; + > 20 │ a.b[`$c`]; │ ^^^^ - 18 │ a.b["_d"]; - 19 │ + 21 │ a.b["_d"]; + 22 │ class C { ["a"] = 0 } - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. - 15 15 │ [`b`]: d - 16 16 │ }; - 17 │ - a.b[`$c`]; - 17 │ + a.b.$c; - 18 18 │ a.b["_d"]; - 19 19 │ + 18 18 │ "b": d + 19 19 │ }; + 20 │ - a.b[`$c`]; + 20 │ + a.b.$c; + 21 21 │ a.b["_d"]; + 22 22 │ class C { ["a"] = 0 } ``` ``` -invalid.js:18:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.js:21:5 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! The computed expression can be simplified without the use of a string literal. - 16 │ }; - 17 │ a.b[`$c`]; - > 18 │ a.b["_d"]; + 19 │ }; + 20 │ a.b[`$c`]; + > 21 │ a.b["_d"]; │ ^^^^ - 19 │ + 22 │ class C { ["a"] = 0 } + 23 │ class C { "a" = 0 } + + i Suggested fix: Use a literal key instead. + + 19 19 │ }; + 20 20 │ a.b[`$c`]; + 21 │ - a.b["_d"]; + 21 │ + a.b._d; + 22 22 │ class C { ["a"] = 0 } + 23 23 │ class C { "a" = 0 } + + +``` + +``` +invalid.js:22:12 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 20 │ a.b[`$c`]; + 21 │ a.b["_d"]; + > 22 │ class C { ["a"] = 0 } + │ ^^^ + 23 │ class C { "a" = 0 } + 24 │ class C { ["a"](){} } + + i Suggested fix: Use a literal key instead. + + 22 │ class·C·{·["a"]·=·0·} + │ -- -- + +``` + +``` +invalid.js:23:11 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 21 │ a.b["_d"]; + 22 │ class C { ["a"] = 0 } + > 23 │ class C { "a" = 0 } + │ ^^^ + 24 │ class C { ["a"](){} } + 25 │ class C { "a"(){} } + + i Suggested fix: Use a literal key instead. + + 23 │ class·C·{·"a"·=·0·} + │ - - + +``` + +``` +invalid.js:24:12 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 22 │ class C { ["a"] = 0 } + 23 │ class C { "a" = 0 } + > 24 │ class C { ["a"](){} } + │ ^^^ + 25 │ class C { "a"(){} } + 26 │ class C { get ["a"](){} } + + i Suggested fix: Use a literal key instead. + + 24 │ class·C·{·["a"](){}·} + │ -- -- + +``` + +``` +invalid.js:25:11 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 23 │ class C { "a" = 0 } + 24 │ class C { ["a"](){} } + > 25 │ class C { "a"(){} } + │ ^^^ + 26 │ class C { get ["a"](){} } + 27 │ class C { get "a"(){} } + + i Suggested fix: Use a literal key instead. + + 25 │ class·C·{·"a"(){}·} + │ - - + +``` + +``` +invalid.js:26:16 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 24 │ class C { ["a"](){} } + 25 │ class C { "a"(){} } + > 26 │ class C { get ["a"](){} } + │ ^^^ + 27 │ class C { get "a"(){} } + 28 │ class C { set ["a"](x){} } + + i Suggested fix: Use a literal key instead. + + 26 │ class·C·{·get·["a"](){}·} + │ -- -- + +``` + +``` +invalid.js:27:15 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 25 │ class C { "a"(){} } + 26 │ class C { get ["a"](){} } + > 27 │ class C { get "a"(){} } + │ ^^^ + 28 │ class C { set ["a"](x){} } + 29 │ class C { set "a"(x){} } + + i Suggested fix: Use a literal key instead. + + 27 │ class·C·{·get·"a"(){}·} + │ - - + +``` + +``` +invalid.js:28:16 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 26 │ class C { get ["a"](){} } + 27 │ class C { get "a"(){} } + > 28 │ class C { set ["a"](x){} } + │ ^^^ + 29 │ class C { set "a"(x){} } + 30 │ a = { + + i Suggested fix: Use a literal key instead. + + 28 │ class·C·{·set·["a"](x){}·} + │ -- -- + +``` + +``` +invalid.js:29:15 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 27 │ class C { get "a"(){} } + 28 │ class C { set ["a"](x){} } + > 29 │ class C { set "a"(x){} } + │ ^^^ + 30 │ a = { + 31 │ ["1+1"]: 2 + + i Suggested fix: Use a literal key instead. + + 29 │ class·C·{·set·"a"(x){}·} + │ - - + +``` + +``` +invalid.js:31:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 29 │ class C { set "a"(x){} } + 30 │ a = { + > 31 │ ["1+1"]: 2 + │ ^^^^^ + 32 │ } + 33 │ a = { + + i Suggested fix: Use a literal key instead. + + 31 │ → ["1+1"]:·2 + │ - - + +``` + +``` +invalid.js:34:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 32 │ } + 33 │ a = { + > 34 │ [`1+1`]: 2 + │ ^^^^^ + 35 │ } - i Suggested fix: Replace it with a static expression. + i Suggested fix: Use a literal key instead. - 16 16 │ }; - 17 17 │ a.b[`$c`]; - 18 │ - a.b["_d"]; - 18 │ + a.b._d; - 19 19 │ + 32 32 │ } + 33 33 │ a = { + 34 │ - → [`1+1`]:·2 + 34 │ + → "1+1":·2 + 35 35 │ } ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts new file mode 100644 index 00000000000..221ab2e95f8 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts @@ -0,0 +1,35 @@ +export interface I { + ["p1"]: number + + "p2": number + + get ["p3"](): number + + get "p4"(): number + + set ["p3"](x: number) + + set "p4"(x: number) + + ["m1"](): void + + "m2"(): void +} + +export type T = { + ["p1"]: number + + "p2": number + + get ["p3"](): number + + get "p4"(): number + + set ["p3"](x: number) + + set "p4"(x: number) + + ["m1"](): void + + "m2"(): void +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts.snap new file mode 100644 index 00000000000..701f3bc7cae --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/invalid.ts.snap @@ -0,0 +1,348 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.ts +--- +# Input +```js +export interface I { + ["p1"]: number + + "p2": number + + get ["p3"](): number + + get "p4"(): number + + set ["p3"](x: number) + + set "p4"(x: number) + + ["m1"](): void + + "m2"(): void +} + +export type T = { + ["p1"]: number + + "p2": number + + get ["p3"](): number + + get "p4"(): number + + set ["p3"](x: number) + + set "p4"(x: number) + + ["m1"](): void + + "m2"(): void +} + +``` + +# Diagnostics +``` +invalid.ts:2:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 1 │ export interface I { + > 2 │ ["p1"]: number + │ ^^^^ + 3 │ + 4 │ "p2": number + + i Suggested fix: Use a literal key instead. + + 2 │ → ["p1"]:·number + │ -- -- + +``` + +``` +invalid.ts:4:2 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 2 │ ["p1"]: number + 3 │ + > 4 │ "p2": number + │ ^^^^ + 5 │ + 6 │ get ["p3"](): number + + i Suggested fix: Use a literal key instead. + + 4 │ → "p2":·number + │ - - + +``` + +``` +invalid.ts:6:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 4 │ "p2": number + 5 │ + > 6 │ get ["p3"](): number + │ ^^^^ + 7 │ + 8 │ get "p4"(): number + + i Suggested fix: Use a literal key instead. + + 6 │ → get·["p3"]():·number + │ -- -- + +``` + +``` +invalid.ts:8:6 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 6 │ get ["p3"](): number + 7 │ + > 8 │ get "p4"(): number + │ ^^^^ + 9 │ + 10 │ set ["p3"](x: number) + + i Suggested fix: Use a literal key instead. + + 8 │ → get·"p4"():·number + │ - - + +``` + +``` +invalid.ts:10:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 8 │ get "p4"(): number + 9 │ + > 10 │ set ["p3"](x: number) + │ ^^^^ + 11 │ + 12 │ set "p4"(x: number) + + i Suggested fix: Use a literal key instead. + + 10 │ → set·["p3"](x:·number) + │ -- -- + +``` + +``` +invalid.ts:12:6 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 10 │ set ["p3"](x: number) + 11 │ + > 12 │ set "p4"(x: number) + │ ^^^^ + 13 │ + 14 │ ["m1"](): void + + i Suggested fix: Use a literal key instead. + + 12 │ → set·"p4"(x:·number) + │ - - + +``` + +``` +invalid.ts:14:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 12 │ set "p4"(x: number) + 13 │ + > 14 │ ["m1"](): void + │ ^^^^ + 15 │ + 16 │ "m2"(): void + + i Suggested fix: Use a literal key instead. + + 14 │ → ["m1"]():·void + │ -- -- + +``` + +``` +invalid.ts:16:2 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 14 │ ["m1"](): void + 15 │ + > 16 │ "m2"(): void + │ ^^^^ + 17 │ } + 18 │ + + i Suggested fix: Use a literal key instead. + + 16 │ → "m2"():·void + │ - - + +``` + +``` +invalid.ts:20:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 19 │ export type T = { + > 20 │ ["p1"]: number + │ ^^^^ + 21 │ + 22 │ "p2": number + + i Suggested fix: Use a literal key instead. + + 20 │ → ["p1"]:·number + │ -- -- + +``` + +``` +invalid.ts:22:2 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 20 │ ["p1"]: number + 21 │ + > 22 │ "p2": number + │ ^^^^ + 23 │ + 24 │ get ["p3"](): number + + i Suggested fix: Use a literal key instead. + + 22 │ → "p2":·number + │ - - + +``` + +``` +invalid.ts:24:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 22 │ "p2": number + 23 │ + > 24 │ get ["p3"](): number + │ ^^^^ + 25 │ + 26 │ get "p4"(): number + + i Suggested fix: Use a literal key instead. + + 24 │ → get·["p3"]():·number + │ -- -- + +``` + +``` +invalid.ts:26:6 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 24 │ get ["p3"](): number + 25 │ + > 26 │ get "p4"(): number + │ ^^^^ + 27 │ + 28 │ set ["p3"](x: number) + + i Suggested fix: Use a literal key instead. + + 26 │ → get·"p4"():·number + │ - - + +``` + +``` +invalid.ts:28:7 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 26 │ get "p4"(): number + 27 │ + > 28 │ set ["p3"](x: number) + │ ^^^^ + 29 │ + 30 │ set "p4"(x: number) + + i Suggested fix: Use a literal key instead. + + 28 │ → set·["p3"](x:·number) + │ -- -- + +``` + +``` +invalid.ts:30:6 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 28 │ set ["p3"](x: number) + 29 │ + > 30 │ set "p4"(x: number) + │ ^^^^ + 31 │ + 32 │ ["m1"](): void + + i Suggested fix: Use a literal key instead. + + 30 │ → set·"p4"(x:·number) + │ - - + +``` + +``` +invalid.ts:32:3 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 30 │ set "p4"(x: number) + 31 │ + > 32 │ ["m1"](): void + │ ^^^^ + 33 │ + 34 │ "m2"(): void + + i Suggested fix: Use a literal key instead. + + 32 │ → ["m1"]():·void + │ -- -- + +``` + +``` +invalid.ts:34:2 lint/nursery/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified without the use of a string literal. + + 32 │ ["m1"](): void + 33 │ + > 34 │ "m2"(): void + │ ^^^^ + 35 │ } + 36 │ + + i Suggested fix: Use a literal key instead. + + 34 │ → "m2"():·void + │ - - + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js index c33bf5f0200..914686503de 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js @@ -10,3 +10,7 @@ a["a-b"] a[`a-b`] a[`time range`]; a[`time${range}`]; +class C { a = 0 } +class C { a(){} } +class C { get a(){} } +class C { set a(x){} } \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js.snap index 536af327a99..819ea4e0b5d 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.js.snap @@ -16,7 +16,10 @@ a["a-b"] a[`a-b`] a[`time range`]; a[`time${range}`]; - +class C { a = 0 } +class C { a(){} } +class C { get a(){} } +class C { set a(x){} } ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts new file mode 100644 index 00000000000..e5616097dd0 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts @@ -0,0 +1,19 @@ +export interface I { + p1: number + + get p2(): number + + set p2(x: number) + + m1(): void +} + +export type T = { + p1: number + + get p2(): number + + set p2(x: number) + + m1(): void +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts.snap new file mode 100644 index 00000000000..be2e2d66d34 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useLiteralKeys/valid.ts.snap @@ -0,0 +1,29 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.ts +--- +# Input +```js +export interface I { + p1: number + + get p2(): number + + set p2(x: number) + + m1(): void +} + +export type T = { + p1: number + + get p2(): number + + set p2(x: number) + + m1(): void +} + +``` + + diff --git a/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs b/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs index ea069120f7c..7dba4442dff 100644 --- a/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs +++ b/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs @@ -1,8 +1,7 @@ use crate::prelude::*; -use crate::js::expressions::computed_member_expression::AnyJsComputedMemberLike; use crate::parentheses::NeedsParentheses; -use rome_js_syntax::{JsComputedMemberAssignment, JsSyntaxNode}; +use rome_js_syntax::{AnyJsComputedMember, JsComputedMemberAssignment, JsSyntaxNode}; #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsComputedMemberAssignment; @@ -13,7 +12,7 @@ impl FormatNodeRule for FormatJsComputedMemberAssign node: &JsComputedMemberAssignment, f: &mut JsFormatter, ) -> FormatResult<()> { - AnyJsComputedMemberLike::from(node.clone()).fmt(f) + AnyJsComputedMember::from(node.clone()).fmt(f) } fn needs_parentheses(&self, item: &JsComputedMemberAssignment) -> bool { diff --git a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs index 38268d558a1..ead1d15f098 100644 --- a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs @@ -4,10 +4,9 @@ use crate::js::expressions::static_member_expression::member_chain_callee_needs_ use crate::parentheses::NeedsParentheses; use rome_formatter::{format_args, write}; use rome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, JsComputedMemberAssignment, - JsComputedMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, + AnyJsComputedMember, AnyJsExpression, AnyJsLiteralExpression, JsComputedMemberExpression, + JsSyntaxKind, JsSyntaxNode, }; -use rome_rowan::{declare_node_union, SyntaxResult}; #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsComputedMemberExpression; @@ -18,7 +17,7 @@ impl FormatNodeRule for FormatJsComputedMemberExpres node: &JsComputedMemberExpression, f: &mut JsFormatter, ) -> FormatResult<()> { - AnyJsComputedMemberLike::from(node.clone()).fmt(f) + AnyJsComputedMember::from(node.clone()).fmt(f) } fn needs_parentheses(&self, item: &JsComputedMemberExpression) -> bool { @@ -26,11 +25,7 @@ impl FormatNodeRule for FormatJsComputedMemberExpres } } -declare_node_union! { - pub(crate) AnyJsComputedMemberLike = JsComputedMemberExpression | JsComputedMemberAssignment -} - -impl Format for AnyJsComputedMemberLike { +impl Format for AnyJsComputedMember { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { write!(f, [self.object().format()])?; @@ -39,10 +34,10 @@ impl Format for AnyJsComputedMemberLike { } /// Formats the lookup portion (everything except the object) of a computed member like. -pub(crate) struct FormatComputedMemberLookup<'a>(&'a AnyJsComputedMemberLike); +pub(crate) struct FormatComputedMemberLookup<'a>(&'a AnyJsComputedMember); impl<'a> FormatComputedMemberLookup<'a> { - pub(crate) fn new(member_like: &'a AnyJsComputedMemberLike) -> Self { + pub(crate) fn new(member_like: &'a AnyJsComputedMember) -> Self { Self(member_like) } } @@ -78,53 +73,6 @@ impl Format for FormatComputedMemberLookup<'_> { } } -impl AnyJsComputedMemberLike { - fn object(&self) -> SyntaxResult { - match self { - AnyJsComputedMemberLike::JsComputedMemberExpression(expression) => expression.object(), - AnyJsComputedMemberLike::JsComputedMemberAssignment(assignment) => assignment.object(), - } - } - - fn l_brack_token(&self) -> SyntaxResult { - match self { - AnyJsComputedMemberLike::JsComputedMemberExpression(expression) => { - expression.l_brack_token() - } - AnyJsComputedMemberLike::JsComputedMemberAssignment(assignment) => { - assignment.l_brack_token() - } - } - } - - fn optional_chain_token(&self) -> Option { - match self { - AnyJsComputedMemberLike::JsComputedMemberExpression(expression) => { - expression.optional_chain_token() - } - AnyJsComputedMemberLike::JsComputedMemberAssignment(_) => None, - } - } - - fn member(&self) -> SyntaxResult { - match self { - AnyJsComputedMemberLike::JsComputedMemberExpression(expression) => expression.member(), - AnyJsComputedMemberLike::JsComputedMemberAssignment(assignment) => assignment.member(), - } - } - - fn r_brack_token(&self) -> SyntaxResult { - match self { - AnyJsComputedMemberLike::JsComputedMemberExpression(expression) => { - expression.r_brack_token() - } - AnyJsComputedMemberLike::JsComputedMemberAssignment(assignment) => { - assignment.r_brack_token() - } - } - } -} - impl NeedsParentheses for JsComputedMemberExpression { fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool { if self.is_optional_chain() && matches!(parent.kind(), JsSyntaxKind::JS_NEW_EXPRESSION) { diff --git a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs index d74f9674015..1c12e06ca43 100644 --- a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs @@ -1,13 +1,12 @@ use crate::prelude::*; -use crate::js::expressions::computed_member_expression::AnyJsComputedMemberLike; use crate::parentheses::NeedsParentheses; use crate::JsLabels; use rome_formatter::{format_args, write}; use rome_js_syntax::{ - AnyJsAssignment, AnyJsAssignmentPattern, AnyJsExpression, AnyJsName, JsAssignmentExpression, - JsInitializerClause, JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind, - JsSyntaxNode, JsSyntaxToken, + AnyJsAssignment, AnyJsAssignmentPattern, AnyJsComputedMember, AnyJsExpression, AnyJsName, + JsAssignmentExpression, JsInitializerClause, JsStaticMemberAssignment, + JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, }; use rome_rowan::{declare_node_union, AstNode, SyntaxResult}; @@ -136,7 +135,7 @@ impl AnyJsStaticMemberLike { } AnyJsStaticMemberLike::can_cast(parent.kind()) - || AnyJsComputedMemberLike::can_cast(parent.kind()) + || AnyJsComputedMember::can_cast(parent.kind()) } None => false, }; @@ -147,7 +146,7 @@ impl AnyJsStaticMemberLike { let first_non_static_member_ancestor = self.syntax().ancestors().find(|parent| { !(AnyJsStaticMemberLike::can_cast(parent.kind()) - || AnyJsComputedMemberLike::can_cast(parent.kind())) + || AnyJsComputedMember::can_cast(parent.kind())) }); let layout = match first_non_static_member_ancestor.and_then(AnyJsExpression::cast) { diff --git a/crates/rome_js_syntax/src/expr_ext.rs b/crates/rome_js_syntax/src/expr_ext.rs index 4e039b1405d..4c9ef708931 100644 --- a/crates/rome_js_syntax/src/expr_ext.rs +++ b/crates/rome_js_syntax/src/expr_ext.rs @@ -4,11 +4,11 @@ use crate::static_value::{QuotedString, StaticStringValue, StaticValue}; use crate::{ AnyJsCallArgument, AnyJsExpression, AnyJsLiteralExpression, AnyJsTemplateElement, JsArrayExpression, JsArrayHole, JsAssignmentExpression, JsBinaryExpression, JsCallExpression, - JsComputedMemberExpression, JsLiteralMemberName, JsLogicalExpression, JsNewExpression, - JsNumberLiteralExpression, JsObjectExpression, JsPostUpdateExpression, JsReferenceIdentifier, - JsRegexLiteralExpression, JsStaticMemberExpression, JsStringLiteralExpression, JsSyntaxKind, - JsSyntaxToken, JsTemplateChunkElement, JsTemplateExpression, JsUnaryExpression, - OperatorPrecedence, T, + JsComputedMemberAssignment, JsComputedMemberExpression, JsLiteralMemberName, + JsLogicalExpression, JsNewExpression, JsNumberLiteralExpression, JsObjectExpression, + JsPostUpdateExpression, JsReferenceIdentifier, JsRegexLiteralExpression, + JsStaticMemberExpression, JsStringLiteralExpression, JsSyntaxKind, JsSyntaxToken, + JsTemplateChunkElement, JsTemplateExpression, JsUnaryExpression, OperatorPrecedence, T, }; use crate::{JsPreUpdateExpression, JsSyntaxKind::*}; use core::iter; @@ -857,6 +857,57 @@ impl JsComputedMemberExpression { } } +declare_node_union! { + pub AnyJsComputedMember = JsComputedMemberExpression | JsComputedMemberAssignment +} + +impl AnyJsComputedMember { + pub fn object(&self) -> SyntaxResult { + match self { + AnyJsComputedMember::JsComputedMemberExpression(expression) => expression.object(), + AnyJsComputedMember::JsComputedMemberAssignment(assignment) => assignment.object(), + } + } + + pub fn l_brack_token(&self) -> SyntaxResult { + match self { + AnyJsComputedMember::JsComputedMemberExpression(expression) => { + expression.l_brack_token() + } + AnyJsComputedMember::JsComputedMemberAssignment(assignment) => { + assignment.l_brack_token() + } + } + } + + pub fn optional_chain_token(&self) -> Option { + match self { + AnyJsComputedMember::JsComputedMemberExpression(expression) => { + expression.optional_chain_token() + } + AnyJsComputedMember::JsComputedMemberAssignment(_) => None, + } + } + + pub fn member(&self) -> SyntaxResult { + match self { + AnyJsComputedMember::JsComputedMemberExpression(expression) => expression.member(), + AnyJsComputedMember::JsComputedMemberAssignment(assignment) => assignment.member(), + } + } + + pub fn r_brack_token(&self) -> SyntaxResult { + match self { + AnyJsComputedMember::JsComputedMemberExpression(expression) => { + expression.r_brack_token() + } + AnyJsComputedMember::JsComputedMemberAssignment(assignment) => { + assignment.r_brack_token() + } + } + } +} + declare_node_union! { pub AnyJsMemberExpression = JsStaticMemberExpression | JsComputedMemberExpression } diff --git a/website/src/pages/lint/rules/useLiteralKeys.md b/website/src/pages/lint/rules/useLiteralKeys.md index 2df8925e228..2de63b68228 100644 --- a/website/src/pages/lint/rules/useLiteralKeys.md +++ b/website/src/pages/lint/rules/useLiteralKeys.md @@ -23,7 +23,7 @@ a.b["c"]; ^^^ 2 │ - Suggested fix: Replace it with a static expression. + Suggested fix: Use a literal key instead. 1 - a.b["c"]; 1+ a.b.c; @@ -43,7 +43,7 @@ a.c[`d`] ^^^ 2 │ - Suggested fix: Replace it with a static expression. + Suggested fix: Use a literal key instead. 1 - a.c[`d`] 1+ a.c.d @@ -63,7 +63,7 @@ a.c[`d`] = "something" ^^^ 2 │ - Suggested fix: Replace it with a static expression. + Suggested fix: Use a literal key instead. 1 - a.c[`d`]·=·"something" 1+ a.c.d·=·"something" @@ -87,7 +87,7 @@ a = { 3 │ } 4 │ - Suggested fix: Replace it with a static expression. + Suggested fix: Use a literal key instead. 2 │ ['b']:·d -- --