fix(immutable-data): treat Object.entries({}).sort() as immediate mut…

…ation fix #773
eslint-functional · Mar 10, 2024 · 245886f · 245886f
1 parent cfd6f8f
commit 245886f
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 27 deletions.
diff --git a/src/rules/immutable-data.ts b/src/rules/immutable-data.ts
@@ -7,21 +7,21 @@ import { type RuleContext } from "@typescript-eslint/utils/ts-eslint";
 import { deepmerge } from "deepmerge-ts";
 
 import {
-  type IgnoreAccessorPatternOption,
-  type IgnoreIdentifierPatternOption,
-  type IgnoreClassesOption,
-  shouldIgnorePattern,
-  shouldIgnoreClasses,
   ignoreAccessorPatternOptionSchema,
   ignoreClassesOptionSchema,
   ignoreIdentifierPatternOptionSchema,
+  shouldIgnoreClasses,
+  shouldIgnorePattern,
+  type IgnoreAccessorPatternOption,
+  type IgnoreClassesOption,
+  type IgnoreIdentifierPatternOption,
 } from "#eslint-plugin-functional/options";
 import { isExpected } from "#eslint-plugin-functional/utils/misc";
 import {
   createRule,
   getTypeOfNode,
-  type RuleResult,
   type NamedCreateRuleMetaWithCategory,
+  type RuleResult,
 } from "#eslint-plugin-functional/utils/rule";
 import {
   findRootIdentifier,
@@ -163,6 +163,22 @@ const objectConstructorMutatorFunctions = new Set([
   "setPrototypeOf",
 ]);
 
+/**
+ * Object constructor functions that return a new array.
+ */
+const objectConstructorNewObjectReturningMethods = [
+  "create",
+  "entries",
+  "fromEntries",
+  "getOwnPropertyDescriptor",
+  "getOwnPropertyDescriptors",
+  "getOwnPropertyNames",
+  "getOwnPropertySymbols",
+  "groupBy",
+  "keys",
+  "values",
+];
+
 /**
  * Check if the given assignment expression violates this rule.
  */
@@ -330,27 +346,55 @@ function isInChainCallAndFollowsNew(
   node: TSESTree.MemberExpression,
   context: Readonly<RuleContext<keyof typeof errorMessages, Options>>,
 ): boolean {
-  return (
-    // Check for: [0, 1, 2]
-    isArrayExpression(node.object) ||
-    // Check for: new Array()
-    (isNewExpression(node.object) &&
-      isArrayConstructorType(getTypeOfNode(node.object.callee, context))) ||
-    (isCallExpression(node.object) &&
-      isMemberExpression(node.object.callee) &&
-      isIdentifier(node.object.callee.property) &&
-      // Check for: Array.from(iterable)
-      ((arrayConstructorFunctions.some(
+  // Check for: [0, 1, 2]
+  if (isArrayExpression(node.object)) {
+    return true;
+  }
+
+  // Check for: new Array()
+  if (
+    isNewExpression(node.object) &&
+    isArrayConstructorType(getTypeOfNode(node.object.callee, context))
+  ) {
+    return true;
+  }
+
+  if (
+    isCallExpression(node.object) &&
+    isMemberExpression(node.object.callee) &&
+    isIdentifier(node.object.callee.property)
+  ) {
+    // Check for: Array.from(iterable)
+    if (
+      arrayConstructorFunctions.some(
+        isExpected(node.object.callee.property.name),
+      ) &&
+      isArrayConstructorType(getTypeOfNode(node.object.callee.object, context))
+    ) {
+      return true;
+    }
+
+    // Check for: array.slice(0)
+    if (
+      arrayNewObjectReturningMethods.some(
+        isExpected(node.object.callee.property.name),
+      )
+    ) {
+      return true;
+    }
+
+    // Check for: Object.entries(object)
+    if (
+      objectConstructorNewObjectReturningMethods.some(
         isExpected(node.object.callee.property.name),
       ) &&
-        isArrayConstructorType(
-          getTypeOfNode(node.object.callee.object, context),
-        )) ||
-        // Check for: array.slice(0)
-        arrayNewObjectReturningMethods.some(
-          isExpected(node.object.callee.property.name),
-        )))
-  );
+      isObjectConstructorType(getTypeOfNode(node.object.callee.object, context))
+    ) {
+      return true;
+    }
+  }
+
+  return false;
 }
 
 /**

diff --git a/tests/rules/immutable-data/ts/array/invalid.ts b/tests/rules/immutable-data/ts/array/invalid.ts
@@ -412,7 +412,7 @@ const tests: Array<
       [0, 1, 2].shift();
       [0, 1, 2].sort();
       [0, 1, 2].splice(0, 1, 9);
-      [0, 1, 2].unshift(6)
+      [0, 1, 2].unshift(6);
     `,
     optionsSet: [[{ ignoreImmediateMutation: false }]],
     errors: [

diff --git a/tests/rules/immutable-data/ts/array/valid.ts b/tests/rules/immutable-data/ts/array/valid.ts
@@ -2,8 +2,8 @@ import dedent from "dedent";
 
 import { type rule } from "#eslint-plugin-functional/rules/immutable-data";
 import {
-  type ValidTestCaseSet,
   type OptionsOf,
+  type ValidTestCaseSet,
 } from "#eslint-plugin-functional/tests/helpers/util";
 
 const tests: Array<ValidTestCaseSet<OptionsOf<typeof rule>>> = [
@@ -308,6 +308,28 @@ const tests: Array<ValidTestCaseSet<OptionsOf<typeof rule>>> = [
     `,
     optionsSet: [[{ ignoreNonConstDeclarations: true }]],
   },
+  {
+    code: dedent`
+      [0, 1, 2].copyWithin(0, 1, 2);
+      [0, 1, 2].fill(3);
+      [0, 1, 2].pop();
+      [0, 1, 2].push(3);
+      [0, 1, 2].reverse();
+      [0, 1, 2].shift();
+      [0, 1, 2].sort();
+      [0, 1, 2].splice(0, 1, 9);
+      [0, 1, 2].unshift(6);
+    `,
+    optionsSet: [[{ ignoreImmediateMutation: true }]],
+  },
+  {
+    code: dedent`
+      Object.entries({}).sort();
+      Object.keys({}).sort();
+      Object.values({}).sort();
+    `,
+    optionsSet: [[{ ignoreImmediateMutation: true }]],
+  },
 ];
 
 export default tests;