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

Handle jaif for constructor with empty ASTPath #292

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lnsun
Copy link
Member

@lnsun lnsun commented Jan 19, 2021

Jaif writer skips records with empty ASTPath but constructor records are created with an empty path thus get skipped.
Updating this behaviour will allow injecting infereed annotations to constructors by including these records into generated jaif file.

@lnsun lnsun requested a review from wmdietl January 19, 2021 01:52
@@ -310,7 +310,12 @@ private void buildClassEntries() {
// TODO: this is not a feature but a workaround of a bug:
Copy link
Member

Choose a reason for hiding this comment

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

Note that this is just a hacky work-around.
I think it would be better to fix all the places that incorrectly have an empty ASTPath. Instead of a silent continue here, we should raise an error when we still find an empty ASTPath.
Also note the duplication between the change here and the one above. Do we really need both? If so, maybe the getConstructorRecord method should be called in more places?

As discussed, please first add a test case to make sure we're fixing this error.

@wmdietl wmdietl assigned lnsun and unassigned wmdietl Jan 26, 2021
@wmdietl
Copy link
Member

wmdietl commented Jan 31, 2022

Look at relation to #129

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