Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

sadikovi
Copy link
Contributor

@sadikovi sadikovi commented May 30, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This PR fixes the scope of classes with generic type being an array, e.g. Foo<int[]>. It scopes properly with the actual assignment, but variable declaration was broken (IMHO, we just forgot to add it).

Fix is simply adding missing [ and ] to the pattern. It looked like when I added a test case I was using the wrong scopes, so I changed them to actual correct ones.

Before:
before

After:
after

Alternate Designs

None were considered, due to relatively simple change.

Benefits

Fixes a bug with scoping generics with arrays.

Possible Drawbacks

None were found.

Applicable Issues

Closes #139

@sadikovi sadikovi changed the title fix class fields with arrays as generic type Fix class fields with arrays as generic type May 30, 2018
@sadikovi sadikovi requested a review from winstliu May 30, 2018 06:51
@sadikovi
Copy link
Contributor Author

@50Wliu could you have a look at this PR? Thanks!

Copy link
Contributor

@winstliu winstliu left a comment

Choose a reason for hiding this comment

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

Does this need to be changed anywhere else (for example, lookaheads)?

@sadikovi
Copy link
Contributor Author

I also tested the fix manually on different examples of initialising variables, so far this change has been enough to provide correct scope and highlighting.

@sadikovi
Copy link
Contributor Author

sadikovi commented Jun 1, 2018

I do not think we need to change in other places, since manual tests worked and unit tests pass. I suggest we address any other issues in follow-up PRs.

@winstliu
Copy link
Contributor

winstliu commented Jun 1, 2018

Sounds good.

@sadikovi sadikovi merged commit f4fc17b into atom:master Jun 2, 2018
@sadikovi sadikovi deleted the fix-class-fields-with-array branch June 2, 2018 07:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing scopes on new and punctuation

2 participants