`SimplifyElseBranch` puts comment in not the best location

[InAnYan]

What version of OpenRewrite are you using?

I am using

• Maven/Gradle plugin v7.32.1
• rewrite-recipe-bom:3.30.0

How are you running OpenRewrite?

I am using the Gradle plugin.

What is the smallest, simplest way to reproduce the problem?

• The real-world example: https://github.com/JabRef/jabref/pull/15665/changes/#diff-db2f9211ebcdff5b3e3a68cbb6c1f69202b33d85cfae155215326fc4ddad5701R140.

Sorry for not providing an MWE, because of the lack of time.

I guess a simpler version is:

Before:

if (<condition>) {
    do1();
} else {
    // Comment.
    if (<condition 2>) {
        do2();
    }
}

What did you expect to see?

Not sure if this is a better version.

if (<condition>) {
    do1();
} else if (<condition 2>) {
    // Comment.
    do2();
}

What did you see instead?

if (<condition>) {
    do1();
} else // Comment.
    if (<condition 2>) {
        do2();
}

Which is technically right, but a bit crazy. I have never seen such a placement of a comment.

It's not really a bug, but a discussion on the style and approach.

What is the full stack trace of any errors you encountered?

No errors.

Are you interested in contributing a fix to OpenRewrite?

Yes, but I don't have time right now. Maybe later

[timtebeek]

Hey thanks for the report! Agree that it looks a little odd, if technically correct. Moving the comment into the if body would bring some complications as well if there was already a comment there; there's not a clear cut guaranteed improvement in all cases here I'm afraid, safe for perhaps not maing any change at all. 🤔

[abhinav-1504]

Hi @timtebeek , I’d like to take this issue.
I’m currently investigating the comment attachment/formatting behavior in SimplifyElseBranch and will work on a fix along with a reproducible test case.

[timtebeek]

Thanks for the offer to help; based on the above we're not clear yet on what the best way forward is. What are you thinking to do before you dive into changing the code?

[abhinav-1504]

Thanks @timtebeek. Before changing the implementation, I’d first like to understand how comments are currently attached/preserved during the else -> else if transformation and identify the different edge cases involved.

My current thought is to avoid transforming the branch when a leading comment inside the else block would end up attached to the else token itself, since that seems to produce the unusual formatting reported here.

I’ll first add a minimal reproducible test covering:

if (a) {
    do1();
} else {
    // Comment.
    if (b) {
        do2();
    }
}

Then I’ll explore a few approaches:

1. Preserve the current transformation only when no leading comments are present.
2. Move the comment safely into the nested if body when formatting remains unambiguous.
3. Skip the simplification entirely in ambiguous/comment-sensitive cases.

I'll investigate which option fits best with existing formatting/comment preservation behavior across Rewrite recipes and share my findings here before opening a PR.