fix: #3260 improve type definitions and documentation on the callback…

… indices of map, filter, and forEach
josdejong · Nov 20, 2024 · 67ddc72 · 67ddc72
1 parent e00d367
commit 67ddc72
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 25 deletions.
diff --git a/src/function/matrix/filter.js b/src/function/matrix/filter.js
@@ -9,6 +9,13 @@ export const createFilter = /* #__PURE__ */ factory(name, dependencies, ({ typed
   /**
    * Filter the items in an array or one dimensional matrix.
    *
+   * The callback is invoked with three arguments: the current value,
+   * the current index, and the matrix operated upon.
+   * Note that because the matrix/array might be
+   * multidimensional, the "index" argument is always an array of numbers giving
+   * the index in each dimension. This is true even for vectors: the "index"
+   * argument is an array of length 1, rather than simply a number.
+   *
    * Syntax:
    *
    *    math.filter(x, test)

diff --git a/src/function/matrix/forEach.js b/src/function/matrix/forEach.js
@@ -9,6 +9,13 @@ export const createForEach = /* #__PURE__ */ factory(name, dependencies, ({ type
   /**
    * Iterate over all elements of a matrix/array, and executes the given callback function.
    *
+   * The callback is invoked with three arguments: the current value,
+   * the current index, and the matrix operated upon.
+   * Note that because the matrix/array might be
+   * multidimensional, the "index" argument is always an array of numbers giving
+   * the index in each dimension. This is true even for vectors: the "index"
+   * argument is an array of length 1, rather than simply a number.
+   *
    * Syntax:
    *
    *    math.forEach(x, callback)

diff --git a/test/typescript-tests/testTypes.ts b/test/typescript-tests/testTypes.ts
@@ -1292,8 +1292,20 @@ Matrices examples
 
   // map matrix
   {
+    const arr = [1, 2, 3]
     assert.deepStrictEqual(
-      math.map([1, 2, 3], function (value) {
+      math.map(arr, function (value) {
+        return value * value
+      }),
+      [1, 4, 9]
+    )
+
+    assert.deepStrictEqual(
+      math.map(arr, function (value, index, self) {
+        const indexValue = index[0]
+        expectTypeOf(indexValue).toMatchTypeOf<number>()
+        assert.deepStrictEqual(self, arr)
+
         return value * value
       }),
       [1, 4, 9]
@@ -1302,8 +1314,9 @@ Matrices examples
 
   // filter matrix
   {
+    const arr = [6, -2, -1, 4, 3]
     assert.deepStrictEqual(
-      math.filter([6, -2, -1, 4, 3], function (x) {
+      math.filter(arr, function (x) {
         return x > 0
       }),
       [6, 4, 3]
@@ -1312,6 +1325,31 @@ Matrices examples
       math.filter(['23', 'foo', '100', '55', 'bar'], /\d+/),
       ['23', '100', '55']
     )
+    assert.deepStrictEqual(
+      math.filter(arr, function (x, index, self) {
+        const indexValue = index[0]
+        expectTypeOf(indexValue).toMatchTypeOf<number>()
+        assert.deepStrictEqual(self, arr)
+
+        return x > 0
+      }),
+      [6, 4, 3]
+    )
+  }
+
+  // forEach matrix
+  {
+    const arr = [6, -2, -1, 4, 3]
+
+    const output: number[] = []
+    math.forEach(arr, function (x, index, self) {
+      const indexValue = index[0]
+      expectTypeOf(indexValue).toMatchTypeOf<number>()
+      assert.deepStrictEqual(self, arr)
+      output.push(x)
+    })
+
+    assert.deepStrictEqual(output, arr)
   }
 
   // concat matrix

diff --git a/test/unit-tests/function/matrix/forEach.test.js b/test/unit-tests/function/matrix/forEach.test.js
@@ -20,33 +20,32 @@ describe('forEach', function () {
     const output = []
     math.forEach([1, 2, 3], math.typed('callback', {
       number: function (value) {
-        output.push(value + 2)
+        output.push([value, arguments.length])
       }
     }))
-    assert.deepStrictEqual(output, [3, 4, 5])
+    assert.deepStrictEqual(output, [
+      [1, 1],
+      [2, 1],
+      [3, 1]
+    ])
   })
 
   it('should invoke a typed function with correct number of arguments (2)', function () {
+    const arr = [1, 2, 3]
     const output = []
-    math.forEach([1, 2, 3], math.typed('callback', {
+    math.forEach(arr, math.typed('callback', {
       'number, Array': function (value, index) {
-        output.push(value + 2)
-      }
-    }))
-    assert.deepStrictEqual(output, [3, 4, 5])
-  })
-
-  it('should invoke a typed function with correct number of arguments (3)', function () {
-    const output = []
-    math.forEach([1, 2, 3], math.typed('callback', {
-      'number, Array, Array': function (value, index, array) {
-        output.push(value + 2)
+        output.push([value, index, arguments.length])
       }
     }))
-    assert.deepStrictEqual(output, [3, 4, 5])
+    assert.deepStrictEqual(output, [
+      [1, [0], 2],
+      [2, [1], 2],
+      [3, [2], 2]
+    ])
   })
 
-  it('should invoke callback with parameters value, index, obj', function () {
+  it('should invoke callback with 3 parameters (value, index, obj)', function () {
     const arr = [[1, 2, 3], [4, 5, 6]]
     const output = []
 
@@ -64,6 +63,17 @@ describe('forEach', function () {
     ])
   })
 
+  it('should invoke callback with 3 parameters when not providing explicit arguments', function () {
+    const arr = [1, 2, 3]
+    const output = []
+
+    math.forEach(arr, function () {
+      output.push(arguments.length)
+    })
+
+    assert.deepStrictEqual(output, [3, 3, 3])
+  })
+
   it('should throw an error if called with unsupported type', function () {
     assert.throws(function () { math.forEach(1, function () {}) })
     assert.throws(function () { math.forEach('arr', function () {}) })

diff --git a/types/index.d.ts b/types/index.d.ts
@@ -1940,8 +1940,7 @@ export interface MathJsInstance extends MathJsFactory {
       | ((
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
           value: any,
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          index: any,
+          index: number[],
           matrix: MathCollection | string[]
         ) => boolean)
       | RegExp
@@ -1965,7 +1964,7 @@ export interface MathJsInstance extends MathJsFactory {
   forEach<T extends MathCollection>(
     x: T,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    callback: (value: any, index: any, matrix: T) => void
+    callback: (value: any, index: number[], matrix: T) => void
   ): void
 
   /**
@@ -5714,8 +5713,7 @@ export interface MathJsChain<TValue> {
       | ((
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
           value: any,
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          index: any,
+          index: number[],
           matrix: MathCollection | string[]
         ) => boolean)
       | RegExp
@@ -5734,7 +5732,7 @@ export interface MathJsChain<TValue> {
   forEach<T extends MathCollection>(
     this: MathJsChain<T>,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    callback: (value: any, index: any, matrix: T) => void
+    callback: (value: any, index: number[], matrix: T) => void
   ): void
 
   /**
@@ -5764,7 +5762,7 @@ export interface MathJsChain<TValue> {
   map<T extends MathCollection>(
     this: MathJsChain<T>,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    callback: (value: any, index: any, matrix: T) => MathType | string
+    callback: (value: any, index: number[], matrix: T) => MathType | string
   ): MathJsChain<T>
 
   /**