-
Notifications
You must be signed in to change notification settings - Fork 466
ChopIfTooLong for methodInvocation chains #6112
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.
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
base: main
Are you sure you want to change the base?
Conversation
rewrite-java/src/main/java/org/openrewrite/java/format/WrapMethodChains.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/test/java/org/openrewrite/java/format/MinimumViableSpacingVisitorTest.java
Outdated
Show resolved
Hide resolved
# Conflicts: # rewrite-java/src/main/java/org/openrewrite/java/format/WrapMethodChains.java
rewrite-java/src/main/java/org/openrewrite/java/format/WrapMethodChains.java
Outdated
Show resolved
Hide resolved
…hodChains.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
class LengthCalculator { | ||
|
||
public static int computeTreeLineLength(J tree, 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 isCompilationUnit = parent.getValue() instanceof J.CompilationUnit; | ||
if (!hasNewLine && !isCompilationUnit) { | ||
return computeTreeLineLength(parent.getValue(), parent); | ||
} | ||
} else { | ||
throw new RuntimeException("Unable to calculate length due to unexpected cursor value: " + cursorValue.getClass()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a service mechanism for supplying language-specific implementations of concepts that appear in multiple languages. This strikes me as a good candidate for that pattern. See JavaSourceFile.service().
This is how a JavaVisitor
can visit a kotlin source file and have maybeAddImport()
use the kotlin version of AddImport
to do it.
I see this as the default SourcePositionService
. While you don't need to do it right now, it could be a reasonable place to put a getLineNumber()
or getStartColumn()
/getEndColumn()
methods which are sometimes requested. Methods like these could be relevant in any language or data format.
There could be Groovy/Kotlin variations which, for example, return false
more often in needsSemicolon
as those languages only require them to separate multiple statements on the same line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took this approach from ColumnPositionCalculator
already present. Is that then also a good candidate?
This method though does not getLineNumber
but gets the length of the current element (including its newlines) starting from the first element that starts on it's line.
Counting the prefix (indentation only) of that element + the length. I struggled to find an explanatory name for that. Any suggestions?
if (method.getName().getComments().isEmpty() && method.getName().getPrefix().getWhitespace().contains("\n")) { | ||
return method.withName(method.getName().withPrefix(Space.EMPTY)); | ||
} | ||
return super.visitMethodInvocation(method, p); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, thanks!
Also support ChopIfTooLong for the recently added WrapMethodChains formatter.