Skip to content

Commit 30a60ba

Browse files
timtebeekTeamModerne
timtebeek
and
TeamModerne
authored
OpenRewrite recipe best practices (#869)
* OpenRewrite recipe best practices Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.recipes.rewrite.OpenRewriteRecipeBestPractices?organizationId=QUxML01vZGVybmUvTW9kZXJuZSArIE9wZW5SZXdyaXRl Co-authored-by: Moderne <team@moderne.io> * Use rewrite-java-25 test runtime, revert preview-feature unnamed lambda parameters `_` unnamed variables are still a preview feature when compiled without `--enable-preview`, so the recipe-best-practices run that introduced `_ ->` broke `compileTestJava`. Revert those two lambda parameters back to `cu ->` and bump the test runtime parser to rewrite-java-25. --------- Co-authored-by: Moderne <team@moderne.io>
1 parent 98d350e commit 30a60ba

12 files changed

Lines changed: 71 additions & 103 deletions

‎build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ dependencies {
4141

4242
testImplementation("com.google.code.gson:gson:latest.release")
4343

44-
testRuntimeOnly("org.openrewrite:rewrite-java-21")
44+
testRuntimeOnly("org.openrewrite:rewrite-java-25")
4545
testRuntimeOnly("com.google.code.findbugs:jsr305:latest.release")
4646
}
4747

‎src/test/java/org/openrewrite/staticanalysis/AnnotateNullableParametersTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ public PersonBuilder setName(@Nullable String name) {
864864

865865
@Test
866866
void provideAdditionalNullCheckingMethods() {
867-
List<String> additionalNullCheckingMethods = List.of("org.my.util.Text hasText(java.lang.String)", "org.my.util.Text isEmptyOrNull(java.lang.String)");
867+
var additionalNullCheckingMethods = List.of("org.my.util.Text hasText(java.lang.String)", "org.my.util.Text isEmptyOrNull(java.lang.String)");
868868
rewriteRun(
869869
spec -> spec.recipe(new AnnotateNullableParameters(null, additionalNullCheckingMethods)),
870870
//language=java
@@ -912,7 +912,7 @@ public PersonBuilder setInfo(@Nullable String name, @Nullable String lastName) {
912912

913913
@Test
914914
void unchangedWhenParameterDereferencedBeforeNullCheckingMethod() {
915-
List<String> additionalNullCheckingMethods = List.of("org.my.util.Text isEmptyOrNull(java.lang.String)");
915+
var additionalNullCheckingMethods = List.of("org.my.util.Text isEmptyOrNull(java.lang.String)");
916916
rewriteRun(
917917
spec -> spec.recipe(new AnnotateNullableParameters(null, additionalNullCheckingMethods)),
918918
//language=java

‎src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@
2828
import org.openrewrite.test.RewriteTest;
2929
import org.openrewrite.test.SourceSpec;
3030

31-
import static java.util.Collections.emptySet;
32-
import static java.util.Collections.singletonList;
31+
import java.util.List;
32+
import java.util.Set;
33+
3334
import static org.openrewrite.java.Assertions.java;
3435

3536
@SuppressWarnings({"ConstantConditions", "EnhancedSwitchMigration", "SwitchStatementWithTooFewBranches", "DefaultNotLastCaseInSwitch"})
@@ -135,9 +136,9 @@ class Test {
135136
void skipIfLastAndSharedWithCase() {
136137
rewriteRun(
137138
spec -> spec.parser(
138-
JavaParser.fromJavaVersion().styles(singletonList(new NamedStyles(
139-
Tree.randomId(), "test", "test", "test", emptySet(),
140-
singletonList(new DefaultComesLastStyle(true)))))
139+
JavaParser.fromJavaVersion().styles(List.of(new NamedStyles(
140+
Tree.randomId(), "test", "test", "test", Set.of(),
141+
List.of(new DefaultComesLastStyle(true)))))
141142
),
142143
//language=java
143144
java(

‎src/test/java/org/openrewrite/staticanalysis/EmptyBlockTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
import org.openrewrite.test.RecipeSpec;
2525
import org.openrewrite.test.RewriteTest;
2626

27-
import static java.util.Collections.emptySet;
28-
import static java.util.Collections.singleton;
27+
import java.util.Set;
28+
2929
import static org.openrewrite.Tree.randomId;
3030
import static org.openrewrite.java.Assertions.java;
3131

@@ -82,9 +82,9 @@ public class A {
8282
"Checkstyle",
8383
"Checkstyle",
8484
"Checkstyle defaults to only preserving blocks but the recipe should support other configurations.",
85-
emptySet(),
86-
singleton(Checkstyle.emptyBlock().withBlockPolicy(
87-
EmptyBlockStyle.BlockPolicy.TEXT))
85+
Set.of(),
86+
Set.of(Checkstyle.emptyBlock().withBlockPolicy(
87+
EmptyBlockStyle.BlockPolicy.TEXT))
8888
)
8989
)
9090
)

‎src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@
2626
import org.openrewrite.test.RecipeSpec;
2727
import org.openrewrite.test.RewriteTest;
2828

29-
import static java.util.Collections.emptySet;
30-
import static java.util.Collections.singletonList;
29+
import java.util.List;
30+
import java.util.Set;
31+
3132
import static org.openrewrite.java.Assertions.java;
3233

3334
@SuppressWarnings({"EnhancedSwitchMigration", "ConstantConditions", "StatementWithEmptyBody", "SwitchStatementWithTooFewBranches", "ReassignedVariable", "UnusedAssignment"})
@@ -202,10 +203,10 @@ public class A {
202203
void checkLastCaseGroupAddsBreakToLastCase() {
203204
rewriteRun(
204205
spec -> spec.parser(JavaParser.fromJavaVersion().styles(
205-
singletonList(
206+
List.of(
206207
new NamedStyles(
207-
Tree.randomId(), "test", "test", "test", emptySet(),
208-
singletonList(new FallThroughStyle(true)))))
208+
Tree.randomId(), "test", "test", "test", Set.of(),
209+
List.of(new FallThroughStyle(true)))))
209210
),
210211
//language=java
211212
java(

‎src/test/java/org/openrewrite/staticanalysis/HiddenFieldTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
import org.openrewrite.test.RecipeSpec;
2727
import org.openrewrite.test.RewriteTest;
2828

29+
import java.util.List;
30+
import java.util.Set;
2931
import java.util.function.Consumer;
3032
import java.util.function.UnaryOperator;
3133

32-
import static java.util.Collections.emptySet;
33-
import static java.util.Collections.singletonList;
3434
import static org.openrewrite.java.Assertions.java;
3535

3636
@SuppressWarnings("UnnecessaryLocalVariable")
@@ -69,10 +69,10 @@ public A(String field1) {
6969

7070
private static Consumer<RecipeSpec> hiddenFieldStyle(UnaryOperator<HiddenFieldStyle> with) {
7171
return spec -> spec.parser(JavaParser.fromJavaVersion().styles(
72-
singletonList(
72+
List.of(
7373
new NamedStyles(
74-
Tree.randomId(), "test", "test", "test", emptySet(),
75-
singletonList(with.apply(Checkstyle.hiddenFieldStyle())))))
74+
Tree.randomId(), "test", "test", "test", Set.of(),
75+
List.of(with.apply(Checkstyle.hiddenFieldStyle())))))
7676
);
7777
}
7878

‎src/test/java/org/openrewrite/staticanalysis/HideUtilityClassConstructorTest.java‎

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,9 @@
2626
import org.openrewrite.test.SourceSpec;
2727

2828
import java.util.List;
29+
import java.util.Set;
2930
import java.util.function.Consumer;
3031

31-
import static java.util.Collections.emptySet;
32-
import static java.util.Collections.singletonList;
3332
import static org.openrewrite.Tree.randomId;
3433
import static org.openrewrite.java.Assertions.java;
3534
import static org.openrewrite.java.Assertions.version;
@@ -41,9 +40,7 @@ public void defaults(RecipeSpec spec) {
4140
spec.recipe(new HideUtilityClassConstructor());
4241
}
4342

44-
/**
45-
* Should be a utility class since all methods are static, but class has public constructor
46-
*/
43+
/// Should be a utility class since all methods are static, but class has public constructor
4744
@DocumentExample
4845
@Test
4946
void changePublicConstructorToPrivate() {
@@ -222,9 +219,7 @@ public static void utility() {
222219
);
223220
}
224221

225-
/**
226-
* Should be a utility class since all methods are static, but class has no constructor (default/package-private)
227-
*/
222+
/// Should be a utility class since all methods are static, but class has no constructor (default/package-private)
228223
@Test
229224
void addPrivateConstructorWhenOnlyDefaultConstructor() {
230225
rewriteRun(
@@ -274,9 +269,7 @@ public static void main(String[] args) {
274269
);
275270
}
276271

277-
/**
278-
* Should be a utility class since all fields are static, but class has public constructor
279-
*/
272+
/// Should be a utility class since all fields are static, but class has public constructor
280273
@Test
281274
void identifyUtilityClassesOnlyStaticFields() {
282275
rewriteRun(
@@ -302,9 +295,7 @@ private A() {
302295
);
303296
}
304297

305-
/**
306-
* Not a utility class since the class implements an interface
307-
*/
298+
/// Not a utility class since the class implements an interface
308299
@Test
309300
void identifyNonUtilityClassesWhenImplementsInterface() {
310301
rewriteRun(
@@ -333,9 +324,7 @@ public static void utility() {
333324
);
334325
}
335326

336-
/**
337-
* Not a utility class since the class extends another
338-
*/
327+
/// Not a utility class since the class extends another
339328
@Test
340329
void identifyNonUtilityClassesWhenExtendsClass() {
341330
rewriteRun(
@@ -363,9 +352,7 @@ public static void doSomething() {
363352
);
364353
}
365354

366-
/**
367-
* Not a utility class since some fields are static, but at least one non-static
368-
*/
355+
/// Not a utility class since some fields are static, but at least one non-static
369356
@Test
370357
void identifyNonUtilityClassesMixedFields() {
371358
rewriteRun(
@@ -385,9 +372,7 @@ public A() {
385372
);
386373
}
387374

388-
/**
389-
* Should be a utility class since all methods are static, but class has public constructor
390-
*/
375+
/// Should be a utility class since all methods are static, but class has public constructor
391376
@Test
392377
void identifyUtilityClassesOnlyStaticMethods() {
393378
rewriteRun(
@@ -423,9 +408,7 @@ public static void utility(String[] args) {
423408
);
424409
}
425410

426-
/**
427-
* Inner class should be a utility class since all it's methods are static, but it has public constructor
428-
*/
411+
/// Inner class should be a utility class since all it's methods are static, but it has public constructor
429412
@Test
430413
void identifyUtilityClassesInnerStaticClasses() {
431414
rewriteRun(
@@ -465,9 +448,7 @@ public void utility() {
465448
);
466449
}
467450

468-
/**
469-
* Not a utility class since some methods are static, but at least one non-static
470-
*/
451+
/// Not a utility class since some methods are static, but at least one non-static
471452
@Test
472453
void identifyNonUtilityClassesMixedMethods() {
473454
rewriteRun(
@@ -604,9 +585,9 @@ public static String someBean() {
604585

605586
private static Consumer<RecipeSpec> hideUtilityClassConstructor(String... ignoreIfAnnotatedBy) {
606587
return spec -> spec.parser(JavaParser.fromJavaVersion().styles(
607-
singletonList(
588+
List.of(
608589
new NamedStyles(
609-
randomId(), "test", "test", "test", emptySet(), singletonList(
590+
randomId(), "test", "test", "test", Set.of(), List.of(
610591
new HideUtilityClassConstructorStyle(List.of(ignoreIfAnnotatedBy))
611592
)
612593
)

‎src/test/java/org/openrewrite/staticanalysis/NeedBracesTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
import org.openrewrite.test.RecipeSpec;
2727
import org.openrewrite.test.RewriteTest;
2828

29+
import java.util.List;
30+
import java.util.Set;
2931
import java.util.function.Consumer;
3032
import java.util.function.UnaryOperator;
3133

32-
import static java.util.Collections.emptySet;
33-
import static java.util.Collections.singletonList;
3434
import static org.openrewrite.java.Assertions.java;
3535

3636
@SuppressWarnings({
@@ -157,10 +157,10 @@ static void addToForEach(int[] arr) {
157157

158158
private static Consumer<RecipeSpec> needsBraces(UnaryOperator<NeedBracesStyle> with) {
159159
return spec -> spec.parser(JavaParser.fromJavaVersion().styles(
160-
singletonList(
160+
List.of(
161161
new NamedStyles(
162-
Tree.randomId(), "test", "test", "test", emptySet(),
163-
singletonList(with.apply(Checkstyle.needBracesStyle())))))
162+
Tree.randomId(), "test", "test", "test", Set.of(),
163+
List.of(with.apply(Checkstyle.needBracesStyle())))))
164164
);
165165
}
166166

‎src/test/java/org/openrewrite/staticanalysis/OperatorWrapTest.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@
2929
import org.openrewrite.test.SourceSpec;
3030

3131
import java.util.List;
32+
import java.util.Set;
3233
import java.util.function.Consumer;
3334
import java.util.function.UnaryOperator;
3435

35-
import static java.util.Collections.emptySet;
36-
import static java.util.Collections.singletonList;
3736
import static org.assertj.core.api.Assertions.assertThat;
3837
import static org.openrewrite.java.Assertions.java;
3938

@@ -77,10 +76,10 @@ private static List<NamedStyles> operatorWrapStyle() {
7776
}
7877

7978
private static List<NamedStyles> operatorWrapStyle(UnaryOperator<OperatorWrapStyle> with) {
80-
return singletonList(
79+
return List.of(
8180
new NamedStyles(
82-
Tree.randomId(), "test", "test", "test", emptySet(),
83-
singletonList(with.apply(Checkstyle.operatorWrapStyle()))
81+
Tree.randomId(), "test", "test", "test", Set.of(),
82+
List.of(with.apply(Checkstyle.operatorWrapStyle()))
8483
)
8584
);
8685
}

‎src/test/java/org/openrewrite/staticanalysis/ReplaceRedundantFormatWithPrintfTest.java‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,7 @@ void test(String arg) {
222222
);
223223
}
224224

225-
/**
226-
* Tests that the code generation used for JavaTemplate behaves correctly given a parameterized type.
227-
*/
225+
/// Tests that the code generation used for JavaTemplate behaves correctly given a parameterized type.
228226
@Test
229227
void doesNotFailWhenArgHasParameterizedType() {
230228
rewriteRun(
@@ -250,9 +248,7 @@ void test() {
250248
);
251249
}
252250

253-
/**
254-
* Tests that the code generation used for JavaTemplate behaves correctly given a primitive type.
255-
*/
251+
/// Tests that the code generation used for JavaTemplate behaves correctly given a primitive type.
256252
@Test
257253
void doesNotFailWhenArgHasPrimitiveType() {
258254
rewriteRun(
@@ -278,9 +274,7 @@ void test() {
278274
);
279275
}
280276

281-
/**
282-
* Tests that the code generation used for JavaTemplate behaves correctly given a template parameter.
283-
*/
277+
/// Tests that the code generation used for JavaTemplate behaves correctly given a template parameter.
284278
@Test
285279
void doesNotFailWhenArgHasTemplateParameter() {
286280
rewriteRun(

0 commit comments

Comments
 (0)
⚡