feat(eslint-plugin): add rule no-void-expression (typescript-eslint#2605)

phaux · web-flow · commit c8a4dadeab8a · 2020-11-02T15:15:27.000-08:00
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -221,6 +221,15 @@ module.exports = {
         '@typescript-eslint/internal/plugin-test-formatting': 'error',
       },
     },
+    // files which list all the things
+    {
+      files: ['packages/eslint-plugin/src/rules/index.ts'],
+      rules: {
+        // enforce alphabetical ordering
+        'sort-keys': 'error',
+        'import/order': ['error', { alphabetize: { order: 'asc' } }],
+      },
+    },
     // tools and tests
     {
       files: ['**/tools/**/*.ts', '**/tests/**/*.ts'],
diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
@@ -117,6 +117,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
 | [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md)                                           | Enforces naming conventions for everything across a codebase                                                            |                    |          | :thought_balloon: |
 | [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md)                                           | Requires that `.toString()` is only called on objects which provide useful information when stringified                 |                    |          | :thought_balloon: |
 | [`@typescript-eslint/no-confusing-non-null-assertion`](./docs/rules/no-confusing-non-null-assertion.md)               | Disallow non-null assertion in locations that may be confusing                                                          |                    | :wrench: |                   |
+| [`@typescript-eslint/no-confusing-void-expression`](./docs/rules/no-confusing-void-expression.md)                     | Requires expressions of type void to appear in statement position                                                       |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md)                                           | Disallow the delete operator with computed key expressions                                                              |                    | :wrench: |                   |
 | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md)                                         | Disallow the declaration of empty interfaces                                                                            | :heavy_check_mark: | :wrench: |                   |
 | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md)                                               | Disallow usage of the `any` type                                                                                        | :heavy_check_mark: | :wrench: |                   |
diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md
@@ -96,7 +96,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
 | [`no-unused-variable`]               | 🌓  | [`@typescript-eslint/no-unused-vars`]                                                                     |
 | [`no-use-before-declare`]            | ✅  | [`@typescript-eslint/no-use-before-define`]                                                               |
 | [`no-var-keyword`]                   | 🌟  | [`no-var`][no-var]                                                                                        |
-| [`no-void-expression`]               | 🛑  | N/A (unrelated to the similarly named ESLint rule `no-void`)                                              |
+| [`no-void-expression`]               | ✅  | [`@typescript-eslint/no-confusing-void-expression`]                                                       |
 | [`prefer-conditional-expression`]    | 🛑  | N/A                                                                                                       |
 | [`prefer-object-spread`]             | 🌟  | [`prefer-object-spread`][prefer-object-spread]                                                            |
 | [`radix`]                            | 🌟  | [`radix`][radix]                                                                                          |
@@ -650,6 +650,8 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
 [`@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
+[`@typescript-eslint/restrict-template-expressions`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/restrict-template-expressions.md
+[`@typescript-eslint/no-confusing-void-expression`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md
 
 <!-- eslint-plugin-import -->
 
diff --git a/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md
@@ -0,0 +1,149 @@
+# Requires expressions of type void to appear in statement position (`no-confusing-void-expression`)
+
+Returning the results of an expression whose type is void can be misleading.
+Attempting to do so is likely a symptom of expecting a different return type from a function.
+Even if used correctly, it can be misleading for other developers,
+who don't know what a particular function does and if its result matters.
+
+This rule provides automatic fixes for most common cases.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```ts
+// somebody forgot that `alert` doesn't return anything
+const response = alert('Are you sure?');
+console.log(alert('Are you sure?'));
+
+// it's not obvious whether the chained promise will contain the response (fixable)
+promise.then(value => window.postMessage(value));
+
+// it looks like we are returning the result of `console.error` (fixable)
+function doSomething() {
+  if (!somethingToDo) {
+    return console.error('Nothing to do!');
+  }
+
+  console.log('Doing a thing...');
+}
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+// just a regular void function in a statement position
+alert('Hello, world!');
+
+// this function returns a boolean value so it's ok
+const response = confirm('Are you sure?');
+console.log(confirm('Are you sure?'));
+
+// now it's obvious that `postMessage` doesn't return any response
+promise.then(value => {
+  window.postMessage(value);
+});
+
+// now it's explicit that we want to log the error and return early
+function doSomething() {
+  if (!somethingToDo) {
+    console.error('Nothing to do!');
+    return;
+  }
+
+  console.log('Doing a thing...');
+}
+
+// using logical expressions for their side effects is fine
+cond && console.log('true');
+cond || console.error('false');
+cond ? console.log('true') : console.error('false');
+```
+
+## Options
+
+An object option can be specified. Each boolean flag makes the rule less strict.
+
+```ts
+type Options = {
+  ignoreArrowShorthand?: boolean;
+  ignoreVoidOperator?: boolean;
+};
+
+const defaults: Options = {
+  ignoreArrowShorthand: false,
+  ignoreVoidOperator: false,
+};
+```
+
+### `ignoreArrowShorthand`
+
+`false` by default.
+
+```json
+{
+  "@typescript-eslint/no-confusing-void-expression": [
+    "error",
+    { "ignoreArrowShorthand": true }
+  ]
+}
+```
+
+It might be undesirable to wrap every arrow function shorthand expression with braces.
+Especially when using Prettier formatter, which spreads such code across 3 lines instead of 1.
+
+Examples of additional **correct** code with this option enabled:
+
+```ts
+promise.then(value => window.postMessage(value));
+```
+
+### `ignoreVoidOperator`
+
+`false` by default.
+
+```json
+{
+  "@typescript-eslint/no-confusing-void-expression": [
+    "error",
+    { "ignoreVoidOperator": true }
+  ]
+}
+```
+
+It might be preferable to only use some distinct syntax
+to explicitly mark the confusing but valid usage of void expressions.
+This option allows void expressions which are explicitly wrapped in the `void` operator.
+This can help avoid confusion among other developers as long as they are made aware of this code style.
+
+This option also changes the automatic fixes for common cases to use the `void` operator.
+It also enables a suggestion fix to wrap the void expression with `void` operator for every problem reported.
+
+Examples of additional **correct** code with this option enabled:
+
+```ts
+// now it's obvious that we don't expect any response
+promise.then(value => void window.postMessage(value));
+
+// now it's explicit that we don't want to return anything
+function doSomething() {
+  if (!somethingToDo) {
+    return void console.error('Nothing to do!');
+  }
+
+  console.log('Doing a thing...');
+}
+
+// we are sure that we want to always log `undefined`
+console.log(void alert('Hello, world!'));
+```
+
+## When Not To Use It
+
+The return type of a function can be inspected by going to its definition or hovering over it in an IDE.
+If you don't care about being explicit about the void type in actual code then don't use this rule.
+Also, if you prefer concise coding style then also don't use it.
+
+## Related to
+
+- TSLint: ['no-void-expression'](https://palantir.github.io/tslint/rules/no-void-expression/)
diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts
@@ -47,6 +47,7 @@ export = {
     '@typescript-eslint/no-array-constructor': 'error',
     '@typescript-eslint/no-base-to-string': 'error',
     '@typescript-eslint/no-confusing-non-null-assertion': 'error',
+    '@typescript-eslint/no-confusing-void-expression': 'error',
     'no-dupe-class-members': 'off',
     '@typescript-eslint/no-dupe-class-members': 'error',
     'no-duplicate-imports': 'off',
diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts
@@ -8,14 +8,12 @@ import braceStyle from './brace-style';
 import classLiteralPropertyStyle from './class-literal-property-style';
 import commaDangle from './comma-dangle';
 import commaSpacing from './comma-spacing';
-import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion';
 import consistentIndexedObjectStyle from './consistent-indexed-object-style';
 import consistentTypeAssertions from './consistent-type-assertions';
 import consistentTypeDefinitions from './consistent-type-definitions';
 import consistentTypeImports from './consistent-type-imports';
 import defaultParamLast from './default-param-last';
 import dotNotation from './dot-notation';
-import enumMembersSpacing from './space-infix-ops';
 import explicitFunctionReturnType from './explicit-function-return-type';
 import explicitMemberAccessibility from './explicit-member-accessibility';
 import explicitModuleBoundaryTypes from './explicit-module-boundary-types';
@@ -30,25 +28,27 @@ import methodSignatureStyle from './method-signature-style';
 import namingConvention from './naming-convention';
 import noArrayConstructor from './no-array-constructor';
 import noBaseToString from './no-base-to-string';
+import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion';
+import noConfusingVoidExpression from './no-confusing-void-expression';
 import noDupeClassMembers from './no-dupe-class-members';
+import noDuplicateImports from './no-duplicate-imports';
 import noDynamicDelete from './no-dynamic-delete';
 import noEmptyFunction from './no-empty-function';
 import noEmptyInterface from './no-empty-interface';
 import noExplicitAny from './no-explicit-any';
-import noImplicitAnyCatch from './no-implicit-any-catch';
-import noExtraneousClass from './no-extraneous-class';
 import noExtraNonNullAssertion from './no-extra-non-null-assertion';
 import noExtraParens from './no-extra-parens';
 import noExtraSemi from './no-extra-semi';
+import noExtraneousClass from './no-extraneous-class';
 import noFloatingPromises from './no-floating-promises';
 import noForInArray from './no-for-in-array';
-import preferLiteralEnumMember from './prefer-literal-enum-member';
+import noImplicitAnyCatch from './no-implicit-any-catch';
 import noImpliedEval from './no-implied-eval';
 import noInferrableTypes from './no-inferrable-types';
 import noInvalidThis from './no-invalid-this';
 import noInvalidVoidType from './no-invalid-void-type';
-import noLossOfPrecision from './no-loss-of-precision';
 import noLoopFunc from './no-loop-func';
+import noLossOfPrecision from './no-loss-of-precision';
 import noMagicNumbers from './no-magic-numbers';
 import noMisusedNew from './no-misused-new';
 import noMisusedPromises from './no-misused-promises';
@@ -83,6 +83,7 @@ import preferEnumInitializers from './prefer-enum-initializers';
 import preferForOf from './prefer-for-of';
 import preferFunctionType from './prefer-function-type';
 import preferIncludes from './prefer-includes';
+import preferLiteralEnumMember from './prefer-literal-enum-member';
 import preferNamespaceKeyword from './prefer-namespace-keyword';
 import preferNullishCoalescing from './prefer-nullish-coalescing';
 import preferOptionalChain from './prefer-optional-chain';
@@ -101,14 +102,14 @@ import restrictTemplateExpressions from './restrict-template-expressions';
 import returnAwait from './return-await';
 import semi from './semi';
 import spaceBeforeFunctionParen from './space-before-function-paren';
+import spaceInfixOps from './space-infix-ops';
 import strictBooleanExpressions from './strict-boolean-expressions';
 import switchExhaustivenessCheck from './switch-exhaustiveness-check';
 import tripleSlashReference from './triple-slash-reference';
 import typeAnnotationSpacing from './type-annotation-spacing';
 import typedef from './typedef';
 import unboundMethod from './unbound-method';
 import unifiedSignatures from './unified-signatures';
-import noDuplicateImports from './no-duplicate-imports';
 
 export default {
   'adjacent-overload-signatures': adjacentOverloadSignatures,
@@ -127,11 +128,11 @@ export default {
   'consistent-type-imports': consistentTypeImports,
   'default-param-last': defaultParamLast,
   'dot-notation': dotNotation,
-  'space-infix-ops': enumMembersSpacing,
   'explicit-function-return-type': explicitFunctionReturnType,
   'explicit-member-accessibility': explicitMemberAccessibility,
   'explicit-module-boundary-types': explicitModuleBoundaryTypes,
   'func-call-spacing': funcCallSpacing,
+  indent: indent,
   'init-declarations': initDeclarations,
   'keyword-spacing': keywordSpacing,
   'lines-between-class-members': linesBetweenClassMembers,
@@ -142,7 +143,9 @@ export default {
   'no-array-constructor': noArrayConstructor,
   'no-base-to-string': noBaseToString,
   'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual,
+  'no-confusing-void-expression': noConfusingVoidExpression,
   'no-dupe-class-members': noDupeClassMembers,
+  'no-duplicate-imports': noDuplicateImports,
   'no-dynamic-delete': noDynamicDelete,
   'no-empty-function': noEmptyFunction,
   'no-empty-interface': noEmptyInterface,
@@ -184,8 +187,8 @@ export default {
   'no-unsafe-member-access': noUnsafeMemberAccess,
   'no-unsafe-return': noUnsafeReturn,
   'no-unused-expressions': noUnusedExpressions,
-  'no-unused-vars-experimental': noUnusedVarsExperimental,
   'no-unused-vars': noUnusedVars,
+  'no-unused-vars-experimental': noUnusedVarsExperimental,
   'no-use-before-define': noUseBeforeDefine,
   'no-useless-constructor': noUselessConstructor,
   'no-var-requires': noVarRequires,
@@ -198,28 +201,27 @@ export default {
   'prefer-namespace-keyword': preferNamespaceKeyword,
   'prefer-nullish-coalescing': preferNullishCoalescing,
   'prefer-optional-chain': preferOptionalChain,
-  'prefer-readonly-parameter-types': preferReadonlyParameterTypes,
   'prefer-readonly': preferReadonly,
+  'prefer-readonly-parameter-types': preferReadonlyParameterTypes,
   'prefer-reduce-type-parameter': preferReduceTypeParameter,
   'prefer-regexp-exec': preferRegexpExec,
   'prefer-string-starts-ends-with': preferStringStartsEndsWith,
   'prefer-ts-expect-error': preferTsExpectError,
   'promise-function-async': promiseFunctionAsync,
+  quotes: quotes,
   'require-array-sort-compare': requireArraySortCompare,
   'require-await': requireAwait,
   'restrict-plus-operands': restrictPlusOperands,
   'restrict-template-expressions': restrictTemplateExpressions,
   'return-await': returnAwait,
+  semi: semi,
   'space-before-function-paren': spaceBeforeFunctionParen,
+  'space-infix-ops': spaceInfixOps,
   'strict-boolean-expressions': strictBooleanExpressions,
   'switch-exhaustiveness-check': switchExhaustivenessCheck,
   'triple-slash-reference': tripleSlashReference,
   'type-annotation-spacing': typeAnnotationSpacing,
+  typedef: typedef,
   'unbound-method': unboundMethod,
   'unified-signatures': unifiedSignatures,
-  'no-duplicate-imports': noDuplicateImports,
-  indent: indent,
-  quotes: quotes,
-  semi: semi,
-  typedef: typedef,
 };
diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts
diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts
