Skip to content

Commit

Permalink
feat(eslint-plugin): add rule no-unsafe-member-access
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Feb 26, 2020
1 parent 4eedd7f commit 05df893
Show file tree
Hide file tree
Showing 9 changed files with 275 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -131,6 +131,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: | |
Expand Down
7 changes: 4 additions & 3 deletions packages/eslint-plugin/ROADMAP.md
Expand Up @@ -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`]<sup>[2]</sup> |
| [`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`] |
Expand All @@ -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] |

<sup>[1]</sup> The ESLint rule also supports silencing with an extra set of parentheses (`if ((foo = bar)) {}`)<br>
<sup>[2]</sup> Only checks member expressions

### Maintainability

Expand All @@ -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] |

<sup>[1]</sup> Only warns when importing deprecated symbols<br>
<sup>[2]</sup> Missing support for blank-line-delimited sections

### Style
Expand Down Expand Up @@ -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] <sup>[2]</sup |
| [`no-unnecessary-else`] | 🌟 | [`no-else-return`][no-else-return] <sup>[2]</sup> |
| [`no-unnecessary-initializer`] | 🌟 | [`no-undef-init`][no-undef-init] |
| [`no-unnecessary-qualifier`] || [`@typescript-eslint/no-unnecessary-qualifier`] |
| [`number-literal-format`] | 🛑 | N/A |
Expand Down Expand Up @@ -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

<!-- eslint-plugin-import -->

Expand Down
43 changes: 43 additions & 0 deletions 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/)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -60,6 +60,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",
Expand Down
22 changes: 12 additions & 10 deletions packages/eslint-plugin/src/rules/index.ts
@@ -1,8 +1,8 @@
import adjacentOverloadSignatures from './adjacent-overload-signatures';
import arrayType from './array-type';
import awaitThenable from './await-thenable';
import banTsIgnore from './ban-ts-ignore';
import banTsComment from './ban-ts-comment';
import banTsIgnore from './ban-ts-ignore';
import banTypes from './ban-types';
import braceStyle from './brace-style';
import camelcase from './camelcase';
Expand All @@ -28,10 +28,10 @@ import noDynamicDelete from './no-dynamic-delete';
import noEmptyFunction from './no-empty-function';
import noEmptyInterface from './no-empty-interface';
import noExplicitAny from './no-explicit-any';
import noExtraneousClass from './no-extraneous-class';
import noExtraNonNullAssertion from './no-extra-non-null-assertion';
import noExtraParens from './no-extra-parens';
import noExtraSemi from './no-extra-semi';
import noExtraneousClass from './no-extraneous-class';
import noFloatingPromises from './no-floating-promises';
import noForInArray from './no-for-in-array';
import noImpliedEval from './no-implied-eval';
Expand All @@ -40,8 +40,8 @@ import noMagicNumbers from './no-magic-numbers';
import noMisusedNew from './no-misused-new';
import noMisusedPromises from './no-misused-promises';
import noNamespace from './no-namespace';
import noNonNullAssertion from './no-non-null-assertion';
import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chain';
import noNonNullAssertion from './no-non-null-assertion';
import noParameterProperties from './no-parameter-properties';
import noRequireImports from './no-require-imports';
import noThisAlias from './no-this-alias';
Expand All @@ -50,8 +50,8 @@ import noTypeAlias from './no-type-alias';
import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare';
import noUnnecessaryCondition from './no-unnecessary-condition';
import noUnnecessaryQualifier from './no-unnecessary-qualifier';
import useDefaultTypeParameter 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';
Expand Down Expand Up @@ -85,13 +85,14 @@ import typeAnnotationSpacing from './type-annotation-spacing';
import typedef from './typedef';
import unboundMethod from './unbound-method';
import unifiedSignatures from './unified-signatures';
import useDefaultTypeParameter from './no-unnecessary-type-arguments';

export default {
'adjacent-overload-signatures': adjacentOverloadSignatures,
'array-type': arrayType,
'await-thenable': awaitThenable,
'ban-ts-ignore': banTsIgnore,
'ban-ts-comment': banTsComment,
'ban-ts-ignore': banTsIgnore,
'ban-types': banTypes,
'brace-style': braceStyle,
camelcase: camelcase,
Expand Down Expand Up @@ -123,28 +124,29 @@ export default {
'no-extraneous-class': noExtraneousClass,
'no-floating-promises': noFloatingPromises,
'no-for-in-array': noForInArray,
'no-inferrable-types': noInferrableTypes,
'no-implied-eval': noImpliedEval,
'no-inferrable-types': noInferrableTypes,
'no-magic-numbers': noMagicNumbers,
'no-misused-new': noMisusedNew,
'no-misused-promises': noMisusedPromises,
'no-namespace': noNamespace,
'no-non-null-assertion': noNonNullAssertion,
'no-non-null-asserted-optional-chain': noNonNullAssertedOptionalChain,
'no-non-null-assertion': noNonNullAssertion,
'no-parameter-properties': noParameterProperties,
'no-require-imports': noRequireImports,
'no-this-alias': noThisAlias,
'no-type-alias': noTypeAlias,
'no-throw-literal': noThrowLiteral,
'no-type-alias': noTypeAlias,
'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare,
'no-unnecessary-condition': noUnnecessaryCondition,
'no-unnecessary-qualifier': noUnnecessaryQualifier,
'no-unnecessary-type-arguments': useDefaultTypeParameter,
'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion,
'no-unsafe-member-access': noUnsafeMemberAccess,
'no-untyped-public-signature': noUntypedPublicSignature,
'no-unused-vars': noUnusedVars,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-unused-expressions': noUnusedExpressions,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-unused-vars': noUnusedVars,
'no-use-before-define': noUseBeforeDefine,
'no-useless-constructor': noUselessConstructor,
'no-var-requires': noVarRequires,
Expand Down
73 changes: 73 additions & 0 deletions packages/eslint-plugin/src/rules/no-unsafe-member-access.ts
@@ -0,0 +1,73 @@
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<TSESTree.Node, State>();

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 state = util.isAnyType(tsNode, checker) ? 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,
};
},
});
23 changes: 23 additions & 0 deletions packages/eslint-plugin/src/util/astUtils.ts
Expand Up @@ -132,15 +132,38 @@ function isAwaitKeyword(
return node?.type === AST_TOKEN_TYPES.Identifier && node.value === 'await';
}

/**
* Checks if a node is the null literal
*/
function isNullLiteral(
node: TSESTree.Node | undefined | null,
): node is TSESTree.NullLiteral {
if (!node) {
return false;
}
return node.type === AST_NODE_TYPES.Literal && node.value === null;
}

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,
isConstructor,
isIdentifier,
isLogicalOrOperator,
isMemberOrOptionalMemberExpression,
isNonNullAssertionPunctuator,
isNotNonNullAssertionPunctuator,
isNotOptionalChainPunctuator,
isNullLiteral,
isOptionalChainPunctuator,
isOptionalOptionalChain,
isSetter,
Expand Down
33 changes: 33 additions & 0 deletions packages/eslint-plugin/src/util/types.ts
Expand Up @@ -290,3 +290,36 @@ export function getEqualsKind(operator: string): EqualsKind | undefined {
return undefined;
}
}

/**
* @returns true if the type is `any`
*/
export function isAnyType(node: ts.Node, checker: ts.TypeChecker): boolean {
const type = checker.getTypeAtLocation(node);
return isTypeFlagSet(type, ts.TypeFlags.Any);
}

/**
* @returns true if the type is `any[]` or `readonly any[]`
*/
export function isAnyArrayType(
node: ts.Node,
checker: ts.TypeChecker,
): boolean {
const type = checker.getTypeAtLocation(node);
return (
checker.isArrayType(type) &&
isTypeReference(type) &&
isTypeFlagSet(checker.getTypeArguments(type)[0], ts.TypeFlags.Any)
);
}

/**
* @returns true if the type is `any`, `any[]` or `readonly any[]`
*/
export function isAnyOrAnyArrayType(
node: ts.Node,
checker: ts.TypeChecker,
): boolean {
return isAnyType(node, checker) || isAnyArrayType(node, checker);
}

0 comments on commit 05df893

Please sign in to comment.