From 41b7b15050badf85a271d6e367ac73a8d8e8db8c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 6 Mar 2020 17:04:14 -0800 Subject: [PATCH 1/8] feat(eslint-plugin): add rule no-unsafe-assignment --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-unsafe-assignment.md | 59 ++++++++ .../docs/rules/no-unsafe-call.md | 2 +- packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-unsafe-assignment.ts | 133 +++++++++++++++++ packages/eslint-plugin/src/util/types.ts | 28 ++-- .../tests/rules/no-unsafe-assignment.test.ts | 137 ++++++++++++++++++ .../src/ts-estree/ts-estree.ts | 2 +- 9 files changed, 354 insertions(+), 11 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-unsafe-assignment.md create mode 100644 packages/eslint-plugin/src/rules/no-unsafe-assignment.ts create mode 100644 packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index fa049b2cfb2..d8cb74e8659 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -133,6 +133,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Enforces that type arguments will not be used if not required | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unsafe-assignment`](./docs/rules/no-unsafe-assignment.md) | Disallows assigning any to variables and properties | | | :thought_balloon: | | [`@typescript-eslint/no-unsafe-call`](./docs/rules/no-unsafe-call.md) | Disallows calling an any type value | | | :thought_balloon: | | [`@typescript-eslint/no-unsafe-member-access`](./docs/rules/no-unsafe-member-access.md) | Disallows member access on any typed variables | | | :thought_balloon: | | [`@typescript-eslint/no-unsafe-return`](./docs/rules/no-unsafe-return.md) | Disallows returning any from a function | | | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md new file mode 100644 index 00000000000..8cc1b10402a --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md @@ -0,0 +1,59 @@ +# Disallows returning any from a function (`no-unsafe-return`) + +Despite your best intentions, the `any` type can sometimes leak into your codebase. +Assigning an `any` typed value to a variable can be hard to pick up on, particularly if it leaks in from an external library. Operations on the variable will not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase. + +## Rule Details + +This rule disallows the assigning `any` to a variable, and assigning `any[]` to an array destructuring. +This rule also compares the assigned type to the variable's declared/inferred return type to ensure you don't return an unsafe `any` in a generic position to a receiver that's expecting a specific type. For example, it will error if you return `Set` from a function declared as returning `Set`. + +Examples of **incorrect** code for this rule: + +```ts +const x = 1 as any, + y = 1 as any; +const [x] = 1 as any; +const [x] = [] as any[]; + +function foo(a = 1 as any) {} +class Foo { + constructor(private a = 1 as any) {} +} +class Foo { + private a = 1 as any; +} + +// generic position examples +const x: Set = new Set(); +const x: Map = new Map(); +const x: Set = new Set(); +const x: Set>> = new Set>>(); +``` + +Examples of **correct** code for this rule: + +```ts +const x = 1, + y = 1; +const [x] = [1]; + +function foo(a = 1) {} +class Foo { + constructor(private a = 1) {} +} +class Foo { + private a = 1; +} + +// generic position examples +const x: Set = new Set(); +const x: Map = new Map(); +const x: Set = new Set(); +const x: Set>> = new Set>>(); +``` + +## Related to + +- [`no-explicit-any`](./no-explicit-any.md) +- TSLint: [`no-unsafe-any`](https://palantir.github.io/tslint/rules/no-unsafe-any/) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-call.md b/packages/eslint-plugin/docs/rules/no-unsafe-call.md index 3c18b7c1ee2..490ff78ced5 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-call.md +++ b/packages/eslint-plugin/docs/rules/no-unsafe-call.md @@ -1,7 +1,7 @@ # Disallows calling an any type value (`no-unsafe-call`) Despite your best intentions, the `any` type can sometimes leak into your codebase. -Member access on `any` typed variables is not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase. +The arguments to, and return value of calling an `any` typed variable are not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase. ## Rule Details diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index c5575d4970d..b3e3d40e155 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -62,6 +62,7 @@ "@typescript-eslint/no-unnecessary-qualifier": "error", "@typescript-eslint/no-unnecessary-type-arguments": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", + "@typescript-eslint/no-unsafe-assignment": "error", "@typescript-eslint/no-unsafe-call": "error", "@typescript-eslint/no-unsafe-member-access": "error", "@typescript-eslint/no-unsafe-return": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 29db58b681a..a466330ed17 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -54,6 +54,7 @@ import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; +import noUnsafeAssignment from './no-unsafe-assignment'; import noUnsafeCall from './no-unsafe-call'; import noUnsafeMemberAccess from './no-unsafe-member-access'; import noUnsafeReturn from './no-unsafe-return'; @@ -149,6 +150,7 @@ export default { 'no-unnecessary-qualifier': noUnnecessaryQualifier, 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, + 'no-unsafe-assignment': noUnsafeAssignment, 'no-unsafe-call': noUnsafeCall, 'no-unsafe-member-access': noUnsafeMemberAccess, 'no-unsafe-return': noUnsafeReturn, diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts new file mode 100644 index 00000000000..51fe78c2798 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -0,0 +1,133 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +const enum ComparisonType { + /** Do no assignment comparison */ + None, + /** Use the receiver's type for comparison */ + Basic, + /** Use the sender's contextual type for comparison */ + Contextual, +} + +export default util.createRule({ + name: 'no-unsafe-assignment', + meta: { + type: 'problem', + docs: { + description: 'Disallows assigning any to variables and properties', + category: 'Possible Errors', + recommended: false, + requiresTypeChecking: true, + }, + messages: { + anyAssignment: 'Unsafe assignment of an any value', + unsafeArrayPattern: 'Unsafe array destructuring of an any array value', + unsafeAssignment: + 'Unsafe asignment of type {{sender}} to a variable of type {{receiver}}', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context); + const checker = program.getTypeChecker(); + + function checkAssignment( + receiverNode: TSESTree.Node, + senderNode: TSESTree.Node, + reportingNode: TSESTree.Node, + comparisonType: ComparisonType, + ): void { + const receiverType = checker.getTypeAtLocation( + esTreeNodeToTSNodeMap.get(receiverNode), + ); + const senderType = checker.getTypeAtLocation( + esTreeNodeToTSNodeMap.get(senderNode), + ); + + if (util.isTypeAnyType(senderType)) { + return context.report({ + node: reportingNode, + messageId: 'anyAssignment', + }); + } + + if ( + receiverNode.type === AST_NODE_TYPES.ArrayPattern && + util.isTypeAnyArrayType(senderType, checker) + ) { + return context.report({ + node: reportingNode, + messageId: 'unsafeArrayPattern', + }); + } + + if (comparisonType === ComparisonType.None) { + return; + } + + const result = util.isUnsafeAssignment(senderType, receiverType, checker); + if (!result) { + return; + } + + const { sender, receiver } = result; + return context.report({ + node: reportingNode, + messageId: 'unsafeAssignment', + data: { + sender: checker.typeToString(sender), + receiver: checker.typeToString(receiver), + }, + }); + } + + function getComparisonType( + typeAnnotation: TSESTree.TSTypeAnnotation | undefined, + ): ComparisonType { + return typeAnnotation + ? // if there's a type annotation, we can do a comparison + ComparisonType.Basic + : // no type annotation means the variable's type will just be inferred, thus equal + ComparisonType.None; + } + + return { + 'VariableDeclarator[init != null]'( + node: TSESTree.VariableDeclarator, + ): void { + checkAssignment( + node.id, + node.init!, + node, + getComparisonType(node.id.typeAnnotation), + ); + }, + 'ClassProperty[value != null]'(node: TSESTree.ClassProperty): void { + checkAssignment( + node.key, + node.value!, + node, + getComparisonType(node.typeAnnotation), + ); + }, + 'AssignmentExpression[operator = "="], AssignmentPattern'( + node: TSESTree.AssignmentExpression | TSESTree.AssignmentPattern, + ): void { + checkAssignment( + node.left, + node.right, + node, + // the variable already has some form of a type to compare against + ComparisonType.Basic, + ); + }, + + // TODO - { x: 1 } + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index f602a1e6a27..aff148c063c 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -304,6 +304,24 @@ export function isTypeAnyType(type: ts.Type): boolean { return isTypeFlagSet(type, ts.TypeFlags.Any); } +/** + * @returns true if the type is `any[]` + */ +export function isTypeAnyArrayType( + type: ts.Type, + checker: ts.TypeChecker, +): boolean { + return ( + checker.isArrayType(type) && + isTypeAnyType( + // getTypeArguments was only added in TS3.7 + checker.getTypeArguments + ? checker.getTypeArguments(type)[0] + : (type.typeArguments ?? [])[0], + ) + ); +} + export const enum AnyType { Any, AnyArray, @@ -321,15 +339,7 @@ export function isAnyOrAnyArrayTypeDiscriminated( if (isTypeAnyType(type)) { return AnyType.Any; } - if ( - checker.isArrayType(type) && - isTypeAnyType( - // getTypeArguments was only added in TS3.7 - checker.getTypeArguments - ? checker.getTypeArguments(type)[0] - : (type.typeArguments ?? [])[0], - ) - ) { + if (isTypeAnyArrayType(type, checker)) { return AnyType.AnyArray; } return AnyType.Safe; diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts new file mode 100644 index 00000000000..b7a10612e06 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -0,0 +1,137 @@ +import rule from '../../src/rules/no-unsafe-assignment'; +import { + RuleTester, + batchedSingleLineTests, + getFixturesRootDir, +} from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: getFixturesRootDir(), + }, +}); + +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', + 'function foo(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]', + // this is not checked, because there's no annotation to compare it with + 'const x = new Set();', + ], + invalid: [ + ...batchedSingleLineTests({ + code: ` +const x = (1 as any); +const x = (1 as any), y = 1; +function foo(a = (1 as any)) {} +class Foo { constructor(private a = (1 as any)) {} } +class Foo { private a = (1 as any) } + `, + errors: [ + { + messageId: 'anyAssignment', + line: 2, + column: 7, + endColumn: 21, + }, + { + messageId: 'anyAssignment', + line: 3, + column: 7, + endColumn: 21, + }, + { + messageId: 'anyAssignment', + line: 4, + column: 14, + endColumn: 28, + }, + { + messageId: 'anyAssignment', + line: 5, + column: 33, + endColumn: 47, + }, + { + messageId: 'anyAssignment', + line: 6, + column: 13, + endColumn: 35, + }, + ], + }), + ...batchedSingleLineTests({ + code: ` +const [x] = (1 as any); +const [x] = [] as any[]; + `, + errors: [ + { + messageId: 'anyAssignment', + line: 2, + column: 7, + endColumn: 23, + }, + { + messageId: 'unsafeArrayPattern', + line: 3, + column: 7, + endColumn: 24, + }, + ], + }), + ...batchedSingleLineTests({ + code: ` +const x: Set = new Set(); +const x: Map = new Map(); +const x: Set = new Set(); +const x: Set>> = new Set>>(); + `, + errors: [ + { + messageId: 'unsafeAssignment', + data: { + sender: 'Set', + receiver: 'Set', + }, + line: 2, + }, + { + messageId: 'unsafeAssignment', + data: { + sender: 'Map', + receiver: 'Map', + }, + line: 3, + }, + { + messageId: 'unsafeAssignment', + data: { + sender: 'Set', + receiver: 'Set', + }, + line: 4, + }, + { + messageId: 'unsafeAssignment', + data: { + sender: 'Set>>', + receiver: 'Set>>', + }, + line: 5, + }, + ], + }), + ], +}); diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index cbf7787c805..35935ade9f9 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -766,7 +766,7 @@ export interface AssignmentExpression extends BinaryExpressionBase { export interface AssignmentPattern extends BaseNode { type: AST_NODE_TYPES.AssignmentPattern; left: BindingName; - right?: Expression; + right: Expression; typeAnnotation?: TSTypeAnnotation; optional?: boolean; decorators?: Decorator[]; From 69ec84063ba774ced983d764f62115481782339c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 11 Mar 2020 09:30:14 -0700 Subject: [PATCH 2/8] 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, + }, + ], + }), ], }); From b8c925038acd02f1017a449dc2656a3b08f10350 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 17:06:05 -0700 Subject: [PATCH 3/8] test: assignment pattern --- .../tests/rules/no-unsafe-assignment.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) 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 7818f063b1f..4abc4fd8a2e 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -43,6 +43,10 @@ class Foo { 'const [x] = [1];', 'const [x, ...y] = [1, 2, 3, 4, 5];', 'const [x, ...y] = [1];', + 'function foo(x = 1) {}', + 'function foo([x] = [1]) {}', + 'function foo([x, ...y] = [1, 2, 3, 4, 5]) {}', + 'function foo([x, ...y] = [1]) {}', // this is not checked, because there's no annotation to compare it with 'const x = new Set();', ], @@ -244,5 +248,52 @@ const [[[[x]]]] = [[[[1 as any]]]]; }, ], }), + ...batchedSingleLineTests({ + code: noFormat` +function foo([x] = [1] as [any]) {} +function foo([x, ...y] = [1, 2] as [1, any]) {} +function foo([x, y, ...z] = [1, 2, 3] as [1, any, any]) {} +function foo([x, ...y] = [1, 2, 3, 4] as [1, 2, 3, any]) {} +function foo([[[[x]]]] = [[[[1 as any]]]]) {} + `, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 2, + column: 15, + endColumn: 16, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 3, + column: 18, + endColumn: 22, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 18, + endColumn: 19, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 4, + column: 21, + endColumn: 25, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 5, + column: 18, + endColumn: 22, + }, + { + messageId: 'unsafeArrayPatternFromTuple', + line: 6, + column: 18, + endColumn: 19, + }, + ], + }), ], }); From fcbaf9163fe345f22861c203c91b3551b39723a1 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 18:38:58 -0700 Subject: [PATCH 4/8] feat: check object pattern --- .../src/rules/no-unsafe-assignment.ts | 196 ++++++++++++---- packages/eslint-plugin/src/util/types.ts | 7 +- .../tests/rules/no-unsafe-assignment.test.ts | 213 +++++++----------- 3 files changed, 229 insertions(+), 187 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index 555814c1523..448d6dc08a5 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -39,25 +39,26 @@ export default util.createRule({ const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context); const checker = program.getTypeChecker(); + // returns true if the assignment reported function checkArrayDestructureHelper( receiverNode: TSESTree.Node, senderNode: TSESTree.Node, ): boolean { if (receiverNode.type !== AST_NODE_TYPES.ArrayPattern) { - return true; + return false; } - const senderType = checker.getTypeAtLocation( - esTreeNodeToTSNodeMap.get(senderNode), - ); + const senderTsNode = esTreeNodeToTSNodeMap.get(senderNode); + const senderType = checker.getTypeAtLocation(senderTsNode); - return checkArrayDestructure(receiverNode, senderType); + return checkArrayDestructure(receiverNode, senderType, senderTsNode); } - // returns true if the assignment is safe + // returns true if the assignment reported function checkArrayDestructure( receiverNode: TSESTree.ArrayPattern, senderType: ts.Type, + senderNode: ts.Node, ): boolean { // any array // const [x] = ([] as any[]); @@ -89,45 +90,136 @@ export default util.createRule({ } 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; + // don't handle rests as they're not a 1:1 assignment + continue; } - const senderType = tupleElements[receiverIndex]; - if (receiverElement.type === AST_NODE_TYPES.ArrayPattern) { - didReport = checkArrayDestructure(receiverElement, senderType); + const senderType = tupleElements[receiverIndex] as ts.Type | undefined; + if (!senderType) { + continue; + } + + // check for the any type first so we can handle [[[x]]] = [any] + if (util.isTypeAnyType(senderType)) { + context.report({ + node: receiverElement, + messageId: 'unsafeArrayPatternFromTuple', + }); + // we want to report on every invalid element in the tuple + didReport = true; + } else if (receiverElement.type === AST_NODE_TYPES.ArrayPattern) { + didReport = checkArrayDestructure( + receiverElement, + senderType, + senderNode, + ); + } else if (receiverElement.type === AST_NODE_TYPES.ObjectPattern) { + didReport = checkObjectDestructure( + receiverElement, + senderType, + senderNode, + ); + } + } + + return didReport; + } + + // returns true if the assignment reported + function checkObjectDestructureHelper( + receiverNode: TSESTree.Node, + senderNode: TSESTree.Node, + ): boolean { + if (receiverNode.type !== AST_NODE_TYPES.ObjectPattern) { + return false; + } + + const senderTsNode = esTreeNodeToTSNodeMap.get(senderNode); + const senderType = checker.getTypeAtLocation(senderTsNode); + + return checkObjectDestructure(receiverNode, senderType, senderTsNode); + } + + // returns true if the assignment reported + function checkObjectDestructure( + receiverNode: TSESTree.ObjectPattern, + senderType: ts.Type, + senderNode: ts.Node, + ): boolean { + const properties = new Map( + senderType + .getProperties() + .map(property => [ + property.getName(), + checker.getTypeOfSymbolAtLocation(property, senderNode), + ]), + ); + + let didReport = false; + for ( + let receiverIndex = 0; + receiverIndex < receiverNode.properties.length; + receiverIndex += 1 + ) { + const receiverProperty = receiverNode.properties[receiverIndex]; + if (receiverProperty.type === AST_NODE_TYPES.RestElement) { + // don't bother checking rest + continue; + } + + let key: string; + if (receiverProperty.computed === false) { + key = + receiverProperty.key.type === AST_NODE_TYPES.Identifier + ? receiverProperty.key.name + : String(receiverProperty.key.value); + } else if (receiverProperty.key.type === AST_NODE_TYPES.Literal) { + key = String(receiverProperty.key.value); + } else if ( + receiverProperty.key.type === AST_NODE_TYPES.TemplateLiteral && + receiverProperty.key.quasis.length === 1 + ) { + key = String(receiverProperty.key.quasis[0].value.cooked); } 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; - } + // can't figure out the name, so skip it + continue; + } + + const senderType = properties.get(key); + if (!senderType) { + continue; + } + + // check for the any type first so we can handle {x: {y: z}} = {x: any} + if (util.isTypeAnyType(senderType)) { + context.report({ + node: receiverProperty.value, + messageId: 'unsafeArrayPatternFromTuple', + }); + didReport = true; + } else if ( + receiverProperty.value.type === AST_NODE_TYPES.ArrayPattern + ) { + didReport = checkArrayDestructure( + receiverProperty.value, + senderType, + senderNode, + ); + } else if ( + receiverProperty.value.type === AST_NODE_TYPES.ObjectPattern + ) { + didReport = checkObjectDestructure( + receiverProperty.value, + senderType, + senderNode, + ); } } return didReport; } - // returns true if the assignment is safe + // returns true if the assignment reported function checkAssignment( receiverNode: TSESTree.Node, senderNode: TSESTree.Node, @@ -146,16 +238,16 @@ export default util.createRule({ node: reportingNode, messageId: 'anyAssignment', }); - return false; + return true; } if (comparisonType === ComparisonType.None) { - return true; + return false; } const result = util.isUnsafeAssignment(senderType, receiverType, checker); if (!result) { - return true; + return false; } const { sender, receiver } = result; @@ -167,7 +259,7 @@ export default util.createRule({ receiver: checker.typeToString(receiver), }, }); - return false; + return true; } function getComparisonType( @@ -184,15 +276,22 @@ export default util.createRule({ 'VariableDeclarator[init != null]'( node: TSESTree.VariableDeclarator, ): void { - const isSafe = checkAssignment( + const init = util.nullThrows( + node.init, + util.NullThrowsReasons.MissingToken(node.type, 'init'), + ); + let didReport = checkAssignment( node.id, - node.init!, + init, node, getComparisonType(node.id.typeAnnotation), ); - if (isSafe) { - checkArrayDestructureHelper(node.id, node.init!); + if (!didReport) { + didReport = checkArrayDestructureHelper(node.id, init); + } + if (!didReport) { + checkObjectDestructureHelper(node.id, init); } }, 'ClassProperty[value != null]'(node: TSESTree.ClassProperty): void { @@ -206,7 +305,7 @@ export default util.createRule({ 'AssignmentExpression[operator = "="], AssignmentPattern'( node: TSESTree.AssignmentExpression | TSESTree.AssignmentPattern, ): void { - const isSafe = checkAssignment( + let didReport = checkAssignment( node.left, node.right, node, @@ -214,8 +313,11 @@ export default util.createRule({ ComparisonType.Basic, ); - if (isSafe) { - checkArrayDestructureHelper(node.left, node.right); + if (!didReport) { + didReport = checkArrayDestructureHelper(node.left, node.right); + } + if (!didReport) { + checkObjectDestructureHelper(node.left, node.right); } }, // Property(node): void { diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 55f3ba12f9f..591613ac250 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -370,6 +370,10 @@ export function isUnsafeAssignment( receiver: ts.Type, checker: ts.TypeChecker, ): false | { sender: ts.Type; receiver: ts.Type } { + if (isTypeAnyType(type) && !isTypeAnyType(receiver)) { + return { sender: type, receiver }; + } + if (isTypeReference(type) && isTypeReference(receiver)) { // TODO - figure out how to handle cases like this, // where the types are assignable, but not the same type @@ -406,9 +410,6 @@ export function isUnsafeAssignment( return false; } - if (isTypeAnyType(type) && !isTypeAnyType(receiver)) { - return { sender: type, receiver }; - } return false; } 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 4abc4fd8a2e..362197de5d5 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -1,3 +1,4 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../../src/rules/no-unsafe-assignment'; import { RuleTester, @@ -5,6 +6,64 @@ import { getFixturesRootDir, noFormat, } from '../RuleTester'; +import { + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule, +} from '../../src/util'; + +type Options = InferOptionsTypeFromRule; +type MessageIds = InferMessageIdsTypeFromRule; +type InvalidTest = TSESLint.InvalidTestCase; + +function assignmentTest( + tests: [string, number, number, boolean?][], +): InvalidTest[] { + return tests.reduce( + (acc, [assignment, column, endColumn, skipAssignmentExpression]) => { + // VariableDeclaration + acc.push({ + code: `const ${assignment}`, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 1, + column: column + 6, + endColumn: endColumn + 6, + }, + ], + }); + // AssignmentPattern + acc.push({ + code: `function foo(${assignment}) {}`, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 1, + column: column + 13, + endColumn: endColumn + 13, + }, + ], + }); + // AssignmentExpression + if (skipAssignmentExpression !== true) { + acc.push({ + code: `(${assignment})`, + errors: [ + { + messageId: 'unsafeArrayPatternFromTuple', + line: 1, + column: column + 1, + endColumn: endColumn + 1, + }, + ], + }); + } + + return acc; + }, + [], + ); +} const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -154,146 +213,26 @@ 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]]]]; - `, + ...assignmentTest([ + ['[x] = [1] as [any]', 2, 3], + ['[[[[x]]]] = [[[[1 as any]]]]', 5, 6], + ['[[[[x]]]] = [1 as any]', 2, 9, true], + ['[{x}] = [{x: 1}] as [{x: any}]', 3, 4], + ]), + { + // TS treats the assignment pattern weirdly in this case + code: '[[[[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, - }, - ], - }), - ...batchedSingleLineTests({ - code: noFormat` -function foo([x] = [1] as [any]) {} -function foo([x, ...y] = [1, 2] as [1, any]) {} -function foo([x, y, ...z] = [1, 2, 3] as [1, any, any]) {} -function foo([x, ...y] = [1, 2, 3, 4] as [1, 2, 3, any]) {} -function foo([[[[x]]]] = [[[[1 as any]]]]) {} - `, - errors: [ - { - messageId: 'unsafeArrayPatternFromTuple', - line: 2, - column: 15, - endColumn: 16, - }, - { - messageId: 'unsafeArrayPatternFromTuple', - line: 3, - column: 18, - endColumn: 22, - }, - { - messageId: 'unsafeArrayPatternFromTuple', - line: 4, - column: 18, - endColumn: 19, - }, - { - messageId: 'unsafeArrayPatternFromTuple', - line: 4, - column: 21, - endColumn: 25, - }, - { - messageId: 'unsafeArrayPatternFromTuple', - line: 5, - column: 18, - endColumn: 22, - }, - { - messageId: 'unsafeArrayPatternFromTuple', - line: 6, - column: 18, - endColumn: 19, + messageId: 'unsafeAssignment', }, ], - }), + }, + ...assignmentTest([ + ['{x} = {x: 1} as {x: any}', 2, 3], + ['{x: y} = {x: 1} as {x: any}', 5, 6], + ['{x: {y}} = {x: {y: 1}} as {x: {y: any}}', 6, 7], + ['{x: [y]} = {x: {y: 1}} as {x: [any]}', 6, 7], + ]), ], }); From b1d1a92557590c9c84715cff99e944bf91816e6a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 21:01:40 -0700 Subject: [PATCH 5/8] feat: properties --- .../docs/rules/no-unsafe-assignment.md | 2 +- .../src/rules/no-unsafe-assignment.ts | 30 ++++++++------- packages/eslint-plugin/src/util/types.ts | 4 ++ .../tests/rules/no-unsafe-assignment.test.ts | 38 +++++++++++++++++-- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md index dfc798826d7..9344c59e9cc 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md +++ b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md @@ -1,4 +1,4 @@ -# Disallows returning any from a function (`no-unsafe-return`) +# Disallows assigning any to variables and properties (`no-unsafe-return`) Despite your best intentions, the `any` type can sometimes leak into your codebase. Assigning an `any` typed value to a variable can be hard to pick up on, particularly if it leaks in from an external library. Operations on the variable will not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase. diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index 448d6dc08a5..742b304c28d 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -222,13 +222,16 @@ export default util.createRule({ // returns true if the assignment reported function checkAssignment( receiverNode: TSESTree.Node, - senderNode: TSESTree.Node, + senderNode: TSESTree.Expression, reportingNode: TSESTree.Node, comparisonType: ComparisonType, ): boolean { - const receiverType = checker.getTypeAtLocation( - esTreeNodeToTSNodeMap.get(receiverNode), - ); + const receiverTsNode = esTreeNodeToTSNodeMap.get(receiverNode); + const receiverType = + comparisonType === ComparisonType.Contextual + ? util.getContextualType(checker, receiverTsNode as ts.Expression) ?? + checker.getTypeAtLocation(receiverTsNode) + : checker.getTypeAtLocation(receiverTsNode); const senderType = checker.getTypeAtLocation( esTreeNodeToTSNodeMap.get(senderNode), ); @@ -320,14 +323,15 @@ export default util.createRule({ checkObjectDestructureHelper(node.left, node.right); } }, - // Property(node): void { - // checkAssignment( - // node.key, - // node.value, - // node, - // ComparisonType.Contextual, // TODO - is this required??? - // ); - // }, + // object pattern props are checked via assignments + ':not(ObjectPattern) > Property'(node: TSESTree.Property): void { + if (node.value.type === AST_NODE_TYPES.AssignmentPattern) { + // handled by other selector + return; + } + + checkAssignment(node.key, node.value, node, ComparisonType.Contextual); + }, // 'JSXAttribute[value != null]'(node: TSESTree.JSXAttribute): void { // if (!node.value) { // return; @@ -336,7 +340,7 @@ export default util.createRule({ // node.name, // node.value, // node, - // ComparisonType.Basic, // TODO + // ComparisonType.Contextual, // TODO // ); // }, }; diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 591613ac250..ae6ee3843aa 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -1,6 +1,7 @@ import { isCallExpression, isJsxExpression, + isIdentifier, isNewExpression, isParameterDeclaration, isPropertyDeclaration, @@ -8,6 +9,7 @@ import { isUnionOrIntersectionType, isVariableDeclaration, unionTypeParts, + isPropertyAssignment, } from 'tsutils'; import * as ts from 'typescript'; @@ -440,6 +442,8 @@ export function getContextualType( return parent.type ? checker.getTypeFromTypeNode(parent.type) : undefined; } else if (isJsxExpression(parent)) { return checker.getContextualType(parent); + } else if (isPropertyAssignment(parent) && isIdentifier(node)) { + return checker.getContextualType(node); } else if ( ![ts.SyntaxKind.TemplateSpan, ts.SyntaxKind.JsxExpression].includes( parent.kind, 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 362197de5d5..f75d817ceb3 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -108,6 +108,8 @@ class Foo { 'function foo([x, ...y] = [1]) {}', // this is not checked, because there's no annotation to compare it with 'const x = new Set();', + 'const x = { y: 1 };', + 'const x: { y: number } = { y: 1 };', ], invalid: [ ...batchedSingleLineTests({ @@ -152,8 +154,8 @@ class Foo { private a = (1 as any) } ], }), ...batchedSingleLineTests({ - code: noFormat` -const [x] = (1 as any); + code: ` +const [x] = 1 as any; const [x] = [] as any[]; `, errors: [ @@ -161,7 +163,7 @@ const [x] = [] as any[]; messageId: 'anyAssignment', line: 2, column: 7, - endColumn: 23, + endColumn: 21, }, { messageId: 'unsafeArrayPattern', @@ -234,5 +236,35 @@ const x: Set>> = new Set>>(); ['{x: {y}} = {x: {y: 1}} as {x: {y: any}}', 6, 7], ['{x: [y]} = {x: {y: 1}} as {x: [any]}', 6, 7], ]), + ...batchedSingleLineTests({ + code: ` +const x = { y: 1 as any }; +const x = { y: { z: 1 as any } }; +const x: { y: Set>> } = { y: new Set>>() }; +const x = { ...(1 as any) }; + `, + errors: [ + { + messageId: 'anyAssignment', + line: 2, + }, + { + messageId: 'anyAssignment', + line: 3, + }, + { + messageId: 'unsafeAssignment', + line: 4, + data: { + sender: 'Set>>', + receiver: 'Set>>', + }, + }, + { + messageId: 'unsafeObjectSpread', + line: 5, + }, + ], + }), ], }); From d6f08d2610cd8952f1b8cf79b3fdd64c4eb1c1ba Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 21:10:01 -0700 Subject: [PATCH 6/8] feat: array spreads --- .../src/rules/no-unsafe-assignment.ts | 14 +++++++++++++ .../tests/rules/no-unsafe-assignment.test.ts | 20 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index 742b304c28d..6460e4d38ef 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -31,6 +31,7 @@ export default util.createRule({ 'Unsafe array destructuring of a tuple element with an any value', unsafeAssignment: 'Unsafe asignment of type {{sender}} to a variable of type {{receiver}}', + unsafeArraySpread: 'Unsafe spread of an any value in an array', }, schema: [], }, @@ -332,6 +333,19 @@ export default util.createRule({ checkAssignment(node.key, node.value, node, ComparisonType.Contextual); }, + 'ArrayExpression > SpreadElement'(node: TSESTree.SpreadElement): void { + const resetNode = esTreeNodeToTSNodeMap.get(node.argument); + const restType = checker.getTypeAtLocation(resetNode); + if ( + util.isTypeAnyType(restType) || + util.isTypeAnyArrayType(restType, checker) + ) { + context.report({ + node: node, + messageId: 'unsafeArraySpread', + }); + } + }, // 'JSXAttribute[value != null]'(node: TSESTree.JSXAttribute): void { // if (!node.value) { // return; 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 f75d817ceb3..8350d951f8d 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -110,6 +110,7 @@ class Foo { 'const x = new Set();', 'const x = { y: 1 };', 'const x: { y: number } = { y: 1 };', + 'const x = [...[1, 2, 3]];', ], invalid: [ ...batchedSingleLineTests({ @@ -230,6 +231,22 @@ const x: Set>> = new Set>>(); }, ], }, + ...batchedSingleLineTests({ + code: ` +const x = [...(1 as any)]; +const x = [...([] as any[])]; + `, + errors: [ + { + messageId: 'unsafeArraySpread', + line: 2, + }, + { + messageId: 'unsafeArraySpread', + line: 3, + }, + ], + }), ...assignmentTest([ ['{x} = {x: 1} as {x: any}', 2, 3], ['{x: y} = {x: 1} as {x: any}', 5, 6], @@ -261,7 +278,8 @@ const x = { ...(1 as any) }; }, }, { - messageId: 'unsafeObjectSpread', + // spreading an any widens the object type to any + messageId: 'anyAssignment', line: 5, }, ], From 9f942965fb3516c7b118dcdd40f015233171f22a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 21:31:22 -0700 Subject: [PATCH 7/8] feat: JSX attributes --- .../src/rules/no-unsafe-assignment.ts | 30 ++++++++------ .../tests/rules/no-unsafe-assignment.test.ts | 39 +++++++++++++++++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index 6460e4d38ef..60f7b78acc0 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -346,17 +346,25 @@ export default util.createRule({ }); } }, - // 'JSXAttribute[value != null]'(node: TSESTree.JSXAttribute): void { - // if (!node.value) { - // return; - // } - // checkAssignment( - // node.name, - // node.value, - // node, - // ComparisonType.Contextual, // TODO - // ); - // }, + 'JSXAttribute[value != null]'(node: TSESTree.JSXAttribute): void { + const value = util.nullThrows( + node.value, + util.NullThrowsReasons.MissingToken(node.type, 'value'), + ); + if ( + value.type !== AST_NODE_TYPES.JSXExpressionContainer || + value.expression.type === AST_NODE_TYPES.JSXEmptyExpression + ) { + return; + } + + checkAssignment( + node.name, + value.expression, + value.expression, + ComparisonType.Contextual, + ); + }, }; }, }); 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 8350d951f8d..708969310ed 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -111,6 +111,14 @@ class Foo { 'const x = { y: 1 };', 'const x: { y: number } = { y: 1 };', 'const x = [...[1, 2, 3]];', + { + code: ` +type Props = { a: string }; +declare function Foo(props: Props): never; +; + `, + filename: 'react.tsx', + }, ], invalid: [ ...batchedSingleLineTests({ @@ -228,6 +236,9 @@ const x: Set>> = new Set>>(); errors: [ { messageId: 'unsafeAssignment', + line: 1, + column: 1, + endColumn: 23, }, ], }, @@ -240,10 +251,14 @@ const x = [...([] as any[])]; { messageId: 'unsafeArraySpread', line: 2, + column: 12, + endColumn: 25, }, { messageId: 'unsafeArraySpread', line: 3, + column: 12, + endColumn: 28, }, ], }), @@ -264,14 +279,20 @@ const x = { ...(1 as any) }; { messageId: 'anyAssignment', line: 2, + column: 13, + endColumn: 24, }, { messageId: 'anyAssignment', line: 3, + column: 18, + endColumn: 29, }, { messageId: 'unsafeAssignment', line: 4, + column: 43, + endColumn: 70, data: { sender: 'Set>>', receiver: 'Set>>', @@ -281,8 +302,26 @@ const x = { ...(1 as any) }; // spreading an any widens the object type to any messageId: 'anyAssignment', line: 5, + column: 7, + endColumn: 28, }, ], }), + { + code: ` +type Props = { a: string }; +declare function Foo(props: Props): never; +; + `, + filename: 'react.tsx', + errors: [ + { + messageId: 'anyAssignment', + line: 4, + column: 9, + endColumn: 17, + }, + ], + }, ], }); From 7b4d11873c97445193b8784485a278e8ccfe18f1 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Apr 2020 21:40:39 -0700 Subject: [PATCH 8/8] docs: docs --- packages/eslint-plugin/docs/rules/no-unsafe-assignment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md index 9344c59e9cc..5cb6e8d61c2 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md +++ b/packages/eslint-plugin/docs/rules/no-unsafe-assignment.md @@ -1,4 +1,4 @@ -# Disallows assigning any to variables and properties (`no-unsafe-return`) +# Disallows assigning any to variables and properties (`no-unsafe-assignment`) Despite your best intentions, the `any` type can sometimes leak into your codebase. Assigning an `any` typed value to a variable can be hard to pick up on, particularly if it leaks in from an external library. Operations on the variable will not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase.