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: fix PositionBuilder if modifier is followed by generic with out whitespace e.g public static<T> #3552

Merged
merged 2 commits into from
Aug 31, 2020

Conversation

Strum355
Copy link
Contributor

@Strum355 Strum355 commented Aug 27, 2020

The following line would result in o2 being of a value that would cause modifierName to include the generic type param, causing the looking into explicitModifiersByName to return null. https://github.com/google/gson/blob/master/gson/src/test/java/com/google/gson/ParameterizedTypeFixtures.java#L129

If a space would be between static and <T>, the modifier position is extracted properly.

@Strum355 Strum355 marked this pull request as draft August 27, 2020 11:30
@Strum355 Strum355 force-pushed the generic-modifier-pos branch 2 times, most recently from 097c68f to e9aab13 Compare August 27, 2020 20:21
@Strum355 Strum355 marked this pull request as ready for review August 27, 2020 20:23
@monperrus
Copy link
Collaborator

Thanks a lot for the bug fix.

What is the cause of the assertion changes? What if you add new method n at the end of the test file instead?

@Strum355
Copy link
Contributor Author

Ah those were because of the added import. Its no longer references in the test class so I will remove it and revert the previous assertions

@monperrus
Copy link
Collaborator

monperrus commented Aug 28, 2020 via email

@Strum355
Copy link
Contributor Author

Done that 👍 only getEndLine() and getSourceEnd() assertions had to be changed :)

@monperrus
Copy link
Collaborator

Thanks for the new commits.

getEndLine() and getSourceEnd() assertions had to be changed :)

because of a change of behavior or a change of test data? in the latter case, we would try to avoid this.

Thanks!

@Strum355
Copy link
Contributor Author

The latter case because a new method was added that caused the issue that this PR addresses. As a new method was added to the test input class, the source length (in chars and lines) was increased. The change in assertion numbers is simply to reflect this addition

@monperrus monperrus changed the title Fixed PositionBuilder failing if modifier is followed by generic with out whitespace e.g public static<T> fix: fix PositionBuilder if modifier is followed by generic with out whitespace e.g public static<T> Aug 31, 2020
@monperrus monperrus merged commit 1f0c035 into INRIA:master Aug 31, 2020
@monperrus
Copy link
Collaborator

As a new method was added to the test input class, the source length (in chars and lines) was increased.

FYI, we avoid spurious assertion changes, so in this case, one option is to put the code in a new test input class.

@monperrus
Copy link
Collaborator

Thanks a lot for your contribution @Strum355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants