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

Apply code polishing to leverage various Java 17 structures. #3134

Closed
wants to merge 2 commits into from
Closed

Conversation

shin-mallang
Copy link
Contributor

I fixed methods that didn't use instanceof pattern matching to use it.

I didn't apply it to the equal method because it seemed to generate without using instanceof pattern matching.

I also didn't apply it to the following part of HibernateUtils because it was ambiguous to apply, but if you have a good recommendation, I will apply it.

if (query instanceof Query) {
    return ((Query<?>) query).getQueryString();
}

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 31, 2023
@shin-mallang
Copy link
Contributor Author

shin-mallang commented Aug 31, 2023

And I have a question.

#3053 (comment)
Contrary to what you mentioned here, I found that instanceof was not applied correctly.

One example is the applySorting(Sort sort, @Nullable String alias) method that exists inside the JSqlParserQueryEnhancer.

The implementation of that code includes the following code.

if (selectStatement.getSelectBody()instanceof SetOperationList setOperationList) {
	return applySortingToSetOperationList(setOperationList, sort);
} else if (!(selectStatement.getSelectBody() instanceof PlainSelect)) {
	return queryString;
}

If that's not what you intended, may I modify this to make it conform to the convention.

@shin-mallang shin-mallang changed the title Using instancesof pattern matching Using instancesof pattern matching. Aug 31, 2023
gregturn pushed a commit that referenced this pull request Sep 5, 2023
gregturn pushed a commit that referenced this pull request Sep 5, 2023
gregturn added a commit that referenced this pull request Sep 5, 2023
@gregturn gregturn self-assigned this Sep 5, 2023
@gregturn gregturn added type: task A general task in: core Issues in core support and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 5, 2023
@gregturn
Copy link
Contributor

gregturn commented Sep 5, 2023

if (selectStatement.getSelectBody()instanceof SetOperationList setOperationList) {
	return applySortingToSetOperationList(setOperationList, sort);
} else if (!(selectStatement.getSelectBody() instanceof PlainSelect)) {
	return queryString;
}

Looks like the !( prefix somehow breaks IntelliJ and the Adapter for Eclipse Code Formatter of removing the space and allows it to format properly. Anyway, I've applied your code polishings to the code base and applied an additional commit on top of your existing ones along with a little bit of reformatting to the changelogs.

Thank you for your efforts @shin-mallang!

@gregturn gregturn closed this Sep 5, 2023
@gregturn gregturn changed the title Using instancesof pattern matching. Apply code polishing to leverage various Java 17 structures. Sep 5, 2023
gregturn pushed a commit that referenced this pull request Sep 5, 2023
gregturn pushed a commit that referenced this pull request Sep 5, 2023
gregturn added a commit that referenced this pull request Sep 5, 2023
@gregturn gregturn added this to the 3.1.4 (2023.0.4) milestone Sep 5, 2023
@gregturn
Copy link
Contributor

gregturn commented Sep 5, 2023

Merged to main. Backported to 3.1.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants