Skip to content
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5d6ea62
Mergeable `if` statements should be combined - RSPEC-S1066
olwispe Aug 30, 2025
4551ac6
Show correct handling of binary in inner condition
timtebeek Sep 8, 2025
a81f904
Inline repeated assignments and add early return
timtebeek Sep 8, 2025
48c6fbe
Show comment lost in current iteration.
timtebeek Sep 8, 2025
5b33886
Preserve comments associated with outer `if`
olwispe Sep 13, 2025
002a234
Move inner `if` to a new line
olwispe Sep 13, 2025
f5d67af
Handle comments associated with inner conditional
olwispe Sep 27, 2025
48b17e9
Update src/main/java/org/openrewrite/staticanalysis/CombineMergeableI…
olwispe Oct 15, 2025
cba1bca
Fix misplaced return in previous suggestion
olwispe Oct 15, 2025
4cac248
Update src/main/java/org/openrewrite/staticanalysis/CombineMergeableI…
olwispe Jan 29, 2026
998d891
Update src/main/java/org/openrewrite/staticanalysis/CombineMergeableI…
olwispe Jan 29, 2026
b82b1ab
Update CombineMergeableIfStatements.java
olwispe Jan 29, 2026
f0c659e
Updated recipes.csv as suggested
olwispe Jan 29, 2026
8e0cbdb
Polish
timtebeek Mar 24, 2026
488123b
Fix indentation bug with non-4-space indent styles
timtebeek Mar 24, 2026
b0dc48a
Add working-set to .gitignore for Moderne CLI
timtebeek Mar 24, 2026
f68a5f2
Merge branch 'main' into combine-mergeable-if-statements-rspec-s1066
timtebeek Mar 30, 2026
4a8f4db
Apply suggestions from code review
timtebeek Mar 30, 2026
9be7658
Use ShiftFormat.indent() instead of custom shiftLeft method
timtebeek Mar 30, 2026
5c530f3
Use autoFormat instead of ShiftFormat for thenPart indentation
timtebeek Mar 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* Licensed under the Moderne Source Available License (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import lombok.Getter;
import lombok.RequiredArgsConstructor;
Comment thread
olwispe marked this conversation as resolved.
import org.jspecify.annotations.Nullable;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Recipe;
import org.openrewrite.Tree;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.style.IntelliJ;
import org.openrewrite.java.style.TabsAndIndentsStyle;
import org.openrewrite.java.tree.Comment;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaSourceFile;
import org.openrewrite.java.tree.Space;
import org.openrewrite.java.tree.Statement;
import org.openrewrite.java.tree.TextComment;
import org.openrewrite.style.Style;

import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;

import static java.util.Collections.emptyList;
import static java.util.Collections.singleton;
import static java.util.Objects.requireNonNull;
import static org.openrewrite.java.format.ShiftFormat.indent;

public class CombineMergeableIfStatements extends Recipe {

private static final String CONTINUATION_KEY = "continuationAfterLogicalAnd";

@Getter
final String displayName = "Mergeable `if` statements should be combined";
@Getter
Comment thread
olwispe marked this conversation as resolved.
Outdated
Comment thread
olwispe marked this conversation as resolved.
Outdated
Comment thread
timtebeek marked this conversation as resolved.
Outdated
final String description = "Mergeable `if` statements should be combined.";
@Getter
final Set<String> tags = singleton("RSPEC-S1066");

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.If visitIf(J.If iff, ExecutionContext ctx) {
J.If outerIf = super.visitIf(iff, ctx);

if (outerIf.getElsePart() == null) {
// thenPart is either a single if or a block with a single if
J.Block outerBlock = null;
J.If innerIf = null;
if (outerIf.getThenPart() instanceof J.If) {
innerIf = (J.If) outerIf.getThenPart();
} else if (outerIf.getThenPart() instanceof J.Block) {
outerBlock = (J.Block) outerIf.getThenPart();
List<Statement> statements = outerBlock.getStatements();
if (statements.size() == 1 && statements.get(0) instanceof J.If) {
innerIf = (J.If) statements.get(0);
}
}

if (innerIf != null && innerIf.getElsePart() == null) {
// thenPart of outer if is replaced with thenPart of innerIf
// combine conditions with logical AND : correct parenthesizing is handled by JavaTemplate
Expression outerCondition = outerIf.getIfCondition().getTree();
Expression innerCondition = innerIf.getIfCondition().getTree();

UUID innerIfId = Tree.randomId();
getCursor().getRoot().putMessage(innerIfId.toString(), innerIf.getComments());
UUID outerBlockId = Tree.randomId();
getCursor().getRoot().putMessage(outerBlockId.toString(),
Optional.ofNullable(outerBlock).map(J::getComments).orElse(emptyList()));

doAfterVisit(new MergedConditionalVisitor<>());
return JavaTemplate.<J.If>apply(
String.format("#{any()} /*%s,%s,%s*/&& #{any()}", CONTINUATION_KEY, innerIfId, outerBlockId),
getCursor(),
outerCondition.getCoordinates().replace(),
outerCondition,
innerCondition)
.withThenPart(indent(innerIf.getThenPart(), getCursor(), -1));
}
}

return outerIf;
}
};
}

@RequiredArgsConstructor
private static class MergedConditionalVisitor<P> extends JavaIsoVisitor<P> {

@Nullable
private TabsAndIndentsStyle tabsAndIndentsStyle;

@Override
public @Nullable J visit(@Nullable Tree tree, P p) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
tabsAndIndentsStyle = Style.from(TabsAndIndentsStyle.class, cu, IntelliJ::tabsAndIndents);
}
return super.visit(tree, p);
}

@Override
public Space visitSpace(@Nullable Space space, Space.Location loc, P p) {
Space s = super.visitSpace(space, loc, p);
if (s.getComments().size() == 1 &&
s.getComments().get(0) instanceof TextComment) {
TextComment onlyComment = (TextComment) s.getComments().get(0);
if (onlyComment.isMultiline() &&
onlyComment.getText().startsWith(CONTINUATION_KEY) &&
getCursor().firstEnclosingOrThrow(J.Binary.class).getOperator() == J.Binary.Type.And) {
final String[] arr = onlyComment.getText().split(",");
final String innerIfId = arr[1];
final String outerBlockId = arr[2];
List<Comment> innerIfComments = Optional.ofNullable(getCursor().getRoot().<List<Comment>>pollMessage(innerIfId)).orElse(emptyList());
List<Comment> outerBlockComments = Optional.ofNullable(getCursor().getRoot().<List<Comment>>pollMessage(outerBlockId)).orElse(emptyList());

getCursor().putMessageOnFirstEnclosing(J.Binary.class, CONTINUATION_KEY, innerIfComments);
s = s.withComments(outerBlockComments);
}
}

return s;
}

@Override
public J.Binary visitBinary(J.Binary binary, P p) {
J.Binary b = super.visitBinary(binary, p);

List<Comment> comments = getCursor().pollMessage(CONTINUATION_KEY);
if (comments != null) {
final String outerIfIndent = getCursor().firstEnclosingOrThrow(J.If.class).getPrefix().getIndent();
final String continuationIndent = continuationIndent(requireNonNull(tabsAndIndentsStyle), outerIfIndent);

if (comments.isEmpty()) {
b = b.withRight(b.getRight()
.withPrefix(Space.format("\n" + continuationIndent)));
} else {
b = b.withRight(b.getRight()
.withComments(ListUtils.map(comments, c -> replaceIndent(c, continuationIndent))));
}
}

return b;
}

private Comment replaceIndent(Comment comment, String newIndent) {
Comment c = comment.withSuffix(replaceLastLineWithIndent(comment.getSuffix(), newIndent));
if (c.isMultiline() && c instanceof TextComment) {
TextComment tc = (TextComment) c;
c = tc.withText(replaceTextIndent(tc.getText(), newIndent));
}
return c;
}

private String replaceTextIndent(final String text, final String newIndent) {
final StringBuilder sb = new StringBuilder();
boolean found = false;
for (final char ch : text.toCharArray()) {
if (ch == ' ' || ch == '\t') {
if (!found) {
sb.append(ch);
}
} else if (ch == '\r' || ch == '\n') {
sb.append(ch);
found = true;
} else {
if (found) {
sb.append(newIndent);
if (ch == '*') {
sb.append(' ');
}
}
sb.append(ch);
found = false;
}
}
if (found) {
sb.append(newIndent);
sb.append(' ');
}
return sb.toString();
}

private String replaceLastLineWithIndent(String whitespace, String indent) {
int idx = whitespace.length() - 1;
while (idx >= 0) {
char c = whitespace.charAt(idx);
if (c == '\r' || c == '\n') {
break;
}
idx--;
}
if (idx >= 0) {
return whitespace.substring(0, idx + 1) + indent;
}
return whitespace;
}

private String continuationIndent(TabsAndIndentsStyle tabsAndIndents, String currentIndent) {
char c;
int len;
if (tabsAndIndents.getUseTabCharacter()) {
c = '\t';
len = tabsAndIndents.getContinuationIndent() / tabsAndIndents.getTabSize();
} else {
c = ' ';
len = tabsAndIndents.getContinuationIndent();
}

StringBuilder sb = new StringBuilder(currentIndent);
for (int i = 0; i < len; i++) {
sb.append(c);
}
return sb.toString();
}
}
}
1 change: 1 addition & 0 deletions src/main/resources/META-INF/rewrite/recipes.csv
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.CatchClauseOnlyRethrows,Catch clause should do more than just rethrow,A `catch` clause that only rethrows the caught exception is unnecessary. Letting the exception bubble up as normal achieves the same result with less code.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ChainStringBuilderAppendCalls,Chain `StringBuilder.append()` calls,"String concatenation within calls to `StringBuilder.append()` causes unnecessary memory allocation. Except for concatenations of String literals, which are joined together at compile time. Replaces inefficient concatenations with chained calls to `StringBuilder.append()`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.CollectionToArrayShouldHaveProperType,'Collection.toArray()' should be passed an array of the proper type,"Using `Collection.toArray()` without parameters returns an `Object[]`, which requires casting. It is more efficient and clearer to use `Collection.toArray(new T[0])` instead.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.CombineMergeableIfStatements,Mergeable `if` statements should be combined,Mergeable `if` statements should be combined.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.CombineSemanticallyEqualCatchBlocks,Combine semantically equal catch blocks,Combine catches in a try that contain semantically equivalent blocks. No change will be made when a caught exception exists if combining catches may change application behavior or type attribution is missing.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.CompareEnumsWithEqualityOperator,Enum values should be compared with "==",Replaces `Enum equals(java.lang.Object)` with `Enum == java.lang.Object`. An `!Enum equals(java.lang.Object)` will change to `!=`.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ControlFlowIndentation,Control flow statement indentation,"Program flow control statements like `if`, `while`, and `for` can omit curly braces when they apply to only a single statement. This recipe ensures that any statements which follow that statement are correctly indented to show they are not part of the flow control statement.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
Expand Down
Loading