Enhancement: [no-confusing-void-expression] Add option to ignore void<->void
Before You File a Proposal Please Confirm You Have Done The Following...
- [X] I have searched for related issues and found none that match my proposal.
- [X] I have searched the current rule list and found no rules that match my proposal.
- [X] I have read the FAQ and my problem is not listed.
My proposal is suitable for this project
- [X] I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).
Link to the rule's documentation
https://typescript-eslint.io/rules/no-confusing-void-expression
Description
Following from https://github.com/typescript-eslint/typescript-eslint/pull/8375#pullrequestreview-1897027584: we tried enabling @typescript-eslint/no-confusing-void-expression internally and very quickly found it to be inconvenient:
Looking at the changes - TBH I don't really like this rule. I don't think any of these changes improve clarity or readability of code.
The biggest issue I see is that if the function is implicitly typed or explicitly marked as returning
voidthe rule still requires you to change things.For example this code should pass the rule, IMO:
function returnsVoid(): void {} function test1(): void { return returnsVoid(); // should be fine } const test2 = (): void => returnsVoid(); // should be fineBecause there's nothing confusing about that code - the expressions are all typed as
voidand the functions are typed asvoid.Also changes like
() => expr=>() => { expr }actively reduce readability, IMO, esp when considering the above.
Agreed. @bradzacher and I chatted briefly, and Brad suggested:
- Adding an option to ignore the
void<->voidreturn case - Making that option the default in v8
Agreed! Filing this issue to add that option. We can file a followup if & when it gets in.
Fail
declare function returnsVoid(): void;
function outerReturnsVoid(): void {
return returnsVoid();
}
Pass
declare function returnsString(): string;
function outerReturnsString(): string {
return returnsString();
}
Additional Info
No response
@JoshuaKGoldberg @bradzacher I'm currently working on resolving the issue, but I have one question.
The issue was about implementing an option to ignore the rule when the Void type is specified, but it also be ignore rule when assigning a function type like () => void?
For example, should the following cases be ignore rule? (cc. @auvred @yeonjuan )
type Foo = () => void;
const test: Foo = () => console.log('foo')
declare function foo(arg: () => void): void;
foo(() => console.log());
type HigherOrderType = () => () => () => void;
const x: HigherOrderType = () => () => () => console.log();
interface Foo {
foo: () => void
}
function bar(): Foo {
return {
foo: () => console.log(),
};
}
const q = {
foo: { bar: () => console.log() },
} as {
foo: { bar: () => void }
}
Hmm, yeah, I'd think that if there's an explicit void return type then it's ok. So all those cases would be fine.
We should wait to hear from @bradzacher too, if Brad has time.
I'd agree - if the contextual type includes a void return type then we should ideally treat it the same as an explicit void return type annotation (eg ignore it).
Thank you. When applying the new option, I will work to ignore the rule in the case mentioned in the comment above!
Is this option doc good?
### `ignoreReturnInVoidFunction`
Explicit void return type in a function signature might be enough to indicate
that the function will not return anything,
even though the function body might contain a return with an expression.
In such case, it's easy to deduce that the returned value must be of type void too.
This option allows returning void expressions from functions
if the function itself is annotated with a void return type.
This option also changes the automatic fix for void expressions returned from void functions
to add an explicit void return type annotation to the function signature.
Examples of additional **correct** code with this option enabled:
```ts option='{ "ignoreReturnInVoidFunction": true }' showPlaygroundButton
// now it's obvious that we don't expect any response
const promise = fetch('...')
.then(resp => resp.json())
.then((data): void => window.postMessage({ data }));
// now it's explicit that we don't want to return anything
function doSomething(): void {
if (Math.random() < 0.1) {
return console.error('Nothing to do!');
}
console.log('Doing a thing...');
}
```
If so, I can send PR later.
For the option name: since we'd like to eventually have it enabled by default, how about checkReturnInVoidFunction? I'm a fan of having optional things default to falsy, not default to truthy.
Looks like the option implemented in #10067 ignores too much. Examples:
declare function onError<T>(callback: () => T): void;
onError(() => console.log('oops'));
const p = Promise.resolve(1).then(() => console.log())
I was expecting that it would require adding (): void => to the arrow functions. It also doesn't mention that a possible fix is to add void type annotation in functions that it does report, and it still autofixes them to add braces.
Maybe leave this open so we can have both: ignore all returns or ignore returns in void functions with literal void type annotation (as described in a comment above).
If not, maybe rename this option to simply ignoreReturn (all) and simplify it dramatically? That would make sense if that rule wants to be in strict config since all void returns are correct and just a stylistic preference. Also option name should be in singular form because all others already are.
@phaux Could you kindly open a new issue with playground links for each of the concerns?