From f12c11e1eb8d37ffbbe9a5dca10c09d853470fce Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 27 May 2019 16:39:07 -0400 Subject: [PATCH 01/12] feat(eslint-plugin): added new rule no-dynamic-delete Adds the equivalent of TSLint's `no-dynamic-delete`. --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 3 +- .../docs/rules/no-dynamic-delete.md | 44 +++++++ .../src/rules/no-dynamic-delete.ts | 115 ++++++++++++++++++ .../tests/rules/no-dynamic-delete.test.ts | 90 ++++++++++++++ 5 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/docs/rules/no-dynamic-delete.md create mode 100644 packages/eslint-plugin/src/rules/no-dynamic-delete.ts create mode 100644 packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 6b605d31b57b..7553b3e2b210 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -130,6 +130,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | | [`@typescript-eslint/no-angle-bracket-type-assertion`](./docs/rules/no-angle-bracket-type-assertion.md) | Enforces the use of `as Type` assertions instead of `` assertions | :heavy_check_mark: | | | | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Bans usage of the delete operator with computed key expressions | :heavy_check_mark: | | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index b2567afe070e..5d98d6a41202 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -57,7 +57,7 @@ | [`no-duplicate-super`] | 🌟 | [`constructor-super`][constructor-super] | | [`no-duplicate-switch-case`] | 🌟 | [`no-duplicate-case`][no-duplicate-case] | | [`no-duplicate-variable`] | 🌟 | [`no-redeclare`][no-redeclare] | -| [`no-dynamic-delete`] | πŸ›‘ | N/A | +| [`no-dynamic-delete`] | βœ… | [`@typescript-eslint/no-dynamic-delete`] | | [`no-empty`] | 🌟 | [`no-empty`][no-empty] | | [`no-eval`] | 🌟 | [`no-eval`][no-eval] | | [`no-floating-promises`] | πŸ›‘ | N/A ([relevant plugin][plugin:promise]) | @@ -609,6 +609,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/prefer-interface`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-interface.md [`@typescript-eslint/no-array-constructor`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-array-constructor.md [`@typescript-eslint/prefer-function-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-function-type.md +[`@typescript-eslint/no-dynamic-delete`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-dynamic-delete.md [`@typescript-eslint/no-for-in-array`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-for-in-array.md [`@typescript-eslint/no-unnecessary-qualifier`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md [`@typescript-eslint/semi`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/semi.md diff --git a/packages/eslint-plugin/docs/rules/no-dynamic-delete.md b/packages/eslint-plugin/docs/rules/no-dynamic-delete.md new file mode 100644 index 000000000000..e316f20f3ab8 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-dynamic-delete.md @@ -0,0 +1,44 @@ +# Disallow the delete operator with computed key expressions (no-dynamic-delete) + +Deleting dynamically computed keys can be dangerous and in some cases not well optimized. + +## Rule Details + +Using the `delete` operator on keys that aren't runtime constants could be a sign that you're using the wrong data structures. +Using `Object`s with added and removed keys can cause occasional edge case bugs, such as if a key is named `"hasOwnProperty"`. +Consider using a `Map` or `Set` if you’re storing collections of objects. + +Examples of **correct** code wth this rule: + +```ts +const container: { [i: string]: number } = { + /* ... */ +}; + +// Constant runtime lookups by string index +delete container.aaa; + +// Constants that must be accessed by [] +delete container[7]; +delete container['-Infinity']; +``` + +Examples of **incorrect** code with this rule: + +```ts +// Can be replaced with the constant equivalents, such as container.aaa +delete container['aaa']; +delete container['Infinity']; +``` + +## When Not To Use It + +When you know your keys are safe to delete, this rule can be unnecessary. +Some environments such as older browsers might not support `Map` and `Set`. + +Do not consider this rule as performance advice before profiling your code's bottlenecks. +Even repeated minor performance slowdowns likely do not significantly affect your application's general perceived speed. + +## Related to + +- TSLint: [no-dynamic-delete](https://palantir.github.io/tslint/rules/no-dynamic-delete) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts new file mode 100644 index 000000000000..5b58d3eb0484 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -0,0 +1,115 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import ts from 'typescript'; +import * as tsutils from 'tsutils'; +import * as util from '../util'; +import { ReportFixFunction } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; + +export default util.createRule({ + name: 'no-dynamic-delete', + meta: { + docs: { + category: 'Best Practices', + description: + 'Bans usage of the delete operator with computed key expressions', + recommended: false, + }, + fixable: 'code', + messages: { + dynamicDelete: 'Do not delete dynamically computed property keys.', + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + const parserServices = util.getParserServices(context); + + function checkDeleteAccessExpression( + esNode: TSESTree.Node, + tsNode: ts.Node, + ): void { + if (!ts.isElementAccessExpression(tsNode)) { + return; + } + + const { argumentExpression } = tsNode; + if (isNecessaryDynamicAccess(argumentExpression)) { + return; + } + + context.report({ + fix: createFixer(argumentExpression), + messageId: 'dynamicDelete', + node: esNode, + }); + } + + function createFixer( + argumentExpression: ts.Expression, + ): ReportFixFunction | undefined { + const start = argumentExpression.getStart() - 1; + const width = argumentExpression.getWidth() + 2; + + if (ts.isPrefixUnaryExpression(argumentExpression)) { + if (!ts.isNumericLiteral(argumentExpression.operand)) { + return undefined; + } + + const convertedOperand = argumentExpression.operand.text; + return fixer => + fixer.replaceTextRange( + [start, start + width], + `[${convertedOperand}]`, + ); + } + + if (ts.isStringLiteral(argumentExpression)) { + return fixer => + fixer.replaceTextRange( + [start, start + width], + `.${argumentExpression.text}`, + ); + } + + return undefined; + } + + return { + 'UnaryExpression[operator=delete]'(node: TSESTree.UnaryExpression) { + if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + return; + } + + checkDeleteAccessExpression( + node.argument.property, + parserServices.esTreeNodeToTSNodeMap.get(node.argument), + ); + }, + }; + }, +}); + +function isNecessaryDynamicAccess(argumentExpression: ts.Expression): boolean { + if (isNumberLike(argumentExpression)) { + return true; + } + + return ( + ts.isStringLiteral(argumentExpression) && + !tsutils.isValidPropertyAccess(argumentExpression.text) + ); +} + +function isNumberLike(node: ts.Node): boolean { + if (ts.isPrefixUnaryExpression(node)) { + return ( + ts.isNumericLiteral(node.operand) && + node.operator === ts.SyntaxKind.MinusToken + ); + } + + return ts.isNumericLiteral(node); +} diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts new file mode 100644 index 000000000000..aaae3f374f20 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts @@ -0,0 +1,90 @@ +import path from 'path'; +import rule from '../../src/rules/no-dynamic-delete'; +import { RuleTester } from '../RuleTester'; + +const rootDir = path.join(process.cwd(), 'tests/fixtures'); +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-dynamic-delete', rule, { + valid: [ + `const container: { [i: string]: 0 } = {}; + delete container.aaa;`, + `const container: { [i: string]: 0 } = {}; + delete container.delete;`, + `const container: { [i: string]: 0 } = {}; + delete container[7];`, + `const container: { [i: string]: 0 } = {}; + delete container[-7];`, + `const container: { [i: string]: 0 } = {}; + delete container['-Infinity'];`, + `const container: { [i: string]: 0 } = {}; + delete container['+Infinity'];`, + ], + invalid: [ + { + code: `const container: { [i: string]: 0 } = {}; + delete container['aaa'];`, + errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container.aaa;`, + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container['aa' + 'b'];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container['delete'];`, + errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container.delete;`, + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container[+7];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container[-Infinity];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container[+Infinity];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container[NaN];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container['NaN'];`, + errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container.NaN;`, + }, + { + code: `const container: { [i: string]: 0 } = {}; + const name = 'name'; + delete container[name];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + const getName = () => 'aaa'; + delete container[getName()];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + ], +}); From 79d5419d857557ddcacfd0aff0245861d3c60c53 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 27 May 2019 20:45:45 -0400 Subject: [PATCH 02/12] Fixed docs per new CI checks --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/src/rules/index.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 7553b3e2b210..72fb53ea126a 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -130,7 +130,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | | [`@typescript-eslint/no-angle-bracket-type-assertion`](./docs/rules/no-angle-bracket-type-assertion.md) | Enforces the use of `as Type` assertions instead of `` assertions | :heavy_check_mark: | | | | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Bans usage of the delete operator with computed key expressions | :heavy_check_mark: | | | +| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Bans usage of the delete operator with computed key expressions | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d62f58d6af3f..fb0a1b7ca5be 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -16,6 +16,7 @@ import memberNaming from './member-naming'; import memberOrdering from './member-ordering'; import noAngleBracketTypeAssertion from './no-angle-bracket-type-assertion'; import noArrayConstructor from './no-array-constructor'; +import noDynamicDelete from './no-dynamic-delete'; import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; import noExtraParens from './no-extra-parens'; @@ -72,6 +73,7 @@ export default { 'member-ordering': memberOrdering, 'no-angle-bracket-type-assertion': noAngleBracketTypeAssertion, 'no-array-constructor': noArrayConstructor, + 'no-dynamic-delete': noDynamicDelete, 'no-empty-interface': noEmptyInterface, 'no-explicit-any': noExplicitAny, 'no-extra-parens': noExtraParens, From 080fc4774eccae5323721beb3ef1618698918cfc Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 27 May 2019 22:07:04 -0400 Subject: [PATCH 03/12] Just for fun, added non member delete expressions in test --- packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts index aaae3f374f20..0b6e7fa40c48 100644 --- a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts @@ -26,6 +26,10 @@ ruleTester.run('no-dynamic-delete', rule, { delete container['-Infinity'];`, `const container: { [i: string]: 0 } = {}; delete container['+Infinity'];`, + `const value = 1; + delete value;`, + `const value = 1; + delete -value;`, ], invalid: [ { From 4329fb93c9ce6119fdd05dd420de96b48680ff23 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 1 Jun 2019 09:35:14 -0400 Subject: [PATCH 04/12] Removed recommended flag; added to all --- packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/no-dynamic-delete.ts | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index cc2f97786718..645aa1e96a8e 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -23,6 +23,7 @@ "@typescript-eslint/no-angle-bracket-type-assertion": "error", "no-array-constructor": "off", "@typescript-eslint/no-array-constructor": "error", + "@typescript-eslint/no-dynamic-delete": "error", "@typescript-eslint/no-empty-interface": "error", "@typescript-eslint/no-explicit-any": "error", "no-extra-parens": "off", diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index 5b58d3eb0484..dbcfd3a14871 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -14,7 +14,6 @@ export default util.createRule({ category: 'Best Practices', description: 'Bans usage of the delete operator with computed key expressions', - recommended: false, }, fixable: 'code', messages: { From c56a2a3b32f5b340bf9d4684406dd64d86df1373 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 1 Jun 2019 09:35:50 -0400 Subject: [PATCH 05/12] Set recommended to false --- packages/eslint-plugin/src/rules/no-dynamic-delete.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index dbcfd3a14871..5b58d3eb0484 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -14,6 +14,7 @@ export default util.createRule({ category: 'Best Practices', description: 'Bans usage of the delete operator with computed key expressions', + recommended: false, }, fixable: 'code', messages: { From e58634e1407daccfae6afe83f68d142f3d2446b7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 15 Jul 2019 09:02:47 -0700 Subject: [PATCH 06/12] Removed uses of the TS language service --- .../src/rules/no-dynamic-delete.ts | 98 ++++++++----------- .../tests/rules/no-dynamic-delete.test.ts | 7 +- 2 files changed, 43 insertions(+), 62 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index 5b58d3eb0484..86bd134ea50f 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -2,7 +2,6 @@ import { TSESTree, AST_NODE_TYPES, } from '@typescript-eslint/experimental-utils'; -import ts from 'typescript'; import * as tsutils from 'tsutils'; import * as util from '../util'; import { ReportFixFunction } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; @@ -25,52 +24,27 @@ export default util.createRule({ }, defaultOptions: [], create(context) { - const parserServices = util.getParserServices(context); - - function checkDeleteAccessExpression( - esNode: TSESTree.Node, - tsNode: ts.Node, - ): void { - if (!ts.isElementAccessExpression(tsNode)) { - return; - } - - const { argumentExpression } = tsNode; - if (isNecessaryDynamicAccess(argumentExpression)) { - return; - } - - context.report({ - fix: createFixer(argumentExpression), - messageId: 'dynamicDelete', - node: esNode, - }); - } - function createFixer( - argumentExpression: ts.Expression, + member: TSESTree.MemberExpression, ): ReportFixFunction | undefined { - const start = argumentExpression.getStart() - 1; - const width = argumentExpression.getWidth() + 2; - - if (ts.isPrefixUnaryExpression(argumentExpression)) { - if (!ts.isNumericLiteral(argumentExpression.operand)) { - return undefined; - } - - const convertedOperand = argumentExpression.operand.text; + if ( + member.property.type === AST_NODE_TYPES.Literal && + typeof member.property.value === 'string' + ) { + const { value } = member.property; return fixer => fixer.replaceTextRange( - [start, start + width], - `[${convertedOperand}]`, + [member.property.range[0] - 1, member.property.range[1] + 1], + `.${value}`, ); } - if (ts.isStringLiteral(argumentExpression)) { + if (member.property.type === AST_NODE_TYPES.Identifier) { + const { name } = member.property; return fixer => fixer.replaceTextRange( - [start, start + width], - `.${argumentExpression.text}`, + [member.property.range[0] - 1, member.property.range[1] + 1], + `.${name}`, ); } @@ -79,37 +53,47 @@ export default util.createRule({ return { 'UnaryExpression[operator=delete]'(node: TSESTree.UnaryExpression) { - if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + if ( + node.argument.type !== AST_NODE_TYPES.MemberExpression || + !node.argument.computed || + isNecessaryDynamicAccess( + diveIntoWrapperExpressions(node.argument.property), + ) + ) { return; } - checkDeleteAccessExpression( - node.argument.property, - parserServices.esTreeNodeToTSNodeMap.get(node.argument), - ); + context.report({ + fix: createFixer(node.argument), + messageId: 'dynamicDelete', + node: node.argument.property, + }); }, }; }, }); -function isNecessaryDynamicAccess(argumentExpression: ts.Expression): boolean { - if (isNumberLike(argumentExpression)) { - return true; +function diveIntoWrapperExpressions( + node: TSESTree.Expression, +): TSESTree.Expression { + if (node.type === AST_NODE_TYPES.UnaryExpression) { + return diveIntoWrapperExpressions(node.argument); } - return ( - ts.isStringLiteral(argumentExpression) && - !tsutils.isValidPropertyAccess(argumentExpression.text) - ); + return node; } -function isNumberLike(node: ts.Node): boolean { - if (ts.isPrefixUnaryExpression(node)) { - return ( - ts.isNumericLiteral(node.operand) && - node.operator === ts.SyntaxKind.MinusToken - ); +function isNecessaryDynamicAccess(property: TSESTree.Expression): boolean { + if (property.type !== AST_NODE_TYPES.Literal) { + return false; } - return ts.isNumericLiteral(node); + if (typeof property.value === 'number') { + return true; + } + + return ( + typeof property.value === 'string' && + !tsutils.isValidPropertyAccess(property.value) + ); } diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts index 0b6e7fa40c48..ef5cdf57dd86 100644 --- a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts @@ -22,6 +22,8 @@ ruleTester.run('no-dynamic-delete', rule, { delete container[7];`, `const container: { [i: string]: 0 } = {}; delete container[-7];`, + `const container: { [i: string]: 0 } = {}; + delete container[+7];`, `const container: { [i: string]: 0 } = {}; delete container['-Infinity'];`, `const container: { [i: string]: 0 } = {}; @@ -51,11 +53,6 @@ ruleTester.run('no-dynamic-delete', rule, { output: `const container: { [i: string]: 0 } = {}; delete container.delete;`, }, - { - code: `const container: { [i: string]: 0 } = {}; - delete container[+7];`, - errors: [{ messageId: 'dynamicDelete' }], - }, { code: `const container: { [i: string]: 0 } = {}; delete container[-Infinity];`, From 22b01c30acc6ead208a41395161e11c4dcc1e3e4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 16 Jul 2019 11:29:47 -0700 Subject: [PATCH 07/12] Imported from utils namespace instead --- packages/eslint-plugin/src/rules/no-dynamic-delete.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index 86bd134ea50f..b68fb105e111 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -1,10 +1,10 @@ import { TSESTree, AST_NODE_TYPES, + TSESLint, } from '@typescript-eslint/experimental-utils'; import * as tsutils from 'tsutils'; import * as util from '../util'; -import { ReportFixFunction } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; export default util.createRule({ name: 'no-dynamic-delete', @@ -26,7 +26,7 @@ export default util.createRule({ create(context) { function createFixer( member: TSESTree.MemberExpression, - ): ReportFixFunction | undefined { + ): TSESLint.ReportFixFunction | undefined { if ( member.property.type === AST_NODE_TYPES.Literal && typeof member.property.value === 'string' From 5c0db7b9e0e5cf8c876b31f587cf6fa32e1946be Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Oct 2019 22:12:32 +0900 Subject: [PATCH 08/12] Used sourceCode API for token positions --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-misused-promises.md | 119 ++++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/configs/base.json | 4 +- .../src/configs/recommended.json | 2 - packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-dynamic-delete.ts | 19 +- .../src/rules/no-misused-promises.ts | 253 ++++++++++++++++ .../src/rules/prefer-readonly.ts | 5 +- .../src/rules/prefer-regexp-exec.ts | 2 +- .../src/rules/triple-slash-reference.ts | 17 +- .../tests/rules/no-dynamic-delete.test.ts | 14 + .../tests/rules/no-misused-promises.test.ts | 282 ++++++++++++++++++ .../experimental-utils/src/ts-eslint/Rule.ts | 1 + 14 files changed, 698 insertions(+), 24 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-misused-promises.md create mode 100644 packages/eslint-plugin/src/rules/no-misused-promises.ts create mode 100644 packages/eslint-plugin/tests/rules/no-misused-promises.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 97c9c53072ed..1be4d323499f 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -153,6 +153,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md) | Disallows magic numbers | | | | | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | | +| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | | | :thought_balloon: | | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | | [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md new file mode 100644 index 000000000000..65e3084ae26f --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -0,0 +1,119 @@ +# Avoid using promises in places not designed to handle them (no-misused-promises) + +This rule forbids using promises in places where the Typescript compiler +allows them but they are not handled properly. These situations can often arise +due to a missing `await` keyword or just a misunderstanding of the way async +functions are handled/awaited. + +## Rule Details + +Examples of **incorrect** code for this rule with `checksConditionals: true`: + +```ts +const promise = Promise.resolve('value'); + +if (promise) { + // Do something +} + +const val = promise ? 123 : 456; + +while (promise) { + // Do something +} +``` + +Examples of **incorrect** code for this rule with `checksVoidReturn: true`: + +```ts +[1, 2, 3].forEach(async value => { + await doSomething(value); +}); + +new Promise(async (resolve, reject) => { + await doSomething(); + resolve(); +}); + +const eventEmitter = new EventEmitter(); +eventEmitter.on('some-event', async () => { + await doSomething(); +}); +``` + +Examples of **correct** code for this rule: + +```ts +const promise = Promise.resolve('value'); + +if (await promise) { + // Do something +} + +const val = (await promise) ? 123 : 456; + +while (await promise) { + // Do something +} + +for (const value of [1, 2, 3]) { + await doSomething(value); +} + +new Promise((resolve, reject) => { + // Do something + resolve(); +}); + +const eventEmitter = new EventEmitter(); +eventEmitter.on('some-event', () => { + doSomething(); +}); +``` + +## Options + +This rule accepts a single option which is an object with `checksConditionals` +and `checksVoidReturn` properties indicating which types of misuse to flag. +Both are enabled by default + +If you don't want functions that return promises where a void return is +expected to be checked, your configuration will look like this: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checksVoidReturn": false + } + ] +} +``` + +Likewise, if you don't want to check conditionals, you can configure the rule +like this: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checksConditionals": false + } + ] +} +``` + +## When Not To Use It + +If you do not use Promises in your codebase or are not concerned with possible +misuses of them outside of what the Typescript compiler will check. + +## Related to + +- [`no-floating-promises`]('./no-floating-promises.md') + +## Further Reading + +- [Typescript void function assignability](https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 74b669ab7c6b..feeffa9ea3c0 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -37,6 +37,7 @@ "no-magic-numbers": "off", "@typescript-eslint/no-magic-numbers": "error", "@typescript-eslint/no-misused-new": "error", + "@typescript-eslint/no-misused-promises": "error", "@typescript-eslint/no-namespace": "error", "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-object-literal-type-assertion": "error", diff --git a/packages/eslint-plugin/src/configs/base.json b/packages/eslint-plugin/src/configs/base.json index 9b6931ad616d..6f56100a6ae7 100644 --- a/packages/eslint-plugin/src/configs/base.json +++ b/packages/eslint-plugin/src/configs/base.json @@ -3,5 +3,7 @@ "parserOptions": { "sourceType": "module" }, - "plugins": ["@typescript-eslint"] + "plugins": [ + "@typescript-eslint" + ] } diff --git a/packages/eslint-plugin/src/configs/recommended.json b/packages/eslint-plugin/src/configs/recommended.json index d26fd25d01c1..0e6d6d9f665f 100644 --- a/packages/eslint-plugin/src/configs/recommended.json +++ b/packages/eslint-plugin/src/configs/recommended.json @@ -24,13 +24,11 @@ "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-object-literal-type-assertion": "error", "@typescript-eslint/no-parameter-properties": "error", - "@typescript-eslint/no-triple-slash-reference": "error", "no-unused-vars": "off", "@typescript-eslint/no-unused-vars": "warn", "no-use-before-define": "off", "@typescript-eslint/no-use-before-define": "error", "@typescript-eslint/no-var-requires": "error", - "@typescript-eslint/prefer-interface": "error", "@typescript-eslint/prefer-namespace-keyword": "error", "@typescript-eslint/type-annotation-spacing": "error" } diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 35f4e1b79488..8b1a6e65acf9 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -28,6 +28,7 @@ import noForInArray from './no-for-in-array'; import noInferrableTypes from './no-inferrable-types'; 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 noObjectLiteralTypeAssertion from './no-object-literal-type-assertion'; @@ -91,6 +92,7 @@ export default { '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-object-literal-type-assertion': noObjectLiteralTypeAssertion, diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index b68fb105e111..a102be438517 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -33,19 +33,13 @@ export default util.createRule({ ) { const { value } = member.property; return fixer => - fixer.replaceTextRange( - [member.property.range[0] - 1, member.property.range[1] + 1], - `.${value}`, - ); + fixer.replaceTextRange(getTokenRange(member.property), `.${value}`); } if (member.property.type === AST_NODE_TYPES.Identifier) { const { name } = member.property; return fixer => - fixer.replaceTextRange( - [member.property.range[0] - 1, member.property.range[1] + 1], - `.${name}`, - ); + fixer.replaceTextRange(getTokenRange(member.property), `.${name}`); } return undefined; @@ -70,6 +64,15 @@ export default util.createRule({ }); }, }; + + function getTokenRange(property: TSESTree.Expression): [number, number] { + const sourceCode = context.getSourceCode(); + + return [ + sourceCode.getTokenBefore(property)!.range[0], + sourceCode.getTokenAfter(property)!.range[1], + ]; + } }, }); diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts new file mode 100644 index 000000000000..7b1181ff179d --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -0,0 +1,253 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import ts from 'typescript'; + +import * as util from '../util'; + +type Options = [ + { + checksConditionals?: boolean; + checksVoidReturn?: boolean; + } +]; + +export default util.createRule({ + name: 'no-misused-promises', + meta: { + docs: { + description: 'Avoid using promises in places not designed to handle them', + category: 'Best Practices', + recommended: false, + }, + messages: { + voidReturn: + 'Promise returned in function argument where a void return was expected.', + conditional: 'Expected non-Promise value in a boolean conditional.', + }, + schema: [ + { + type: 'object', + properties: { + checksConditionals: { + type: 'boolean', + }, + checksVoidReturn: { + type: 'boolean', + }, + }, + }, + ], + type: 'problem', + }, + defaultOptions: [ + { + checksConditionals: true, + checksVoidReturn: true, + }, + ], + + create(context, [{ checksConditionals, checksVoidReturn }]) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + const conditionalChecks: TSESLint.RuleListener = { + ConditionalExpression: checkTestConditional, + DoWhileStatement: checkTestConditional, + ForStatement: checkTestConditional, + IfStatement: checkTestConditional, + LogicalExpression(node) { + // We only check the lhs of a logical expression because the rhs might + // be the return value of a short circuit expression. + checkConditional(node.left); + }, + UnaryExpression(node) { + if (node.operator === '!') { + checkConditional(node.argument); + } + }, + WhileStatement: checkTestConditional, + }; + + const voidReturnChecks: TSESLint.RuleListener = { + CallExpression: checkArguments, + NewExpression: checkArguments, + }; + + function checkTestConditional(node: { test: TSESTree.Expression | null }) { + if (node.test) { + checkConditional(node.test); + } + } + + function checkConditional(node: TSESTree.Expression) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (isAlwaysThenable(checker, tsNode)) { + context.report({ + messageId: 'conditional', + node, + }); + } + } + + function checkArguments( + node: TSESTree.CallExpression | TSESTree.NewExpression, + ) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get< + ts.CallExpression | ts.NewExpression + >(node); + const voidParams = voidFunctionParams(checker, tsNode); + if (voidParams.size === 0) { + return; + } + + for (const [index, argument] of node.arguments.entries()) { + if (!voidParams.has(index)) { + continue; + } + + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(argument); + if (returnsThenable(checker, tsNode as ts.Expression)) { + context.report({ + messageId: 'voidReturn', + node: argument, + }); + } + } + } + + return { + ...(checksConditionals ? conditionalChecks : {}), + ...(checksVoidReturn ? voidReturnChecks : {}), + }; + }, +}); + +// Variation on the thenable check which requires all forms of the type (read: +// alternates in a union) to be thenable. Otherwise, you might be trying to +// check if something is defined or undefined and get caught because one of the +// branches is thenable. +function isAlwaysThenable(checker: ts.TypeChecker, node: ts.Node) { + const type = checker.getTypeAtLocation(node); + + for (const subType of tsutils.unionTypeParts(checker.getApparentType(type))) { + const thenProp = subType.getProperty('then'); + + // If one of the alternates has no then property, it is not thenable in all + // cases. + if (thenProp === undefined) { + return false; + } + + // We walk through each variation of the then property. Since we know it + // exists at this point, we just need at least one of the alternates to + // be of the right form to consider it thenable. + const thenType = checker.getTypeOfSymbolAtLocation(thenProp, node); + let hasThenableSignature = false; + for (const subType of tsutils.unionTypeParts(thenType)) { + for (const signature of subType.getCallSignatures()) { + if ( + signature.parameters.length !== 0 && + isFunctionParam(checker, signature.parameters[0], node) + ) { + hasThenableSignature = true; + break; + } + } + + // We only need to find one variant of the then property that has a + // function signature for it to be thenable. + if (hasThenableSignature) { + break; + } + } + + // If no flavors of the then property are thenable, we don't consider the + // overall type to be thenable + if (!hasThenableSignature) { + return false; + } + } + + // If all variants are considered thenable (i.e. haven't returned false), we + // consider the overall type thenable + return true; +} + +function isFunctionParam( + checker: ts.TypeChecker, + param: ts.Symbol, + node: ts.Node, +): boolean { + const type: ts.Type | undefined = checker.getApparentType( + checker.getTypeOfSymbolAtLocation(param, node), + ); + for (const subType of tsutils.unionTypeParts(type)) { + if (subType.getCallSignatures().length !== 0) { + return true; + } + } + return false; +} + +// Get the positions of parameters which are void functions (and not also +// thenable functions). These are the candidates for the void-return check at +// the current call site. +function voidFunctionParams( + checker: ts.TypeChecker, + node: ts.CallExpression | ts.NewExpression, +) { + const voidReturnIndices = new Set(); + const thenableReturnIndices = new Set(); + const type = checker.getTypeAtLocation(node.expression); + + for (const subType of tsutils.unionTypeParts(type)) { + // Standard function calls and `new` have two different types of signatures + const signatures = ts.isCallExpression(node) + ? subType.getCallSignatures() + : subType.getConstructSignatures(); + for (const signature of signatures) { + for (const [index, parameter] of signature.parameters.entries()) { + const type = checker.getTypeOfSymbolAtLocation( + parameter, + node.expression, + ); + for (const subType of tsutils.unionTypeParts(type)) { + for (const signature of subType.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { + voidReturnIndices.add(index); + } else if ( + tsutils.isThenableType(checker, node.expression, returnType) + ) { + thenableReturnIndices.add(index); + } + } + } + } + } + } + + // If a certain positional argument accepts both thenable and void returns, + // a promise-returning function is valid + for (const thenable of thenableReturnIndices) { + voidReturnIndices.delete(thenable); + } + + return voidReturnIndices; +} + +// Returns true if the expression is a function that returns a thenable +function returnsThenable(checker: ts.TypeChecker, node: ts.Expression) { + const type = checker.getApparentType(checker.getTypeAtLocation(node)); + + for (const subType of tsutils.unionTypeParts(type)) { + for (const signature of subType.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isThenableType(checker, node, returnType)) { + return true; + } + } + } + + return false; +} diff --git a/packages/eslint-plugin/src/rules/prefer-readonly.ts b/packages/eslint-plugin/src/rules/prefer-readonly.ts index 24685d306762..c82aaf45ab34 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly.ts @@ -2,7 +2,10 @@ import * as tsutils from 'tsutils'; import ts from 'typescript'; import * as util from '../util'; import { typeIsOrHasBaseType } from '../util'; -import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; type MessageIds = 'preferReadonly'; diff --git a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts index ffcf5aef9755..dae7b8f13c1f 100644 --- a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts +++ b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts @@ -1,4 +1,4 @@ -import { TSESTree } from '@typescript-eslint/typescript-estree'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createRule, getParserServices, getTypeName } from '../util'; import { getStaticValue } from 'eslint-utils'; diff --git a/packages/eslint-plugin/src/rules/triple-slash-reference.ts b/packages/eslint-plugin/src/rules/triple-slash-reference.ts index 286f445c9661..906efaa7db81 100644 --- a/packages/eslint-plugin/src/rules/triple-slash-reference.ts +++ b/packages/eslint-plugin/src/rules/triple-slash-reference.ts @@ -1,10 +1,5 @@ import * as util from '../util'; -import { - Literal, - Node, - TSExternalModuleReference, -} from '@typescript-eslint/typescript-estree/dist/ts-estree/ts-estree'; -import { TSESTree } from '@typescript-eslint/typescript-estree'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; type Options = [ { @@ -55,14 +50,14 @@ export default util.createRule({ }, ], create(context, [{ lib, path, types }]) { - let programNode: Node; + let programNode: TSESTree.Node; const sourceCode = context.getSourceCode(); const references: ({ comment: TSESTree.Comment; importName: string; })[] = []; - function hasMatchingReference(source: Literal) { + function hasMatchingReference(source: TSESTree.Literal) { references.forEach(reference => { if (reference.importName === source.value) { context.report({ @@ -78,14 +73,14 @@ export default util.createRule({ return { ImportDeclaration(node) { if (programNode) { - const source = node.source as Literal; + const source = node.source as TSESTree.Literal; hasMatchingReference(source); } }, TSImportEqualsDeclaration(node) { if (programNode) { - const source = (node.moduleReference as TSExternalModuleReference) - .expression as Literal; + const source = (node.moduleReference as TSESTree.TSExternalModuleReference) + .expression as TSESTree.Literal; hasMatchingReference(source); } }, diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts index ef5cdf57dd86..e3b4e5366e8b 100644 --- a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts @@ -41,6 +41,13 @@ ruleTester.run('no-dynamic-delete', rule, { output: `const container: { [i: string]: 0 } = {}; delete container.aaa;`, }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container [ 'aaa' ] ;`, + errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container .aaa ;`, + }, { code: `const container: { [i: string]: 0 } = {}; delete container['aa' + 'b'];`, @@ -75,6 +82,13 @@ ruleTester.run('no-dynamic-delete', rule, { output: `const container: { [i: string]: 0 } = {}; delete container.NaN;`, }, + { + code: `const container: { [i: string]: 0 } = {}; + delete container [ 'NaN' ] ;`, + errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container .NaN ;`, + }, { code: `const container: { [i: string]: 0 } = {}; const name = 'name'; diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts new file mode 100644 index 000000000000..5d4c7fcf4b85 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -0,0 +1,282 @@ +import rule from '../../src/rules/no-misused-promises'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-misused-promises', rule, { + valid: [ + `if (true) {}`, + { + code: `if (Promise.resolve()) {}`, + options: [{ checksConditionals: false }], + }, + ` +if (true) {} +else if (false) {} +else {} +`, + { + code: ` +if (Promise.resolve()) {} +else if (Promise.resolve()) {} +else {} +`, + options: [{ checksConditionals: false }], + }, + `for (;;) {}`, + `for (let i; i < 10; i++) {}`, + { + code: `for (let i; Promise.resolve(); i++) {}`, + options: [{ checksConditionals: false }], + }, + `do {} while (true);`, + { + code: `do {} while (Promise.resolve())`, + options: [{ checksConditionals: false }], + }, + `while (true) {}`, + { + code: `while (Promise.resolve()) {}`, + options: [{ checksConditionals: false }], + }, + `true ? 123 : 456`, + { + code: `Promise.resolve() ? 123 : 456`, + options: [{ checksConditionals: false }], + }, + `if (!true) {}`, + { + code: `if (!Promise.resolve()) {}`, + options: [{ checksConditionals: false }], + }, + `(await Promise.resolve()) || false`, + { + code: `Promise.resolve() || false`, + options: [{ checksConditionals: false }], + }, + `(true && await Promise.resolve()) || false`, + { + code: `(true && Promise.resolve()) || false`, + options: [{ checksConditionals: false }], + }, + `false || (true && Promise.resolve())`, + ` +async function test() { + if (await Promise.resolve()) {} +} +`, + ` +async function test() { + const mixed: Promise | undefined = Promise.resolve(); + if (mixed) { + await mixed; + } +} +`, + `if (~Promise.resolve()) {}`, + ` +interface NotQuiteThenable { + then(param: string): void; + then(): void; +} +const value: NotQuiteThenable = { then() {} }; +if (value) {} +`, + `[1, 2, 3].forEach(val => {});`, + { + code: `[1, 2, 3].forEach(async val => {});`, + options: [{ checksVoidReturn: false }], + }, + `new Promise((resolve, reject) => resolve());`, + { + code: `new Promise(async (resolve, reject) => resolve());`, + options: [{ checksVoidReturn: false }], + }, + `Promise.all(['abc', 'def'].map(async val => { await val; }))`, + ` +const fn: (arg: () => Promise | void) => void = () => {}; +fn(() => Promise.resolve()); +`, + ], + + invalid: [ + { + code: `if (Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: ` +if (Promise.resolve()) {} +else if (Promise.resolve()) {} +else {} +`, + errors: [ + { + line: 2, + messageId: 'conditional', + }, + { + line: 3, + messageId: 'conditional', + }, + ], + }, + { + code: `for (let i; Promise.resolve(); i++) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `do {} while (Promise.resolve())`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `while (Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `Promise.resolve() ? 123 : 456`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `if (!Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `Promise.resolve() || false`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `(true && Promise.resolve()) || false`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: ` +[Promise.resolve(), Promise.reject()].forEach( + async val => { await val; } +); +`, + errors: [ + { + line: 3, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +new Promise(async (resolve, reject) => { + await Promise.resolve(); + resolve(); +}); +`, + errors: [ + { + line: 2, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', async (err, res) => { + await res; +}); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', (err, res) => Promise.resolve(res)); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', (err, res) => { + if (err) { + return 'abc'; + } else { + return Promise.resolve(res); + } +}); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + ], +}); diff --git a/packages/experimental-utils/src/ts-eslint/Rule.ts b/packages/experimental-utils/src/ts-eslint/Rule.ts index 2759a3d33d24..bc546e25caee 100644 --- a/packages/experimental-utils/src/ts-eslint/Rule.ts +++ b/packages/experimental-utils/src/ts-eslint/Rule.ts @@ -270,6 +270,7 @@ interface RuleListener { JSXSpreadChild?: RuleFunction; JSXText?: RuleFunction; LabeledStatement?: RuleFunction; + LogicalExpression?: RuleFunction; MemberExpression?: RuleFunction; MetaProperty?: RuleFunction; MethodDefinition?: RuleFunction; From c161ff82ce4722ebee4b9f2cdad6faa778f02979 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Oct 2019 22:54:26 +0900 Subject: [PATCH 09/12] Undid some odd post-master merges --- .../eslint-plugin-tslint/src/rules/config.ts | 2 +- .../docs/rules/generic-type-naming.md | 2 +- .../eslint-plugin/src/rules/array-type.ts | 2 +- packages/eslint-plugin/src/rules/ban-types.ts | 2 +- .../src/rules/class-name-casing.ts | 2 +- .../rules/explicit-function-return-type.ts | 2 +- .../src/rules/func-call-spacing.ts | 2 +- .../src/rules/interface-name-prefix.ts | 2 +- .../src/rules/member-ordering.ts | 2 +- .../src/rules/no-empty-interface.ts | 2 +- .../src/rules/no-explicit-any.ts | 2 +- .../src/rules/no-extraneous-class.ts | 2 +- .../src/rules/no-floating-promises.ts | 2 +- .../src/rules/no-inferrable-types.ts | 2 +- .../src/rules/no-misused-promises.ts | 2 +- .../eslint-plugin/src/rules/no-namespace.ts | 2 +- .../src/rules/no-parameter-properties.ts | 2 +- .../eslint-plugin/src/rules/no-this-alias.ts | 2 +- .../eslint-plugin/src/rules/no-type-alias.ts | 2 +- .../src/rules/no-unnecessary-condition.ts | 2 +- .../rules/no-unnecessary-type-assertion.ts | 2 +- .../src/rules/prefer-readonly.ts | 2 +- .../src/rules/promise-function-async.ts | 2 +- .../src/rules/strict-boolean-expressions.ts | 2 +- .../src/rules/triple-slash-reference.ts | 2 +- .../src/rules/type-annotation-spacing.ts | 2 +- .../src/util/getParserServices.ts | 2 +- .../eslint-plugin/tests/rules/indent/utils.ts | 2 +- .../eslint-plugin/typings/eslint-rules.d.ts | 24 +++++++++---------- 29 files changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/eslint-plugin-tslint/src/rules/config.ts b/packages/eslint-plugin-tslint/src/rules/config.ts index 286aa8395d42..23ff428111d2 100644 --- a/packages/eslint-plugin-tslint/src/rules/config.ts +++ b/packages/eslint-plugin-tslint/src/rules/config.ts @@ -31,7 +31,7 @@ export type Options = [ rules?: RawRulesConfig; rulesDirectory?: string[]; lintFile?: string; - } + }, ]; /** diff --git a/packages/eslint-plugin/docs/rules/generic-type-naming.md b/packages/eslint-plugin/docs/rules/generic-type-naming.md index cda927f2935c..a24bb281d666 100644 --- a/packages/eslint-plugin/docs/rules/generic-type-naming.md +++ b/packages/eslint-plugin/docs/rules/generic-type-naming.md @@ -15,7 +15,7 @@ Examples of **correct** code with a configuration of `'^T[A-Z][a-zA-Z]+$'`: ```typescript type ReadOnly = { - readonly [TKey in keyof TType]: TType[TKey] + readonly [TKey in keyof TType]: TType[TKey]; }; interface SimpleMap { diff --git a/packages/eslint-plugin/src/rules/array-type.ts b/packages/eslint-plugin/src/rules/array-type.ts index 06061bda331c..b9977618c40c 100644 --- a/packages/eslint-plugin/src/rules/array-type.ts +++ b/packages/eslint-plugin/src/rules/array-type.ts @@ -77,7 +77,7 @@ type Options = [ { default: OptionString; readonly?: OptionString; - } + }, ]; type MessageIds = | 'errorStringGeneric' diff --git a/packages/eslint-plugin/src/rules/ban-types.ts b/packages/eslint-plugin/src/rules/ban-types.ts index fe961977ac86..a8c2cec6c06e 100644 --- a/packages/eslint-plugin/src/rules/ban-types.ts +++ b/packages/eslint-plugin/src/rules/ban-types.ts @@ -12,7 +12,7 @@ type Options = [ fixWith?: string; } >; - } + }, ]; type MessageIds = 'bannedTypeMessage'; diff --git a/packages/eslint-plugin/src/rules/class-name-casing.ts b/packages/eslint-plugin/src/rules/class-name-casing.ts index 2769e7a441d6..c71a39fd2ac4 100644 --- a/packages/eslint-plugin/src/rules/class-name-casing.ts +++ b/packages/eslint-plugin/src/rules/class-name-casing.ts @@ -7,7 +7,7 @@ import * as util from '../util'; type Options = [ { allowUnderscorePrefix?: boolean; - } + }, ]; type MessageIds = 'notPascalCased'; diff --git a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts index 61bec813e875..43a1cac41fb4 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -11,7 +11,7 @@ type Options = [ allowTypedFunctionExpressions?: boolean; allowHigherOrderFunctions?: boolean; allowDirectConstAssertionInArrowFunctions?: boolean; - } + }, ]; type MessageIds = 'missingReturnType'; diff --git a/packages/eslint-plugin/src/rules/func-call-spacing.ts b/packages/eslint-plugin/src/rules/func-call-spacing.ts index b64207816549..876761819fd7 100644 --- a/packages/eslint-plugin/src/rules/func-call-spacing.ts +++ b/packages/eslint-plugin/src/rules/func-call-spacing.ts @@ -6,7 +6,7 @@ export type Options = [ 'never' | 'always', { allowNewlines?: boolean; - }? + }?, ]; export type MessageIds = 'unexpected' | 'missing'; diff --git a/packages/eslint-plugin/src/rules/interface-name-prefix.ts b/packages/eslint-plugin/src/rules/interface-name-prefix.ts index fdfd220d3514..13284fc2aa34 100644 --- a/packages/eslint-plugin/src/rules/interface-name-prefix.ts +++ b/packages/eslint-plugin/src/rules/interface-name-prefix.ts @@ -18,7 +18,7 @@ type Options = [ | { prefixWithI: 'always'; allowUnderscorePrefix?: boolean; - } + }, ]; type MessageIds = 'noPrefix' | 'alwaysPrefix'; diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index d16ef184f6a6..72020afc6008 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -13,7 +13,7 @@ type Options = [ classExpressions?: OrderConfig; interfaces?: OrderConfig; typeLiterals?: OrderConfig; - } + }, ]; const allMemberTypes = ['field', 'method', 'constructor'].reduce( diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index 4e844c38a04b..55fbdb5337e3 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -3,7 +3,7 @@ import * as util from '../util'; type Options = [ { allowSingleExtends?: boolean; - } + }, ]; type MessageIds = 'noEmpty' | 'noEmptyWithSuper'; diff --git a/packages/eslint-plugin/src/rules/no-explicit-any.ts b/packages/eslint-plugin/src/rules/no-explicit-any.ts index 7d237884360e..249265a7683c 100644 --- a/packages/eslint-plugin/src/rules/no-explicit-any.ts +++ b/packages/eslint-plugin/src/rules/no-explicit-any.ts @@ -9,7 +9,7 @@ export type Options = [ { fixToUnknown?: boolean; ignoreRestArgs?: boolean; - } + }, ]; export type MessageIds = 'unexpectedAny'; diff --git a/packages/eslint-plugin/src/rules/no-extraneous-class.ts b/packages/eslint-plugin/src/rules/no-extraneous-class.ts index c8fdcb6d8b6b..4756b5928f33 100644 --- a/packages/eslint-plugin/src/rules/no-extraneous-class.ts +++ b/packages/eslint-plugin/src/rules/no-extraneous-class.ts @@ -9,7 +9,7 @@ type Options = [ allowConstructorOnly?: boolean; allowEmpty?: boolean; allowStaticOnly?: boolean; - } + }, ]; type MessageIds = 'empty' | 'onlyStatic' | 'onlyConstructor'; diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 4a059a441a4f..32538271b948 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -6,7 +6,7 @@ import * as util from '../util'; type Options = [ { ignoreVoid?: boolean; - } + }, ]; export default util.createRule({ diff --git a/packages/eslint-plugin/src/rules/no-inferrable-types.ts b/packages/eslint-plugin/src/rules/no-inferrable-types.ts index e8b7447daebe..92a582526795 100644 --- a/packages/eslint-plugin/src/rules/no-inferrable-types.ts +++ b/packages/eslint-plugin/src/rules/no-inferrable-types.ts @@ -8,7 +8,7 @@ type Options = [ { ignoreParameters?: boolean; ignoreProperties?: boolean; - } + }, ]; type MessageIds = 'noInferrableType'; diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index eb05a7b0f5f9..63d0f0bf483c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -8,7 +8,7 @@ type Options = [ { checksConditionals?: boolean; checksVoidReturn?: boolean; - } + }, ]; export default util.createRule({ diff --git a/packages/eslint-plugin/src/rules/no-namespace.ts b/packages/eslint-plugin/src/rules/no-namespace.ts index 3036b9ec6e75..3698b4f941bd 100644 --- a/packages/eslint-plugin/src/rules/no-namespace.ts +++ b/packages/eslint-plugin/src/rules/no-namespace.ts @@ -8,7 +8,7 @@ type Options = [ { allowDeclarations?: boolean; allowDefinitionFiles?: boolean; - } + }, ]; type MessageIds = 'moduleSyntaxIsPreferred'; diff --git a/packages/eslint-plugin/src/rules/no-parameter-properties.ts b/packages/eslint-plugin/src/rules/no-parameter-properties.ts index ef60572cf7c4..9b6bb31e6541 100644 --- a/packages/eslint-plugin/src/rules/no-parameter-properties.ts +++ b/packages/eslint-plugin/src/rules/no-parameter-properties.ts @@ -15,7 +15,7 @@ type Modifier = type Options = [ { allows: Modifier[]; - } + }, ]; type MessageIds = 'noParamProp'; diff --git a/packages/eslint-plugin/src/rules/no-this-alias.ts b/packages/eslint-plugin/src/rules/no-this-alias.ts index e1bd800821dd..8865fad2c3fb 100644 --- a/packages/eslint-plugin/src/rules/no-this-alias.ts +++ b/packages/eslint-plugin/src/rules/no-this-alias.ts @@ -8,7 +8,7 @@ type Options = [ { allowDestructuring?: boolean; allowedNames?: string[]; - } + }, ]; type MessageIds = 'thisAssignment' | 'thisDestructure'; diff --git a/packages/eslint-plugin/src/rules/no-type-alias.ts b/packages/eslint-plugin/src/rules/no-type-alias.ts index 54b9eeb75c18..38816b60a2fa 100644 --- a/packages/eslint-plugin/src/rules/no-type-alias.ts +++ b/packages/eslint-plugin/src/rules/no-type-alias.ts @@ -25,7 +25,7 @@ type Options = [ allowLiterals?: Values; allowMappedTypes?: Values; allowTupleTypes?: Values; - } + }, ]; type MessageIds = 'noTypeAlias' | 'noCompositionAlias'; diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index ddae915cef50..49c8f13401c8 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -51,7 +51,7 @@ type ExpressionWithTest = export type Options = [ { ignoreRhs?: boolean; - } + }, ]; export type MessageId = diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index e7c5585e1df8..61583878bae9 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -16,7 +16,7 @@ import * as util from '../util'; type Options = [ { typesToIgnore?: string[]; - } + }, ]; type MessageIds = 'contextuallyUnnecessary' | 'unnecessaryAssertion'; diff --git a/packages/eslint-plugin/src/rules/prefer-readonly.ts b/packages/eslint-plugin/src/rules/prefer-readonly.ts index b22fb70dd610..280213dde0f3 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly.ts @@ -12,7 +12,7 @@ type MessageIds = 'preferReadonly'; type Options = [ { onlyInlineLambdas?: boolean; - } + }, ]; const functionScopeBoundaries = [ diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 246a7ee274f1..1575cf02dfce 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -9,7 +9,7 @@ type Options = [ checkFunctionDeclarations?: boolean; checkFunctionExpressions?: boolean; checkMethodDeclarations?: boolean; - } + }, ]; type MessageIds = 'missingAsync'; diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 1d734ef240be..e67e4d1f687f 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -17,7 +17,7 @@ type Options = [ { ignoreRhs?: boolean; allowNullable?: boolean; - } + }, ]; export default util.createRule({ diff --git a/packages/eslint-plugin/src/rules/triple-slash-reference.ts b/packages/eslint-plugin/src/rules/triple-slash-reference.ts index f95aa136da37..8ce64e5748af 100644 --- a/packages/eslint-plugin/src/rules/triple-slash-reference.ts +++ b/packages/eslint-plugin/src/rules/triple-slash-reference.ts @@ -6,7 +6,7 @@ type Options = [ lib?: 'always' | 'never'; path?: 'always' | 'never'; types?: 'always' | 'never' | 'prefer-import'; - } + }, ]; type MessageIds = 'tripleSlashReference'; diff --git a/packages/eslint-plugin/src/rules/type-annotation-spacing.ts b/packages/eslint-plugin/src/rules/type-annotation-spacing.ts index 531d6ef84abc..d0f1c5b45a44 100644 --- a/packages/eslint-plugin/src/rules/type-annotation-spacing.ts +++ b/packages/eslint-plugin/src/rules/type-annotation-spacing.ts @@ -15,7 +15,7 @@ type Options = [ after?: boolean; }; }; - }? + }?, ]; type MessageIds = | 'expectedSpaceAfter' diff --git a/packages/eslint-plugin/src/util/getParserServices.ts b/packages/eslint-plugin/src/util/getParserServices.ts index 4094687404d7..d0a184871718 100644 --- a/packages/eslint-plugin/src/util/getParserServices.ts +++ b/packages/eslint-plugin/src/util/getParserServices.ts @@ -4,7 +4,7 @@ import { } from '@typescript-eslint/experimental-utils'; type RequiredParserServices = { - [k in keyof ParserServices]: Exclude + [k in keyof ParserServices]: Exclude; }; /** diff --git a/packages/eslint-plugin/tests/rules/indent/utils.ts b/packages/eslint-plugin/tests/rules/indent/utils.ts index b40f0a8165e2..badfbc1c3c6f 100644 --- a/packages/eslint-plugin/tests/rules/indent/utils.ts +++ b/packages/eslint-plugin/tests/rules/indent/utils.ts @@ -39,7 +39,7 @@ type ProvidedError = [ // actual indent number | string, // node type - AST_NODE_TYPES | AST_TOKEN_TYPES + AST_NODE_TYPES | AST_TOKEN_TYPES, ]; function is2DProvidedErrorArr( diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index d9da8e53f4e7..120b11433cb9 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -16,7 +16,7 @@ declare module 'eslint/lib/rules/arrow-parens' { 'always' | 'as-needed', { requireForBlockBody?: boolean; - }? + }?, ], { ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void; @@ -35,7 +35,7 @@ declare module 'eslint/lib/rules/camelcase' { allow?: string[]; ignoreDestructuring?: boolean; properties?: 'always' | 'never'; - } + }, ], { Identifier(node: TSESTree.Identifier): void; @@ -81,7 +81,7 @@ declare module 'eslint/lib/rules/indent' { flatTernaryExpressions?: boolean; ignoredNodes?: string[]; ignoreComments?: boolean; - })? + })?, ], { '*:exit'(node: TSESTree.Node): void; @@ -163,7 +163,7 @@ declare module 'eslint/lib/rules/no-empty-function' { [ { allow?: string[]; - } + }, ], { FunctionDeclaration(node: TSESTree.FunctionDeclaration): void; @@ -200,7 +200,7 @@ declare module 'eslint/lib/rules/no-magic-numbers' { ignoreNumericLiteralTypes?: boolean; ignoreEnums?: boolean; ignoreReadonlyClassProperties?: boolean; - } + }, ], { Literal(node: TSESTree.Literal): void; @@ -217,7 +217,7 @@ declare module 'eslint/lib/rules/no-redeclare' { [ { builtinGlobals?: boolean; - }? + }?, ], { ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void; @@ -254,7 +254,7 @@ declare module 'eslint/lib/rules/no-shadow' { builtinGlobals?: boolean; hoist: 'all' | 'functions' | 'never'; allow: string[]; - } + }, ], { ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void; @@ -271,7 +271,7 @@ declare module 'eslint/lib/rules/no-undef' { [ { typeof?: boolean; - } + }, ], { ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void; @@ -371,7 +371,7 @@ declare module 'eslint/lib/rules/no-extra-parens' { nestedBinaryExpressions?: boolean; ignoreJSX?: 'none' | 'all' | 'multi-line' | 'single-line'; enforceForArrowConditionals?: boolean; - }? + }?, ], { ArrayExpression(node: TSESTree.ArrayExpression): void; @@ -444,7 +444,7 @@ declare module 'eslint/lib/rules/semi' { { beforeStatementContinuationChars?: 'always' | 'any' | 'never'; omitLastInOneLineBlock?: boolean; - }? + }?, ], { VariableDeclaration(node: TSESTree.VariableDeclaration): void; @@ -474,7 +474,7 @@ declare module 'eslint/lib/rules/quotes' { { allowTemplateLiterals?: boolean; avoidEscape?: boolean; - }? + }?, ], { Literal(node: TSESTree.Literal): void; @@ -498,7 +498,7 @@ declare module 'eslint/lib/rules/brace-style' { '1tbs' | 'stroustrup' | 'allman', { allowSingleLine?: boolean; - }? + }?, ], { BlockStatement(node: TSESTree.BlockStatement): void; From 4187a84359e1b04a03b4bb39fa048cb4f582ff38 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Oct 2019 22:57:46 +0900 Subject: [PATCH 10/12] Improved bad-case docs a bit --- packages/eslint-plugin/docs/rules/no-dynamic-delete.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-dynamic-delete.md b/packages/eslint-plugin/docs/rules/no-dynamic-delete.md index e316f20f3ab8..630046823387 100644 --- a/packages/eslint-plugin/docs/rules/no-dynamic-delete.md +++ b/packages/eslint-plugin/docs/rules/no-dynamic-delete.md @@ -29,6 +29,11 @@ Examples of **incorrect** code with this rule: // Can be replaced with the constant equivalents, such as container.aaa delete container['aaa']; delete container['Infinity']; + +// Dynamic, difficult-to-reason-about lookups +const name = 'name'; +delete container[name]; +delete container[name.toUpperCase()]; ``` ## When Not To Use It From 87fc17dce37acd6d2f999e3b080924a528514b60 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Oct 2019 23:19:25 +0900 Subject: [PATCH 11/12] chore: fixed lint complaints on return types --- .../src/rules/no-dynamic-delete.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index a102be438517..0ee1636ea643 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -31,22 +31,21 @@ export default util.createRule({ member.property.type === AST_NODE_TYPES.Literal && typeof member.property.value === 'string' ) { - const { value } = member.property; - return fixer => - fixer.replaceTextRange(getTokenRange(member.property), `.${value}`); + return createPropertyReplacement( + member.property, + member.property.value, + ); } if (member.property.type === AST_NODE_TYPES.Identifier) { - const { name } = member.property; - return fixer => - fixer.replaceTextRange(getTokenRange(member.property), `.${name}`); + return createPropertyReplacement(member.property, member.property.name); } return undefined; } return { - 'UnaryExpression[operator=delete]'(node: TSESTree.UnaryExpression) { + 'UnaryExpression[operator=delete]'(node: TSESTree.UnaryExpression): void { if ( node.argument.type !== AST_NODE_TYPES.MemberExpression || !node.argument.computed || @@ -65,6 +64,14 @@ export default util.createRule({ }, }; + function createPropertyReplacement( + property: TSESTree.Expression, + replacement: string, + ) { + return (fixer: TSESLint.RuleFixer): TSESLint.RuleFix => + fixer.replaceTextRange(getTokenRange(property), `.${replacement}`); + } + function getTokenRange(property: TSESTree.Expression): [number, number] { const sourceCode = context.getSourceCode(); From 7c65b05b8ee32d3fef5781f82a5348c9649f5543 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Oct 2019 23:28:42 +0900 Subject: [PATCH 12/12] chore: remove duplicate no-empty-function README.md line --- packages/eslint-plugin/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index b687727684b7..7dfc2c135a78 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -161,7 +161,6 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Bans usage of the delete operator with computed key expressions | | :wrench: | | -| [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | | | | | [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | :heavy_check_mark: | | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | |