fix(immutable-data): rework logic, it should work now

fix #692

Author: RebeccaStevens

docs/rules/immutable-data.md

@@ -104,6 +104,10 @@ const sorted = [...original].sort((a, b) => a.localeCompare(b)); // This is OK w
 If true, this rule will ignore any mutations that happen on non-const variables.
 This allow for more easily using mutable data by simply using the `let` keyword instead of `const`.
 
+Note: If a value is referenced by both a `let` and a `const` variable, the `let`
+reference can be modified while the `const` one can't. The may lead to value of
+the `const` variable unexpectedly changing when the `let` one is modified elsewhere.
+
 ### `ignoreClasses`
 
 Ignore mutations inside classes.

src/rules/immutable-data.ts

@@ -5,10 +5,7 @@ import {
 } from "@typescript-eslint/utils/json-schema";
 import { type RuleContext } from "@typescript-eslint/utils/ts-eslint";
 import { deepmerge } from "deepmerge-ts";
-import { isNodeFlagSet } from "ts-api-utils";
-import type ts from "typescript";
 
-import typescript from "#eslint-plugin-functional/conditional-imports/typescript";
 import {
   type IgnoreAccessorPatternOption,
   type IgnoreIdentifierPatternOption,
@@ -30,7 +27,11 @@ import {
   type RuleResult,
   type NamedCreateRuleMetaWithCategory,
 } from "#eslint-plugin-functional/utils/rule";
-import { isInConstructor } from "#eslint-plugin-functional/utils/tree";
+import {
+  findRootIdentifier,
+  isDefinedByMutableVaraible,
+  isInConstructor,
+} from "#eslint-plugin-functional/utils/tree";
 import {
   isArrayConstructorType,
   isArrayExpression,
@@ -166,39 +167,6 @@ const objectConstructorMutatorFunctions = new Set([
   "setPrototypeOf",
 ]);
 
-/**
- * Is the given identifier defined by a mutable variable (let or var)?
- */
-function isDefinedByMutableVaraible(
-  node: TSESTree.Identifier,
-  context: Readonly<RuleContext<keyof typeof errorMessages, Options>>,
-) {
-  if (typescript === undefined) {
-    return true;
-  }
-
-  const tsNode = context.parserServices?.esTreeNodeToTSNodeMap.get(node);
-  const variableDeclaration =
-    tsNode !== undefined &&
-    "flowNode" in tsNode &&
-    typeof tsNode.flowNode === "object" &&
-    tsNode.flowNode !== null &&
-    "node" in tsNode.flowNode &&
-    typeof tsNode.flowNode.node === "object" &&
-    tsNode.flowNode.node !== null &&
-    typescript.isVariableDeclaration(tsNode.flowNode.node as ts.Node)
-      ? (tsNode.flowNode.node as ts.VariableDeclaration)
-      : undefined;
-
-  const variableDeclarationList = variableDeclaration?.parent;
-
-  return (
-    variableDeclarationList === undefined ||
-    !typescript.isVariableDeclarationList(variableDeclarationList) ||
-    !isNodeFlagSet(variableDeclarationList, typescript.NodeFlags.Const)
-  );
-}
-
 /**
  * Check if the given assignment expression violates this rule.
  */
@@ -231,15 +199,17 @@ function checkAssignmentExpression(
     };
   }
 
-  if (
-    ignoreNonConstDeclarations &&
-    isIdentifier(node.left.object) &&
-    isDefinedByMutableVaraible(node.left.object, context)
-  ) {
-    return {
-      context,
-      descriptors: [],
-    };
+  if (ignoreNonConstDeclarations) {
+    const rootIdentifier = findRootIdentifier(node.left.object);
+    if (
+      rootIdentifier !== undefined &&
+      isDefinedByMutableVaraible(rootIdentifier, context)
+    ) {
+      return {
+        context,
+        descriptors: [],
+      };
+    }
   }
 
   return {
@@ -282,15 +252,17 @@ function checkUnaryExpression(
     };
   }
 
-  if (
-    ignoreNonConstDeclarations &&
-    isIdentifier(node.argument.object) &&
-    isDefinedByMutableVaraible(node.argument.object, context)
-  ) {
-    return {
-      context,
-      descriptors: [],
-    };
+  if (ignoreNonConstDeclarations) {
+    const rootIdentifier = findRootIdentifier(node.argument.object);
+    if (
+      rootIdentifier !== undefined &&
+      isDefinedByMutableVaraible(rootIdentifier, context)
+    ) {
+      return {
+        context,
+        descriptors: [],
+      };
+    }
   }
 
   return {
@@ -332,15 +304,17 @@ function checkUpdateExpression(
     };
   }
 
-  if (
-    ignoreNonConstDeclarations &&
-    isIdentifier(node.argument.object) &&
-    isDefinedByMutableVaraible(node.argument.object, context)
-  ) {
-    return {
-      context,
-      descriptors: [],
-    };
+  if (ignoreNonConstDeclarations) {
+    const rootIdentifier = findRootIdentifier(node.argument.object);
+    if (
+      rootIdentifier !== undefined &&
+      isDefinedByMutableVaraible(rootIdentifier, context)
+    ) {
+      return {
+        context,
+        descriptors: [],
+      };
+    }
   }
 
   return {
@@ -424,15 +398,25 @@ function checkCallExpression(
     arrayMutatorMethods.has(node.callee.property.name) &&
     (!ignoreImmediateMutation ||
       !isInChainCallAndFollowsNew(node.callee, context)) &&
-    isArrayType(getTypeOfNode(node.callee.object, context)) &&
-    (!ignoreNonConstDeclarations ||
-      !isIdentifier(node.callee.object) ||
-      !isDefinedByMutableVaraible(node.callee.object, context))
+    isArrayType(getTypeOfNode(node.callee.object, context))
   ) {
-    return {
-      context,
-      descriptors: [{ node, messageId: "array" }],
-    };
+    if (ignoreNonConstDeclarations) {
+      const rootIdentifier = findRootIdentifier(node.callee.object);
+      if (
+        rootIdentifier === undefined ||
+        !isDefinedByMutableVaraible(rootIdentifier, context)
+      ) {
+        return {
+          context,
+          descriptors: [{ node, messageId: "array" }],
+        };
+      }
+    } else {
+      return {
+        context,
+        descriptors: [{ node, messageId: "array" }],
+      };
+    }
   }
 
   // Non-array object mutation (ex. Object.assign on identifier)?
@@ -448,15 +432,25 @@ function checkCallExpression(
       ignoreIdentifierPattern,
       ignoreAccessorPattern,
     ) &&
-    isObjectConstructorType(getTypeOfNode(node.callee.object, context)) &&
-    (!ignoreNonConstDeclarations ||
-      !isIdentifier(node.callee.object) ||
-      !isDefinedByMutableVaraible(node.callee.object, context))
+    isObjectConstructorType(getTypeOfNode(node.callee.object, context))
   ) {
-    return {
-      context,
-      descriptors: [{ node, messageId: "object" }],
-    };
+    if (ignoreNonConstDeclarations) {
+      const rootIdentifier = findRootIdentifier(node.callee.object);
+      if (
+        rootIdentifier === undefined ||
+        !isDefinedByMutableVaraible(rootIdentifier, context)
+      ) {
+        return {
+          context,
+          descriptors: [{ node, messageId: "object" }],
+        };
+      }
+    } else {
+      return {
+        context,
+        descriptors: [{ node, messageId: "object" }],
+      };
+    }
   }
 
   return {

src/utils/tree.ts

@@ -1,5 +1,10 @@
 import { type TSESTree } from "@typescript-eslint/utils";
+import { getParserServices } from "@typescript-eslint/utils/eslint-utils";
+import { type RuleContext } from "@typescript-eslint/utils/ts-eslint";
 
+import typescript from "#eslint-plugin-functional/conditional-imports/typescript";
+
+import { type BaseOptions } from "./rule";
 import {
   isBlockStatement,
   isCallExpression,
@@ -19,6 +24,7 @@ import {
   isTSTypeAnnotation,
   isTSTypeLiteral,
   isTSTypeReference,
+  isVariableDeclaration,
 } from "./type-guards";
 
 /**
@@ -261,3 +267,44 @@ export function getKeyOfValueInObjectExpression(
 
   return objectExpressionProp.key.name;
 }
+
+/**
+ * Is the given identifier defined by a mutable variable (let or var)?
+ */
+export function isDefinedByMutableVaraible<
+  Context extends RuleContext<string, BaseOptions>,
+>(node: TSESTree.Identifier, context: Context) {
+  const services = getParserServices(context);
+  const symbol = services.getSymbolAtLocation(node);
+  const variableDeclaration = symbol?.valueDeclaration;
+  if (
+    variableDeclaration === undefined ||
+    !typescript!.isVariableDeclaration(variableDeclaration)
+  ) {
+    return true;
+  }
+
+  const variableDeclarator =
+    context.parserServices?.tsNodeToESTreeNodeMap.get(variableDeclaration);
+  if (
+    variableDeclarator?.parent === undefined ||
+    !isVariableDeclaration(variableDeclarator.parent)
+  ) {
+    return true;
+  }
+
+  return variableDeclarator.parent.kind !== "const";
+}
+
+/**
+ * Get the root identifier of an expression.
+ */
+export function findRootIdentifier(node: TSESTree.Expression) {
+  if (isIdentifier(node)) {
+    return node;
+  }
+  if (isMemberExpression(node)) {
+    return findRootIdentifier(node.object);
+  }
+  return undefined;
+}