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-unnecessary-boolean-literal-compare] False positive and unsafe fix with exactOptionalPropertyTypes #9105

Closed
4 tasks done
andersk opened this issue May 16, 2024 · 6 comments
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@andersk
Copy link
Contributor

andersk commented May 16, 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://typescript-eslint.io/play/#ts=5.3.2&fileType=.tsx&code=MYewdgzgLgBCAOUCW4IC4YG8YDMA2AhgOYD8GARiCHgKYFgwC%2BMAZFroaRVbfTAD4wArmAAmNHEjA1RTGAF4sjANwAoUJGo0AdHhBEAFAmSpt%2BYgvmKcBPBBoBKNUA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0ARhw5IhTWo3yJoQ%2BLTIcAtsRGJ0UJdA7RI4MAF8QZoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEk6AHgIbR64HGOoAKmsiWeAJ4AVQfzKU8mAK7oANOBqQSU5CwpgpsxQF8QOoA&tokens=false

Repro Code

const options: { flag?: boolean } & { flag?: boolean | undefined } = {};
console.log(options.flag === false);

ESLint Config

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-boolean-literal-compare": "error"
  }
}

tsconfig

{
  "compilerOptions": {
    "exactOptionalPropertyTypes": true,
    "strict": true
  }
}

Expected Result

No errors. I haven’t changed the default options { allowComparingNullableBooleansToTrue: true, allowComparingNullableBooleansToFalse: true }, so comparing boolean | undefined to false should be allowed.

Actual Result

  2:13  error  This expression unnecessarily compares a boolean value to a boolean instead of using it directly  @typescript-eslint/no-unnecessary-boolean-literal-compare

The rule automatically applies an incorrect fix options.flag === false!options.flag, which changes the output of the code from false to true.

Additional Info

No response

@andersk andersk added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels May 16, 2024
@bradzacher
Copy link
Member

This is actually working as expected.

When you intersect two union types the resulting type is the intersection of the unions - i.e. the types that exist in both unions.
For example - type T = (number | undefined) & number; - T === number.

When you intersect two object types TS will include all the unique properties between the two types, and then it will intersect the common properties. So what you end up with in your example is boolean & (boolean | undefined) - i.e. boolean.

Which is why you can write options.flag.valueOf() without any guards and TS will not error.

When we inspect the types for .flag, TS tells us its type is just boolean. It says there's no possibility of it being undefined. Hence the rule errors.

If you tweak the types slightly you'll see that rules like no-unnecessary-condition will also error here because the types tell us there's no undefined.

IDK if the behaviour is correct from TS's side here - surely it should be erroring on the property / saying the type is undefined at read time?
I don't know for sure - would require an issue to discuss with them.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party external This issue is with another package, not typescript-eslint itself and removed triage Waiting for maintainers to take a look labels May 16, 2024
@andersk
Copy link
Contributor Author

andersk commented May 16, 2024

Both members of the intersection have flag?: …, not flag: …. So the property is allowed to be absent.

Which is why you can write options.flag.valueOf() without any guards and TS will not error.

This does seem like a TypeScript bug.

When we inspect the types for .flag, TS tells us its type is just boolean. It says there's no possibility of it being undefined. Hence the rule errors.

That shows (property) flag?: boolean. The ? indicates that TypeScript knows the property might be absent.

@kirkwaiblinger

This comment was marked as outdated.

@andersk
Copy link
Contributor Author

andersk commented May 16, 2024

It looks like this is the related TypeScript bug:

@kirkwaiblinger
Copy link
Member

Disregard my comment, didn't realize --exactOptionalPropertyTypes is set

@bradzacher
Copy link
Member

That shows (property) flag?: boolean

Yes - that's because TS's "intellisense printer" shows you the type AND the context. So when it shows you the context (the property name) it also needs to show the "divider". When it prints the divider it also inspects the flags for the property to determine if it should also print the ? in addition to the standard :.


microsoft/TypeScript#58174 looks to be the culprit - that's why it works without the intersection but breaks it with it.

So I'll close this as a bug in TS.

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label May 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

3 participants