Skip to content

Commit 821100b

Browse files
committed
Fix FinalizePrivateFields breaking compilation when field is read in a lambda initializer
Skip private fields whose only assignment is in the constructor body when they are read inside a lambda or anonymous class in an instance field initializer or instance initializer block. Since initializers run before the constructor body, finalizing such fields breaks javac's definite assignment analysis. Fixes #861
1 parent e097fab commit 821100b

2 files changed

Lines changed: 114 additions & 0 deletions

File tree

src/main/java/org/openrewrite/staticanalysis/FinalizePrivateFields.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,18 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
8787
.map(Map.Entry::getKey)
8888
.collect(toSet());
8989

90+
// Skip fields only assigned in the constructor body that are read inside a lambda or
91+
// anonymous class in an instance initializer: finalizing them breaks javac's definite
92+
// assignment analysis, since initializers run before the constructor body.
93+
Set<JavaType.Variable> uninitializedFinalizable = privateFields.stream()
94+
.filter(v -> v.getInitializer() == null && v.getVariableType() != null)
95+
.map(J.VariableDeclarations.NamedVariable::getVariableType)
96+
.filter(privateFieldsToBeFinalized::contains)
97+
.collect(toSet());
98+
if (!uninitializedFinalizable.isEmpty()) {
99+
privateFieldsToBeFinalized.removeAll(findFieldsReadInDeferredInitializer(classDecl, uninitializedFinalizable));
100+
}
101+
90102
return super.visitClassDeclaration(classDecl, ctx);
91103
}
92104

@@ -154,6 +166,57 @@ private static boolean isInnerClass(J.ClassDeclaration classDecl) {
154166
return classDecl.getType() != null && classDecl.getType().getOwningClass() != null;
155167
}
156168

169+
private static Set<JavaType.Variable> findFieldsReadInDeferredInitializer(J.ClassDeclaration classDecl, Set<JavaType.Variable> candidates) {
170+
Set<JavaType.Variable> found = new HashSet<>();
171+
JavaIsoVisitor<Set<JavaType.Variable>> reader = new JavaIsoVisitor<Set<JavaType.Variable>>() {
172+
int lambdaOrAnonDepth;
173+
174+
@Override
175+
public J.Lambda visitLambda(J.Lambda lambda, Set<JavaType.Variable> set) {
176+
lambdaOrAnonDepth++;
177+
J.Lambda l = super.visitLambda(lambda, set);
178+
lambdaOrAnonDepth--;
179+
return l;
180+
}
181+
182+
@Override
183+
public J.NewClass visitNewClass(J.NewClass newClass, Set<JavaType.Variable> set) {
184+
boolean anonymous = newClass.getBody() != null;
185+
if (anonymous) {
186+
lambdaOrAnonDepth++;
187+
}
188+
J.NewClass nc = super.visitNewClass(newClass, set);
189+
if (anonymous) {
190+
lambdaOrAnonDepth--;
191+
}
192+
return nc;
193+
}
194+
195+
@Override
196+
public J.Identifier visitIdentifier(J.Identifier identifier, Set<JavaType.Variable> set) {
197+
if (lambdaOrAnonDepth > 0 && candidates.contains(identifier.getFieldType())) {
198+
set.add(identifier.getFieldType());
199+
}
200+
return super.visitIdentifier(identifier, set);
201+
}
202+
};
203+
for (Statement stmt : classDecl.getBody().getStatements()) {
204+
if (stmt instanceof J.VariableDeclarations) {
205+
J.VariableDeclarations vd = (J.VariableDeclarations) stmt;
206+
if (!vd.hasModifier(J.Modifier.Type.Static)) {
207+
for (J.VariableDeclarations.NamedVariable v : vd.getVariables()) {
208+
if (v.getInitializer() != null) {
209+
reader.visit(v.getInitializer(), found);
210+
}
211+
}
212+
}
213+
} else if (stmt instanceof J.Block && !((J.Block) stmt).isStatic()) {
214+
reader.visit(stmt, found);
215+
}
216+
}
217+
return found;
218+
}
219+
157220
private static class CollectPrivateFieldsAssignmentCounts extends JavaIsoVisitor<Map<JavaType.Variable, Integer>> {
158221

159222
/**

src/test/java/org/openrewrite/staticanalysis/FinalizePrivateFieldsTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,57 @@ public class Reproducer {
897897
);
898898
}
899899

900+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/861")
901+
@Test
902+
void fieldReadByLambdaInInstanceFieldInitializer() {
903+
rewriteRun(
904+
//language=java
905+
java(
906+
"""
907+
import java.util.function.Function;
908+
909+
public class Foo {
910+
private String name;
911+
protected Function<Throwable, Void> logAndAccept =
912+
throwable -> {
913+
System.out.println(name);
914+
return null;
915+
};
916+
917+
public Foo(String name) {
918+
this.name = name;
919+
}
920+
}
921+
"""
922+
)
923+
);
924+
}
925+
926+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/861")
927+
@Test
928+
void fieldReadByAnonymousClassInInstanceFieldInitializer() {
929+
rewriteRun(
930+
//language=java
931+
java(
932+
"""
933+
public class Foo {
934+
private String name;
935+
protected Runnable runner = new Runnable() {
936+
@Override
937+
public void run() {
938+
System.out.println(name);
939+
}
940+
};
941+
942+
public Foo(String name) {
943+
this.name = name;
944+
}
945+
}
946+
"""
947+
)
948+
);
949+
}
950+
900951
@Test
901952
void keepIndentation() {
902953
rewriteRun(

0 commit comments

Comments
 (0)