Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Expand Up @@ -16,7 +16,6 @@
package org.openrewrite.style;

public enum LineWrapSetting {
DoNotWrap, WrapAlways;
// Eventually we would add values like WrapIfTooLong or ChopIfTooLong

DoNotWrap, WrapAlways, ChopIfTooLong;
// Eventually we would add values like WrapIfTooLong
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class WrappingAndBracesTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("builder", "newBuilder")),
new WrappingAndBracesStyle.Annotations(WrapAlways),
Expand Down Expand Up @@ -925,6 +926,7 @@ final String method4(){
void annotationWrappingWithNulls() {
rewriteRun(spec ->
spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(DoNotWrap, emptyList()),
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P
return c;
}

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P p) {
if (method.getName().getComments().isEmpty() && method.getName().getPrefix().getWhitespace().contains("\n")) {
return method.withName(method.getName().withPrefix(Space.EMPTY));
}
return super.visitMethodInvocation(method, p);
Comment on lines 102 to 105
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A method invocation meeting this condition could be contained within another method invocation which meets this condition. In that circumstance the inner method would not be reformatted as no traversal into sub-elements happens when this condition is matched.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, thanks!

}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, P p) {
J.MethodDeclaration m = super.visitMethodDeclaration(method, p);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
import org.openrewrite.Cursor;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
Expand All @@ -41,7 +42,7 @@ public class WrapMethodChains<P> extends JavaIsoVisitor<P> {
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P ctx) {
J.MethodInvocation m = super.visitMethodInvocation(method, ctx);

if (style.getChainedMethodCalls().getWrap() == LineWrapSetting.WrapAlways) {
if (style.getChainedMethodCalls().getWrap() == LineWrapSetting.WrapAlways || style.getChainedMethodCalls().getWrap() == LineWrapSetting.ChopIfTooLong) {
List<MethodMatcher> matchers = style.getChainedMethodCalls().getBuilderMethods().stream()
.map(name -> String.format("*..* %s(..)", name))
.map(MethodMatcher::new)
Expand All @@ -58,6 +59,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P ctx
return m;
}

// Not long enough to wrap
if (style.getChainedMethodCalls().getWrap() == LineWrapSetting.ChopIfTooLong && calculateLineLength(getCursor()) <= style.getHardWrapAt()) {
return m;
}

//Only update the whitespace, preserving comments
if (after.getComments().isEmpty()) {
after = after.withWhitespace("\n");
Expand All @@ -73,6 +79,21 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P ctx
return m;
}

private int calculateLineLength(Cursor cursor) {
Object cursorValue = cursor.getValue();
if (cursorValue instanceof J) {
J j = (J) cursorValue;
boolean hasNewLine = j.getPrefix().getWhitespace().contains("\n") || j.getComments().stream().anyMatch(c -> c.getSuffix().contains("\n"));
Cursor parent = cursor.getParentTreeCursor();
boolean isRoot = Cursor.ROOT_VALUE == parent.getValue();
if (!hasNewLine && !isRoot) {
return calculateLineLength(parent);
}
return j.print(cursor).replaceAll("^.*\\n", "").length() + 1;
}
throw new RuntimeException("Unable to calculate length due to unexpected cursor value: " + cursorValue.getClass());
}

private J.@Nullable MethodInvocation findChainStarterInChain(J.MethodInvocation method, List<MethodMatcher> matchers) {
Expression current = method;
while (current instanceof J.MethodInvocation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public static SpacesStyle spaces() {

public static WrappingAndBracesStyle wrappingAndBraces() {
return new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(DoNotWrap, emptyList()),
new WrappingAndBracesStyle.Annotations(WrapAlways),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
@With
public class WrappingAndBracesStyle implements JavaStyle {

int hardWrapAt;
IfStatement ifStatement;
ChainedMethodCalls chainedMethodCalls;
@Nullable Annotations classAnnotations;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public static class Builder {
new NamedStyles(UUID.randomUUID(), "junit", "Unit Test style", "Only used in unit tests", emptySet(),
List.of(
new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, asList("builder", "newBuilder", "stream")),
new WrappingAndBracesStyle.Annotations(WrapAlways),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package org.openrewrite.java.format;

import org.junit.jupiter.api.Test;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.test.RewriteTest.toRecipe;

class MinimumViableSpacingVisitorTest implements RewriteTest {

@Override
public void defaults(RecipeSpec spec) {
spec
.parser(JavaParser.fromJavaVersion().dependsOn("""
package com.example;

public class MyObject {
public static Builder builder() { return new Builder(); }
public static Builder newBuilder() { return new Builder(); }
public static class Builder {
Builder name(String n) { return this; }
Builder age(int a) { return this; }
Builder items(java.util.List<String> items) { return this; }
Builder nested(MyObject nested) { return this; }
MyObject build() { return new MyObject(); }
}
}
"""))
.recipe(toRecipe(() -> new MinimumViableSpacingVisitor<>(null)));
}

@Test
void reformatChainedMethodInvocationToSingleLine() {
rewriteRun(
java(
"""
package com.example;
class Test {
void test() {
MyObject myObject = MyObject.builder()
.

name("John");
}
}
""",
"""
package com.example;
class Test {
void test() {
MyObject myObject = MyObject.builder()
.name("John");
}
}
"""
)
);
}

@Test
void doNotReformatChainedMethodInvocationToSingleLineWhenCommentInPrefixOfName() {
rewriteRun(
java(
"""
package com.example;
class Test {
void test() {
MyObject myObject = MyObject.builder()
. //Some comment
name("John");
}
}
"""
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package org.openrewrite.java.format;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.style.WrappingAndBracesStyle;
import org.openrewrite.test.RecipeSpec;
Expand All @@ -24,8 +26,7 @@
import java.util.Arrays;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.style.LineWrapSetting.DoNotWrap;
import static org.openrewrite.style.LineWrapSetting.WrapAlways;
import static org.openrewrite.style.LineWrapSetting.*;
import static org.openrewrite.test.RewriteTest.toRecipe;

class WrapMethodChainsTest implements RewriteTest {
Expand All @@ -50,6 +51,7 @@ public static class Builder {
"""))
.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(
new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("builder", "newBuilder")),
new WrappingAndBracesStyle.Annotations(WrapAlways),
Expand Down Expand Up @@ -473,6 +475,7 @@ void test() {
void formatStreamChain() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -569,6 +572,7 @@ void test() {
void formatStreamWithMultilineFilterLambda() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -641,6 +645,7 @@ static class Item {
void formatStreamWithMultipleMultilineLambdas() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -715,6 +720,7 @@ boolean isValid() {
void formatStreamWithMixedLambdaStyles() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -773,6 +779,7 @@ List<Integer> process(List<String> items) {
void formatStreamWithComplexNestedLambda() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -848,6 +855,7 @@ static class Employee {
void formatStreamWithMethodReferencesAndLambdas() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -909,6 +917,7 @@ String preprocess(String s) {
void formatStreamWithPeekAndMultilineLambda() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -964,6 +973,7 @@ List<String> process(List<String> items) {
void preserveAlreadyFormattedStreamWithMultilineLambda() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -1038,6 +1048,7 @@ static class Item {
void formatStreamWithReduceMultilineLambda() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("stream")),
null,
Expand Down Expand Up @@ -1087,6 +1098,7 @@ Integer sum(List<Integer> numbers) {
void formatStreamInBuilderArgument() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
120,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(WrapAlways, Arrays.asList("builder", "stream")),
null,
Expand Down Expand Up @@ -1129,4 +1141,71 @@ MyObject process(List<String> items) {
)
);
}

@Test
void chopIfTooLong() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
70,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(ChopIfTooLong, Arrays.asList("builder", "stream")),
null,
null,
null,
null,
null,
null)))),
java(
"""
package com.example;

class Test {
void test() {
MyObject obj = MyObject.builder().name("test").age(25).build();
}
}
""",
"""
package com.example;

class Test {
void test() {
MyObject obj = MyObject.builder()
.name("test")
.age(25)
.build();
Comment on lines +1173 to +1176
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat surprised by the left aligned new lines here; is the expectation that a formatter is run afterwards?

}
}
"""
)
);
}

@ParameterizedTest
@ValueSource(ints = {71, 72})
void doNotChopIfNotTooLong(int length) {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new WrappingAndBracesVisitor<>(new WrappingAndBracesStyle(
length,
new WrappingAndBracesStyle.IfStatement(false),
new WrappingAndBracesStyle.ChainedMethodCalls(ChopIfTooLong, Arrays.asList("builder", "stream")),
null,
null,
null,
null,
null,
null)))),
java(
"""
package com.example;

class Test {
void test() {
MyObject obj = MyObject.builder().name("test").age(25).build();
}
}
"""
)
);
}
}
Loading