Skip to content

Commit

Permalink
fix(lint/useLiteralKeys): unescaped newline false positive (#3210)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sec-ant
Sec-ant committed Jun 14, 2024
1 parent 4c4e502 commit 743d805
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 34 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
60 changes: 50 additions & 10 deletions crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,19 @@ declare_rule! {

impl Rule for UseLiteralKeys {
type Query = Ast<AnyJsMember>;
type State = (TextRange, String);
type State = (TextRange, String, bool);
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> 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()?;
Expand All @@ -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<Self>, (range, _): &Self::State) -> Option<RuleDiagnostic> {
fn diagnostic(
_ctx: &RuleContext<Self>,
(range, _, is_computed_member_name): &Self::State,
) -> Option<RuleDiagnostic> {
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<Self>, (_, identifier): &Self::State) -> Option<JsRuleAction> {
fn action(ctx: &RuleContext<Self>, (_, identifier, _): &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let mut mutation = ctx.root().begin();
match node {
Expand Down Expand Up @@ -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
}
14 changes: 14 additions & 0 deletions crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
139 changes: 126 additions & 13 deletions crates/biome_js_analyze/tests/specs/complexity/useLiteralKeys/invalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down Expand Up @@ -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"];
Expand All @@ -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 }
Expand All @@ -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"](){} }
Expand All @@ -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"](){} }
Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -600,11 +615,109 @@ 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.
34 │ a?.["b"]?.['c']?.d?.e?.["f"]
│ -- --
```

```
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 │ };
```
Loading

0 comments on commit 743d805

Please sign in to comment.