-
-
Notifications
You must be signed in to change notification settings - Fork 353
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 crash on handling parameterized ProblemReferenceBinding #3964
Merged
monperrus
merged 9 commits into
INRIA:master
from
slarse:issue/3951-not-allowed-javaletter
Jun 2, 2021
Merged
fix: Fix crash on handling parameterized ProblemReferenceBinding #3964
monperrus
merged 9 commits into
INRIA:master
from
slarse:issue/3951-not-allowed-javaletter
Jun 2, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
slarse
changed the title
wip: fix: Fix incorrect parsing parameterized ProblemReferenceBinding
wip: fix: Fix incorrect parsing parameterized ProblemTypeBinding
May 28, 2021
slarse
changed the title
wip: fix: Fix incorrect parsing parameterized ProblemTypeBinding
wip: fix: Fix incorrect parsing parameterized ProblemReferenceBinding
May 28, 2021
Waiting for #3966 |
This turns out to be a massive pain. Resolving anything beyond the simple name of the type is really error-prone, as JDT provides next to no information. |
slarse
changed the title
wip: fix: Fix incorrect parsing parameterized ProblemReferenceBinding
wip: fix: Fix crash on handling parameterized ProblemReferenceBinding
Jun 1, 2021
slarse
changed the title
wip: fix: Fix crash on handling parameterized ProblemReferenceBinding
review: fix: Fix crash on handling parameterized ProblemReferenceBinding
Jun 1, 2021
monperrus
changed the title
review: fix: Fix crash on handling parameterized ProblemReferenceBinding
fix: Fix crash on handling parameterized ProblemReferenceBinding
Jun 2, 2021
Thanks @slarse I feel your pain, having myself spent a lot of time workarounding |
Merged
woutersmeenk
pushed a commit
to woutersmeenk/spoon
that referenced
this pull request
Aug 29, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #3951
The problem is described in the issue. It's very rare for JDT to parse a parameterized type reference to a ProblemReferenceBinding (even when the actual referenced type can't be resolved). In the particular case of error code 15 (using a type that isn't closable on a try-with-resources), it does however resolve the reference to a ProblemReferenceBinding with type parameters in the name, which caused the crash.
This PR does not attempt to parse the type arguments, it only fixes the crash and lets the pre-existing logic for creating type references from problem bindings do its thing. In other words, a type reference is created with the correct simple name, and that's about it. A warning that the type arguments are ignored is also issued.
I found a way to actually resolve the correct type binding in the particular corner case represented in the test case I added, which I may add in a later PR as that coincidentally triggers the long-standing bug reported in #3708.