Skip to content

Commit 5695ab6

Browse files
steve-aom-elliotttimtebeek
steve-aom-elliott
and
timtebeek
authored
Handling cases for AddCommentToMethodInvocations where single line comments would break code and simplifying recipe. (#5793)
* Handling cases where single line comments would break code and improving the formatting of the comments themselves. * Heavily simplified behaviour of recipe so that it always produces a single line block-style comment (it would have needed to be able to detect whether it was safe to create multiple lines otherwise) * Slight polish * Use a simple replace for non regex --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent ae2e27c commit 5695ab6

File tree

2 files changed

+75
-145
lines changed

2 files changed

+75
-145
lines changed

‎rewrite-java/src/main/java/org/openrewrite/java/AddCommentToMethodInvocations.java‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,13 @@
1717

1818
import lombok.EqualsAndHashCode;
1919
import lombok.Value;
20-
import org.jspecify.annotations.Nullable;
2120
import org.openrewrite.*;
2221
import org.openrewrite.internal.ListUtils;
2322
import org.openrewrite.java.search.UsesMethod;
24-
import org.openrewrite.java.tree.Comment;
25-
import org.openrewrite.java.tree.J;
26-
import org.openrewrite.java.tree.TextComment;
23+
import org.openrewrite.java.tree.*;
2724
import org.openrewrite.marker.Markers;
2825

2926
import java.util.List;
30-
import java.util.regex.Matcher;
31-
import java.util.regex.Pattern;
3227

3328
@Value
3429
@EqualsAndHashCode(callSuper = false)
@@ -53,14 +48,6 @@ public String getDescription() {
5348
example = "java.util.List add*(..)")
5449
String methodPattern;
5550

56-
@Option(displayName = "Multiline",
57-
description = "Comments use by default single line // but they can use multiline /* */.",
58-
required = false)
59-
@Nullable
60-
Boolean isMultiline;
61-
62-
private static final Pattern NEWLINE = Pattern.compile("\\R");
63-
6451
@Override
6552
public Validated<Object> validate() {
6653
return super.validate().and(MethodMatcher.validate(methodPattern));
@@ -73,16 +60,15 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
7360
@Override
7461
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
7562
J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
76-
7763
if (methodMatcher.matches(m)) {
78-
String methodPrefixWhitespace = m.getPrefix().getWhitespace();
79-
80-
boolean createMultiline = Boolean.TRUE.equals(isMultiline);
81-
Matcher matcher = NEWLINE.matcher(comment);
82-
String newCommentText = matcher.find() ? matcher.replaceAll(createMultiline ? methodPrefixWhitespace : " ") : comment;
83-
64+
String prefixWhitespace = m.getPrefix().getWhitespace();
65+
String newCommentText = comment.trim()
66+
/* First Line * Second Line */
67+
.replaceAll("\\R", " * ")
68+
// Prevent closing the comment early
69+
.replace("*/", "*");
8470
if (doesNotHaveComment(newCommentText, m.getComments())) {
85-
TextComment textComment = new TextComment(createMultiline, newCommentText, methodPrefixWhitespace, Markers.EMPTY);
71+
TextComment textComment = new TextComment(true, " " + newCommentText + " ", prefixWhitespace, Markers.EMPTY);
8672
return m.withComments(ListUtils.concat(m.getComments(), textComment));
8773
}
8874
}
@@ -92,7 +78,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
9278
private boolean doesNotHaveComment(String lookFor, List<Comment> comments) {
9379
for (Comment c : comments) {
9480
if (c instanceof TextComment &&
95-
lookFor.equals(((TextComment) c).getText())) {
81+
lookFor.trim().equals(((TextComment) c).getText().trim())) {
9682
return false;
9783
}
9884
}

‎rewrite-java/src/test/java/org/openrewrite/java/AddCommentToMethodInvocationsTest.java‎

Lines changed: 66 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@
2323
import static org.openrewrite.java.Assertions.java;
2424

2525
class AddCommentToMethodInvocationsTest implements RewriteTest {
26-
private static final String SHORT_COMMENT = " Short comment to add";
27-
private static final String LONG_COMMENT = " This is a very long comment to add. The comment uses multiline comments, not single line.";
26+
private static final String SHORT_COMMENT = "Short comment to add";
27+
private static final String LONG_COMMENT = "This is a very long comment to add.\nThe comment uses multiple lines.";
28+
private static final String HEAVY_WRAP_COMMENT = "\nLine 1\nLine 2\nLine 3\n";
2829

2930
@Override
3031
public void defaults(RecipeSpec spec) {
@@ -33,6 +34,12 @@ public void defaults(RecipeSpec spec) {
3334
package foo;
3435
public class Foo {
3536
public void bar(String arg) {}
37+
public boolean gar() {
38+
return true;
39+
}
40+
public String har(boolean arg) {
41+
return "";
42+
}
3643
}
3744
"""
3845
));
@@ -42,58 +49,36 @@ public void bar(String arg) {}
4249
@Test
4350
void addSingleLineComment() {
4451
rewriteRun(
45-
spec -> spec.recipe(new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo bar(..)", false)),
52+
spec -> spec.recipes(
53+
new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo bar(..)"),
54+
new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo gar(..)")
55+
),
4656
//language=java
4757
java(
4858
"""
4959
import foo.Foo;
50-
60+
5161
class Other {
5262
void method() {
5363
Foo foo = new Foo();
64+
// Existing Comment
5465
foo.bar("a");
66+
boolean gar = /* Existing Comment */ foo.gar();
67+
String har = foo.har(/* Existing Comment */foo.gar());
5568
}
5669
}
5770
""",
5871
"""
5972
import foo.Foo;
60-
61-
class Other {
62-
void method() {
63-
Foo foo = new Foo();
64-
// Short comment to add
65-
foo.bar("a");
66-
}
67-
}
68-
"""
69-
)
70-
);
71-
}
72-
73-
@Test
74-
void addLongComment() {
75-
rewriteRun(
76-
spec -> spec.recipe(new AddCommentToMethodInvocations(LONG_COMMENT, "foo.Foo bar(..)", true)),
77-
//language=java
78-
java(
79-
"""
80-
import foo.Foo;
81-
82-
class Other {
83-
void method() {
84-
Foo foo = new Foo();
85-
foo.bar("a");
86-
}
87-
}
88-
""",
89-
"""
90-
import foo.Foo;
91-
73+
9274
class Other {
9375
void method() {
9476
Foo foo = new Foo();
95-
/* This is a very long comment to add. The comment uses multiline comments, not single line.*/
77+
// Existing Comment
78+
/* Short comment to add */
9679
foo.bar("a");
80+
boolean gar = /* Existing Comment */ /* Short comment to add */ foo.gar();
81+
String har = foo.har(/* Existing Comment *//* Short comment to add */foo.gar());
9782
}
9883
}
9984
"""
@@ -104,62 +89,40 @@ void method() {
10489
@Test
10590
void addMultilineComment() {
10691
rewriteRun(
107-
spec -> spec.recipe(new AddCommentToMethodInvocations("\nLine 1\nLine 2\nLine 3\n", "foo.Foo bar(..)", true)),
92+
spec -> spec.recipes(
93+
new AddCommentToMethodInvocations(LONG_COMMENT, "foo.Foo bar(..)"),
94+
new AddCommentToMethodInvocations(LONG_COMMENT, "foo.Foo gar(..)")
95+
),
10896
//language=java
10997
java(
11098
"""
11199
import foo.Foo;
112-
113-
class Other {
114-
void method() {
115-
Foo foo = new Foo();
116-
foo.bar("a");
117-
}
118-
}
119-
""",
120-
"""
121-
import foo.Foo;
122-
100+
123101
class Other {
124102
void method() {
125103
Foo foo = new Foo();
126104
/*
127-
Line 1
128-
Line 2
129-
Line 3
130-
*/
131-
foo.bar("a");
132-
}
133-
}
134-
"""
135-
)
136-
);
137-
}
138-
139-
@Test
140-
void addMultilineCommentOnSingleLine() {
141-
rewriteRun(
142-
spec -> spec.recipe(new AddCommentToMethodInvocations("\nLine 1\nLine 2\nLine 3\n", "foo.Foo bar(..)", false)),
143-
//language=java
144-
java(
145-
"""
146-
import foo.Foo;
147-
148-
class Other {
149-
void method() {
150-
Foo foo = new Foo();
105+
* Existing Comment
106+
*/
151107
foo.bar("a");
108+
boolean gar = /* Existing Comment */ foo.gar();
109+
String har = foo.har(/* Existing Comment */foo.gar());
152110
}
153111
}
154112
""",
155113
"""
156114
import foo.Foo;
157-
115+
158116
class Other {
159117
void method() {
160118
Foo foo = new Foo();
161-
// Line 1 Line 2 Line 3\s
119+
/*
120+
* Existing Comment
121+
*/
122+
/* This is a very long comment to add. * The comment uses multiple lines. */
162123
foo.bar("a");
124+
boolean gar = /* Existing Comment */ /* This is a very long comment to add. * The comment uses multiple lines. */ foo.gar();
125+
String har = foo.har(/* Existing Comment *//* This is a very long comment to add. * The comment uses multiple lines. */foo.gar());
163126
}
164127
}
165128
"""
@@ -168,33 +131,36 @@ void method() {
168131
}
169132

170133
@Test
171-
void addSingleLineCommentToExistingSingleLineComments() {
134+
void addAndSimplifyHeavilyWrappedComment() {
172135
rewriteRun(
173-
spec -> spec.recipe(new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo bar(..)", false)),
136+
spec -> spec.recipes(
137+
new AddCommentToMethodInvocations(HEAVY_WRAP_COMMENT, "foo.Foo bar(..)"),
138+
new AddCommentToMethodInvocations(HEAVY_WRAP_COMMENT, "foo.Foo gar(..)")
139+
),
174140
//language=java
175141
java(
176142
"""
177143
import foo.Foo;
178-
144+
179145
class Other {
180146
void method() {
181147
Foo foo = new Foo();
182-
// Existing single line comment
183-
// Another existing single line comment
184148
foo.bar("a");
149+
boolean gar = foo.gar();
150+
String har = foo.har(foo.gar());
185151
}
186152
}
187153
""",
188154
"""
189155
import foo.Foo;
190-
156+
191157
class Other {
192158
void method() {
193159
Foo foo = new Foo();
194-
// Existing single line comment
195-
// Another existing single line comment
196-
// Short comment to add
160+
/* Line 1 * Line 2 * Line 3 */
197161
foo.bar("a");
162+
boolean gar = /* Line 1 * Line 2 * Line 3 */ foo.gar();
163+
String har = foo.har(/* Line 1 * Line 2 * Line 3 */foo.gar());
198164
}
199165
}
200166
"""
@@ -203,37 +169,24 @@ void method() {
203169
}
204170

205171
@Test
206-
void addSingleLineCommentToExistingMultiLineComment() {
172+
void doesNotAddCommentIfAlreadyAdded() {
207173
rewriteRun(
208-
spec -> spec.recipe(new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo bar(..)", false)),
174+
spec -> spec.recipes(
175+
new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo bar(..)"),
176+
new AddCommentToMethodInvocations(SHORT_COMMENT, "foo.Foo gar(..)")
177+
),
209178
//language=java
210179
java(
211180
"""
212181
import foo.Foo;
213-
182+
214183
class Other {
215184
void method() {
216185
Foo foo = new Foo();
217-
/*
218-
* Existing multi line
219-
* comment
220-
*/
221-
foo.bar("a");
222-
}
223-
}
224-
""",
225-
"""
226-
import foo.Foo;
227-
228-
class Other {
229-
void method() {
230-
Foo foo = new Foo();
231-
/*
232-
* Existing multi line
233-
* comment
234-
*/
235186
// Short comment to add
236187
foo.bar("a");
188+
boolean gar = /* Short comment to add */ foo.gar();
189+
String har = foo.har(/* Short comment to add */foo.gar());
237190
}
238191
}
239192
"""
@@ -242,36 +195,27 @@ void method() {
242195
}
243196

244197
@Test
245-
void addLongCommentToExistingMultiLineComment() {
198+
void escapeClosingTag() {
246199
rewriteRun(
247-
spec -> spec.recipe(new AddCommentToMethodInvocations(LONG_COMMENT, "foo.Foo bar(..)", true)),
200+
spec -> spec.recipe(new AddCommentToMethodInvocations("this is a */ terrible idea", "foo.Foo bar(..)")
201+
),
248202
//language=java
249203
java(
250204
"""
251205
import foo.Foo;
252-
206+
253207
class Other {
254-
void method() {
255-
Foo foo = new Foo();
256-
/*
257-
* Existing multi line
258-
* comment
259-
*/
208+
void method(Foo foo) {
260209
foo.bar("a");
261210
}
262211
}
263212
""",
264213
"""
265214
import foo.Foo;
266-
215+
267216
class Other {
268-
void method() {
269-
Foo foo = new Foo();
270-
/*
271-
* Existing multi line
272-
* comment
273-
*/
274-
/* This is a very long comment to add. The comment uses multiline comments, not single line.*/
217+
void method(Foo foo) {
218+
/* this is a * terrible idea */
275219
foo.bar("a");
276220
}
277221
}

0 commit comments

Comments
 (0)