From 3813539f7cb11e3f1c5ded511d4024f67362811c Mon Sep 17 00:00:00 2001 From: Jente Sondervorst Date: Tue, 17 Feb 2026 11:00:40 +0100 Subject: [PATCH 1/3] Prevent LombokValToFinalVar from removing star imports LombokValToFinalVar unconditionally called `maybeRemoveImport("lombok.var")` in `visitCompilationUnit` on every file matching the `MaybeUsesImport` precondition. This matched files with `import lombok.*;` even when no val/var was used. In multi-module projects with incomplete type information, `maybeRemoveImport` could not determine that other lombok types (like @Data, @Getter) were still referenced, and removed the entire star import. Now only calls `maybeRemoveImport` when there is an explicit `import lombok.var;` statement, leaving star imports untouched. Fixes #962 --- .../migrate/lombok/LombokValToFinalVar.java | 12 +- .../lombok/LombokValToFinalVarTest.java | 136 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java index b54e4a368f..ca7e561ee7 100644 --- a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java +++ b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java @@ -61,7 +61,17 @@ public TreeVisitor getVisitor() { private static class LombokValToFinalVarVisitor extends JavaIsoVisitor { @Override public J.CompilationUnit visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext ctx) { - maybeRemoveImport(LOMBOK_VAR); + // Only remove explicit `import lombok.var;`, not from star imports like `import lombok.*;` + // which may also cover other lombok types still in use. With incomplete type info + // (e.g. in multi-module projects), maybeRemoveImport on a star import can incorrectly + // remove the entire import. + for (J.Import imp : compilationUnit.getImports()) { + if (!imp.isStatic() && LOMBOK_VAR.equals(imp.getTypeName()) && + !"*".equals(imp.getQualid().getSimpleName())) { + maybeRemoveImport(LOMBOK_VAR); + break; + } + } return super.visitCompilationUnit(compilationUnit, ctx); } diff --git a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java index 680ff5157a..5520ca3253 100755 --- a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java @@ -21,6 +21,7 @@ import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.version; @@ -113,6 +114,141 @@ void bar() { ); } + @Test + void preserveStarImportWithoutVarUsage() { + //language=java + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion()) + .typeValidationOptions(TypeValidation.none()), + version( + java( + """ + import lombok.*; + + @AllArgsConstructor + @NoArgsConstructor + @ToString + @Data + @EqualsAndHashCode + public class AuthHeaders { + String authHeader; + String token; + } + """ + ), + 17 + ) + ); + } + + @Test + void preserveStarImportWithVarUsage() { + //language=java + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion()) + .typeValidationOptions(TypeValidation.none()), + version( + java( + """ + import lombok.*; + + @Data + class A { + void bar() { + var foo = "foo"; + } + } + """ + ), + 17 + ) + ); + } + + @Test + void removeExplicitVarImport() { + //language=java + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion()) + .typeValidationOptions(TypeValidation.none()), + version( + java( + """ + import lombok.var; + class A { + void bar() { + var foo = "foo"; + } + } + """, + """ + class A { + void bar() { + var foo = "foo"; + } + } + """ + ), + 17 + ) + ); + } + + @Test + void removeStarImportWhenOnlyValUsed() { + //language=java + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion()) + .typeValidationOptions(TypeValidation.none()), + version( + java( + """ + import lombok.*; + class A { + void bar() { + val foo = "foo"; + } + } + """, + """ + class A { + void bar() { + final var foo = "foo"; + } + } + """ + ), + 17 + ) + ); + } + + @Test + void starImportRemainsWhenOnlyVarUsed() { + // When `import lombok.*;` exists only for `var`, the star import remains unused after + // the recipe runs. This is acceptable: removing a star import with incomplete type info + // (e.g. in multi-module projects) risks breaking compilation by dropping imports that + // other lombok types still need. An unused import is preferable to broken code. + //language=java + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion()) + .typeValidationOptions(TypeValidation.none()), + version( + java( + """ + import lombok.*; + class A { + void bar() { + var foo = "foo"; + } + } + """ + ), + 17 + ) + ); + } + @Nested @SuppressWarnings({"StatementWithEmptyBody", "RedundantOperationOnEmptyContainer"}) class ValInLoop { From cf89ffde4008bfddd3a93c0dc7df9d287b98c9e5 Mon Sep 17 00:00:00 2001 From: Jente Sondervorst Date: Tue, 17 Feb 2026 11:06:24 +0100 Subject: [PATCH 2/3] Apply suggestions from code review --- .../java/migrate/lombok/LombokValToFinalVarTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java index 5520ca3253..44ca317d1a 100755 --- a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java @@ -227,7 +227,7 @@ void bar() { void starImportRemainsWhenOnlyVarUsed() { // When `import lombok.*;` exists only for `var`, the star import remains unused after // the recipe runs. This is acceptable: removing a star import with incomplete type info - // (e.g. in multi-module projects) risks breaking compilation by dropping imports that + // risks breaking compilation by dropping imports that // other lombok types still need. An unused import is preferable to broken code. //language=java rewriteRun( From c55978fd0566922c11620647ceeb2ab7d45a0d9a Mon Sep 17 00:00:00 2001 From: Jente Sondervorst Date: Tue, 17 Feb 2026 11:06:46 +0100 Subject: [PATCH 3/3] Apply suggestion from @Jenson3210 --- .../openrewrite/java/migrate/lombok/LombokValToFinalVar.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java index ca7e561ee7..2ba6a51c4b 100644 --- a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java +++ b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java @@ -61,10 +61,6 @@ public TreeVisitor getVisitor() { private static class LombokValToFinalVarVisitor extends JavaIsoVisitor { @Override public J.CompilationUnit visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext ctx) { - // Only remove explicit `import lombok.var;`, not from star imports like `import lombok.*;` - // which may also cover other lombok types still in use. With incomplete type info - // (e.g. in multi-module projects), maybeRemoveImport on a star import can incorrectly - // remove the entire import. for (J.Import imp : compilationUnit.getImports()) { if (!imp.isStatic() && LOMBOK_VAR.equals(imp.getTypeName()) && !"*".equals(imp.getQualid().getSimpleName())) {