From ef6a8417fe10938b24442d41ffde70314e998338 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 6 Mar 2020 17:04:14 -0800 Subject: [PATCH] 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 | 18 ++- .../tests/rules/no-unsafe-assignment.test.ts | 137 ++++++++++++++++++ .../src/ts-estree/ts-estree.ts | 2 +- 9 files changed, 349 insertions(+), 6 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 b7927757924..c39d27ee814 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -132,6 +132,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 7a9cdb79f54..dffeaabfd48 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 2626767b84c..57b8659aa76 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -61,6 +61,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 486b8a97945..80821ad1b21 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -53,6 +53,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'; @@ -147,6 +148,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 924c68e151d..4a2078ddd17 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -304,6 +304,19 @@ 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(checker.getTypeArguments(type)[0]) + ); +} + export const enum AnyType { Any, AnyArray, @@ -321,10 +334,7 @@ export function isAnyOrAnyArrayTypeDiscriminated( if (isTypeAnyType(type)) { return AnyType.Any; } - if ( - checker.isArrayType(type) && - isTypeAnyType(checker.getTypeArguments(type)[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 94d4b7c2440..dc44200c404 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[];