Skip to content

Extracted the Block Trait so other Traits/recipes can use it #6090

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Extracted the Block Trait so other Traits/recipes can use it #6090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ac83b8
Extracted the Block Trait so other Traits/recipes can use it
Jenson3210 Sep 30, 2025
83a7849
Migrated the Block Trait to use a concept of Line (CommentLine and St…
Jenson3210 Oct 1, 2025
cf0b9df
cleanup
Jenson3210 Oct 1, 2025
6f8792d
Added 2 more tests to show behavior or filterLine, mapLine and Line.r…
Jenson3210 Oct 1, 2025
12786f4
reviewbot
Jenson3210 Oct 1, 2025
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
1 change: 1 addition & 0 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
return super.visit(tree, ctx);
}

//select.doSomething(arg1, arg2);
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
Expand Down
266 changes: 18 additions & 248 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependencies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,18 @@
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.marker.ImplicitReturn;
import org.openrewrite.java.tree.*;
import org.openrewrite.trait.SimpleTraitMatcher;
import org.openrewrite.java.trait.Block;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Statement;
import org.openrewrite.trait.Trait;
import org.openrewrite.trait.VisitFunction2;

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Predicate;

import static java.util.Collections.emptyList;

@RequiredArgsConstructor
public class GradleDependencies implements Trait<J.MethodInvocation> {
private static final MethodMatcher DEPENDENCY_DSL_MATCHER = new MethodMatcher("DependencyHandlerSpec *(..)");
Expand All @@ -68,7 +64,20 @@ public class GradleDependencies implements Trait<J.MethodInvocation> {
J.Lambda lambda = (J.Lambda) expression;
if (lambda.getBody() instanceof J.Block) {
return new Block.Matcher().get(lambda.getBody(), new Cursor(cursor, lambda))
.map(block -> block.mapStatements(mapper))
.map(block -> block.mapStatements(statement -> {
if (statement instanceof J.Return && ((J.Return) statement).getExpression() instanceof Statement) {
Statement originalStatement = (Statement) ((J.Return) statement).getExpression();
Statement newStatement = mapper.apply(originalStatement);
if (originalStatement == newStatement) {
return statement;
} else if (newStatement instanceof Expression) {
return ((J.Return) statement).withExpression((Expression) newStatement);
} else if (newStatement == null) {
return null;
}
}
return mapper.apply(statement);
}))
.map(Trait::getTree)
.filter(block -> !(block.getStatements().isEmpty() && block.getComments().isEmpty() && block.getEnd().getComments().isEmpty()))
.map(lambda::withBody)
Expand Down Expand Up @@ -132,243 +141,4 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) {
return null;
}
}

@RequiredArgsConstructor
private static class Block implements Trait<J.Block> {
@Getter
private final Cursor cursor;

public Block mapStatements(Function<Statement, @Nullable Statement> mapper) {
return mapStatements(mapper, (J.Return::withExpression));
}

public Block mapStatements(Function<Statement, @Nullable Statement> mapper, BiFunction<J.Return, Expression, J.Return> returnMapper) {
J.Block block = getTree();
List<Statement> statements = block.getStatements();
List<Statement> newStatements = new LinkedList<>();
LinkedList<Comment> comments = new LinkedList<>();
String whitespace = null;
boolean lastStatementWasKept = false;
for (Statement statement : statements) {
if (statement instanceof J.Return && ((J.Return) statement).getExpression() instanceof Statement) {
statement = ((J.Return) statement).getExpression().withPrefix(statement.getPrefix());
}
Statement mappedStatement = mapper.apply(statement);
if (mappedStatement != null) {
// We want to keep the element (it might have been changed though).
List<Comment> statementComments = mappedStatement.getComments();
if (!comments.isEmpty() && whitespace != null) {
//A previous statement was removed, it had comments and we now need to add these comments to the current element.
Comment last = comments.removeLast();
if (mappedStatement.getPrefix().getWhitespace().contains("\n")) {
// We need to add the prefix of the current statement as the suffix of the last comment of the previous block.
comments.addLast(last.withSuffix(mappedStatement.getPrefix().getWhitespace()));
comments.addAll(statementComments);
} else {
// Add the prefix of the current statement as the suffix of the last comment which is on the same line. (multiple multiline comments could be present)
int j = 0;
while (j < statementComments.size() && !statementComments.get(j).getSuffix().contains("\n")) {
j++;
}
comments.add(last.withSuffix(statementComments.get(j).getSuffix()));
// Now that we have suffixed the correct comment, we can add remaining comments of the current statement
if (j + 1 < statementComments.size()) {
comments.addAll(statementComments.subList(j + 1, statementComments.size()));
}
}
//make sure that the previous removed element's whitespace get used and the newly calculated list of comments containing also the previous removed element's comments.
statement = mappedStatement.withPrefix(mappedStatement.getPrefix().withWhitespace(whitespace).withComments(comments));
} else if (!lastStatementWasKept && !statement.getPrefix().getWhitespace().contains("\n") && whitespace != null) {
int startIndex = 0;
int j = 0;
while (j < statementComments.size() && !statementComments.get(j).getSuffix().contains("\n")) {
j++;
}
if (j + 1 <= statementComments.size()) {
startIndex = j + 1;
}
if (startIndex < statementComments.size()) {
if (startIndex > 0) {
statement = statement.withPrefix(statement.getPrefix().withWhitespace(statementComments.get(startIndex - 1).getSuffix()).withComments(statementComments.subList(startIndex, statementComments.size())));
}
} else {
statement = statement.withPrefix(statement.getPrefix().withWhitespace(whitespace).withComments(emptyList()));
}
}
comments = new LinkedList<>();
whitespace = null;
newStatements.add(statement);
lastStatementWasKept = true;
} else {
List<Comment> statementComments = statement.getComments();
if (!comments.isEmpty() && whitespace != null) {
int startIndex = 0;
int endIndex = findIndexOfLastCommentOnNewLine(statementComments) + 1;
if (!statement.getPrefix().getWhitespace().contains("\n")) {
int j = 0;
while (j < statementComments.size() && !statementComments.get(j).getSuffix().contains("\n")) {
j++;
}
Comment last = comments.removeLast();
comments.add(last.withSuffix(statementComments.get(j).getSuffix()));
if (j + 1 <= statementComments.size()) {
startIndex = j + 1;
}
}
if (startIndex < endIndex) {
comments.addAll(statementComments.subList(startIndex, endIndex));
}
} else {
if (lastStatementWasKept) {
comments.addAll(statementComments.subList(0, findIndexOfLastCommentOnNewLine(statementComments) + 1));
whitespace = statement.getPrefix().getWhitespace();
} else {
int startIndex = findIndexOfFirstCommentOnNewLine(statementComments, statement.getPrefix());
comments.addAll(statementComments.subList(startIndex, findIndexOfLastCommentOnNewLine(statementComments) + 1));
if (startIndex == 0) {
whitespace = statement.getPrefix().getWhitespace();
} else {
whitespace = statementComments.get(startIndex - 1).getSuffix();
}
}
}
lastStatementWasKept = false;
}
}

newStatements = ListUtils.mapLast(newStatements, (Function<Statement, Statement>) newLast -> {
Statement currentLast = statements.get(statements.size() - 1);
if (currentLast instanceof J.Return && !(newLast instanceof J.Return)) {
if (newLast instanceof Expression) {
J.Return currentReturn = (J.Return) currentLast;
Space statementPrefix = currentReturn.getMarkers().findFirst(ImplicitReturn.class).isPresent() ? Space.EMPTY : Space.SINGLE_SPACE;
J.Return mappedReturn = returnMapper.apply(currentReturn, newLast.withPrefix(statementPrefix));
if (mappedReturn.getExpression() instanceof Statement) {
if (mapper.apply((Statement) mappedReturn.getExpression()) == null) {
throw new IllegalArgumentException("The return statement replacement result should not be one that gets filtered out to avoid cyclic changes that result in the entire block being cleared. Did you return something from the old return that still return null when the mapper would be applied?");
}
} else if (mapper.apply(mappedReturn) == null) {
throw new IllegalArgumentException("The return statement replacement result should not be one that gets filtered out to avoid cyclic changes that result in the entire block being cleared. Did you return something from the old return that still return null when the mapper would be applied?");
}
return mappedReturn
.withPrefix(newLast.getPrefix());
}
}
return newLast;
});
if (statements.equals(newStatements) && comments.isEmpty()) {
return this;
}

if (!lastStatementWasKept) {
block = block.withEnd(buildEnd(block.getEnd(), comments, whitespace));
}

block = block.withStatements(newStatements);

return new Block(new Cursor(this.cursor.getParent(), block));
}

private Space buildEnd(Space end, List<Comment> comments, @Nullable String endWhitespace) {
List<Comment> newComments = new ArrayList<>();
String endBlockWhitespace = null;
if (!comments.isEmpty()) {
for (Comment comment : comments) {
if (comment instanceof TextComment) {
newComments.add(comment);
if (endBlockWhitespace == null) {
endBlockWhitespace = endWhitespace;
}
}
}
}

List<Comment> existingEndComments = new ArrayList<>();
boolean eolComments = !end.getWhitespace().contains("\n");
String whitespace = null;
if (!eolComments) {
whitespace = end.getWhitespace();
}
if (!end.getComments().isEmpty()) {
for (Comment comment : end.getComments()) {
if (comment instanceof TextComment) {
if (!eolComments) {
existingEndComments.add(comment);
} else if (comment.getSuffix().contains("\n")) {
eolComments = false;
}
if (whitespace == null) {
whitespace = comment.getSuffix();
}
}
}
}
if (!newComments.isEmpty()) {
newComments.set(newComments.size() - 1, newComments.get(newComments.size() - 1).withSuffix(whitespace == null ? "" : whitespace));
} else {
endBlockWhitespace = whitespace;
}

newComments.addAll(existingEndComments);

return end.withWhitespace(endBlockWhitespace == null ? end.getWhitespace() : endBlockWhitespace).withComments(newComments);
}

private static int findIndexOfFirstCommentOnNewLine(List<Comment> comments, Space prefix) {
if (prefix.getWhitespace().contains("\n")) {
return 0;
}
int index = 0;
while (index < comments.size()) {
Comment comment = comments.get(index);
index++;
if (comment.getSuffix().contains("\n")) {
break;
}
}
return index;
}

private static int findIndexOfLastCommentOnNewLine(List<Comment> comments) {
int index = comments.size() - 1;
while (index >= 0) {
Comment comment = comments.get(index);
if (comment.isMultiline()) {
if (comment.getSuffix().contains("\n")) {
return index;
} else {
index--;
}
} else {
return index;
}
}
return index;
}

private static class Matcher extends SimpleTraitMatcher<Block> {
@Override
public <P> TreeVisitor<? extends Tree, P> asVisitor(VisitFunction2<Block, P> visitor) {
return new JavaVisitor<P>() {
@Override
public J visitBlock(J.Block block, P p) {
Block trait = test(getCursor());
return trait != null ?
(J) visitor.visit(trait, p) :
super.visitBlock(block, p);
}
};
}

@Override
protected @Nullable Block test(Cursor cursor) {
Object object = cursor.getValue();
if (object instanceof J.Block) {
return new Block(cursor);
}

return null;
}
}
}
}
6 changes: 3 additions & 3 deletions rewrite-gradle/src/test/java/org/openrewrite/gradle/trait/GradleDependenciesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ public void defaults(RecipeSpec spec) {
@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
Optional<GradleDependencies> meybeDependenciesBlock = new GradleDependencies.Matcher().get(mi, getCursor().getParent());
Optional<GradleDependencies> maybeDependenciesBlock = new GradleDependencies.Matcher().get(mi, getCursor().getParent());

if (meybeDependenciesBlock.isPresent()) {
return meybeDependenciesBlock
if (maybeDependenciesBlock.isPresent()) {
return maybeDependenciesBlock
.map(deps -> deps.removeDependency("javax.validation", "validation-api"))
.map(Trait::getTree)
.orElse(null);
Expand Down
Loading