fix(eslint-plugin): [no-unnecessary-condition] handle left-hand optional with exactOptionalPropertyTypes option #8249
Conversation
|
Thanks for the PR, @yeonjuan! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
f4a4bfc to
00ff844
Compare
fcfa73f to
f2145b1
Compare
| // TODO - figure out how to handle when computed key used. | ||
| /* | ||
| declare const foo: { bar: { baz?: number }; }; | ||
| declare const baz: 'baz'; | ||
| foo.bar[baz] ??= 1; | ||
| */ |
There was a problem hiding this comment.
[Feature] Yeah we'll have to do something here to make this PR mergable.
Have you tried getStaticValue? That's kind of the standard utility we've been going with.
For really complex cases where folks are doing funky stuff like (1 * 1) + "other" then it's fine to not handle them.
There was a problem hiding this comment.
Hi @JoshuaKGoldberg, While looking into handling the computed key case, I found another false positive which occurs regardless of the exactOptionalPropertyTypes.
enum Keys {
A = 'A',
B = 'B',
}
type Foo = {
[Keys.A]: number | null;
[Keys.B]: number;
};
declare const foo: Foo;
declare const key: Keys;
foo[key] ??= 1; // Unnecessary conditional, expected left-hand side of `??` operator to be possibly null or undefined. 11:1 - 11:9
let bar = foo[key];
bar ??= 1; // No ErrorI think this false positive and the #6635 can be handled together, without branching based on exactOptionalPropertyTypes.
I'd like to come back to it after it's been triaged, what do you think about this false positive?
There was a problem hiding this comment.
Yeah that makes sense to me. Nice find!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8249 +/- ##
==========================================
+ Coverage 87.70% 87.71% +0.01%
==========================================
Files 397 397
Lines 13950 13963 +13
Branches 4055 4061 +6
==========================================
+ Hits 12235 12248 +13
Misses 1403 1403
Partials 312 312
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…nal with exactOptionalPropertyTypes option
…nal with exactOptionalPropertyTypes option (typescript-eslint#8249) * fix(eslint-plugin): [no-unnecessary-condition] handle left-hand optional with exactOptionalPropertyTypes option * typo: remove the 's' --------- Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
PR Checklist
??=operator andexactOptionalPropertyTypescompiler option #6635Overview
This PR fixes a false positive issue that occurs when using the exactOptionalPropertyTypes option.
Playground
tsconfig.json
{ "compilerOptions": { "exactOptionalPropertyTypes": true, "strictNullChecks": true } }code.ts
When the exactOptionalPropertyTypes option is true, typescript removes
undefinedtype of the expression on the left-hand side in assignment. This seems to be the way typescript is intended to provide the functionality of exactOptionalPropertyTypes.Playground
In this PR, I added code to get the property's Symbol from the member expression's object type and check if whether it's optional.
This implementation has a limitation in that it cannot handle well for using computed key. If it should be handled, I'll need to modify the code a bit more, and I'd love to hear feedback.