Skip to content

Enhancement: [no-confusing-void-expression] ignoreVoidReturningFunctions should suggest adding void annotation #10290

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

Closed
4 tasks done
phaux opened this issue Nov 5, 2024 · 2 comments
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@phaux
Copy link
Contributor

phaux commented Nov 5, 2024

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • 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://deploy-preview-10067--typescript-eslint.netlify.app/rules/no-confusing-void-expression#ignorevoidreturningfunctions

Description

When given code:

function foo() {
  if (cond) return console.log()
}

The error says to move the console.log before return and autofix does it automatically, but when ignoreVoidReturningFunctions is enabled a simpler fix would be to simply add the void annotation to the function return type.

I think it should mention this possibility in every error message and autofix should do whatever requires less changes. For example for arrow functions it could preserve the current behavior and fix by adding braces.

Fail

// no changes

Pass

// no changes

Additional Info

Continued from #8538

@phaux phaux added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Nov 5, 2024
@JoshuaKGoldberg
Copy link
Member

Huh, I'm surprised nobody's commented on this till now. Sorry about that!

...though I'm -1 personally on trying to be clever with the fix. In cases like the example code, yes, a : void on the function would make sense. But what if it's a big function with an inferred type that happens to be a union including | undefined? What if it's also a recursive function?

We don't generally try to be smart and choose between better options in fixes, because it often ends up being a difficult trail of "well, in this case, it's 51% better to switch to...".

@phaux I'm tempted to close this out, but want to hear from you first. Am I horribly misinterpreting / missing something?

@JoshuaKGoldberg JoshuaKGoldberg added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Dec 7, 2024
@phaux
Copy link
Contributor Author

phaux commented Dec 7, 2024

@JoshuaKGoldberg I was interested in adding this fix but since #10289 is not going to be changed then I personally will not probably use this option so I guess it can be closed.

@phaux phaux closed this as completed Dec 7, 2024
@kirkwaiblinger kirkwaiblinger closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2024
@kirkwaiblinger kirkwaiblinger removed the awaiting response Issues waiting for a reply from the OP or another party label Dec 8, 2024
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Dec 19, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

3 participants