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: Correct parenthesis around type-casting for refactored code when using SniperJavaPrettyPrinter #6215

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

Conversation

rishivijayv
Copy link
Contributor

@rishivijayv rishivijayv commented Mar 4, 2025

Fix #4335

Context and Fix

When a piece of code involving a type-cast is refactored and then printed using SniperJavaPrettyPrinter (where the cast itself is to be printed from the source-code) , we seem to be printing extra parenthesis around the type-cast, leading to non-compilable java code.

I believe this was occurring because when processing a CAST fragment specifically, we by default seem to add extra (...) pairs around it here.

Then, when the scan between those 2 additions visits the SniperJavaPrettyPrinter scan here, we eventually get to a call to print the fragment's source-code as is -- which involves the (...) that already come with a cast. I believe this is will always be the case, since explicit casting (that I believe we are interested with here) will always be enclosed within its own (...) for it to be valid Java source code (according to references like this: https://www.w3schools.com/java/java_type_casting.asp).

So, in SniperJavaPrettyPrinter, instead of just printing the fragment's source-code as is, I've added a helper function which can be used to modify the source-code being printed to account for such "default" behaviour being done by our pretty-printing code. So, all calls to fragment.getSourceCode() in SniperJavaPrettyPrinter should have been replaced with this helper now. For now it just involves special logic to handle CAST type-fragments, but I can see that being expanded in the future if there are other "special cases" like this one.

Testing

I've recreated the example found in the original github issue (changing var to valid Java types that seem to be required according to a list in this test file: link -- with var the testPrinterTokenListener in PrinterTest was failing). Now I believe there are no longer extra parenthesis around (Double).

I think this fix might also resolve #4221? I'm not entirely sure how to go about testing that though, since from my understanding SniperJavaPrettyPrinter really comes in to play whenever there is a change/refactoring from the original source code to the new one, and in #4221 I can't see what the change being made is, just the Expected and the Original. The use of non-standard Java library packages (OracleConnecteion, etc) also makes it a little hard to recreate exactly. If there is a way/need to recreate that test or something that is similar, let me know how and I'll add/verify that test here!

If there are any other tests that I should add, please let me know as well! All the existing tests pass, which I think is a good sign 😄

Additional Notes

I was also investigating another SniperJavaPrettyPrinter bug related to extra spaces being added around type-casts. The source code still seems compilable (which imo makes this fix a little more important), but I think the explicit addition of ( brackets in the enterCtExpression function of DefaultJavaPrettyPrinter might be making things difficult. Here, the fix works in the confines of that explicit addition. More info can be found on my comment here: #3911 (comment)

cc @algomaster99 @monperrus @I-Al-Istannen would appreciate your thoughts on this whenever you get the time 🙏🏽 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant