From ea7295c1ef4f3a46f3c8b33fa878d7b9c0ff3ffc Mon Sep 17 00:00:00 2001
From: Tyler Cosgrove <Vivalldi@users.noreply.github.com>
Date: Fri, 22 Sep 2023 10:36:31 -0400
Subject: [PATCH] chore(js_analyze): bolster `noAccumulatingSpread` against
 false positives (#330)

---
 .../nursery/no_accumulating_spread.rs         | 27 ++++++++++++++++++-
 .../nursery/noAccumulatingSpread/valid.jsonc  |  9 ++++++-
 .../noAccumulatingSpread/valid.jsonc.snap     | 15 +++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_accumulating_spread.rs b/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_accumulating_spread.rs
index 1b3cc921cc53..61f6cc5f406a 100644
--- a/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_accumulating_spread.rs
+++ b/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_accumulating_spread.rs
@@ -5,7 +5,7 @@ use biome_js_syntax::{
     AnyJsFunction, AnyJsMemberExpression, JsCallArgumentList, JsCallArguments, JsCallExpression,
     JsFormalParameter, JsParameterList, JsParameters, JsSpread,
 };
-use biome_rowan::AstNode;
+use biome_rowan::{AstNode, AstSeparatedList};
 
 use crate::semantic_services::Semantic;
 
@@ -97,15 +97,40 @@ fn is_known_accumulator(node: &JsSpread, model: &SemanticModel) -> Option<bool>
         .parent::<JsParameterList>()
         .and_then(|list| list.parent::<JsParameters>())
         .and_then(|parameters| parameters.parent::<AnyJsFunction>())?;
+
+    // Known accumulators need at least 2 arguments and no more than 4. (accumulator, value, index, array)
+    let param_count = function
+        .parameters()
+        .ok()?
+        .as_js_parameters()?
+        .items()
+        .iter()
+        .count();
+    if !(2..=4).contains(&param_count) {
+        return None;
+    }
+
     let call_expression = function
         .parent::<JsCallArgumentList>()
         .and_then(|arguments| arguments.parent::<JsCallArguments>())
         .and_then(|arguments| arguments.parent::<JsCallExpression>())?;
+
+    // The accumulator function should be a part of a call expression. This call expression should
+    // have no more than 2 arguments. (callback, initialValue)
+    let arg_count = call_expression.arguments().ok()?.args().iter().count();
+    if arg_count > 2 {
+        return None;
+    }
+
     let callee = call_expression.callee().ok()?;
     let member_expression = AnyJsMemberExpression::cast_ref(callee.syntax())?;
+
+    // We only care about `.reduce` and `.reduceRight`.
     let member_name = member_expression.member_name()?;
     if !matches!(member_name.text(), "reduce" | "reduceRight") {
         return None;
     }
+
+    // Finally check that the spread references the first parameter.
     Some(parameter.syntax().index() == 0)
 }
diff --git a/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc
index 4b158f104d4d..b15c08e3eac6 100644
--- a/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc
+++ b/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc
@@ -13,5 +13,12 @@
 
 	// Object - Allow spreading the item into the accumulator
 	"foo.reduce((acc, bar) => {acc[bar.key] = { ...bar.value }; return acc;}, {})",
-	"foo.reduceRight((acc, bar) => {acc[bar.key] = { ...bar.value }; return acc;}, {})"
+	"foo.reduceRight((acc, bar) => {acc[bar.key] = { ...bar.value }; return acc;}, {})",
+
+	// Callbacks with wrong number of parameters
+	"foo.reduce((acc,value,index,array,somethingExtra) => [...acc, value], [])",
+	"foo.reduce((acc) => [...acc], [])",
+
+	// Wrong number of arguments to known method (reduce can have 1 or 2 args, but not more)
+	"foo.reduce((acc, bar) => [...acc, bar], [], 123)"
 ]
diff --git a/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc.snap
index 596b1cf9de11..58f07bc339ba 100644
--- a/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc.snap
+++ b/crates/biome_js_analyze/tests/specs/nursery/noAccumulatingSpread/valid.jsonc.snap
@@ -42,4 +42,19 @@ foo.reduce((acc, bar) => {acc[bar.key] = { ...bar.value }; return acc;}, {})
 foo.reduceRight((acc, bar) => {acc[bar.key] = { ...bar.value }; return acc;}, {})
 ```
 
+# Input
+```js
+foo.reduce((acc,value,index,array,somethingExtra) => [...acc, value], [])
+```
+
+# Input
+```js
+foo.reduce((acc) => [...acc], [])
+```
+
+# Input
+```js
+foo.reduce((acc, bar) => [...acc, bar], [], 123)
+```
+