From 743d805345ad963d8c02c5c4c31599b44ff0280c Mon Sep 17 00:00:00 2001 From: Ze-Zheng Wu Date: Fri, 14 Jun 2024 22:10:17 +0800 Subject: [PATCH] fix(lint/useLiteralKeys): unescaped newline false positive (#3210) --- CHANGELOG.md | 12 ++ .../src/lint/complexity/use_literal_keys.rs | 60 ++++++-- .../complexity/useLiteralKeys/invalid.js | 14 ++ .../complexity/useLiteralKeys/invalid.js.snap | 139 ++++++++++++++++-- .../complexity/useLiteralKeys/invalid.ts.snap | 20 +-- .../specs/complexity/useLiteralKeys/valid.js | 6 +- .../complexity/useLiteralKeys/valid.js.snap | 5 + 7 files changed, 222 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fff40e2cd731..34d23cf98b26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,18 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - The [`noUnmatchableAnbSelector`](https://biomejs.dev/linter/rules/no-unmatchable-anb-selector/) rule is now able to catch unmatchable `an+b` selectors like `0n+0` or `-0n+0`. Contributed by @Sec-ant. - The [`useHookAtTopLevel`](https://biomejs.dev/linter/rules/use-hook-at-top-level/) rule now recognizes properties named as hooks like `foo.useFoo()`. Contributed by @ksnyder9801 - Fix [#3092](https://github.com/biomejs/biome/issues/3092), prevent warning for `Custom properties (--*)`. Contributed by @chansuke +- Fix a false positive in the [`useLiteralKeys`](https://biomejs.dev/linter/rules/use-literal-keys/) rule. ([#3160](https://github.com/biomejs/biome/issues/3160)) + + This rule now ignores the following kind of computed member name: + + ```js + const a = { + [`line1 + line2`]: true, + }; + ``` + + Contributed by @Sec-ant ### Parser diff --git a/crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs b/crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs index fb6467495aaf..d10f99b5678d 100644 --- a/crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs +++ b/crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs @@ -62,15 +62,19 @@ declare_rule! { impl Rule for UseLiteralKeys { type Query = Ast; - type State = (TextRange, String); + type State = (TextRange, String, bool); type Signals = Option; type Options = (); fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); + let mut is_computed_member_name = false; let inner_expression = match node { AnyJsMember::AnyJsComputedMember(computed_member) => computed_member.member().ok()?, - AnyJsMember::JsComputedMemberName(member) => member.expression().ok()?, + AnyJsMember::JsComputedMemberName(member) => { + is_computed_member_name = true; + member.expression().ok()? + } }; let value = inner_expression.as_static_value()?; let value = value.as_string_constant()?; @@ -79,27 +83,47 @@ impl Rule for UseLiteralKeys { // The first is a regular property. // The second is a special property that changes the object prototype. // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto - if matches!(node, AnyJsMember::JsComputedMemberName(_)) && value == "__proto__" { + if is_computed_member_name && value == "__proto__" { return None; } - // A computed property `["something"]` can always be simplified to a string literal "something". - if matches!(node, AnyJsMember::JsComputedMemberName(_)) || is_js_ident(value) { - return Some((inner_expression.range(), value.to_string())); + // A computed property `["something"]` can always be simplified to a string literal "something", + // unless it is a template literal inside that contains unescaped new line characters: + // + // const a = { + // `line1 + // line2`: true + // } + // + if (is_computed_member_name && !has_unescaped_new_line(value)) || is_js_ident(value) { + return Some(( + inner_expression.range(), + value.to_string(), + is_computed_member_name, + )); } None } - fn diagnostic(_ctx: &RuleContext, (range, _): &Self::State) -> Option { + fn diagnostic( + _ctx: &RuleContext, + (range, _, is_computed_member_name): &Self::State, + ) -> Option { Some(RuleDiagnostic::new( rule_category!(), range, - markup! { - "The computed expression can be simplified without the use of a string literal." + if *is_computed_member_name { + markup! { + "The computed expression can be simplified to a string literal." + } + } else { + markup! { + "The computed expression can be simplified without the use of a string literal." + } }, )) } - fn action(ctx: &RuleContext, (_, identifier): &Self::State) -> Option { + fn action(ctx: &RuleContext, (_, identifier, _): &Self::State) -> Option { let node = ctx.query(); let mut mutation = ctx.root().begin(); match node { @@ -161,3 +185,19 @@ impl Rule for UseLiteralKeys { declare_node_union! { pub AnyJsMember = AnyJsComputedMember | JsComputedMemberName } + +fn has_unescaped_new_line(text: &str) -> bool { + let mut iter = text.as_bytes().iter(); + while let Some(c) = iter.next() { + match c { + b'\\' => { + iter.next(); + } + b'\n' => { + return true; + } + _ => {} + } + } + false +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js index c16c38296455..98eb4427ac6f 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js +++ b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js @@ -32,3 +32,17 @@ a = { // optional chain a?.["b"]?.['c']?.d?.e?.["f"] +a = { + ["line1\ + line2"]: true, +}; +a = { + [`line1\ + line2`]: true, +}; +a = { + ["line1\nline2"]: true, +}; +a = { + [`line1\nline2`]: true, +}; \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js.snap b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js.snap index 1f64c90ae13b..c93afb333cc6 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js.snap @@ -38,7 +38,20 @@ a = { // optional chain a?.["b"]?.['c']?.d?.e?.["f"] - +a = { + ["line1\ + line2"]: true, +}; +a = { + [`line1\ + line2`]: true, +}; +a = { + ["line1\nline2"]: true, +}; +a = { + [`line1\nline2`]: true, +}; ``` # Diagnostics @@ -327,7 +340,7 @@ invalid.js:10:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:12:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 10 │ a.b[c["d"]] = "something"; 11 │ a = { @@ -351,7 +364,7 @@ invalid.js:12:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:15:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 13 │ }; 14 │ a = { @@ -423,7 +436,7 @@ invalid.js:18:5 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:19:12 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 17 │ a.b[`$c`]; 18 │ a.b["_d"]; @@ -442,7 +455,7 @@ invalid.js:19:12 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:20:12 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 18 │ a.b["_d"]; 19 │ class C { ["a"] = 0 } @@ -461,7 +474,7 @@ invalid.js:20:12 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:21:16 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 19 │ class C { ["a"] = 0 } 20 │ class C { ["a"](){} } @@ -480,7 +493,7 @@ invalid.js:21:16 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:22:16 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 20 │ class C { ["a"](){} } 21 │ class C { get ["a"](){} } @@ -499,7 +512,7 @@ invalid.js:22:16 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:24:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 22 │ class C { set ["a"](x){} } 23 │ a = { @@ -518,7 +531,7 @@ invalid.js:24:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:27:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 25 │ } 26 │ a = { @@ -542,7 +555,7 @@ invalid.js:27:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.js:30:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 28 │ } 29 │ a = { @@ -566,7 +579,8 @@ invalid.js:34:5 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ 33 │ // optional chain > 34 │ a?.["b"]?.['c']?.d?.e?.["f"] │ ^^^ - 35 │ + 35 │ a = { + 36 │ ["line1\ i Unsafe fix: Use a literal key instead. @@ -583,7 +597,8 @@ invalid.js:34:12 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ 33 │ // optional chain > 34 │ a?.["b"]?.['c']?.d?.e?.["f"] │ ^^^ - 35 │ + 35 │ a = { + 36 │ ["line1\ i Unsafe fix: Use a literal key instead. @@ -600,7 +615,8 @@ invalid.js:34:25 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ 33 │ // optional chain > 34 │ a?.["b"]?.['c']?.d?.e?.["f"] │ ^^^ - 35 │ + 35 │ a = { + 36 │ ["line1\ i Unsafe fix: Use a literal key instead. @@ -608,3 +624,100 @@ invalid.js:34:25 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ │ -- -- ``` + +``` +invalid.js:36:4 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified to a string literal. + + 34 │ a?.["b"]?.['c']?.d?.e?.["f"] + 35 │ a = { + > 36 │ ["line1\ + │ ^^^^^^^ + > 37 │ line2"]: true, + │ ^^^^^^ + 38 │ }; + 39 │ a = { + + i Unsafe fix: Use a literal key instead. + + 34 34 │ a?.["b"]?.['c']?.d?.e?.["f"] + 35 35 │ a = { + 36 │ - ··["line1\ + 37 │ - ··line2"]:·true, + 36 │ + ··"line1\ + 37 │ + ··line2":·true, + 38 38 │ }; + 39 39 │ a = { + + +``` + +``` +invalid.js:40:4 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified to a string literal. + + 38 │ }; + 39 │ a = { + > 40 │ [`line1\ + │ ^^^^^^^ + > 41 │ line2`]: true, + │ ^^^^^^ + 42 │ }; + 43 │ a = { + + i Unsafe fix: Use a literal key instead. + + 38 38 │ }; + 39 39 │ a = { + 40 │ - ··[`line1\ + 41 │ - ··line2`]:·true, + 40 │ + ··"line1\ + 41 │ + ··line2":·true, + 42 42 │ }; + 43 43 │ a = { + + +``` + +``` +invalid.js:44:4 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified to a string literal. + + 42 │ }; + 43 │ a = { + > 44 │ ["line1\nline2"]: true, + │ ^^^^^^^^^^^^^^ + 45 │ }; + 46 │ a = { + + i Unsafe fix: Use a literal key instead. + + 44 │ ··["line1\nline2"]:·true, + │ - - + +``` + +``` +invalid.js:47:4 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The computed expression can be simplified to a string literal. + + 45 │ }; + 46 │ a = { + > 47 │ [`line1\nline2`]: true, + │ ^^^^^^^^^^^^^^ + 48 │ }; + + i Unsafe fix: Use a literal key instead. + + 45 45 │ }; + 46 46 │ a = { + 47 │ - ··[`line1\nline2`]:·true, + 47 │ + ··"line1\nline2":·true, + 48 48 │ }; + + +``` diff --git a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.ts.snap b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.ts.snap index 3ab82b583c74..a5989965fb3d 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.ts.snap @@ -34,7 +34,7 @@ export type T = { ``` invalid.ts:2:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 1 │ export interface I { > 2 │ ["p1"]: number @@ -52,7 +52,7 @@ invalid.ts:2:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━ ``` invalid.ts:4:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 2 │ ["p1"]: number 3 │ @@ -71,7 +71,7 @@ invalid.ts:4:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━ ``` invalid.ts:6:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 4 │ get ["p2"](): number 5 │ @@ -90,7 +90,7 @@ invalid.ts:6:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━ ``` invalid.ts:8:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 6 │ set ["p2"](x: number) 7 │ @@ -109,7 +109,7 @@ invalid.ts:8:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━ ``` invalid.ts:10:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 8 │ ["m1"](): void 9 │ @@ -128,7 +128,7 @@ invalid.ts:10:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.ts:14:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 13 │ export type T = { > 14 │ ["p1"]: number @@ -146,7 +146,7 @@ invalid.ts:14:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.ts:16:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 14 │ ["p1"]: number 15 │ @@ -165,7 +165,7 @@ invalid.ts:16:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.ts:18:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 16 │ get ["p2"](): number 17 │ @@ -184,7 +184,7 @@ invalid.ts:18:7 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.ts:20:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 18 │ set ["p2"](x: number) 19 │ @@ -203,7 +203,7 @@ invalid.ts:20:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━ ``` invalid.ts:22:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! The computed expression can be simplified without the use of a string literal. + ! The computed expression can be simplified to a string literal. 20 │ ["m1"](): void 21 │ diff --git a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js index e300b6ed043c..346a3b5d4f56 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js +++ b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js @@ -31,4 +31,8 @@ a = { // Exception a = { ["__proto__"]: null, -} \ No newline at end of file +} +a = { + [`line1 + line2`]: true, +}; diff --git a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js.snap b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js.snap index 11b4bd1966b3..2828f91d04ac 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/valid.js.snap @@ -38,4 +38,9 @@ a = { a = { ["__proto__"]: null, } +a = { + [`line1 + line2`]: true, +}; + ```