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

feat(parser): handle optional chaining in scope analysis #1169

Merged
merged 6 commits into from Nov 11, 2019

Conversation

Nizarius
Copy link
Contributor

@Nizarius Nizarius commented Nov 4, 2019

Fixes #1090
Fixes #1116
Fixes #1104

@typescript-eslint
Copy link
Contributor

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.

@bradzacher bradzacher changed the title Fixed optional chaining parsing #1090 #1116 #1104 feat(parser): handle optional chaining in scope analysis Nov 4, 2019
@bradzacher bradzacher added the enhancement New feature or request label Nov 4, 2019
@codecov
Copy link

codecov bot commented Nov 4, 2019

Codecov Report

Merging #1169 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            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
Impacted Files Coverage Δ
packages/parser/src/analyze-scope.ts 95.5% <100%> (+0.1%) ⬆️

@Nizarius
Copy link
Contributor Author

Nizarius commented Nov 8, 2019

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.

@bradzacher
Copy link
Member

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.

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.

@Nizarius
Copy link
Contributor Author

Nizarius commented Nov 9, 2019

@bradzacher I've added optionalCallExpression to Referencer and added a lot of new tests to these rules

@deser

This comment has been minimized.

@xwchris

This comment has been minimized.

@bradzacher
Copy link
Member

@deser and @xwchris - please don't comment with comments like that.
It serves no purpose, and just creates spam notifications for everyone (i.e. for myself and the other maintainers, or anyone watching this repo).

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.

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thanks for doing this.

@bradzacher bradzacher merged commit 026ceb9 into typescript-eslint:master Nov 11, 2019
@davidje13
Copy link

davidje13 commented Nov 11, 2019

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 ?. syntax. (#1090 #1104). Is it possible this patch somehow got missed from the released build? I haven't done much investigating yet.

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 .eslintcache file (--cache in the eslint command). Removing that makes it work perfectly when running 2.7.0 :)

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Nov 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
5 participants