Skip to content

Improved support for indentation formatting using records and single arg method declarations #6158

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

Merged
sambsnyd merged 17 commits into from
Oct 22, 2025
Merged

Improved support for indentation formatting using records and single arg method declarations #6158

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e6b5d1f
Fixed an issue where conversion would print markers to the text resul…
Jenson3210 Oct 20, 2025
238dae9
Fixed an issue where conversion would print markers to the text resul…
Jenson3210 Oct 20, 2025
f729120
Minor polish
timtebeek Oct 20, 2025
bf7f540
Change copyright year in PlainTextParserConversionTest
timtebeek Oct 20, 2025
39695f2
Improved support for indentation formatting using records and single …
Jenson3210 Oct 20, 2025
8a43e5f
Merge branch 'main' into indentation-issue
timtebeek Oct 20, 2025
f9b94d4
Update rewrite-java/src/main/java/org/openrewrite/java/format/Minimum…
timtebeek Oct 20, 2025
c8b77a9
comments
Jenson3210 Oct 20, 2025
ef65db4
Merge remote-tracking branch 'origin/indentation-issue' into indentat…
Jenson3210 Oct 20, 2025
ee27e25
Update rewrite-java/src/main/java/org/openrewrite/java/format/TabsAnd…
Jenson3210 Oct 20, 2025
f470ef5
comments
Jenson3210 Oct 20, 2025
6c9e883
Merge remote-tracking branch 'origin/indentation-issue' into indentat…
Jenson3210 Oct 20, 2025
f07a178
fix failing tests
Jenson3210 Oct 21, 2025
7ea5489
Code review comments
Jenson3210 Oct 22, 2025
25fcb20
Code review comments
Jenson3210 Oct 22, 2025
ffbbd06
Moved spaces calculation to SpacesVisitor
Jenson3210 Oct 22, 2025
35de878
Moved spaces calculation to SpacesVisitor
Jenson3210 Oct 22, 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
22 changes: 22 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/format/MinimumViableSpacingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P
}
}

if (c.getPrimaryConstructor() != null) {
c = c.withPrimaryConstructor(ListUtils.map(c.getPrimaryConstructor(), (ix, param) -> {
if (param.getPrefix().getLastWhitespace().contains("\n")) {
return param;
}
if (ix == 0 && param.getComments().isEmpty()) {
return param.withPrefix(param.getPrefix().withWhitespace(""));
}
return param.withPrefix(param.getPrefix().withWhitespace(" "));
}));
}

return c;
}

Expand Down Expand Up @@ -166,6 +178,16 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, P
}
}

m = m.withParameters(ListUtils.map(m.getParameters(), (ix, param) -> {
Comment thread
timtebeek marked this conversation as resolved.
Outdated
if (param.getPrefix().getLastWhitespace().contains("\n")) {
return param;
}
if (ix == 0 && param.getComments().isEmpty()) {
return param.withPrefix(param.getPrefix().withWhitespace(""));
}
return param.withPrefix(param.getPrefix().withWhitespace(" "));
}));

return m;
}

Expand Down
83 changes: 58 additions & 25 deletions rewrite-java/src/main/java/org/openrewrite/java/format/TabsAndIndentsVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Iterator;
import java.util.List;

import static java.util.Collections.emptyList;
import static org.openrewrite.java.format.ColumnPositionCalculator.computeColumnPosition;

public class TabsAndIndentsVisitor<P> extends JavaIsoVisitor<P> {
Expand Down Expand Up @@ -201,6 +202,7 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
Space after;

int indent = getCursor().getNearestMessage("lastIndent", 0);
IndentType indentType = getCursor().getParent().getNearestMessage("indentType", IndentType.ALIGN);
if (right.getElement() instanceof J) {
J elem = (J) right.getElement();
if ((right.getAfter().getLastWhitespace().contains("\n") ||
Expand Down Expand Up @@ -232,27 +234,52 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
JContainer<J> container = getCursor().getParentOrThrow().getValue();
List<J> elements = container.getElements();
J lastArg = elements.get(elements.size() - 1);
if (elements.size() > 1 && style.getMethodDeclarationParameters().getAlignWhenMultiple()) {
J.MethodDeclaration method = getCursor().firstEnclosing(J.MethodDeclaration.class);
if (method != null) {
int alignTo = computeFirstParameterColumn(method);
if (alignTo != -1) {
getCursor().getParentOrThrow().putMessage("lastIndent", alignTo - style.getContinuationIndent());
elem = visitAndCast(elem, p);
getCursor().getParentOrThrow().putMessage("lastIndent", indent);
after = indentTo(right.getAfter(), t == lastArg ? indent : alignTo, loc.getAfterLocation());
//noinspection ConstantConditions
J tree = null;
if (loc == JRightPadded.Location.METHOD_DECLARATION_PARAMETER) {
tree = getCursor().firstEnclosing(J.MethodDeclaration.class);
} else if (loc == JRightPadded.Location.RECORD_STATE_VECTOR) {
Comment thread
Jenson3210 marked this conversation as resolved.
Outdated
tree = getCursor().firstEnclosing(J.ClassDeclaration.class);
getCursor().getParentOrThrow().putMessage("indentType", IndentType.CONTINUATION_INDENT);
}
if (elements.size() > 1) {
try {
if ((loc == JRightPadded.Location.METHOD_DECLARATION_PARAMETER && style.getMethodDeclarationParameters().getAlignWhenMultiple()) ||
((loc == JRightPadded.Location.RECORD_STATE_VECTOR && style.getRecordComponents() != null && style.getRecordComponents().getAlignWhenMultiple()))) {

@timtebeek timtebeek Oct 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

style.getRecordComponents() is not marked as @Nullable; should it be? Or can we remove this guard since we seem to rely on the NoSuchMethodError below.

@Jenson3210 Jenson3210 Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as with the try/catch, we had NPE's, No SuchMethodErrors last time when introducing new style classes. So added like this to be safe. We should then later be able to remove the try/catch's and the if != null statements

@sambsnyd sambsnyd Oct 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it makes more sense for these temporary backwards compatibility null checks to exist within the relevant style class. You can always provide an implementation for getRecordComponents() which returns the default value when the underlying field is null. Then when the null check is no longer relevant there is only one place to remove it from, rather than from every call site.

@Jenson3210 Jenson3210 Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That makes sense in this occasion as we will introduce both the style and the default getter in a single go. It will hence be either a NoSuchMethod or a defaulted getter. Last time we also had the possibilitiy of an old (non-defaulted) getter being parent loaded.
Adapted!

if (tree != null) {
int alignTo = computeFirstParameterColumn(tree);
if (alignTo != -1) {
getCursor().getParentOrThrow().putMessage("lastIndent", alignTo - style.getContinuationIndent());
elem = visitAndCast(elem, p);
getCursor().getParentOrThrow().putMessage("lastIndent", indent);
after = indentTo(right.getAfter(), t == lastArg ? indent : alignTo, loc.getAfterLocation());
} else {
after = right.getAfter();
}
} else {
after = right.getAfter();
}
} else {
after = right.getAfter();
elem = visitAndCast(elem, p);
after = indentTo(right.getAfter(), t == lastArg ? indent : style.getContinuationIndent(), loc.getAfterLocation());
}
} else {
after = right.getAfter();
} catch (NoSuchMethodError error) {
// style.getRecordComponents introduction might give NoSuchMethodError depending on the runtime, the lst build date...
elem = visitAndCast(elem, p);
after = indentTo(right.getAfter(), t == lastArg ? indent : style.getContinuationIndent(), loc.getAfterLocation());
}
} else if (elements.size() > 1) {
elem = visitAndCast(elem, p);
after = indentTo(right.getAfter(), t == lastArg ? indent : style.getContinuationIndent(), loc.getAfterLocation());
} else {
if (elem.getPrefix().getLastWhitespace().contains("\n") && tree != null) {
int alignTo = computeFirstParameterColumn(tree);
if (alignTo != -1) {
getCursor().getParentTreeCursor().putMessage("lastIndent", alignTo - style.getContinuationIndent());
elem = visitAndCast(elem, p);
getCursor().getParentTreeCursor().putMessage("lastIndent", indent);
}
}
after = right.getAfter();
}
getCursor().getParentOrThrow().putMessage("indentType", indentType);
break;
}
case METHOD_INVOCATION_ARGUMENT:
Expand Down Expand Up @@ -339,6 +366,8 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
switch (loc) {
case NEW_CLASS_ARGUMENTS:
case METHOD_INVOCATION_ARGUMENT:
case RECORD_STATE_VECTOR:
case METHOD_DECLARATION_PARAMETER:
if (!elem.getPrefix().getLastWhitespace().contains("\n")) {
JContainer<J> args = getCursor().getParentOrThrow().getValue();
boolean anyOtherArgOnOwnLine = false;
Expand Down Expand Up @@ -389,23 +418,27 @@ public Space visitSpace(Space space, Space.Location loc, P p) {
return (after == right.getAfter() && t == right.getElement()) ? right : new JRightPadded<>(t, after, right.getMarkers());
}

private int computeFirstParameterColumn(J.MethodDeclaration method) {
List<JRightPadded<Statement>> arguments = method.getPadding().getParameters().getPadding().getElements();
private int computeFirstParameterColumn(J tree) {
List<JRightPadded<Statement>> arguments;
if (tree instanceof J.MethodDeclaration) {
arguments = ((J.MethodDeclaration) tree).getPadding().getParameters().getPadding().getElements();
} else if (tree instanceof J.ClassDeclaration) {
JContainer<Statement> primaryConstructorArgs = ((J.ClassDeclaration) tree).getPadding().getPrimaryConstructor();
arguments = primaryConstructorArgs == null ? emptyList() : primaryConstructorArgs.getPadding().getElements();
} else {
throw new IllegalStateException("Unexpected tree type: " + tree.getClass().getSimpleName());
Comment thread
Jenson3210 marked this conversation as resolved.
Outdated
}
J firstArg = arguments.isEmpty() ? null : arguments.get(0).getElement();
if (firstArg == null || firstArg instanceof J.Empty) {
return -1;
}

if (firstArg.getPrefix().getLastWhitespace().contains("\n")) {
int declPrefixLength = getLengthOfWhitespace(method.getPrefix().getLastWhitespace());
int argPrefixLength = getLengthOfWhitespace(firstArg.getPrefix().getLastWhitespace());
//noinspection ConstantConditions to be backwards compatible with older style versions
if (declPrefixLength >= argPrefixLength) {
return declPrefixLength + style.getContinuationIndent();
}
return argPrefixLength;
int declPrefixLength = getLengthOfWhitespace(tree.getPrefix().getLastWhitespace());

return declPrefixLength + style.getContinuationIndent();
} else {
return computeColumnPosition(method, firstArg, getCursor());
return computeColumnPosition(tree, firstArg, getCursor());
}
}

Expand Down
3 changes: 2 additions & 1 deletion rewrite-java/src/main/java/org/openrewrite/java/style/Autodetect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ public TabsAndIndentsStyle getTabsAndIndentsStyle() {
continuationIndent,
false,
new TabsAndIndentsStyle.MethodDeclarationParameters(
multilineAlignedToFirstArgument >= multilineNotAlignedToFirstArgument)
multilineAlignedToFirstArgument >= multilineNotAlignedToFirstArgument),
new TabsAndIndentsStyle.RecordComponents(multilineAlignedToFirstArgument >= multilineNotAlignedToFirstArgument)
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion rewrite-java/src/main/java/org/openrewrite/java/style/IntelliJ.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static ImportLayoutStyle importLayout() {

public static TabsAndIndentsStyle tabsAndIndents() {
return new TabsAndIndentsStyle(false, 4, 4, 8, false,
new TabsAndIndentsStyle.MethodDeclarationParameters(true));
new TabsAndIndentsStyle.MethodDeclarationParameters(true), new TabsAndIndentsStyle.RecordComponents(true));
}

public static BlankLinesStyle blankLines() {
Expand Down
7 changes: 7 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/style/TabsAndIndentsStyle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,20 @@ public class TabsAndIndentsStyle implements JavaStyle {
Boolean indentsRelativeToExpressionStart;

MethodDeclarationParameters methodDeclarationParameters;
RecordComponents recordComponents;

@Value
@With
public static class MethodDeclarationParameters {
Boolean alignWhenMultiple;
}

@Value
@With
public static class RecordComponents {
Boolean alignWhenMultiple;
}

@Override
public Style applyDefaults() {
return StyleHelper.merge(IntelliJ.tabsAndIndents(), this);
Expand Down
Loading
Loading