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

fix(parser): fix crash when visiting decorators in parameters #237

Merged
merged 5 commits into from
Feb 12, 2019
Merged

fix(parser): fix crash when visiting decorators in parameters #237

merged 5 commits into from
Feb 12, 2019

Conversation

armano2
Copy link
Member

@armano2 armano2 commented Feb 10, 2019

This PR is fixes crashes in no-unused-vars for decorators in non identifier function parameters, and fixes issue with no-shadow for this in functions

#122 (comment)

i think we should merge this after #234 (snapshots for scope analysis)

fixes: #207

@armano2 armano2 self-assigned this Feb 10, 2019
@armano2 armano2 added the bug Something isn't working label Feb 10, 2019
@armano2 armano2 changed the title fix(parser): fix visiting decorators in parameters fix(parser): fix crash during visiting decorators in parameters Feb 10, 2019
@codecov
Copy link

codecov bot commented Feb 10, 2019

Codecov Report

Merging #237 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #237      +/-   ##
=========================================
+ Coverage   94.53%   94.6%   +0.07%     
=========================================
  Files          63      63              
  Lines        2761    2762       +1     
  Branches      718     719       +1     
=========================================
+ Hits         2610    2613       +3     
  Misses         57      57              
+ Partials       94      92       -2
Impacted Files Coverage Δ
packages/parser/src/visitor-keys.ts 100% <ø> (ø) ⬆️
packages/eslint-plugin/src/rules/no-unused-vars.ts 92.85% <ø> (+4.97%) ⬆️
packages/parser/src/analyze-scope.ts 95.76% <100%> (+0.09%) ⬆️

@armano2 armano2 changed the title fix(parser): fix crash during visiting decorators in parameters fix(parser): fix crash when visiting decorators in parameters Feb 10, 2019
@armano2 armano2 removed the request for review from JamesHenry February 11, 2019 02:48
@armano2 armano2 requested review from a team and removed request for bradzacher February 11, 2019 18:44
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.

don't know a heap about the scope manager yet, but the CI passes and I don't see anything stupid in the code, so I'm happy with it 😄

@armano2
Copy link
Member Author

armano2 commented Feb 12, 2019

Crash is related to eslint not beeing able to set parent to decorators in parameters.
we was creating references while visiting it with pattern-visitor, but nodes was not traversable.

eslint traverse nodes base on structure defined in visitor-keys, and setts up parent for nodes.

no-unused-vars is relying on parent of nodes set properly

2nd issue was that TSParameterProperty was not visitable and rule itself was reporting unused variables bacause we was not making references for decorators

@armano2 armano2 merged commit 225fc26 into typescript-eslint:master Feb 12, 2019
@armano2 armano2 deleted the no-unused-vars branch February 12, 2019 01:29
kaicataldo pushed a commit to kaicataldo/typescript-eslint that referenced this pull request Aug 27, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[no-shadow] error on this used as parameter function
3 participants