-
Notifications
You must be signed in to change notification settings - Fork 88
732: Remove trailing whitespace #733
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
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceVisitor.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespace.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
…tespace.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/RemoveTrailingWhitespaceTest.java
Show resolved
Hide resolved
input.append("Line ").append(i); | ||
expected.append("Line ").append(i); | ||
|
||
// Add i spaces as trailing whitespace | ||
for (int j = 0; j < i; j++) { | ||
input.append(" "); | ||
} |
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.
input.append("Line ").append(i); | |
expected.append("Line ").append(i); | |
// Add i spaces as trailing whitespace | |
for (int j = 0; j < i; j++) { | |
input.append(" "); | |
} | |
input.append("Line ").append(i).repeat(" ", i); | |
expected.append("Line ").append(i); |
Java 21 has this new repeat
method on StringBuilder
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.
Added in Java 11, IIRC.
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 meant: the tests run in Java 21, so this could be used here, no?
|
||
@Override | ||
public TreeVisitor<?, ExecutionContext> getVisitor() { | ||
return new PlainTextVisitor<ExecutionContext>() { |
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 PlainTextVisitor does not act on Java files. If you would want to "downcast" the Java lst as a text lst, you could build a generic TreeVisitor that does that conversion.
In this case though you do not want to do that as you will loose all rich information of the lst and every recipe that should run on java files after this one will not make any changes as the Lst's at that moment are no longer JavaSourceFiles but PlainText Sourcefiles causing the isAcceptable of the JavaVisitor to skip the converted files.
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.
…tespaceTest.java Co-authored-by: Jente Sondervorst <[email protected]>
"Line with trailing spaces \n" + | ||
"Line without trailing spaces\n" + | ||
"Another line with spaces \n", | ||
"Line with trailing spaces\n" + | ||
"Line without trailing spaces\n" + | ||
"Another line with spaces\n" |
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.
"Line with trailing spaces \n" + | |
"Line without trailing spaces\n" + | |
"Another line with spaces \n", | |
"Line with trailing spaces\n" + | |
"Line without trailing spaces\n" + | |
"Another line with spaces\n" | |
""" | |
Line with trailing spaces \s | |
Line without trailing spaces | |
Another line with spaces \s | |
""", | |
""" | |
Line with trailing spaces | |
Line without trailing spaces | |
Another line with spaces | |
""" |
"Line with trailing tabs\t\t\n" + | ||
"Line without trailing tabs\n" + | ||
"Mixed tabs and spaces\t \n", | ||
"Line with trailing tabs\n" + | ||
"Line without trailing tabs\n" + | ||
"Mixed tabs and spaces\n" |
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.
"Line with trailing tabs\t\t\n" + | |
"Line without trailing tabs\n" + | |
"Mixed tabs and spaces\t \n", | |
"Line with trailing tabs\n" + | |
"Line without trailing tabs\n" + | |
"Mixed tabs and spaces\n" | |
""" | |
Line with trailing tabs | |
Line without trailing tabs | |
Mixed tabs and spaces \s | |
""", | |
""" | |
Line with trailing tabs | |
Line without trailing tabs | |
Mixed tabs and spaces | |
""" |
"First line\n" + | ||
"Second line ", | ||
"First line\n" + | ||
"Second 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.
"First line\n" + | |
"Second line ", | |
"First line\n" + | |
"Second line" | |
""" | |
First line | |
Second line """, | |
""" | |
First line | |
Second line""" |
"Line 1 \n" + | ||
"\n" + | ||
"Line 3\t\n" + | ||
"\n" + | ||
"Line 5", | ||
"Line 1\n" + | ||
"\n" + | ||
"Line 3\n" + | ||
"\n" + | ||
"Line 5" |
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.
"Line 1 \n" + | |
"\n" + | |
"Line 3\t\n" + | |
"\n" + | |
"Line 5", | |
"Line 1\n" + | |
"\n" + | |
"Line 3\n" + | |
"\n" + | |
"Line 5" | |
""" | |
Line 1 \s | |
Line 3 | |
Line 5""", | |
""" | |
Line 1 | |
Line 3 | |
Line 5""" |
"Spaces then tabs \t\t\n" + | ||
"Tabs then spaces\t\t \n" + | ||
"Mixed whitespace \t \t \n", | ||
"Spaces then tabs\n" + | ||
"Tabs then spaces\n" + | ||
"Mixed whitespace\n" |
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.
"Spaces then tabs \t\t\n" + | |
"Tabs then spaces\t\t \n" + | |
"Mixed whitespace \t \t \n", | |
"Spaces then tabs\n" + | |
"Tabs then spaces\n" + | |
"Mixed whitespace\n" | |
""" | |
Spaces then tabs | |
Tabs then spaces \s | |
Mixed whitespace \s | |
""", | |
""" | |
Spaces then tabs | |
Tabs then spaces | |
Mixed whitespace | |
""" |
"Clean line 1\n" + | ||
"Clean line 2\n" + | ||
"Clean line 3" |
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.
"Clean line 1\n" + | |
"Clean line 2\n" + | |
"Clean line 3" | |
""" | |
Clean line 1 | |
Clean line 2 | |
Clean line 3""" |
"line with content\n" + | ||
" \n" + | ||
"another line", | ||
"line with content\n" + | ||
"\n" + | ||
"another 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.
"line with content\n" + | |
" \n" + | |
"another line", | |
"line with content\n" + | |
"\n" + | |
"another line" | |
""" | |
line with content | |
\s | |
another line""", | |
""" | |
line with content | |
another line""" |
" Indented line \n" + | ||
"\tTab indented line\t\n" + | ||
" Mixed indentation\t ", | ||
" Indented line\n" + | ||
"\tTab indented line\n" + | ||
" Mixed indentation" |
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.
" Indented line \n" + | |
"\tTab indented line\t\n" + | |
" Mixed indentation\t ", | |
" Indented line\n" + | |
"\tTab indented line\n" + | |
" Mixed indentation" | |
""" | |
Indented line \s | |
Tab indented line | |
Mixed indentation """, | |
""" | |
Indented line | |
Tab indented line | |
Mixed indentation""" |
What's changed?
Recipe to remove trailing whitespace
What's your motivation?
Fixes #732
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Patterned after other RemoveTrailingWhitespace recipes in openrewrite/rewrite.
Checklist