Skip to content

Commit

Permalink
feat(eslint-plugin): add rule no-unsafe-member-access (#1643)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Mar 3, 2020
1 parent 3b40231 commit 608a750
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 3 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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: | |
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 @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
74 changes: 74 additions & 0 deletions 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<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 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,
};
},
});
10 changes: 10 additions & 0 deletions packages/eslint-plugin/src/util/astUtils.ts
Expand Up @@ -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,
Expand All @@ -231,6 +240,7 @@ export {
isFunctionType,
isIdentifier,
isLogicalOrOperator,
isMemberOrOptionalMemberExpression,
isNonNullAssertionPunctuator,
isNotNonNullAssertionPunctuator,
isNotOptionalChainPunctuator,
Expand Down
7 changes: 7 additions & 0 deletions packages/eslint-plugin/src/util/types.ts
Expand Up @@ -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);
}
85 changes: 85 additions & 0 deletions 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,
},
],
}),
],
});

0 comments on commit 608a750

Please sign in to comment.