Skip to content

Commit d09e755

Browse files
timtebeektimtebeek
committed
Fix nested-class clobbering and move recipe to best-practices
The argument-replacement visitor descended into nested/local classes and rewrote their super(..)/this(..) calls with the outer constructor's argument list. Guard against descending into nested class declarations. Also move ExtractExplicitConstructorInvocationArguments out of the default UpgradeToJava25 upgrade and into JavaBestPractices, since it restyles already-valid code rather than migrating it. Add tests for the nested-class case, varargs bail, statements before super, static-field extraction, and name-collision avoidance.
1 parent ea059e8 commit d09e755

4 files changed

Lines changed: 195 additions & 1 deletion

File tree

‎src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
135135
}
136136
}
137137
return (J.MethodDeclaration) new JavaIsoVisitor<ExecutionContext>() {
138+
@Override
139+
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx2) {
140+
// Do not descend into nested/local classes; their own `super(..)`/`this(..)` calls
141+
// are handled by their own constructor visits, not this one's argument list.
142+
return classDecl;
143+
}
144+
138145
@Override
139146
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx2) {
140147
mi = super.visitMethodInvocation(mi, ctx2);

‎src/main/resources/META-INF/rewrite/java-best-practices.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ recipeList:
3434
# Instance main methods (JEP 512); not moved into the version upgrade as it does not yet work for Spring Boot
3535
# https://github.com/spring-projects/spring-boot/issues/35785
3636
- org.openrewrite.java.migrate.lang.MigrateMainMethodToInstanceMain
37+
# Hoist complex super(..)/this(..) arguments into locals (JEP 513); opt-in, as it restyles working code
38+
- org.openrewrite.java.migrate.lang.ExtractExplicitConstructorInvocationArguments
3739
# Text blocks for strings without newlines (upgrade chain only does convertStringsWithoutNewlines: false)
3840
- org.openrewrite.java.migrate.lang.UseTextBlocks:
3941
convertStringsWithoutNewlines: true

‎src/main/resources/META-INF/rewrite/java-version-25.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ recipeList:
3434
- org.openrewrite.java.migrate.UpgradePluginsForJava25
3535
- org.openrewrite.java.migrate.io.ReplaceSystemOutWithIOPrint
3636
- org.openrewrite.java.migrate.lang.MigrateProcessWaitForDuration
37-
- org.openrewrite.java.migrate.lang.ExtractExplicitConstructorInvocationArguments
3837
- org.openrewrite.java.migrate.lang.ReplaceUnusedVariablesWithUnderscore
3938
- org.openrewrite.java.migrate.util.MigrateInflaterDeflaterToClose
4039
- org.openrewrite.java.migrate.util.MigrateStringReaderToReaderOf

‎src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,192 @@ class Child extends Parent {
313313
);
314314
}
315315

316+
@Test
317+
void doNotClobberLocalClassConstructorInvocation() {
318+
rewriteRun(
319+
//language=java
320+
java(
321+
"""
322+
class Base {
323+
Base(String s) {
324+
}
325+
}
326+
327+
class Parent {
328+
Parent(String name) {
329+
}
330+
}
331+
332+
class Child extends Parent {
333+
Child(String value) {
334+
super(value.trim());
335+
class Local extends Base {
336+
Local() {
337+
super("literal");
338+
}
339+
}
340+
}
341+
}
342+
""",
343+
"""
344+
class Base {
345+
Base(String s) {
346+
}
347+
}
348+
349+
class Parent {
350+
Parent(String name) {
351+
}
352+
}
353+
354+
class Child extends Parent {
355+
Child(String value) {
356+
String name = value.trim();
357+
super(name);
358+
class Local extends Base {
359+
Local() {
360+
super("literal");
361+
}
362+
}
363+
}
364+
}
365+
"""
366+
)
367+
);
368+
}
369+
370+
@Test
371+
void doNotExtractFromVarargsConstructor() {
372+
rewriteRun(
373+
//language=java
374+
java(
375+
"""
376+
class Parent {
377+
Parent(String... values) {
378+
}
379+
}
380+
381+
class Child extends Parent {
382+
Child(String value) {
383+
super(value.trim(), value.strip());
384+
}
385+
}
386+
"""
387+
)
388+
);
389+
}
390+
391+
@Test
392+
void insertDeclarationsAfterExistingStatementsBeforeSuper() {
393+
rewriteRun(
394+
//language=java
395+
java(
396+
"""
397+
class Parent {
398+
Parent(int value) {
399+
}
400+
}
401+
402+
class Child extends Parent {
403+
Child(String value) {
404+
System.out.println("before");
405+
super(Integer.parseInt(value));
406+
}
407+
}
408+
""",
409+
"""
410+
class Parent {
411+
Parent(int value) {
412+
}
413+
}
414+
415+
class Child extends Parent {
416+
Child(String value) {
417+
System.out.println("before");
418+
int value1 = Integer.parseInt(value);
419+
super(value1);
420+
}
421+
}
422+
"""
423+
)
424+
);
425+
}
426+
427+
@Test
428+
void extractStaticFieldArgumentToPreserveOrder() {
429+
rewriteRun(
430+
//language=java
431+
java(
432+
"""
433+
class Parent {
434+
Parent(int a, int b) {
435+
}
436+
}
437+
438+
class Child extends Parent {
439+
static final int CONST = 1;
440+
441+
Child(String value) {
442+
super(CONST, Integer.parseInt(value));
443+
}
444+
}
445+
""",
446+
"""
447+
class Parent {
448+
Parent(int a, int b) {
449+
}
450+
}
451+
452+
class Child extends Parent {
453+
static final int CONST = 1;
454+
455+
Child(String value) {
456+
int a = CONST;
457+
int b = Integer.parseInt(value);
458+
super(a, b);
459+
}
460+
}
461+
"""
462+
)
463+
);
464+
}
465+
466+
@Test
467+
void avoidNameCollisionWithLaterDeclaredVariable() {
468+
rewriteRun(
469+
//language=java
470+
java(
471+
"""
472+
class Parent {
473+
Parent(int value) {
474+
}
475+
}
476+
477+
class Child extends Parent {
478+
Child(String value) {
479+
super(Integer.parseInt(value));
480+
int value1 = 5;
481+
}
482+
}
483+
""",
484+
"""
485+
class Parent {
486+
Parent(int value) {
487+
}
488+
}
489+
490+
class Child extends Parent {
491+
Child(String value) {
492+
int value2 = Integer.parseInt(value);
493+
super(value2);
494+
int value1 = 5;
495+
}
496+
}
497+
"""
498+
)
499+
);
500+
}
501+
316502
@Test
317503
void extractThisDelegationArgument() {
318504
rewriteRun(

0 commit comments

Comments
 (0)