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: print square brackets for manually created array references #4888

Merged

Conversation

algomaster99
Copy link
Contributor

Fixes #4887

The changes modify the constructor of CtArrayTypeReferenceImpl to set the declaration style as TYPE by default.

@algomaster99
Copy link
Contributor Author

@I-Al-Istannen can you please review this?



// act
((CtArrayTypeReferenceImpl<?>) arrayTypeReference).setDeclarationKind(CtArrayTypeReferenceImpl.DeclarationKind.IDENTIFIER);
Copy link
Contributor Author

@algomaster99 algomaster99 Sep 6, 2022

Choose a reason for hiding this comment

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

These explicit casts look ugly. I can move the APIs used in the CtArrayTypeReference interface. I do not know why I did not implement it that way in #4341 . If the reviewers think this is a good idea, I can submit a separate PR for this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think "hiding" that functionality makes sense - we probably shouldn't encourage people to make use of it. Otherwise you'd also need to add support for it on return types, i.e. public int method()[] { return new int[10]; } 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

Additionally, I am not sure how much value this offers as attaching the brackets to the name (or after the method...) is not a really common style from what I have gathered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

public int method()[] { return new int[10]; }

Wow. I did not know that is a valid syntax. Maybe we can add support for it if someone ever reports a bug. However, it is so unlikely people shall use this syntax which is good :)

Additionally, I am not sure how much value this offers as attaching the brackets to the name (or after the method...) is not a really common style from what I have gathered.

Right. It does not seem useful from a transformation perspective so we can keep it as is.

@algomaster99 algomaster99 requested review from I-Al-Istannen and SirYwell and removed request for I-Al-Istannen and SirYwell September 6, 2022 21:48
@I-Al-Istannen I-Al-Istannen changed the title fix: attach square brackets to type of array type reference by default fix: print square brackets for manually created array references Sep 8, 2022
@I-Al-Istannen I-Al-Istannen merged commit ff2e047 into INRIA:master Sep 8, 2022
@I-Al-Istannen
Copy link
Collaborator

Thanks :)

@algomaster99 algomaster99 deleted the attach-bracket-in-factory-method branch September 8, 2022 20:02
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.

[Bug]: Pretty printing issue for array type variable
3 participants