diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 19ee2638a8e..a14b3ced2fd 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-member-access`](./docs/rules/no-unsafe-member-access.md) | Disallows member access on any typed variables | | | :thought_balloon: | | [`@typescript-eslint/no-unused-vars-experimental`](./docs/rules/no-unused-vars-experimental.md) | Disallow unused variables and arguments | | | :thought_balloon: | | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :heavy_check_mark: | | | | [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | | :wrench: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 3c7b5b4edc6..32eb363609c 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -90,7 +90,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`no-this-assignment`] | ✅ | [`@typescript-eslint/no-this-alias`] | | [`no-unbound-method`] | ✅ | [`@typescript-eslint/unbound-method`] | | [`no-unnecessary-class`] | ✅ | [`@typescript-eslint/no-extraneous-class`] | -| [`no-unsafe-any`] | 🛑 | N/A | +| [`no-unsafe-any`] | 🌓 | [`@typescript-eslint/no-unsafe-member-access`][2] | | [`no-unsafe-finally`] | 🌟 | [`no-unsafe-finally`][no-unsafe-finally] | | [`no-unused-expression`] | 🌟 | [`no-unused-expressions`][no-unused-expressions] | | [`no-unused-variable`] | 🌓 | [`@typescript-eslint/no-unused-vars`] | @@ -113,6 +113,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] | [1] The ESLint rule also supports silencing with an extra set of parentheses (`if ((foo = bar)) {}`)
+[2] Only checks member expressions ### Maintainability @@ -136,7 +137,6 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`prefer-readonly`] | ✅ | [`@typescript-eslint/prefer-readonly`] | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | -[1] Only warns when importing deprecated symbols
[2] Missing support for blank-line-delimited sections ### Style @@ -174,7 +174,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`no-reference-import`] | ✅ | [`@typescript-eslint/triple-slash-reference`] | | [`no-trailing-whitespace`] | 🌟 | [`no-trailing-spaces`][no-trailing-spaces] | | [`no-unnecessary-callback-wrapper`] | 🛑 | N/A and this might be unsafe (i.e. with `forEach`) | -| [`no-unnecessary-else`] | 🌟 | [`no-else-return`][no-else-return] [2][2] | | [`no-unnecessary-initializer`] | 🌟 | [`no-undef-init`][no-undef-init] | | [`no-unnecessary-qualifier`] | ✅ | [`@typescript-eslint/no-unnecessary-qualifier`] | | [`number-literal-format`] | 🛑 | N/A | @@ -640,6 +640,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/semi`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/semi.md [`@typescript-eslint/no-floating-promises`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md [`@typescript-eslint/no-magic-numbers`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-magic-numbers.md +[`@typescript-eslint/no-unsafe-member-access`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md b/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md new file mode 100644 index 00000000000..e99b2536c16 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md @@ -0,0 +1,43 @@ +# Disallows member access on any typed variables (`no-unsafe-member-access`) + +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. + +## Rule Details + +This rule disallows member access on any variable that is typed as `any`. + +Examples of **incorrect** code for this rule: + +```ts +declare const anyVar: any; +declare const nestedAny: { prop: any }; + +anyVar.a; +anyVar.a.b; +anyVar['a']; +anyVar['a']['b']; + +nestedAny.prop.a; +nestedAny.prop['a']; + +const key = 'a'; +nestedAny.prop[key]; +``` + +Examples of **correct** code for this rule: + +```ts +declare const properlyTyped: { prop: { a: string } }; + +nestedAny.prop.a; +nestedAny.prop['a']; + +const key = 'a'; +nestedAny.prop[key]; +``` + +## 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/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 590ef81df8d..47f4e509068 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-member-access": "error", "no-unused-expressions": "off", "@typescript-eslint/no-unused-expressions": "error", "no-unused-vars": "off", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 3568057787b..f09033aa2d7 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 noUnsafeMemberAccess from './no-unsafe-member-access'; import noUntypedPublicSignature from './no-untyped-public-signature'; import noUnusedExpressions from './no-unused-expressions'; import noUnusedVars from './no-unused-vars'; @@ -144,6 +145,7 @@ export default { 'no-unnecessary-qualifier': noUnnecessaryQualifier, 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, + 'no-unsafe-member-access': noUnsafeMemberAccess, 'no-untyped-public-signature': noUntypedPublicSignature, 'no-unused-expressions': noUnusedExpressions, 'no-unused-vars-experimental': noUnusedVarsExperimental, diff --git a/packages/eslint-plugin/src/rules/no-unsafe-member-access.ts b/packages/eslint-plugin/src/rules/no-unsafe-member-access.ts new file mode 100644 index 00000000000..298cd97561e --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unsafe-member-access.ts @@ -0,0 +1,74 @@ +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +const enum State { + Unsafe = 1, + Safe = 2, +} + +export default util.createRule({ + name: 'no-unsafe-member-access', + meta: { + type: 'problem', + docs: { + description: 'Disallows member access on any typed variables', + category: 'Possible Errors', + recommended: false, + requiresTypeChecking: true, + }, + messages: { + unsafeMemberExpression: + 'Unsafe member access {{property}} on an any value', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context); + const checker = program.getTypeChecker(); + const sourceCode = context.getSourceCode(); + + const stateCache = new Map(); + + function checkMemberExpression( + node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, + ): State { + const cachedState = stateCache.get(node); + if (cachedState) { + return cachedState; + } + + if (util.isMemberOrOptionalMemberExpression(node.object)) { + const objectState = checkMemberExpression(node.object); + if (objectState === State.Unsafe) { + // if the object is unsafe, we know this will be unsafe as well + // we don't need to report, as we have already reported on the inner member expr + stateCache.set(node, objectState); + return objectState; + } + } + + const tsNode = esTreeNodeToTSNodeMap.get(node.object); + const type = checker.getTypeAtLocation(tsNode); + const state = util.isTypeAnyType(type) ? State.Unsafe : State.Safe; + stateCache.set(node, state); + + if (state === State.Unsafe) { + const propertyName = sourceCode.getText(node.property); + context.report({ + node, + messageId: 'unsafeMemberExpression', + data: { + property: node.computed ? `[${propertyName}]` : `.${propertyName}`, + }, + }); + } + + return state; + } + + return { + 'MemberExpression, OptionalMemberExpression': checkMemberExpression, + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/astUtils.ts b/packages/eslint-plugin/src/util/astUtils.ts index a92f00eaa16..3dffeb90e01 100644 --- a/packages/eslint-plugin/src/util/astUtils.ts +++ b/packages/eslint-plugin/src/util/astUtils.ts @@ -221,6 +221,15 @@ function isAwaitKeyword( return node?.type === AST_TOKEN_TYPES.Identifier && node.value === 'await'; } +function isMemberOrOptionalMemberExpression( + node: TSESTree.Node, +): node is TSESTree.MemberExpression | TSESTree.OptionalMemberExpression { + return ( + node.type === AST_NODE_TYPES.MemberExpression || + node.type === AST_NODE_TYPES.OptionalMemberExpression + ); +} + export { isAwaitExpression, isAwaitKeyword, @@ -231,6 +240,7 @@ export { isFunctionType, isIdentifier, isLogicalOrOperator, + isMemberOrOptionalMemberExpression, isNonNullAssertionPunctuator, isNotNonNullAssertionPunctuator, isNotOptionalChainPunctuator, diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index eae3519ea21..f59f3d7c53e 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -290,3 +290,10 @@ export function getEqualsKind(operator: string): EqualsKind | undefined { return undefined; } } + +/** + * @returns true if the type is `any` + */ +export function isTypeAnyType(type: ts.Type): boolean { + return isTypeFlagSet(type, ts.TypeFlags.Any); +} diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-member-access.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-member-access.test.ts new file mode 100644 index 00000000000..641e0492e5a --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unsafe-member-access.test.ts @@ -0,0 +1,85 @@ +import rule from '../../src/rules/no-unsafe-member-access'; +import { + RuleTester, + batchedSingleLineTests, + getFixturesRootDir, +} from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: getFixturesRootDir(), + }, +}); + +ruleTester.run('no-unsafe-member-access', rule, { + valid: [ + 'function foo(x: { a: number }) { x.a }', + 'function foo(x?: { a: number }) { x?.a }', + ], + invalid: [ + ...batchedSingleLineTests({ + code: ` +function foo(x: any) { x.a } +function foo(x: any) { x.a.b.c.d.e.f.g } +function foo(x: { a: any }) { x.a.b.c.d.e.f.g } + `, + errors: [ + { + messageId: 'unsafeMemberExpression', + data: { + property: '.a', + }, + line: 2, + column: 24, + endColumn: 27, + }, + { + messageId: 'unsafeMemberExpression', + data: { + property: '.a', + }, + line: 3, + column: 24, + endColumn: 27, + }, + { + messageId: 'unsafeMemberExpression', + data: { + property: '.b', + }, + line: 4, + column: 31, + endColumn: 36, + }, + ], + }), + ...batchedSingleLineTests({ + code: ` +function foo(x: any) { x['a'] } +function foo(x: any) { x['a']['b']['c'] } + `, + errors: [ + { + messageId: 'unsafeMemberExpression', + data: { + property: "['a']", + }, + line: 2, + column: 24, + endColumn: 30, + }, + { + messageId: 'unsafeMemberExpression', + data: { + property: "['a']", + }, + line: 3, + column: 24, + endColumn: 30, + }, + ], + }), + ], +});