feat(eslint-plugin): add no-meaningless-void-operator rule

[jtbandes]

Report an error on void x when x is of type void, undefined, or never (and automatically delete the void operator).

This rule was first implemented (by me) in https://github.com/foxglove/eslint-plugin. As it could be generally useful I'm proposing to add it to @typescript-eslint/eslint-plugin.

[typescript-eslint]

Thanks for the PR, @jtbandes!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[codecov]

Codecov Report

Merging #3641 (ed70f1c) into master (26de645) will increase coverage by 0.90%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3641      +/-   ##
==========================================
+ Coverage   92.67%   93.57%   +0.90%     
==========================================
  Files         327      148     -179     
  Lines       11357     7890    -3467     
  Branches     3204     2496     -708     
==========================================
- Hits        10525     7383    -3142     
+ Misses        370      162     -208     
+ Partials      462      345     -117     

Flag	Coverage Δ
unittest	93.57% <100.00%> (+0.90%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/eslint-plugin/src/configs/all.ts	100.00% <ø> (ø)
...t-plugin/src/rules/no-meaningless-void-operator.ts	100.00% <100.00%> (ø)
packages/scope-manager/src/lib/es2017.intl.ts
packages/scope-manager/src/lib/es2018.promise.ts
packages/scope-manager/src/lib/esnext.intl.ts
packages/scope-manager/src/scope/TypeScope.ts
packages/scope-manager/src/lib/es2015.reflect.ts
...ges/experimental-utils/src/ts-eslint/RuleTester.ts
...ackages/scope-manager/src/scope/MappedTypeScope.ts
...e-manager/src/scope/FunctionExpressionNameScope.ts
... and 173 more

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit ed70f1c. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx test @typescript-eslint/eslint-plugin-internal
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx run-many --target=typecheck --all --parallel
	nx test @typescript-eslint/parser
	nx test @typescript-eslint/experimental-utils
	nx test @typescript-eslint/scope-manager
	nx test @typescript-eslint/visitor-keys
	nx test @typescript-eslint/typescript-estree
	nx run-many --target=build --all --parallel
	nx test @typescript-eslint/parser
	nx test @typescript-eslint/experimental-utils
	nx test @typescript-eslint/scope-manager
	nx test @typescript-eslint/visitor-keys
	nx test @typescript-eslint/typescript-estree
	nx run-many --target=build --all --parallel

---

Sent with 💌 from NxCloud.

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit c5e9937. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx run-many --target=build --all --parallel
	nx run-many --target=typecheck --all --parallel

---

Sent with 💌 from NxCloud.

[bradzacher]

The rule makes sense to me, and the code LGTM.

One change required: It's not correct or safe to autofix void (never) to (never).
Because typeof void (never) === undefined, but typeof never === never.

This could cause TS build errors - which breaks the cardinal rule of ESLint fixers (don't break the build).

Example - imagine that the client code and server response are out-of-sync due to the client not having refreshed their browser:

function foo(arg: 'a' | 'b') {
  switch (arg) {
    case 'a':
      return 'ayyy';

    case 'b':
      return 'beee';

    default:
      return void arg;
  }
}

function getPayloadFromServer(): 'a' | 'b' {
  return 'c'; // oops - the client code is behind the server code
}
foo(getPayloadFromServer()); // returns `undefined`

If you remove the void here it'll instead return 'c' - which is a change in runtime behaviour.

Obviously the code above is pretty dumb and likely wouldn't exist in production, but it's valid code that we have to be weary of.

---

I'm happy for the never check to be behind an option, however the never fix would have to be a suggestion fixer instead of an autofixer (as it's potentially unsafe)

[jtbandes]

Added a checkNever option which is false by default.

[jtbandes]

Hi @bradzacher – anything else you need from me here? Thanks for your time 🙂

[bradzacher]

thanks for this!