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: prevent appending semicolon after enum constant if it is not there initially #4306

Closed
wants to merge 1 commit into from

Conversation

algomaster99
Copy link
Contributor

Printing enum constants with sniper printer enforces ; at the end even though it is not present before. For example,

public enum Days {
    SUNDAY, MONDAY, TUESDAY, WEDNESDAY,
    THURSDAY, FRIDAY, SATURDAY
}

is changed to

public enum Days {
    SUNDAY, MONDAY, TUESDAY, WEDNESDAY,
    THURSDAY, FRIDAY, SATURDAY;
}

enum is compilable with or without the ; if there are no method declarations afterwards. Source: https://docs.oracle.com/javase/tutorial/java/javaOO/enum.html.

when there are fields and methods, the list of enum constants must end with a semicolon.

I think we should configure the sniper printer to check for ; and print it only if required. The current behaviour is not causing a compilation error, but it helps minimise the diff so we should consider it.

@algomaster99
Copy link
Contributor Author

I have figured out where this happens. However, I am not sure how to fix this yet. I know that this fix would go into SniperJavaPrettyPrinter. This is the place where we build the context, but it spoils the genericity if we also check for such an edge case. @slarse @MartinWitt @nharrand , your thoughts?

@slarse
Copy link
Collaborator

slarse commented Nov 24, 2021

The place where most of the sniper printing magic happens is in AbstractSourceFragmentPrinter::print, and the methods transitively called from there. That's usually where I start debugging when I'm trying to figure stuff like this out.

@algomaster99
Copy link
Contributor Author

@slarse thanks for the pointers.

Keeping this in my backlog.

@algomaster99
Copy link
Contributor Author

Debugged this further. I think the following is a wrong assumption.

/*
* the token did not exist in origin sources. Print spaces made by DJPP
* It can happen e.g. when type parameter like <T> was added. Then bracket tokens are not in origin sources
*/

The comment says that the token is not present in the original sources, however, it always prints them. We need to have a way that prevents that.

@slarse
Copy link
Collaborator

slarse commented Oct 12, 2022

Closing this as stale per the contributing guidelines, feel free to reopen if you can invest time in this again.

@slarse slarse closed this Oct 12, 2022
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