From c5e9937ec90d25f82cac751fa77783fe8dd0fa9a Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Sat, 17 Jul 2021 23:18:38 -0700 Subject: [PATCH 1/4] feat(eslint-plugin): add `no-meaningless-void-operator` rule --- packages/eslint-plugin/README.md | 1 + .../rules/no-meaningless-void-operator.md | 37 +++++++++ packages/eslint-plugin/src/configs/all.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-meaningless-void-operator.ts | 65 +++++++++++++++ .../no-meaningless-void-operator.test.ts | 79 +++++++++++++++++++ 6 files changed, 185 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md create mode 100644 packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts create mode 100644 packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 344764dcb96..bcfdd3a699b 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -128,6 +128,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-implicit-any-catch`](./docs/rules/no-implicit-any-catch.md) | Disallow usage of the implicit `any` type in catch clauses | | :wrench: | | | [`@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 | :white_check_mark: | :wrench: | | | [`@typescript-eslint/no-invalid-void-type`](./docs/rules/no-invalid-void-type.md) | Disallows usage of `void` type outside of generic or return types | | | | +| [`@typescript-eslint/no-meaningless-void-operator`](./docs/rules/no-meaningless-void-operator.md) | Disallow the `void` operator except when used to discard a value | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :white_check_mark: | | | | [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | :white_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :white_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md new file mode 100644 index 00000000000..cde72c90962 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md @@ -0,0 +1,37 @@ +# Disallow the `void` operator except when used to discard a value (`no-meaningless-void-operator`) + +Disallow the `void` operator when its argument is already of type `void`, `undefined`, or `never`. + +## Rule Details + +The `void` operator is a useful tool to convey the programmer's intent to discard a value. For example, it is recommended as one way of suppressing [`@typescript-eslint/no-floating-promises`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md) instead of adding `.catch()` to a promise. + +This rule helps an author catch API changes where previously a value was being discarded at a call site, but the callee changed so it no longer returns a value. When combined with [no-unused-expressions](https://eslint.org/docs/rules/no-unused-expressions), it also helps _readers_ of the code by ensuring consistency: a statement that looks like `void foo();` is **always** discarding a return value, and a statement that looks like `foo();` is **never** discarding a return value. + +Examples of **incorrect** code for this rule: + +```ts +void (() => {})(); + +function foo() {} +void foo(); + +function bar(x: never) { + void x; +} +``` + +Examples of **correct** code for this rule: + +```ts +(() => {})(); + +function foo() {} +foo(); // nothing to discard + +function bar(x: number) { + void x; // discarding a number + return 2; +} +void bar(); // discarding a number +``` diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index b21d14efc9f..e21bb1d20fa 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -78,6 +78,7 @@ export = { '@typescript-eslint/no-loss-of-precision': 'error', 'no-magic-numbers': 'off', '@typescript-eslint/no-magic-numbers': 'error', + '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/no-namespace': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index be78db26f80..55603d64192 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -50,6 +50,7 @@ import noInvalidVoidType from './no-invalid-void-type'; import noLoopFunc from './no-loop-func'; import noLossOfPrecision from './no-loss-of-precision'; import noMagicNumbers from './no-magic-numbers'; +import noMeaninglessVoidOperator from './no-meaningless-void-operator'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; import noNamespace from './no-namespace'; @@ -168,6 +169,7 @@ export default { 'no-loop-func': noLoopFunc, 'no-loss-of-precision': noLossOfPrecision, 'no-magic-numbers': noMagicNumbers, + 'no-meaningless-void-operator': noMeaninglessVoidOperator, 'no-misused-new': noMisusedNew, 'no-misused-promises': noMisusedPromises, 'no-namespace': noNamespace, diff --git a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts new file mode 100644 index 00000000000..6458df99439 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts @@ -0,0 +1,65 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import * as util from '../util'; +import * as ts from 'typescript'; + +export default util.createRule<[], 'meaninglessVoidOperator'>({ + name: 'no-meaningless-void-operator', + meta: { + type: 'suggestion', + docs: { + description: + 'Disallow the `void` operator except when used to discard a value', + category: 'Best Practices', + recommended: false, + suggestion: true, + requiresTypeChecking: true, + }, + fixable: 'code', + messages: { + meaninglessVoidOperator: `void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored`, + }, + schema: [], + }, + defaultOptions: [], + + create(context, _options) { + const parserServices = ESLintUtils.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const sourceCode = context.getSourceCode(); + + return { + [`UnaryExpression[operator="void"]`]( + node: TSESTree.UnaryExpression, + ): void { + const argTsNode = parserServices.esTreeNodeToTSNodeMap.get( + node.argument, + ); + const argType = checker.getTypeAtLocation(argTsNode); + if ( + tsutils + .unionTypeParts(argType) + .every( + part => + part.flags & + (ts.TypeFlags.Void | + ts.TypeFlags.Undefined | + ts.TypeFlags.Never), + ) + ) { + context.report({ + node, + messageId: 'meaninglessVoidOperator', + data: { type: checker.typeToString(argType) }, + fix(fixer) { + return fixer.removeRange([ + sourceCode.getTokens(node)[0].range[0], + sourceCode.getTokens(node)[1].range[0], + ]); + }, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts new file mode 100644 index 00000000000..ab1ee09aa95 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts @@ -0,0 +1,79 @@ +import rule from '../../src/rules/no-meaningless-void-operator'; +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-meaningless-void-operator', rule, { + valid: [ + ` +(() => {})(); + +function foo() {} +foo(); // nothing to discard + +function bar(x: number) { + void x; + return 2; +} +void bar(); // discarding a number + `, + ], + invalid: [ + { + code: 'void (() => {})();', + output: '(() => {})();', + errors: [ + { + messageId: 'meaninglessVoidOperator', + line: 1, + column: 1, + }, + ], + }, + { + code: ` +function foo() {} +void foo(); + `, + output: ` +function foo() {} +foo(); + `, + errors: [ + { + messageId: 'meaninglessVoidOperator', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +function bar(x: never) { + void x; +} + `, + output: ` +function bar(x: never) { + x; +} + `, + errors: [ + { + messageId: 'meaninglessVoidOperator', + line: 3, + column: 3, + }, + ], + }, + ], +}); From 172249fedae5911213e531fb816ffbfc4475845b Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Mon, 2 Aug 2021 11:36:34 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Brad Zacher --- .../eslint-plugin/docs/rules/no-meaningless-void-operator.md | 2 +- .../eslint-plugin/src/rules/no-meaningless-void-operator.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md index cde72c90962..015cb166f26 100644 --- a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md +++ b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md @@ -4,7 +4,7 @@ Disallow the `void` operator when its argument is already of type `void`, `undef ## Rule Details -The `void` operator is a useful tool to convey the programmer's intent to discard a value. For example, it is recommended as one way of suppressing [`@typescript-eslint/no-floating-promises`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md) instead of adding `.catch()` to a promise. +The `void` operator is a useful tool to convey the programmer's intent to discard a value. For example, it is recommended as one way of suppressing [`@typescript-eslint/no-floating-promises`](./no-floating-promises.md) instead of adding `.catch()` to a promise. This rule helps an author catch API changes where previously a value was being discarded at a call site, but the callee changed so it no longer returns a value. When combined with [no-unused-expressions](https://eslint.org/docs/rules/no-unused-expressions), it also helps _readers_ of the code by ensuring consistency: a statement that looks like `void foo();` is **always** discarding a return value, and a statement that looks like `foo();` is **never** discarding a return value. diff --git a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts index 6458df99439..0aa68c19f2c 100644 --- a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts +++ b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts @@ -17,7 +17,7 @@ export default util.createRule<[], 'meaninglessVoidOperator'>({ }, fixable: 'code', messages: { - meaninglessVoidOperator: `void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored`, + meaninglessVoidOperator: "void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored", }, schema: [], }, @@ -29,7 +29,7 @@ export default util.createRule<[], 'meaninglessVoidOperator'>({ const sourceCode = context.getSourceCode(); return { - [`UnaryExpression[operator="void"]`]( + 'UnaryExpression[operator="void"]'( node: TSESTree.UnaryExpression, ): void { const argTsNode = parserServices.esTreeNodeToTSNodeMap.get( From 2154af779e617a9efab4d2081173d33d9548ef43 Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Mon, 2 Aug 2021 11:59:05 -0700 Subject: [PATCH 3/4] fix: move never support behind `checkNever` option --- .../rules/no-meaningless-void-operator.md | 23 ++++-- .../src/rules/no-meaningless-void-operator.ts | 79 +++++++++++++------ .../no-meaningless-void-operator.test.ts | 23 ++++-- 3 files changed, 90 insertions(+), 35 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md index 015cb166f26..4aa69e5714b 100644 --- a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md +++ b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md @@ -1,6 +1,6 @@ # Disallow the `void` operator except when used to discard a value (`no-meaningless-void-operator`) -Disallow the `void` operator when its argument is already of type `void`, `undefined`, or `never`. +Disallow the `void` operator when its argument is already of type `void` or `undefined`. ## Rule Details @@ -15,10 +15,6 @@ void (() => {})(); function foo() {} void foo(); - -function bar(x: never) { - void x; -} ``` Examples of **correct** code for this rule: @@ -35,3 +31,20 @@ function bar(x: number) { } void bar(); // discarding a number ``` + +### Options + +This rule accepts a single object option with the following default configuration: + +```json +{ + "@typescript-eslint/no-meaningless-void-operator": [ + "error", + { + "checkNever": false + } + ] +} +``` + +- `checkNever: true` will suggest removing `void` when the argument has type `never`. diff --git a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts index 0aa68c19f2c..09516812f76 100644 --- a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts +++ b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts @@ -1,9 +1,19 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { + ESLintUtils, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import * as tsutils from 'tsutils'; import * as util from '../util'; import * as ts from 'typescript'; -export default util.createRule<[], 'meaninglessVoidOperator'>({ +type Options = [ + { + checkNever: boolean; + }, +]; + +export default util.createRule({ name: 'no-meaningless-void-operator', meta: { type: 'suggestion', @@ -17,46 +27,67 @@ export default util.createRule<[], 'meaninglessVoidOperator'>({ }, fixable: 'code', messages: { - meaninglessVoidOperator: "void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored", + meaninglessVoidOperator: + "void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored", }, - schema: [], + schema: [ + { + type: 'object', + properties: { + checkNever: { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + ], }, - defaultOptions: [], + defaultOptions: [{ checkNever: false }], - create(context, _options) { + create(context, [{ checkNever }]) { const parserServices = ESLintUtils.getParserServices(context); const checker = parserServices.program.getTypeChecker(); const sourceCode = context.getSourceCode(); return { - 'UnaryExpression[operator="void"]'( - node: TSESTree.UnaryExpression, - ): void { + 'UnaryExpression[operator="void"]'(node: TSESTree.UnaryExpression): void { + const fix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix => { + return fixer.removeRange([ + sourceCode.getTokens(node)[0].range[0], + sourceCode.getTokens(node)[1].range[0], + ]); + }; + const argTsNode = parserServices.esTreeNodeToTSNodeMap.get( node.argument, ); const argType = checker.getTypeAtLocation(argTsNode); + const unionParts = tsutils.unionTypeParts(argType); if ( - tsutils - .unionTypeParts(argType) - .every( - part => - part.flags & - (ts.TypeFlags.Void | - ts.TypeFlags.Undefined | - ts.TypeFlags.Never), - ) + unionParts.every( + part => part.flags & (ts.TypeFlags.Void | ts.TypeFlags.Undefined), + ) + ) { + context.report({ + node, + messageId: 'meaninglessVoidOperator', + data: { type: checker.typeToString(argType) }, + fix, + }); + } else if ( + checkNever && + unionParts.every( + part => + part.flags & + (ts.TypeFlags.Void | ts.TypeFlags.Undefined | ts.TypeFlags.Never), + ) ) { context.report({ node, messageId: 'meaninglessVoidOperator', data: { type: checker.typeToString(argType) }, - fix(fixer) { - return fixer.removeRange([ - sourceCode.getTokens(node)[0].range[0], - sourceCode.getTokens(node)[1].range[0], - ]); - }, + suggest: [{ messageId: 'meaninglessVoidOperator', fix }], }); } }, diff --git a/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts index ab1ee09aa95..24853b01f28 100644 --- a/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts +++ b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts @@ -26,6 +26,11 @@ function bar(x: number) { } void bar(); // discarding a number `, + ` +function bar(x: never) { + void x; +} + `, ], invalid: [ { @@ -57,21 +62,27 @@ foo(); ], }, { + options: [{ checkNever: true }], code: ` function bar(x: never) { void x; } - `, - output: ` -function bar(x: never) { - x; -} - `, + `.trimRight(), errors: [ { messageId: 'meaninglessVoidOperator', line: 3, column: 3, + suggestions: [ + { + messageId: 'meaninglessVoidOperator', + output: ` +function bar(x: never) { + x; +} + `.trimRight(), + }, + ], }, ], }, From 95081fb42dc475f103ec0fe7422fa80370ea58a2 Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Mon, 2 Aug 2021 12:03:34 -0700 Subject: [PATCH 4/4] fix: update message for suggestion --- .../src/rules/no-meaningless-void-operator.ts | 8 ++++++-- .../tests/rules/no-meaningless-void-operator.test.ts | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts index 09516812f76..3640cb34630 100644 --- a/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts +++ b/packages/eslint-plugin/src/rules/no-meaningless-void-operator.ts @@ -13,7 +13,10 @@ type Options = [ }, ]; -export default util.createRule({ +export default util.createRule< + Options, + 'meaninglessVoidOperator' | 'removeVoid' +>({ name: 'no-meaningless-void-operator', meta: { type: 'suggestion', @@ -29,6 +32,7 @@ export default util.createRule({ messages: { meaninglessVoidOperator: "void operator shouldn't be used on {{type}}; it should convey that a return value is being ignored", + removeVoid: "Remove 'void'", }, schema: [ { @@ -87,7 +91,7 @@ export default util.createRule({ node, messageId: 'meaninglessVoidOperator', data: { type: checker.typeToString(argType) }, - suggest: [{ messageId: 'meaninglessVoidOperator', fix }], + suggest: [{ messageId: 'removeVoid', fix }], }); } }, diff --git a/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts index 24853b01f28..5e034f2904b 100644 --- a/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts +++ b/packages/eslint-plugin/tests/rules/no-meaningless-void-operator.test.ts @@ -75,7 +75,7 @@ function bar(x: never) { column: 3, suggestions: [ { - messageId: 'meaninglessVoidOperator', + messageId: 'removeVoid', output: ` function bar(x: never) { x;