From b4cb496fd94ee376b5abd8fe593cd1b3f9764221 Mon Sep 17 00:00:00 2001 From: Justinas Delinda <8914032+minht11@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:23:57 +0300 Subject: [PATCH] fix: cleanup --- .../nursery/use_consistent_new_builtin.rs | 41 +++--- .../useConsistentNewBuiltin/invalid.js | 4 +- .../useConsistentNewBuiltin/invalid.js.snap | 134 +++++++++++++++++- 3 files changed, 148 insertions(+), 31 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/use_consistent_new_builtin.rs b/crates/biome_js_analyze/src/lint/nursery/use_consistent_new_builtin.rs index 4ef4ac866c11..eac0e03146f8 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_consistent_new_builtin.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_consistent_new_builtin.rs @@ -140,7 +140,7 @@ impl BuiltinCreationRule { } } -pub struct UseNewForBuiltinsState { +pub struct UseConsistentNewBuiltinState { name: String, creation_rule: BuiltinCreationRule, } @@ -151,26 +151,28 @@ declare_node_union! { impl Rule for UseConsistentNewBuiltin { type Query = Semantic; - type State = UseNewForBuiltinsState; + type State = UseConsistentNewBuiltinState; type Signals = Option; type Options = (); fn run(ctx: &RuleContext) -> 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 @@ -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::( - node.clone().into(), - call_expression.into(), - ); + mutation + .replace_node::(node.clone().into(), call_expression.into()); Some(JsRuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, - message: markup! { "Remove ""new""." }.to_owned(), + message: markup! { "Remove ""new"" keyword." }.to_owned(), mutation, }) } JsNewOrCallExpression::JsCallExpression(node) => { let new_expression = convert_call_expression_to_new_expression(node)?; - mutation.replace_node_discard_trivia::( - node.clone().into(), - new_expression.into(), - ); + mutation + .replace_node::(node.clone().into(), new_expression.into()); Some(JsRuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, @@ -263,18 +261,17 @@ fn convert_new_expression_to_call_expression(expr: &JsNewExpression) -> Option Option { - 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(), ) diff --git a/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js index cbd5a55199b7..4e2ba875040a 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js +++ b/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js @@ -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() @@ -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 */ } \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js.snap index a3becc171798..f4dffe3ced4b 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/useConsistentNewBuiltin/invalid.js.snap @@ -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 @@ -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. @@ -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·*/ + │ ---- ```