refactor(useArrayLiterals): check all expressions and add a fix (#4416)

CHANGELOG.md

@@ -82,6 +82,32 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 #### Enhancements
 
 - `useExportType` and `useImportType` now ignore TypeScript declaration files ([#4416](https://github.com/biomejs/biome/pull/4416)). Contributed by @Conaclos
+- [useArrayLiterals](https://biomejs.dev/linter/rules/use-array-literals/) now provides a code fix.
+
+  ```diff
+  - const xs = new Array();
+  + const xs = [];
+  ```
+
+  The code fix is currently marked as unsafe.
+  We plan to make it safe in a future release of Biome.
+
+  Contributed by @Conaclos
+
+#### Bug fixes
+
+- [useArrayLiterals](https://biomejs.dev/linter/rules/use-array-literals/) now reports all expressions using the `Array` constructors.
+
+  Previously, the rule reported only use of the `Array` constructor in expressions statements.
+
+  ```js
+  // This was reported
+  new Array();
+  // This was not reported
+  const xs = new Array();
+  ```
+
+  Contributed by @Conaclos
 
 ### Parser
 

crates/biome_js_analyze/src/lint/correctness/use_array_literals.rs

@@ -1,9 +1,15 @@
 use biome_analyze::{
-    context::RuleContext, declare_lint_rule, Ast, FixKind, Rule, RuleDiagnostic, RuleSource,
+    context::RuleContext, declare_lint_rule, ActionCategory, FixKind, Rule, RuleDiagnostic,
+    RuleSource,
 };
 use biome_console::markup;
-use biome_js_syntax::{AnyJsCallArgument, AnyJsExpression, JsCallArguments, JsExpressionStatement};
-use biome_rowan::AstNode;
+use biome_js_factory::make;
+use biome_js_syntax::{
+    global_identifier, AnyJsCallArgument, AnyJsExpression, JsNewOrCallExpression, JsSyntaxKind, T,
+};
+use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt};
+
+use crate::{services::semantic::Semantic, JsRuleAction};
 
 declare_lint_rule! {
     /// Disallow Array constructors.
@@ -16,29 +22,29 @@ declare_lint_rule! {
     /// ### Invalid
     ///
     /// ```js,expect_diagnostic
-    /// Array();
+    /// const xs = Array();
     /// ```
     ///
     /// ```js,expect_diagnostic
-    /// Array(0, 1, 2);
+    /// const xs = Array(0, 1, 2);
     /// ```
     ///
     /// ```js,expect_diagnostic
-    /// new Array(0, 1, 2);
+    /// const xs = new Array(0, 1, 2);
     /// ```
     ///
     /// ```js,expect_diagnostic
-    /// Array(...args);
+    /// const xs = Array(...args);
     /// ```
     ///
     /// ### Valid
     ///
     /// ```js
-    /// Array(500);
+    /// const xs = Array(65000);
     /// ```
     ///
     /// ```js
-    /// [0, 1, 2];
+    /// const xs = [0, 1, 2];
     /// ```
     ///
     pub UseArrayLiterals {
@@ -52,29 +58,41 @@ declare_lint_rule! {
 }
 
 impl Rule for UseArrayLiterals {
-    type Query = Ast<JsExpressionStatement>;
+    type Query = Semantic<JsNewOrCallExpression>;
     type State = ();
     type Signals = Option<Self::State>;
     type Options = ();
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
-        let expression_statement = ctx.query();
-
-        let expression = expression_statement.expression().ok()?;
-
-        match expression {
-            AnyJsExpression::JsCallExpression(call_expression) => {
-                let callee = call_expression.callee().ok()?;
-                let arguments = call_expression.arguments().ok()?;
-                validate(&callee, &arguments)
-            }
-            AnyJsExpression::JsNewExpression(new_expression) => {
-                let callee = new_expression.callee().ok()?;
-                let arguments = new_expression.arguments()?;
-                validate(&callee, &arguments)
-            }
-            _ => None,
+        let node = ctx.query();
+        let callee = node.callee().ok()?.omit_parentheses();
+        let (reference, name) = global_identifier(&callee)?;
+        if name.text() != "Array" || ctx.model().binding(&reference).is_some() {
+            return None;
+        }
+        if callee.syntax() != reference.syntax()
+            && !reference
+                .value_token()
+                .is_ok_and(|name| matches!(name.text_trimmed(), "globalThis" | "window" | "Array"))
+        {
+            return None;
+        }
+        let Some(arguments) = node.arguments() else {
+            return if matches!(node, JsNewOrCallExpression::JsNewExpression(_)) {
+                // Report `new Array`
+                Some(())
+            } else {
+                // ignore `Array`
+                None
+            };
+        };
+        let [arg1, arg2] = arguments.get_arguments_by_index([0, 1]);
+        if arg1.is_some() && arg2.is_none() && !matches!(arg1?, AnyJsCallArgument::JsSpread(_)) {
+            // Ignore `Array(length)`
+            return None;
         }
+        // Report `Array()`, `Array(x, y)`, and `Array(...xs)`
+        Some(())
     }
 
     fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
@@ -84,31 +102,56 @@ impl Rule for UseArrayLiterals {
                 rule_category!(),
                 node.range(),
                 markup! {
-                    "Don't use Array constructors."
+                    "Use an array literal instead of the "<Emphasis>"Array"</Emphasis>" constructor."
                 },
             )
             .note(markup! {
-                "Use of the Array constructor is not allowed except creating sparse arrays of a specified size by giving a single numeric argument."
-            })
-            .note(markup! {
-                "The array literal notation [] is preferable."
+                "The "<Emphasis>"Array"</Emphasis>" constructor is misleading because it can be used to preallocate an array of a given length or to create an array with a given list of elements."
             }),
         )
     }
-}
 
-fn validate(callee: &AnyJsExpression, arguments: &JsCallArguments) -> Option<()> {
-    if callee.text() != "Array" {
-        return None;
-    }
-    let mut args_iter = arguments.args().into_iter();
-    let first_arg = args_iter.next();
-    let second_arg = args_iter.next();
-    if first_arg.is_some()
-        && second_arg.is_none()
-        && !matches!(first_arg?.ok()?, AnyJsCallArgument::JsSpread(_))
-    {
-        return None;
+    fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
+        let node = ctx.query();
+        if node
+            .syntax()
+            .parent()
+            .is_some_and(|parent| parent.kind() == JsSyntaxKind::JS_EXPRESSION_STATEMENT)
+        {
+            // Ignore useless expression statements.
+            // This avoids issues with missing semicolons.
+            return None;
+        }
+        let mut mutation = ctx.root().begin();
+        let new_node = if let Some(args) = node.arguments() {
+            let l_paren_trailing_trivia = args.l_paren_token().ok()?.trailing_trivia().pieces();
+            let r_paren_leading_trivia = args.r_paren_token().ok()?.leading_trivia().pieces();
+            let args = args.args();
+            let items = args
+                .elements()
+                .flat_map(|item| item.into_node())
+                .map(|item| item.into())
+                .collect::<Vec<_>>();
+            let separators = args.separators().flatten().collect::<Vec<_>>();
+            make::js_array_expression(
+                make::token(T!['[']).append_trivia_pieces(l_paren_trailing_trivia),
+                make::js_array_element_list(items, separators),
+                make::token(T![']']).prepend_trivia_pieces(r_paren_leading_trivia),
+            )
+        } else {
+            // `new Array` -> `[]`
+            make::js_array_expression(
+                make::token(T!['[']),
+                make::js_array_element_list([], []),
+                make::token(T![']']),
+            )
+        };
+        mutation.replace_node::<AnyJsExpression>(node.clone().into(), new_node.into());
+        Some(JsRuleAction::new(
+            ActionCategory::QuickFix,
+            ctx.metadata().applicability(),
+            markup! { "Use an array literal." }.to_owned(),
+            mutation,
+        ))
     }
-    Some(())
 }

crates/biome_js_analyze/tests/specs/correctness/useArrayLiterals/invalid.js

@@ -1,11 +1,30 @@
-Array();
+var xs = Array();
 
-Array(0, 1, 2);
+var xs = Array(0, 1, 2);
 
-Array(...args);
+var xs = Array(...args);
 
-new Array();
+var xs = new Array;
 
-new Array(0, 1, 2);
+var xs = new Array();
 
-new Array(...args);
+var xs = new Array(0, 1, 2);
+
+var xs = new Array(...args);
+
+var xs = /**A*/ new /**B*/ Array /**C*/ ( /**D*/ 0 /**E*/, /**F*/ 1 /**G*/) /**H*/;
+
+var xs = (Array)(
+    /* foo */ a,
+    b = c() // bar
+);
+
+var xs = Array?.();
+
+// ASI
+foo
+new Array
+
+var xs = globalThis.Array();
+
+var xs = window.Array();