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.

Sign up for GitHub

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

Jump to bottom

Add getKeyword() to J.Binary.Type, J.Unary.Type, and J.AssignmentOperation.Type #2867

Closed
wants to merge 1 commit into from
Closed

Add getKeyword() to J.Binary.Type, J.Unary.Type, and J.AssignmentOperation.Type #2867

wants to merge 1 commit into from

Conversation

knutwannheden
Copy link
Contributor

Similar to #2630 but without adding any Java fields which would increase the size of the LST.

@knutwannheden knutwannheden self-assigned this Feb 20, 2023
@knutwannheden knutwannheden added the enhancement New feature or request label Feb 20, 2023
@jkschneider
Copy link
Member

Why? We are very guarded about adding API surface area to any LST elements.

@knutwannheden
Add getKeyword() to J.Binary, J.Unary, and J.AssignmentOperation
fd37bfc 
Similar to #2630 but without adding any Java fields which would increase the size of the LST.
@knutwannheden knutwannheden changed the title Add getKeyword() to J.Binary.Type and J.Unary.Type Add getKeyword() to J.Binary.Type, J.Unary.Type, and J.AssignmentOperation.Type Feb 20, 2023
@knutwannheden
Copy link
Contributor Author

Why? We are very guarded about adding API surface area to any LST elements.

The only reason would be to get rid of some duplication. For the J.Binary.Operation case I have another place, where I would use it. But I understand your reasoning and have no problem with closing the PR again.

@knutwannheden
Copy link
Contributor Author

The new use case I mention is in my other open PR for pattern variable support in JavaTemplate: https://github.com/openrewrite/rewrite/pull/2863/files#diff-1642ff64ca8163e1eb1e9f8e8a296966bff9ce05b23eb91dc14466f76231f561R611-R621

@sambsnyd
Copy link
Member

Hmm, I like this change. The fact that there were a few nigh-identical switch statements kicking around, with another to be added in the other PR @knutwannheden references, suggests to me that this is useful.

@sambsnyd sambsnyd reopened this Feb 21, 2023
@jkschneider
Copy link
Member

@sambsnyd I asked to close this because getters that return syntax are bound to not work well across languages in the same grammar island. e.g. or vs. ||.

@knutwannheden knutwannheden deleted the operator-keyword branch February 26, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@knutwannheden knutwannheden

Labels
enhancement New feature or request
Projects
OpenRewrite
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@knutwannheden @jkschneider @sambsnyd