-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Enhancement: [no-confusing-void-expression] Add option to ignore void<->void #8538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@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 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 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 |
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 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 |
@phaux Could you kindly open a new issue with playground links for each of the concerns? |
Before You File a Proposal Please Confirm You Have Done The Following...
My proposal is suitable for this project
Link to the rule's documentation
https://typescript-eslint.io/rules/no-confusing-void-expression
Description
Following from #8375 (review): we tried enabling
@typescript-eslint/no-confusing-void-expression
internally and very quickly found it to be inconvenient:Agreed. @bradzacher and I chatted briefly, and Brad suggested:
void
<->void
return caseAgreed! Filing this issue to add that option. We can file a followup if & when it gets in.
Fail
Pass
Additional Info
No response
The text was updated successfully, but these errors were encountered: