Skip to content
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

Bug: [no-confusing-void-expression] ignoreVoidReturningFunctions ignores generic type returning functions #10289

Open
4 tasks done
phaux opened this issue Nov 5, 2024 · 4 comments
Labels
awaiting response Issues waiting for a reply from the OP or another party bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@phaux
Copy link
Contributor

phaux commented Nov 5, 2024

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

https://deploy-preview-10067--typescript-eslint.netlify.app/play/#ts=5.5.2&fileType=.ts&code=GYVwdgxgLglg9mABAgogJzXNAeAKgPgAoIBDAGzICMSIBrALkUIEpEBefRXZxgNzhgATRAG8AUIkSkK1OiwDcYgL5ixqDFkIt2nCAgDOcMgFMAdGTgBzQgHI4cAA76bzZorF6w%2BqIgftEAAqYALYw%2BmZoxoZkvMaEAIzMplAAFsZgWqwcUgZGZhbWrmJAA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1rI6YDNYyZgHNaANw6UAJvQAexaCiG90YANrhsORNGgdokADSatWLdkiVhnRQDVJUgEqJ8saExEAxWEzL5KvKgY%2BHCIJtgAvuEAuppREUA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

declare function onError<T>(callback: () => T): void;

onError(() => console.log('oops'));

Promise.resolve(1).then(() => console.log("foo"))

ESLint Config

{
  "rules": {
    "@typescript-eslint/no-confusing-void-expression": [
      "error",
      {
        "ignoreVoidReturningFunctions": true
      }
    ]
  }
}

tsconfig

No response

Expected Result

Should report both console.logs

The correct code should be

onError((): void => console.log('oops'));

Promise.resolve(1).then((): void => console.log("foo"))

or

onError<void>(() => console.log('oops'));

Promise.resolve(1).then<void>(() => console.log("foo"))

Actual Result

Doesn't report.

Additional Info

Continued from #8538

@phaux phaux added bug Something isn't working 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

🤔 this strikes me as "working as intended". Per the ignoreVoidReturningFunctions option description (emphasis mine):

Whether to ignore returns from functions with explicit void return types and functions with contextual void return types.

The void return types in this issue are contextually inferred. So this is what the option is asking for, no?

@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 Nov 6, 2024
@phaux
Copy link
Contributor Author

phaux commented Nov 6, 2024

@JoshuaKGoldberg

returns from functions with explicit void return types and functions with contextual void return types

So basically all returns?

Then it should also ignore

function foo() {
  return console.log("foo")
}

because void return is inferred here too.

And if that's the expected behavior then this option should just be called ignoreReturn, because it ignores basically all returns.

@bradzacher
Copy link
Member

There's a complicating factor with your examples in the OP - they use type parameters.
Type parameters are both inferred from the function signature AND they form the contextual type of the function.

This means that when we inspect the context of the function we see that it is expecting a void return type and thus we ignore the cases.

It's a tricky detail of type parameters that is very difficult for us to really separate and analyse for. TS doesn't give us information to determine how the type parameters are filled in - it just tells us their type. So we haven't got much choice other than to just treat them as given.

OTOH a function like this has no contextual type and has no explicit return type

function foo() { return console.log("foo") }

And so it is reported as it's assumed it was an accident. If you explicitly annotate the void return type then we ignore the case.

@phaux
Copy link
Contributor Author

phaux commented Nov 6, 2024

I get it now. The docs are indeed correct.

It's just the fact that one is ignored and other isn't seems very arbitrary.

I would imagine this option to do one of these:

  1. ignore return when containing function has a void type annotation (simple ast check)
  2. ignore return when an explicit void can be found anywhere in the types.
  3. ignore return always.

Right now it does 2 and sometimes 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Issues waiting for a reply from the OP or another party bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

3 participants