Skip to content

Commit

Permalink
fix: cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@minht11
minht11 committed Apr 22, 2024
1 parent 552b30b commit 4be6c6e
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 31 deletions.
41 changes: 19 additions & 22 deletions crates/biome_js_analyze/src/lint/nursery/use_consistent_new_builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl BuiltinCreationRule {
}
}

pub struct UseNewForBuiltinsState {
pub struct UseConsistentNewBuiltinState {
name: String,
creation_rule: BuiltinCreationRule,
}
Expand All @@ -151,26 +151,28 @@ declare_node_union! {

impl Rule for UseConsistentNewBuiltin {
type Query = Semantic<JsNewOrCallExpression>;
type State = UseNewForBuiltinsState;
type State = UseConsistentNewBuiltinState;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let (callee, creation_rule) = extract_callee_and_rule(node)?;

let (reference, name) = global_identifier(&callee.omit_parentheses())?;

let name_text = name.text();

if creation_rule.forbidden_builtins_list().binary_search(&name_text).is_ok() {
return ctx
.model()
.binding(&reference)
.is_none()
.then_some(UseNewForBuiltinsState {
if creation_rule
.forbidden_builtins_list()
.binary_search(&name_text)
.is_ok()
{
return ctx.model().binding(&reference).is_none().then_some(
UseConsistentNewBuiltinState {
name: name_text.to_string(),
creation_rule,
});
},
);
}

None
Expand Down Expand Up @@ -204,24 +206,20 @@ impl Rule for UseConsistentNewBuiltin {
JsNewOrCallExpression::JsNewExpression(node) => {
let call_expression = convert_new_expression_to_call_expression(node)?;

mutation.replace_node_discard_trivia::<AnyJsExpression>(
node.clone().into(),
call_expression.into(),
);
mutation
.replace_node::<AnyJsExpression>(node.clone().into(), call_expression.into());
Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Remove "<Emphasis>"new"</Emphasis>"." }.to_owned(),
message: markup! { "Remove "<Emphasis>"new"</Emphasis>" keyword." }.to_owned(),
mutation,
})
}
JsNewOrCallExpression::JsCallExpression(node) => {
let new_expression = convert_call_expression_to_new_expression(node)?;

mutation.replace_node_discard_trivia::<AnyJsExpression>(
node.clone().into(),
new_expression.into(),
);
mutation
.replace_node::<AnyJsExpression>(node.clone().into(), new_expression.into());
Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
Expand Down Expand Up @@ -263,18 +261,17 @@ fn convert_new_expression_to_call_expression(expr: &JsNewExpression) -> Option<J
}

fn convert_call_expression_to_new_expression(expr: &JsCallExpression) -> Option<JsNewExpression> {
let mut callee: AnyJsExpression = expr.callee().ok()?;
let mut callee = expr.callee().ok()?;
let leading_trivia_pieces = callee.syntax().first_leading_trivia()?.pieces();

let new_token = make::token(JsSyntaxKind::NEW_KW)
.with_leading_trivia_pieces(leading_trivia_pieces)
.with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]);

// Remove leading trivia from the callee.
callee = callee.with_leading_trivia_pieces([])?;

Some(
make::js_new_expression(new_token.clone(), callee)
make::js_new_expression(new_token, callee)
.with_arguments(expr.arguments().ok()?)
.build(),
)
Expand Down
4 changes: 2 additions & 2 deletions crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ FinalizationRegistry()
window.Object({})
globalThis.Object()
function foo() {
return /** Comment */ globalThis.Object({ foo: 'bar' })
return /** Start */ globalThis.Object({ foo: 'bar' }) /** End */
}

new String()
Expand All @@ -38,5 +38,5 @@ new Boolean()
new window.String(123)
new globalThis.String()
function foo() {
return new globalThis.String("foo")
return /** Start */ new globalThis.String("foo") /** End */
}
134 changes: 127 additions & 7 deletions crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,17 @@ FinalizationRegistry()
window.Object({})
globalThis.Object()
function foo() {
return /** Comment */ globalThis.Object({ foo: 'bar' })
return /** Start */ globalThis.Object({ foo: 'bar' }) /** End */
}

new String()
new Number()
new Boolean()
new window.String(123)
new globalThis.String()
function foo() {
return /** Start */ new globalThis.String("foo") /** End */
}
```

# Diagnostics
Expand Down Expand Up @@ -599,7 +607,7 @@ invalid.js:30:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━
> 30 │ globalThis.Object()
│ ^^^^^^^^^^^^^^^^^^^
31 │ function foo() {
32 │ return /** Comment */ globalThis.Object({ foo: 'bar' })
32 │ return /** Start */ globalThis.Object({ foo: 'bar' }) /** End */
i Unsafe fix: Add new keyword.
Expand All @@ -609,20 +617,132 @@ invalid.js:30:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━
```

```
invalid.js:32:27 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:32:25 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use new Object() instead of Object().
30 │ globalThis.Object()
31 │ function foo() {
> 32 │ return /** Comment */ globalThis.Object({ foo: 'bar' })
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 32 │ return /** Start */ globalThis.Object({ foo: 'bar' }) /** End */
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
33 │ }
34 │
i Unsafe fix: Add new keyword.
32 │ ····return·/**·Comment·*/·new·globalThis.Object({·foo:·'bar'·})
│ ++++
32 │ ····return·/**·Start·*/·new·globalThis.Object({·foo:·'bar'·})·/**·End·*/
│ ++++
```

```
invalid.js:35:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use String() instead of new String().
33 │ }
34 │
> 35 │ new String()
│ ^^^^^^^^^^^^
36 │ new Number()
37 │ new Boolean()
i Unsafe fix: Remove new keyword.
35 │ new·String()
│ ----
```

```
invalid.js:36:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use Number() instead of new Number().
35 │ new String()
> 36 │ new Number()
│ ^^^^^^^^^^^^
37 │ new Boolean()
38 │ new window.String(123)
i Unsafe fix: Remove new keyword.
36 │ new·Number()
│ ----
```

```
invalid.js:37:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use Boolean() instead of new Boolean().
35 │ new String()
36 │ new Number()
> 37 │ new Boolean()
│ ^^^^^^^^^^^^^
38 │ new window.String(123)
39 │ new globalThis.String()
i Unsafe fix: Remove new keyword.
37 │ new·Boolean()
│ ----
```

```
invalid.js:38:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use String() instead of new String().
36 │ new Number()
37 │ new Boolean()
> 38 │ new window.String(123)
│ ^^^^^^^^^^^^^^^^^^^^^^
39 │ new globalThis.String()
40 │ function foo() {
i Unsafe fix: Remove new keyword.
38 │ new·window.String(123)
│ ----
```

```
invalid.js:39:1 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use String() instead of new String().
37 │ new Boolean()
38 │ new window.String(123)
> 39 │ new globalThis.String()
│ ^^^^^^^^^^^^^^^^^^^^^^^
40 │ function foo() {
41 │ return /** Start */ new globalThis.String("foo") /** End */
i Unsafe fix: Remove new keyword.
39 │ new·globalThis.String()
│ ----
```

```
invalid.js:41:25 lint/nursery/useConsistentNewBuiltin FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use String() instead of new String().
39 │ new globalThis.String()
40 │ function foo() {
> 41 │ return /** Start */ new globalThis.String("foo") /** End */
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
42 │ }
i Unsafe fix: Remove new keyword.
41 │ ····return·/**·Start·*/·new·globalThis.String("foo")·/**·End·*/
│ ----
```

0 comments on commit 4be6c6e

Please sign in to comment.