diff --git a/CHANGELOG.md b/CHANGELOG.md index 7760cd9..86deac8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +* readonly-array rule with the option ignore-prefix set will now ignore nested arrays within an ignored variable. See [#132](https://github.com/jonaskello/tslint-immutable/issues/132). See PR [#133](https://github.com/jonaskello/tslint-immutable/pull/133) + ## [v5.5.1] - 2019-03-22 * no-array-mutation now checks nested arrays. See [#134](https://github.com/jonaskello/tslint-immutable/issues/134). See PR [#135](https://github.com/jonaskello/tslint-immutable/pull/135) diff --git a/src/noArrayMutationRule.ts b/src/noArrayMutationRule.ts index 2ad8c40..819bd72 100644 --- a/src/noArrayMutationRule.ts +++ b/src/noArrayMutationRule.ts @@ -5,8 +5,7 @@ import { isAssignmentKind } from "tsutils/util"; import { createInvalidNode, CheckNodeResult, - createCheckNodeTypedRule, - InvalidNode + createCheckNodeTypedRule } from "./shared/check-node"; import * as Ignore from "./shared/ignore"; import { isAccessExpression, AccessExpression } from "./shared/typeguard"; @@ -88,18 +87,10 @@ const newArrayReturningMethods: ReadonlyArray = [ const constructorFunctions = ["from", "of"]; function checkTypedNode( - node: ts.BinaryExpression, - ctx: Lint.WalkContext, - checker: ts.TypeChecker -): CheckNodeResult { - return { invalidNodes: getInvalidNodes(node, ctx, checker) }; -} - -function getInvalidNodes( node: ts.Node, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if (utils.isBinaryExpression(node)) { return checkBinaryExpression(node, ctx, checker); } @@ -119,7 +110,8 @@ function getInvalidNodes( if (utils.isCallExpression(node)) { return checkCallExpression(node, ctx, checker); } - return []; + + return { invalidNodes: [] }; } /** @@ -130,7 +122,7 @@ function checkBinaryExpression( node: ts.BinaryExpression, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if ( !Ignore.isIgnoredPrefix( node.getText(node.getSourceFile()), @@ -144,10 +136,10 @@ function checkBinaryExpression( ); if (isArrayType(leftExpressionType)) { - return [createInvalidNode(node, [])]; + return { invalidNodes: [createInvalidNode(node, [])] }; } } - return []; + return { invalidNodes: [] }; } /** @@ -157,7 +149,7 @@ function checkDeleteExpression( node: ts.DeleteExpression, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if ( !Ignore.isIgnoredPrefix( node.expression.getText(node.getSourceFile()), @@ -170,10 +162,10 @@ function checkDeleteExpression( ); if (isArrayType(expressionType)) { - return [createInvalidNode(node, [])]; + return { invalidNodes: [createInvalidNode(node, [])] }; } } - return []; + return { invalidNodes: [] }; } /** @@ -183,7 +175,7 @@ function checkPrefixUnaryExpression( node: ts.PrefixUnaryExpression, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if ( !Ignore.isIgnoredPrefix( node.operand.getText(node.getSourceFile()), @@ -197,10 +189,10 @@ function checkPrefixUnaryExpression( ); if (isArrayType(operandExpressionType)) { - return [createInvalidNode(node, [])]; + return { invalidNodes: [createInvalidNode(node, [])] }; } } - return []; + return { invalidNodes: [] }; } /** @@ -210,7 +202,7 @@ function checkPostfixUnaryExpression( node: ts.PostfixUnaryExpression, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if ( !Ignore.isIgnoredPrefix( node.getText(node.getSourceFile()), @@ -224,10 +216,10 @@ function checkPostfixUnaryExpression( ); if (isArrayType(operandExpressionType)) { - return [createInvalidNode(node, [])]; + return { invalidNodes: [createInvalidNode(node, [])] }; } } - return []; + return { invalidNodes: [] }; } /** @@ -237,7 +229,7 @@ function checkCallExpression( node: ts.CallExpression, ctx: Lint.WalkContext, checker: ts.TypeChecker -): ReadonlyArray { +): CheckNodeResult { if ( !Ignore.isIgnoredPrefix( node.getText(node.getSourceFile()), @@ -258,10 +250,10 @@ function checkCallExpression( ); if (isArrayType(expressionType)) { - return [createInvalidNode(node, [])]; + return { invalidNodes: [createInvalidNode(node, [])] }; } } - return []; + return { invalidNodes: [] }; } /** diff --git a/src/noLetRule.ts b/src/noLetRule.ts index 1a150bb..6849416 100644 --- a/src/noLetRule.ts +++ b/src/noLetRule.ts @@ -3,7 +3,6 @@ import * as Lint from "tslint"; import * as utils from "tsutils/typeguard/2.8"; import * as Ignore from "./shared/ignore"; import { - InvalidNode, createInvalidNode, CheckNodeResult, createCheckNodeRule @@ -21,27 +20,34 @@ function checkNode( node: ts.Node, ctx: Lint.WalkContext ): CheckNodeResult { - const variableStatementFailures = checkVariableStatement(node, ctx); - const forStatementsFailures = checkForStatements(node, ctx); + const results = [ + checkVariableStatement(node, ctx), + checkForStatements(node, ctx) + ]; + return { - invalidNodes: [...variableStatementFailures, ...forStatementsFailures] + invalidNodes: results.reduce( + (merged, result) => [...merged, ...result.invalidNodes], + [] + ), + skipChildren: results.some(result => result.skipChildren === true) }; } function checkVariableStatement( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { if (utils.isVariableStatement(node)) { return checkDeclarationList(node.declarationList, ctx); } - return []; + return { invalidNodes: [] }; } function checkForStatements( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { if ( (utils.isForStatement(node) || utils.isForInStatement(node) || @@ -52,13 +58,13 @@ function checkForStatements( ) { return checkDeclarationList(node.initializer, ctx); } - return []; + return { invalidNodes: [] }; } function checkDeclarationList( declarationList: ts.VariableDeclarationList, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { if (Lint.isNodeFlagSet(declarationList, ts.NodeFlags.Let)) { // It is a let declaration, now check each variable that is declared const invalidVariableDeclarationNodes = []; @@ -94,7 +100,7 @@ function checkDeclarationList( addFix = false; } } - return invalidVariableDeclarationNodes; + return { invalidNodes: invalidVariableDeclarationNodes }; } - return []; + return { invalidNodes: [] }; } diff --git a/src/readonlyArrayRule.ts b/src/readonlyArrayRule.ts index af0c9ff..cddb779 100644 --- a/src/readonlyArrayRule.ts +++ b/src/readonlyArrayRule.ts @@ -3,7 +3,6 @@ import * as Lint from "tslint"; import * as utils from "tsutils/typeguard/2.8"; import * as Ignore from "./shared/ignore"; import { - InvalidNode, createInvalidNode, CheckNodeResult, createCheckNodeRule @@ -28,20 +27,26 @@ function checkNode( node: ts.Node, ctx: Lint.WalkContext ): CheckNodeResult { + const results = [ + checkArrayType(node, ctx), + checkTypeReference(node, ctx), + checkImplicitType(node, ctx), + checkTupleType(node, ctx) + ]; + return { - invalidNodes: [ - ...checkArrayType(node, ctx), - ...checkTypeReference(node, ctx), - ...checkImplicitType(node, ctx), - ...checkTupleType(node, ctx) - ] + invalidNodes: results.reduce( + (merged, result) => [...merged, ...result.invalidNodes], + [] + ), + skipChildren: results.some(result => result.skipChildren === true) }; } function checkArrayType( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { // We need to check both shorthand syntax "number[]"... if (utils.isArrayTypeNode(node)) { // Ignore arrays decleared with readonly keyword. @@ -50,14 +55,17 @@ function checkArrayType( utils.isTypeOperatorNode(node.parent) && node.parent.operator === ts.SyntaxKind.ReadonlyKeyword ) { - return []; + return { invalidNodes: [] }; } if ( node.parent && Ignore.shouldIgnorePrefix(node.parent, ctx.options, ctx.sourceFile) ) { - return []; + return { + invalidNodes: [], + skipChildren: true + }; } if ( @@ -66,33 +74,35 @@ function checkArrayType( utils.isParameterDeclaration(node.parent) && node.parent.dotDotDotToken ) { - return []; + return { invalidNodes: [] }; } if (ctx.options.ignoreReturnType && checkIsReturnTypeOrNestedWithIn(node)) { - return []; + return { invalidNodes: [] }; } const [major, minor] = ts.version .split(".") .map(n => Number.parseInt(n, 10)); - return [ - createInvalidNode( - node, - major > 3 || (major === 3 && minor >= 4) - ? getReadonlyKeywordFix(node, ctx) - : getReadonlyArrayFix(node, ctx) - ) - ]; + return { + invalidNodes: [ + createInvalidNode( + node, + major > 3 || (major === 3 && minor >= 4) + ? getReadonlyKeywordFix(node, ctx) + : getReadonlyArrayFix(node, ctx) + ) + ] + }; } - return []; + return { invalidNodes: [] }; } function checkTypeReference( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { // ...and type reference "Array" if ( utils.isTypeReferenceNode(node) && @@ -102,28 +112,36 @@ function checkTypeReference( node.parent && Ignore.shouldIgnorePrefix(node.parent, ctx.options, ctx.sourceFile) ) { - return []; + return { + invalidNodes: [], + skipChildren: true + }; } if (ctx.options.ignoreReturnType && checkIsReturnTypeOrNestedWithIn(node)) { - return []; + return { invalidNodes: [] }; } - return [ - createInvalidNode(node, [ - new Lint.Replacement(node.getStart(ctx.sourceFile), 0, "Readonly") - ]) - ]; + return { + invalidNodes: [ + createInvalidNode(node, [ + new Lint.Replacement(node.getStart(ctx.sourceFile), 0, "Readonly") + ]) + ] + }; } - return []; + return { invalidNodes: [] }; } export function checkImplicitType( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { if (Ignore.shouldIgnorePrefix(node, ctx.options, ctx.sourceFile)) { - return []; + return { + invalidNodes: [], + skipChildren: true + }; } // Check if the initializer is used to set an implicit array type if ( @@ -134,17 +152,19 @@ export function checkImplicitType( const nameText = node.name.getText(ctx.sourceFile); let typeArgument = "any"; - return [ - createInvalidNode(node.name, [ - new Lint.Replacement( - node.name.end - length, - length, - `${nameText}: ReadonlyArray<${typeArgument}>` - ) - ]) - ]; + return { + invalidNodes: [ + createInvalidNode(node.name, [ + new Lint.Replacement( + node.name.end - length, + length, + `${nameText}: ReadonlyArray<${typeArgument}>` + ) + ]) + ] + }; } - return []; + return { invalidNodes: [] }; } function checkIsReturnTypeOrNestedWithIn(node: ts.Node): boolean { @@ -173,7 +193,7 @@ function isUntypedAndHasArrayLiteralExpressionInitializer( function checkTupleType( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { const [major, minor] = ts.version.split(".").map(n => Number.parseInt(n, 10)); // Only applies to ts 3.4 and newer. @@ -185,13 +205,17 @@ function checkTupleType( utils.isTypeOperatorNode(node.parent) && node.parent.operator === ts.SyntaxKind.ReadonlyKeyword ) { - return []; + return { invalidNodes: [] }; } - return [createInvalidNode(node, getReadonlyKeywordFix(node, ctx))]; + return { + invalidNodes: [ + createInvalidNode(node, getReadonlyKeywordFix(node, ctx)) + ] + }; } } - return []; + return { invalidNodes: [] }; } function getReadonlyKeywordFix( diff --git a/src/readonlyKeywordRule.ts b/src/readonlyKeywordRule.ts index 8bcca7b..656f1a3 100644 --- a/src/readonlyKeywordRule.ts +++ b/src/readonlyKeywordRule.ts @@ -3,7 +3,6 @@ import * as Lint from "tslint"; import * as utils from "tsutils/typeguard/2.8"; import * as Ignore from "./shared/ignore"; import { - InvalidNode, createInvalidNode, CheckNodeResult, createCheckNodeRule @@ -28,13 +27,13 @@ function checkNode( node: ts.Node, ctx: Lint.WalkContext ): CheckNodeResult { - return { invalidNodes: checkPropertySignatureAndIndexSignature(node, ctx) }; + return checkPropertySignatureAndIndexSignature(node, ctx); } function checkPropertySignatureAndIndexSignature( node: ts.Node, ctx: Lint.WalkContext -): ReadonlyArray { +): CheckNodeResult { if ( (utils.isPropertySignature(node) || utils.isIndexSignatureDeclaration(node) || @@ -47,16 +46,18 @@ function checkPropertySignatureAndIndexSignature( ) { // Check if ignore-prefix applies if (Ignore.shouldIgnorePrefix(node, ctx.options, ctx.sourceFile)) { - return []; + return { invalidNodes: [] }; } const start = utils.isIndexSignatureDeclaration(node) ? node.getStart(ctx.sourceFile) : node.name.getStart(ctx.sourceFile); - return [ - createInvalidNode(node, [new Lint.Replacement(start, 0, "readonly ")]) - ]; + return { + invalidNodes: [ + createInvalidNode(node, [new Lint.Replacement(start, 0, "readonly ")]) + ] + }; } - return []; + return { invalidNodes: [] }; } diff --git a/test/rules/no-array-mutation/ignore-prefix/test.ts.lint b/test/rules/no-array-mutation/ignore-prefix/test.ts.lint index aa25250..8386bf3 100644 --- a/test/rules/no-array-mutation/ignore-prefix/test.ts.lint +++ b/test/rules/no-array-mutation/ignore-prefix/test.ts.lint @@ -1,5 +1,6 @@ const x = [0, 1, 2]; const mutableY = [0, 1, 2]; +const mutable2D = [[0], [1], [2]]; // Disallowed array mutation patterns @@ -18,11 +19,19 @@ x[0]++; // Allowed prefix mutableY[0] = 4; +mutable2D[0] = [3]; +mutable2D[0][0] = [1]; delete mutableY[0]; +delete mutable2D[0]; +delete mutable2D[0][0]; mutableY[0]++; +mutable2D[0]++; +mutable2D[0][0]++; ++mutableY[0]; +++mutable2D[0]; +++mutable2D[0][0]; [failure]: Mutating an array is not allowed. diff --git a/test/rules/readonly-array/ignore-prefix/nested.ts.lint b/test/rules/readonly-array/ignore-prefix/nested.ts.lint new file mode 100644 index 0000000..3afdf7a --- /dev/null +++ b/test/rules/readonly-array/ignore-prefix/nested.ts.lint @@ -0,0 +1,9 @@ +const _2DArray: number[][] = [[0], [1], [2]]; + ~~~~~~~~~~ [failure] + ~~~~~~~~ [failure] + +const mutable2DArray: number[][] = [[0], [1], [2]]; + +const mutableObjectArray: { foo: number[] } = { foo: [0] }; + +[failure]: Only ReadonlyArray allowed.