Skip to content

Commit

Permalink
fix(immutable-data): rework logic, it should work now
Browse files Browse the repository at this point in the history 
fix #692
  • Loading branch information
@RebeccaStevens
RebeccaStevens committed Jul 24, 2023
1 parent 147d82f commit 9644994
Show file tree
Hide file tree
Showing 7 changed files with 508 additions and 129 deletions.
4 changes: 4 additions & 0 deletions docs/rules/immutable-data.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
154 changes: 74 additions & 80 deletions src/rules/immutable-data.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?
Expand All @@ -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 {
Expand Down
47 changes: 47 additions & 0 deletions src/utils/tree.ts
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -19,6 +24,7 @@ import {
isTSTypeAnnotation,
isTSTypeLiteral,
isTSTypeReference,
isVariableDeclaration,
} from "./type-guards";

/**
Expand Down Expand Up @@ -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;
}
Loading

0 comments on commit 9644994

Please sign in to comment.