Mergeable if statements should be combined - RSPEC-S1066

[olwispe]

What's changed?

This PR implements RSPEC-S1066 which was requested in :

• Collapsible if statements should be merged. RSPEC-1066 #45
• Replace nested ifs with one if and conditions joined with && #537

What's your motivation?

Among others, help migrate to pattern matching for instanceof when combined with org.openrewrite.staticanalysis.InstanceOfPatternMatch :

if (o instanceof String) {
  String s = (String) o;
  if (s.isEmpty()) {
     System.out.println("OK");
  }
}

if (o instanceof String s && s.isEmpty()) {
  System.out.println("OK");
}

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

Any additional context

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[timtebeek]

Great to see @olwispe ! Added some light polish and I've been looking at the changes at scale.

Looking at these two examples I wonder if perhaps we should move the inner if conditional to a new line, and keep the comments in more of their original position.

I've pushed a quick test update that shows a comment lost. What are your thoughts to the above proposal?

[olwispe]

Thanks for your comments.
I tend to prefer to have each merged conditional on a new line as you suggest and therefore keep the comments in their original position.

[github-actions]

This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in two weeks. PRs may be reopened when there is renewed interest.

[olwispe]

Hi @timtebeek!
I've implemented your earlier suggestions which were quite tricky to implement given my knowledge of OpenRewrite.
Anyway, could you review these changes?

[timtebeek]

Moderne Validation Summary — Apache Organization

Ran CombineMergeableIfStatements against the Apache Maven organization (87 repos, 81 succeeded, 6 skipped due to missing LSTs).

Results

• 36 out of 81 repositories produced changes
• ~127 files modified across those 36 repos
• Top repos by changes: maven-2 (14 files), maven-scm (10 files), maven-doxia (10 files), maven-indexer (8 files), maven-wagon (6 files)

Additionally tested individually against apache/accumulo (30 files changed).

Correctness

✅ Repos with standard 4-space indentation: All patches look correct. Conditions are properly merged with &&, comments are preserved, and the then body is correctly dedented by one level.

❌ Repos with 2-space indentation (e.g., apache/accumulo): The recipe produces incorrect indentation in the merged if body. The indent(innerIf.getThenPart(), getCursor(), -1) call on line 107 dedents by 4 spaces (default IntelliJ style) instead of 2 spaces (the actual indent used in the file). This causes the body to shift left by too much, breaking compilation.

Example (accumulo, 2-space indent):

// Before (correct):
      if (!extentsToHost.isEmpty()) {
        if (...) {
          requestTabletHosting(context, extentsToHost.values());
        }
      }

// After (incorrect — body shifted too far left):
      if (!extentsToHost.isEmpty() &&
              (...)) {
      requestTabletHosting(context, extentsToHost.values());  // should be 8 spaces, got 6
    }

Root Cause

ShiftFormat.indent() uses the file's TabsAndIndentsStyle (defaulting to IntelliJ's 4-space indent), but repos like accumulo use 2-space indentation without an explicit style marker. The -1 level shift therefore removes 4 spaces instead of 2.

[timtebeek]

Fix Applied: Indentation bug with non-4-space indent

Pushed commit 488123b1 to fix the indentation issue found during validation.

Problem

ShiftFormat.indent(-1) always dedented by 4 spaces (IntelliJ default), breaking repos that use 2-space indentation (like Apache Accumulo). The body of the merged if statement would shift too far left, producing malformed code.

Fix

Replaced indent(-1) with a custom shiftLeft() method that computes the actual indent difference between the inner and outer if statements by measuring their whitespace. This works correctly regardless of indent style (2-space, 4-space, tabs, etc.).

Test

Added combineMergeableIfStatementsWithTwoSpaceIndent test case that verifies the recipe produces correct indentation with 2-space indent style.