-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Bug: [no-unnecessary-condition] False positive Checking Lookup Types #6264
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
The lint rule specifically ignores types marked as "type parameters", which It is flagged as an index access type, which is part of the "type variable" flag - so maybe we need to update our code to use that flag instead? Unsure of the ramifications. |
I ran into this issue as well with variants of the following code: function getElem(dict: Record<string, { foo: string }>, key: string) {
if (dict[key]) { // Unnecessary conditional, value is always truthy. 2:9 - 2:18
return dict[key].foo;
} else {
return '';
}
} Essentially the rule is very overconfident when it comes to index signatures, which can never really be guaranteed. It's unfortunate because the only workaround I found was to wrap the type in function getElem(dict: Partial<Record<string, { foo: string }>>, key: string) {
if (dict[key]) {
return dict[key].foo; // Error 2532: Object is possibly 'undefined'. 3:16 - 3:25
} else {
return '';
}
}
function getElem2(dict: Partial<Record<string, { foo: string }>>, key: string) {
const elem = dict[key];
if (elem) {
return elem.foo; // OK
} else {
return '';
}
} As an aside, setting |
@ehoogeveen-medweb Narrowing doesn't work with expression indexers. You can only narrow static variables, which is why your revised version works. For this, see microsoft/TypeScript#10530 and its many duplicates. The rule only uses whatever TS tells it as the expression's type—see #2883. This issue is specifically about generically typed variables. |
@Josh-Cena I know narrowing doesn't work, but the problem is that this rule claims that |
@ehoogeveen-medweb In your example Just in case, I included test cases for your example in the PR I just created. |
@uhyo Right, it's a limitation of TypeScript that For example there's no general way to add the information "the value contained in variable A is a key of type T" to type T (it works for literal types but not for anything wider or generic), so something as simple as Similarly TypeScript does not have a way to dynamically track or represent the minimum size of arrays, so things like You can work around these things to some extent using helper functions, but on some level those functions have to lie about the type and allow unchecked access within a given scope (for example you can make a helper function like I think on a conceptual level index signature properties should be considered as optional properties (at least with For this rule I think your PR is exactly what is needed: We can't know for sure that the condition is unnecessary for an indexed access type, so those shouldn't trigger an error. In general I think it would be great if there was a rule that... checked for unchecked indexed access on objects. But if TypeScript itself struggles to do that then it's probably an unreasonable thing to request out of typescript-eslint :) |
@ehoogeveen-medweb This issue is not about your case. It's about the case where the expression is of a generic type that provably may include |
@Josh-Cena I understand, and I'm glad that this issue has been fixed. I was just trying to explain why enabling I may attempt to create a rule that gives the same protection as |
Before You File a Bug Report Please Confirm You Have Done The Following...
Playground Link
https://typescript-eslint.io/play/#ts=4.9.3&sourceType=module&code=KYDwDg9gTgLgBAMwK4DsDGMCWEVwObAwA8A8gEYBWANHANLACecoMwKAJgM5wDWjECOOQoA+ABQRKALiGUafBjPoMAlHADeAKDhw0OTvABuAQwA2SYHAC8cSRQDaCgLoBubXEyCxJ88DVadHShCJChcHws3HQBfdxgACygIAHc4FGBUgFEoJKgxAHIAIRISAFkAQnyVTVjNAhgxdUQICBkUJFNTOGiaACIEFt6VFyA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0yHJgBNK+SpPRRE0aB2iRwYAL4gtQA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA
Repro Code
ESLint Config
tsconfig
Expected Result
No error.
Actual Result
The rule flags
if (value)
as error because it thinksvalue
is always truthy.Additional Info
The type of value is
Obj[Key]
here. Maybe the rule wrongly thinks that generic lookup types are always truthy.The text was updated successfully, but these errors were encountered: