Skip to content

Commit 0c1793f

Browse files
authored
Prevent LombokValToFinalVar from removing star imports (#986)
* 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 * Apply suggestions from code review * Apply suggestion from @Jenson3210
1 parent e91f59e commit 0c1793f

2 files changed

Lines changed: 143 additions & 1 deletion

File tree

src/main/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVar.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,13 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
6161
private static class LombokValToFinalVarVisitor extends JavaIsoVisitor<ExecutionContext> {
6262
@Override
6363
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext ctx) {
64-
maybeRemoveImport(LOMBOK_VAR);
64+
for (J.Import imp : compilationUnit.getImports()) {
65+
if (!imp.isStatic() && LOMBOK_VAR.equals(imp.getTypeName()) &&
66+
!"*".equals(imp.getQualid().getSimpleName())) {
67+
maybeRemoveImport(LOMBOK_VAR);
68+
break;
69+
}
70+
}
6571
return super.visitCompilationUnit(compilationUnit, ctx);
6672
}
6773

src/test/java/org/openrewrite/java/migrate/lombok/LombokValToFinalVarTest.java

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.openrewrite.java.JavaParser;
2222
import org.openrewrite.test.RecipeSpec;
2323
import org.openrewrite.test.RewriteTest;
24+
import org.openrewrite.test.TypeValidation;
2425

2526
import static org.openrewrite.java.Assertions.java;
2627
import static org.openrewrite.java.Assertions.version;
@@ -113,6 +114,141 @@ void bar() {
113114
);
114115
}
115116

117+
@Test
118+
void preserveStarImportWithoutVarUsage() {
119+
//language=java
120+
rewriteRun(
121+
spec -> spec.parser(JavaParser.fromJavaVersion())
122+
.typeValidationOptions(TypeValidation.none()),
123+
version(
124+
java(
125+
"""
126+
import lombok.*;
127+
128+
@AllArgsConstructor
129+
@NoArgsConstructor
130+
@ToString
131+
@Data
132+
@EqualsAndHashCode
133+
public class AuthHeaders {
134+
String authHeader;
135+
String token;
136+
}
137+
"""
138+
),
139+
17
140+
)
141+
);
142+
}
143+
144+
@Test
145+
void preserveStarImportWithVarUsage() {
146+
//language=java
147+
rewriteRun(
148+
spec -> spec.parser(JavaParser.fromJavaVersion())
149+
.typeValidationOptions(TypeValidation.none()),
150+
version(
151+
java(
152+
"""
153+
import lombok.*;
154+
155+
@Data
156+
class A {
157+
void bar() {
158+
var foo = "foo";
159+
}
160+
}
161+
"""
162+
),
163+
17
164+
)
165+
);
166+
}
167+
168+
@Test
169+
void removeExplicitVarImport() {
170+
//language=java
171+
rewriteRun(
172+
spec -> spec.parser(JavaParser.fromJavaVersion())
173+
.typeValidationOptions(TypeValidation.none()),
174+
version(
175+
java(
176+
"""
177+
import lombok.var;
178+
class A {
179+
void bar() {
180+
var foo = "foo";
181+
}
182+
}
183+
""",
184+
"""
185+
class A {
186+
void bar() {
187+
var foo = "foo";
188+
}
189+
}
190+
"""
191+
),
192+
17
193+
)
194+
);
195+
}
196+
197+
@Test
198+
void removeStarImportWhenOnlyValUsed() {
199+
//language=java
200+
rewriteRun(
201+
spec -> spec.parser(JavaParser.fromJavaVersion())
202+
.typeValidationOptions(TypeValidation.none()),
203+
version(
204+
java(
205+
"""
206+
import lombok.*;
207+
class A {
208+
void bar() {
209+
val foo = "foo";
210+
}
211+
}
212+
""",
213+
"""
214+
class A {
215+
void bar() {
216+
final var foo = "foo";
217+
}
218+
}
219+
"""
220+
),
221+
17
222+
)
223+
);
224+
}
225+
226+
@Test
227+
void starImportRemainsWhenOnlyVarUsed() {
228+
// When `import lombok.*;` exists only for `var`, the star import remains unused after
229+
// the recipe runs. This is acceptable: removing a star import with incomplete type info
230+
// risks breaking compilation by dropping imports that
231+
// other lombok types still need. An unused import is preferable to broken code.
232+
//language=java
233+
rewriteRun(
234+
spec -> spec.parser(JavaParser.fromJavaVersion())
235+
.typeValidationOptions(TypeValidation.none()),
236+
version(
237+
java(
238+
"""
239+
import lombok.*;
240+
class A {
241+
void bar() {
242+
var foo = "foo";
243+
}
244+
}
245+
"""
246+
),
247+
17
248+
)
249+
);
250+
}
251+
116252
@Nested
117253
@SuppressWarnings({"StatementWithEmptyBody", "RedundantOperationOnEmptyContainer"})
118254
class ValInLoop {

0 commit comments

Comments
 (0)