feat(parser): handle optional chaining in scope analysis #1169
Conversation
|
Thanks for the PR, @Nizarius! 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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
Codecov Report
@@ Coverage Diff @@
## master #1169 +/- ##
==========================================
+ Coverage 93.96% 93.96% +<.01%
==========================================
Files 120 120
Lines 5201 5207 +6
Branches 1442 1443 +1
==========================================
+ Hits 4887 4893 +6
Misses 179 179
Partials 135 135
|
|
Guys, can you please give me any update on this PR? It looks like crucial bug but in 4 days no comments (or merging) were provided. |
Myself and the other maintainers volunteer our time to this project. We do not always have time every week to review PRs. We'll get to it as soon as we can. |
There was a problem hiding this comment.
mostly LGTM.
A few comments.
Also (similar to #1175) could you please add more test cases across every one of these rules?
Fixture examples for cases to ensure are handled correctly:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call-with-parens.src.ts
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call.src.ts
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-element-access-with-parens.src.ts
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-element-access.src.ts
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-with-parens.src.ts
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/shared-fixtures/fixtures/typescript/basics/optional-chain.src.ts
|
@bradzacher I've added optionalCallExpression to Referencer and added a lot of new tests to these rules |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@deser and @xwchris - please don't comment with comments like that. Let me be clear: it doesn't bump the priority, it just spams people. As I mentioned before, this project is maintained by volunteers. We will get to a review when we're able. |
|
Using the just-published 2.7.0 (which was released after this patch) I'm still seeing "no-restricted-globals" and "@typescript-eslint/no-use-before-define" errors when using Edit: looks like the code added in this PR is present in the build I'm using. I'm going to investigate further. Edit2: My issue was due to a |
Fixes #1090
Fixes #1116
Fixes #1104