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-condition false negative inside string and number conditionals #7926

Closed
4 tasks done
guilhermesimoes opened this issue Nov 14, 2023 · 1 comment
Closed
4 tasks done
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on

Comments

@guilhermesimoes
Copy link

guilhermesimoes commented Nov 14, 2023

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.2.2&fileType=.tsx&code=CYUwxgNghgTiAEEQBd4CMD2GIC51aSgDsBuAWACgBLAM3gApNsBKeAb0vnloaYlY4UuXMBiIBnbCAB0EDAHNGBeAB8V8AOQ0sG5uSHwAvpWMVKoSLARJURAK4BbPPYdoQMfT3ouBnbnW9HAXgAehD4cQALDDsIYHhQZHBUdxgMGHhI9xA-ETFJJFkFQIdVdS0dPVDwqJi4hJRk%2BFT0zOy-U1NzcGg4RBQI5Bg8cSGqInlPANGYXwMvGeCwiOjY%2BMSmloysuFz4UQkpIsUZss1tDF0SapW69cawFJg07faDTqA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0yHJgBNK%2BSpPRRE0aB2iRwYAL4gtQA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

declare let str: string;
if (str) {
  if (str) {
    console.log(str || 'foo');
  }
}

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-unnecessary-condition": "error",
  },
};

tsconfig

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected Result

The error "Unnecessary conditional, value is always truthy" should be reported on lines 3 and 4

Actual Result

No errors are detected

Additional Info

I discovered this because I was trying out eslint-plugin-sonarjs, and that plugin has a similar rule called no-gratuitous-expression that does detect these cases. The code for that rule is available here.

@guilhermesimoes guilhermesimoes 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 14, 2023
@bradzacher
Copy link
Member

This is working as expected.
The no-unnecessary-condition rule is powered solely by types.

Under-the-hood within TS the boolean type is actually defined as a union type (true | false) so it follows logically that TS can refine boolean to true or false based on a conditional check.

However other primitives are not and cannot be defined like that. Which means that the result of refining string via a conditional is always going to be string.
There's talk of "negated types" (eg microsoft/TypeScript#4196) which would allow TS to refine this case to eg string & not '' but currently that feature doesn't exist.
TS also doesn't narrow the false branch to values like '' or 0 | NaN - presumably for simplicity's sake.


The rule is already complicated enough going through all of the logic required to evaluate types - and I don't think there's much value in adding a lot of additional complexity to catch a relatively rare case - especially when there are other rules that already exist and catch the case.

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2023
@bradzacher bradzacher added wontfix This will not be worked on and removed triage Waiting for team members to take a look labels Nov 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants