From 059591af0507162092a36630b57619686b513f70 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 25 Mar 2024 23:05:26 +1300 Subject: [PATCH] feat(immutable-data): add option for `ignoreNonConstDeclarations` to `treatParametersAsConst` (#794) fix #724 --- docs/rules/immutable-data.md | 10 +- src/rules/immutable-data.ts | 139 ++++++++++---- src/utils/tree.ts | 31 +++- .../rules/immutable-data/ts/object/invalid.ts | 170 ++++++++++++++++++ tests/rules/immutable-data/ts/object/valid.ts | 84 +++++++++ 5 files changed, 398 insertions(+), 36 deletions(-) diff --git a/docs/rules/immutable-data.md b/docs/rules/immutable-data.md index bbff8ff2c..d572cde6f 100644 --- a/docs/rules/immutable-data.md +++ b/docs/rules/immutable-data.md @@ -63,7 +63,11 @@ This rule accepts an options object of the following type: type Options = { ignoreClasses: boolean | "fieldsOnly"; ignoreImmediateMutation: boolean; - ignoreNonConstDeclarations: boolean; + ignoreNonConstDeclarations: + | boolean + | { + treatParametersAsConst: boolean; + }; ignoreIdentifierPattern?: string[] | string; ignoreAccessorPattern?: string[] | string; }; @@ -110,6 +114,10 @@ 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. +#### `treatParametersAsConst` + +If true, parameters won't be ignored, while other non-const variables will be. + ### `ignoreClasses` Ignore mutations inside classes. diff --git a/src/rules/immutable-data.ts b/src/rules/immutable-data.ts index 8cf02efac..035125e87 100644 --- a/src/rules/immutable-data.ts +++ b/src/rules/immutable-data.ts @@ -61,7 +61,11 @@ type Options = [ IgnoreClassesOption & IgnoreIdentifierPatternOption & { ignoreImmediateMutation: boolean; - ignoreNonConstDeclarations: boolean; + ignoreNonConstDeclarations: + | boolean + | { + treatParametersAsConst: boolean; + }; }, ]; @@ -80,7 +84,20 @@ const schema: JSONSchema4[] = [ type: "boolean", }, ignoreNonConstDeclarations: { - type: "boolean", + oneOf: [ + { + type: "boolean", + }, + { + type: "object", + properties: { + treatParametersAsConst: { + type: "boolean", + }, + }, + additionalProperties: false, + }, + ], }, } satisfies JSONSchema4ObjectSchema["properties"], ), @@ -230,11 +247,23 @@ function checkAssignmentExpression( }; } - if (ignoreNonConstDeclarations) { + if (ignoreNonConstDeclarations !== false) { const rootIdentifier = findRootIdentifier(node.left.object); if ( rootIdentifier !== undefined && - isDefinedByMutableVariable(rootIdentifier, context) + isDefinedByMutableVariable( + rootIdentifier, + context, + (variableNode) => + ignoreNonConstDeclarations === true || + !ignoreNonConstDeclarations.treatParametersAsConst || + shouldIgnorePattern( + variableNode, + context, + ignoreIdentifierPattern, + ignoreAccessorPattern, + ), + ) ) { return { context, @@ -283,11 +312,23 @@ function checkUnaryExpression( }; } - if (ignoreNonConstDeclarations) { + if (ignoreNonConstDeclarations !== false) { const rootIdentifier = findRootIdentifier(node.argument.object); if ( rootIdentifier !== undefined && - isDefinedByMutableVariable(rootIdentifier, context) + isDefinedByMutableVariable( + rootIdentifier, + context, + (variableNode) => + ignoreNonConstDeclarations === true || + !ignoreNonConstDeclarations.treatParametersAsConst || + shouldIgnorePattern( + variableNode, + context, + ignoreIdentifierPattern, + ignoreAccessorPattern, + ), + ) ) { return { context, @@ -335,11 +376,23 @@ function checkUpdateExpression( }; } - if (ignoreNonConstDeclarations) { + if (ignoreNonConstDeclarations !== false) { const rootIdentifier = findRootIdentifier(node.argument.object); if ( rootIdentifier !== undefined && - isDefinedByMutableVariable(rootIdentifier, context) + isDefinedByMutableVariable( + rootIdentifier, + context, + (variableNode) => + ignoreNonConstDeclarations === true || + !ignoreNonConstDeclarations.treatParametersAsConst || + shouldIgnorePattern( + variableNode, + context, + ignoreIdentifierPattern, + ignoreAccessorPattern, + ), + ) ) { return { context, @@ -473,18 +526,29 @@ function checkCallExpression( !isInChainCallAndFollowsNew(node.callee, context)) && isArrayType(getTypeOfNode(node.callee.object, context)) ) { - if (ignoreNonConstDeclarations) { - const rootIdentifier = findRootIdentifier(node.callee.object); - if ( - rootIdentifier === undefined || - !isDefinedByMutableVariable(rootIdentifier, context) - ) { - return { - context, - descriptors: [{ node, messageId: "array" }], - }; - } - } else { + if (ignoreNonConstDeclarations === false) { + return { + context, + descriptors: [{ node, messageId: "array" }], + }; + } + const rootIdentifier = findRootIdentifier(node.callee.object); + if ( + rootIdentifier === undefined || + !isDefinedByMutableVariable( + rootIdentifier, + context, + (variableNode) => + ignoreNonConstDeclarations === true || + !ignoreNonConstDeclarations.treatParametersAsConst || + shouldIgnorePattern( + variableNode, + context, + ignoreIdentifierPattern, + ignoreAccessorPattern, + ), + ) + ) { return { context, descriptors: [{ node, messageId: "array" }], @@ -507,18 +571,29 @@ function checkCallExpression( ) && isObjectConstructorType(getTypeOfNode(node.callee.object, context)) ) { - if (ignoreNonConstDeclarations) { - const rootIdentifier = findRootIdentifier(node.callee.object); - if ( - rootIdentifier === undefined || - !isDefinedByMutableVariable(rootIdentifier, context) - ) { - return { - context, - descriptors: [{ node, messageId: "object" }], - }; - } - } else { + if (ignoreNonConstDeclarations === false) { + return { + context, + descriptors: [{ node, messageId: "object" }], + }; + } + const rootIdentifier = findRootIdentifier(node.callee.object); + if ( + rootIdentifier === undefined || + !isDefinedByMutableVariable( + rootIdentifier, + context, + (variableNode) => + ignoreNonConstDeclarations === true || + !ignoreNonConstDeclarations.treatParametersAsConst || + shouldIgnorePattern( + variableNode, + context, + ignoreIdentifierPattern, + ignoreAccessorPattern, + ), + ) + ) { return { context, descriptors: [{ node, messageId: "object" }], diff --git a/src/utils/tree.ts b/src/utils/tree.ts index d8e30c375..d80d9bc43 100644 --- a/src/utils/tree.ts +++ b/src/utils/tree.ts @@ -213,6 +213,18 @@ export function isArgument(node: TSESTree.Node): boolean { ); } +/** + * Is the given node a parameter? + */ +export function isParameter(node: TSESTree.Node): boolean { + return ( + node.parent !== undefined && + isFunctionLike(node.parent) && + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + node.parent.params.includes(node as any) + ); +} + /** * Is the given node a getter function? */ @@ -273,14 +285,27 @@ export function getKeyOfValueInObjectExpression( */ export function isDefinedByMutableVariable< Context extends RuleContext, ->(node: TSESTree.Identifier, context: Context) { +>( + node: TSESTree.Identifier, + context: Context, + treatParametersAsMutable: (node: TSESTree.Node) => boolean, +): boolean { const services = getParserServices(context); const symbol = services.getSymbolAtLocation(node); const variableDeclaration = symbol?.valueDeclaration; + + if (variableDeclaration === undefined) { + return true; + } + const variableDeclarationNode = + services.tsNodeToESTreeNodeMap.get(variableDeclaration); if ( - variableDeclaration === undefined || - !typescript!.isVariableDeclaration(variableDeclaration) + variableDeclarationNode !== undefined && + isParameter(variableDeclarationNode) ) { + return treatParametersAsMutable(variableDeclarationNode); + } + if (!typescript!.isVariableDeclaration(variableDeclaration)) { return true; } diff --git a/tests/rules/immutable-data/ts/object/invalid.ts b/tests/rules/immutable-data/ts/object/invalid.ts index 41ad753f2..90e4542c4 100644 --- a/tests/rules/immutable-data/ts/object/invalid.ts +++ b/tests/rules/immutable-data/ts/object/invalid.ts @@ -511,6 +511,176 @@ const tests: Array< }, ], }, + { + code: dedent` + function y(x: {a: 1, b: object}) { + x.foo = "bar"; + x["foo"] = "bar"; + x.a += 1; + x.a -= 1; + x.a *= 1; + x.a /= 1; + x.a %= 1; + x.a <<= 1; + x.a >>= 1; + x.a >>>= 1; + x.a &= 1; + x.a |= 1; + x.a ^= 1; + x.a **= 1; + delete x.a; + delete x["a"]; + x.a++; + x.a--; + ++x.a; + --x.a; + if (x.a = 2) {} + if (x.a++) {} + Object.assign(x, { c: "world" }); + Object.assign(x.b, { c: "world" }); + Object.defineProperties(x, { d: { value: 2, writable: false }}); + Object.defineProperty(x, "e", { value: 3, writable: false }); + Object.setPrototypeOf(x, null); + } + `, + optionsSet: [ + [{ ignoreNonConstDeclarations: { treatParametersAsConst: true } }], + ], + errors: [ + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 2, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 3, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 4, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 5, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 6, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 7, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 8, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 9, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 10, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 11, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 12, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 13, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 14, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 15, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UnaryExpression, + line: 16, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UnaryExpression, + line: 17, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UpdateExpression, + line: 18, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UpdateExpression, + line: 19, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UpdateExpression, + line: 20, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UpdateExpression, + line: 21, + column: 3, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.AssignmentExpression, + line: 22, + column: 7, + }, + { + messageId: "generic", + type: AST_NODE_TYPES.UpdateExpression, + line: 23, + column: 7, + }, + ], + }, { code: dedent` (mutable_foo as Bar).baz = "hello world"; diff --git a/tests/rules/immutable-data/ts/object/valid.ts b/tests/rules/immutable-data/ts/object/valid.ts index 2f0655b5b..c41ade6b4 100644 --- a/tests/rules/immutable-data/ts/object/valid.ts +++ b/tests/rules/immutable-data/ts/object/valid.ts @@ -195,6 +195,90 @@ const tests: Array>> = [ `, optionsSet: [[{ ignoreNonConstDeclarations: true }]], }, + { + code: dedent` + function y(x: {a: 1, b: object}) { + x.foo = "bar"; + x["foo"] = "bar"; + x.a += 1; + x.a -= 1; + x.a *= 1; + x.a /= 1; + x.a %= 1; + x.a <<= 1; + x.a >>= 1; + x.a >>>= 1; + x.a &= 1; + x.a |= 1; + x.a ^= 1; + x.a **= 1; + delete x.a; + delete x["a"]; + x.a++; + x.a--; + ++x.a; + --x.a; + if (x.a = 2) {} + if (x.a++) {} + Object.assign(x, { c: "world" }); + Object.assign(x.b, { c: "world" }); + Object.defineProperties(x, { d: { value: 2, writable: false }}); + Object.defineProperty(x, "e", { value: 3, writable: false }); + Object.setPrototypeOf(x, null); + } + `, + optionsSet: [ + [{ ignoreNonConstDeclarations: true }], + [ + { + ignoreNonConstDeclarations: { + treatParametersAsConst: false, + }, + }, + ], + ], + }, + { + code: dedent` + function y(mutable_x: {a: 1, b: object}) { + mutable_x.foo = "bar"; + mutable_x["foo"] = "bar"; + mutable_x.a += 1; + mutable_x.a -= 1; + mutable_x.a *= 1; + mutable_x.a /= 1; + mutable_x.a %= 1; + mutable_x.a <<= 1; + mutable_x.a >>= 1; + mutable_x.a >>>= 1; + mutable_x.a &= 1; + mutable_x.a |= 1; + mutable_x.a ^= 1; + mutable_x.a **= 1; + delete mutable_x.a; + delete mutable_x["a"]; + mutable_x.a++; + mutable_x.a--; + ++mutable_x.a; + --mutable_x.a; + if (mutable_x.a = 2) {} + if (mutable_x.a++) {} + Object.assign(mutable_x, { c: "world" }); + Object.assign(mutable_x.b, { c: "world" }); + Object.defineProperties(mutable_x, { d: { value: 2, writable: false }}); + Object.defineProperty(mutable_x, "e", { value: 3, writable: false }); + Object.setPrototypeOf(mutable_x, null); + } + `, + optionsSet: [ + [ + { + ignoreNonConstDeclarations: { treatParametersAsConst: true }, + ignoreAccessorPattern: "mutable*", + }, + ], + ], + }, { code: dedent` (mutable_foo as Bar).baz = "hello world";