From 69ec84063ba774ced983d764f62115481782339c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 11 Mar 2020 09:30:14 -0700 Subject: [PATCH] feat: check array patterns --- .../docs/rules/no-unsafe-assignment.md | 3 + .../src/rules/no-unsafe-assignment.ts | 147 +++++++++++++++--- packages/eslint-plugin/src/util/types.ts | 16 +- .../tests/rules/no-unsafe-assignment.test.ts | 135 ++++++++++++++-- 4 files changed, 267 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md index 8cc1b10402a..dfc798826d7 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md +++ b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md @@ -15,6 +15,8 @@ const x = 1 as any, y = 1 as any; const [x] = 1 as any; const [x] = [] as any[]; +const [x] = [1 as any]; +[x] = [1] as [any]; function foo(a = 1 as any) {} class Foo { @@ -37,6 +39,7 @@ Examples of **correct** code for this rule: const x = 1, y = 1; const [x] = [1]; +[x] = [1] as [number]; function foo(a = 1) {} class Foo { diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index 51fe78c2798..555814c1523 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -2,6 +2,7 @@ import { TSESTree, AST_NODE_TYPES, } from '@typescript-eslint/experimental-utils'; +import * as ts from 'typescript'; import * as util from '../util'; const enum ComparisonType { @@ -26,6 +27,8 @@ export default util.createRule({ messages: { anyAssignment: 'Unsafe assignment of an any value', unsafeArrayPattern: 'Unsafe array destructuring of an any array value', + unsafeArrayPatternFromTuple: + 'Unsafe array destructuring of a tuple element with an any value', unsafeAssignment: 'Unsafe asignment of type {{sender}} to a variable of type {{receiver}}', }, @@ -36,12 +39,101 @@ export default util.createRule({ const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context); const checker = program.getTypeChecker(); + function checkArrayDestructureHelper( + receiverNode: TSESTree.Node, + senderNode: TSESTree.Node, + ): boolean { + if (receiverNode.type !== AST_NODE_TYPES.ArrayPattern) { + return true; + } + + const senderType = checker.getTypeAtLocation( + esTreeNodeToTSNodeMap.get(senderNode), + ); + + return checkArrayDestructure(receiverNode, senderType); + } + + // returns true if the assignment is safe + function checkArrayDestructure( + receiverNode: TSESTree.ArrayPattern, + senderType: ts.Type, + ): boolean { + // any array + // const [x] = ([] as any[]); + if (util.isTypeAnyArrayType(senderType, checker)) { + context.report({ + node: receiverNode, + messageId: 'unsafeArrayPattern', + }); + return false; + } + + if (!checker.isTupleType(senderType)) { + return true; + } + + const tupleElements = util.getTypeArguments(senderType, checker); + + // tuple with any + // const [x] = [1 as any]; + let didReport = false; + for ( + let receiverIndex = 0; + receiverIndex < receiverNode.elements.length; + receiverIndex += 1 + ) { + const receiverElement = receiverNode.elements[receiverIndex]; + if (!receiverElement) { + continue; + } + + if (receiverElement.type === AST_NODE_TYPES.RestElement) { + // const [...x] = [1, 2, 3 as any]; + // check the remaining elements to see if one of them is typed as any + for ( + let senderIndex = receiverIndex; + senderIndex < tupleElements.length; + senderIndex += 1 + ) { + const senderType = tupleElements[senderIndex]; + if (senderType && util.isTypeAnyType(senderType)) { + context.report({ + node: receiverElement, + messageId: 'unsafeArrayPatternFromTuple', + }); + return false; + } + } + // rest element must be the last one in a destructure + return true; + } + + const senderType = tupleElements[receiverIndex]; + if (receiverElement.type === AST_NODE_TYPES.ArrayPattern) { + didReport = checkArrayDestructure(receiverElement, senderType); + } else { + if (senderType && util.isTypeAnyType(senderType)) { + context.report({ + node: receiverElement, + messageId: 'unsafeArrayPatternFromTuple', + }); + // we want to report on every invalid element in the tuple + didReport = true; + } + } + } + + return didReport; + } + + // returns true if the assignment is safe function checkAssignment( receiverNode: TSESTree.Node, senderNode: TSESTree.Node, reportingNode: TSESTree.Node, comparisonType: ComparisonType, - ): void { + ): boolean { const receiverType = checker.getTypeAtLocation( esTreeNodeToTSNodeMap.get(receiverNode), ); @@ -50,33 +142,24 @@ export default util.createRule({ ); if (util.isTypeAnyType(senderType)) { - return context.report({ + context.report({ node: reportingNode, messageId: 'anyAssignment', }); - } - - if ( - receiverNode.type === AST_NODE_TYPES.ArrayPattern && - util.isTypeAnyArrayType(senderType, checker) - ) { - return context.report({ - node: reportingNode, - messageId: 'unsafeArrayPattern', - }); + return false; } if (comparisonType === ComparisonType.None) { - return; + return true; } const result = util.isUnsafeAssignment(senderType, receiverType, checker); if (!result) { - return; + return true; } const { sender, receiver } = result; - return context.report({ + context.report({ node: reportingNode, messageId: 'unsafeAssignment', data: { @@ -84,6 +167,7 @@ export default util.createRule({ receiver: checker.typeToString(receiver), }, }); + return false; } function getComparisonType( @@ -100,12 +184,16 @@ export default util.createRule({ 'VariableDeclarator[init != null]'( node: TSESTree.VariableDeclarator, ): void { - checkAssignment( + const isSafe = checkAssignment( node.id, node.init!, node, getComparisonType(node.id.typeAnnotation), ); + + if (isSafe) { + checkArrayDestructureHelper(node.id, node.init!); + } }, 'ClassProperty[value != null]'(node: TSESTree.ClassProperty): void { checkAssignment( @@ -118,16 +206,37 @@ export default util.createRule({ 'AssignmentExpression[operator = "="], AssignmentPattern'( node: TSESTree.AssignmentExpression | TSESTree.AssignmentPattern, ): void { - checkAssignment( + const isSafe = checkAssignment( node.left, node.right, node, // the variable already has some form of a type to compare against ComparisonType.Basic, ); - }, - // TODO - { x: 1 } + if (isSafe) { + checkArrayDestructureHelper(node.left, node.right); + } + }, + // Property(node): void { + // checkAssignment( + // node.key, + // node.value, + // node, + // ComparisonType.Contextual, // TODO - is this required??? + // ); + // }, + // 'JSXAttribute[value != null]'(node: TSESTree.JSXAttribute): void { + // if (!node.value) { + // return; + // } + // checkAssignment( + // node.name, + // node.value, + // node, + // ComparisonType.Basic, // TODO + // ); + // }, }; }, }); diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index aff148c063c..55f3ba12f9f 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -297,6 +297,18 @@ export function getEqualsKind(operator: string): EqualsKind | undefined { } } +export function getTypeArguments( + type: ts.TypeReference, + checker: ts.TypeChecker, +): readonly ts.Type[] { + // getTypeArguments was only added in TS3.7 + if (checker.getTypeArguments) { + return checker.getTypeArguments(type); + } + + return type.typeArguments ?? []; +} + /** * @returns true if the type is `any` */ @@ -315,9 +327,7 @@ export function isTypeAnyArrayType( checker.isArrayType(type) && isTypeAnyType( // getTypeArguments was only added in TS3.7 - checker.getTypeArguments - ? checker.getTypeArguments(type)[0] - : (type.typeArguments ?? [])[0], + getTypeArguments(type, checker)[0], ) ); } diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts index b7a10612e06..7818f063b1f 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -3,6 +3,7 @@ import { RuleTester, batchedSingleLineTests, getFixturesRootDir, + noFormat, } from '../RuleTester'; const ruleTester = new RuleTester({ @@ -15,23 +16,39 @@ const ruleTester = new RuleTester({ ruleTester.run('no-unsafe-assignment', rule, { valid: [ - 'const x = 1', - 'const x: number = 1', - 'const x = 1, y = 1', - 'let x', - 'let x = 1, y', + 'const x = 1;', + 'const x: number = 1;', + ` +const x = 1, + y = 1; + `, + 'let x;', + ` +let x = 1, + y; + `, 'function foo(a = 1) {}', - 'class Foo { constructor(private a = 1) {} }', - 'class Foo { private a = 1 }', + ` +class Foo { + constructor(private a = 1) {} +} + `, + ` +class Foo { + private a = 1; +} + `, 'const x: Set = new Set();', 'const x: Set = new Set();', - 'const [x] = [1]', + 'const [x] = [1];', + 'const [x, ...y] = [1, 2, 3, 4, 5];', + 'const [x, ...y] = [1];', // this is not checked, because there's no annotation to compare it with 'const x = new Set();', ], invalid: [ ...batchedSingleLineTests({ - code: ` + code: noFormat` const x = (1 as any); const x = (1 as any), y = 1; function foo(a = (1 as any)) {} @@ -72,7 +89,7 @@ class Foo { private a = (1 as any) } ], }), ...batchedSingleLineTests({ - code: ` + code: noFormat` const [x] = (1 as any); const [x] = [] as any[]; `, @@ -87,12 +104,12 @@ const [x] = [] as any[]; messageId: 'unsafeArrayPattern', line: 3, column: 7, - endColumn: 24, + endColumn: 10, }, ], }), ...batchedSingleLineTests({ - code: ` + code: noFormat` const x: Set = new Set(); const x: Map = new Map(); const x: Set = new Set(); @@ -133,5 +150,99 @@ const x: Set>> = new Set>>(); }, ], }), + ...batchedSingleLineTests({ + code: noFormat` +const [x] = [1 as any]; +const [x, ...y] = [1, 2 as any]; +const [x, y, ...z] = [1, 2 as any, 3 as any]; +const [x, ...y] = [1, 2, 3, 4 as any]; +const [[[[x]]]] = [[[[1 as any]]]]; + `, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 2, + column: 8, + endColumn: 9, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 3, + column: 11, + endColumn: 15, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 11, + endColumn: 12, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 14, + endColumn: 18, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 5, + column: 11, + endColumn: 15, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 6, + column: 11, + endColumn: 12, + }, + ], + }), + ...batchedSingleLineTests({ + code: noFormat` +[x] = [1] as [any]; +[x, ...y] = [1, 2] as [1, any]; +[x, y, ...z] = [1, 2, 3] as [1, any, any]; +[x, ...y] = [1, 2, 3, 4] as [1, 2, 3, any]; +[[[[x]]]] = [[[[1 as any]]]]; + `, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 2, + column: 2, + endColumn: 3, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 3, + column: 5, + endColumn: 9, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 5, + endColumn: 6, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 8, + endColumn: 12, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 5, + column: 5, + endColumn: 9, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 6, + column: 5, + endColumn: 6, + }, + ], + }), ], });