From 26da228421e0e215f96106cb62f154302d8c3724 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:25:17 +0200 Subject: [PATCH 1/4] ObjectFinalizeCallsSuper: declare Throwable when adding super.finalize() The recipe appended `super.finalize()` to any declaration matching `java.lang.Object finalize()` without looking at the throws clause. `Object.finalize()` declares `Throwable`, and an override that does not call the superclass may legally omit it, so the added call left those sources with an unreported checked exception and they no longer compiled. Resolve the nearest supertype declaration of `finalize()` and branch on what it throws. Nothing declared means the added call throws nothing either, so it is appended as before. `Throwable`, which includes reaching `java.lang.Object`, means add `throws Throwable` to the override first, but only when the override declares no clause of its own. Every other case, including a supertype that narrowed the clause to another checked exception and missing type attribution, has no legal way to add the call without inventing exception handling, so those methods are left unchanged. The throws check applies only inside a Java compilation unit, since `Throwable` is unchecked in the other JVM languages this recipe runs on. The recipe therefore declines a few shapes it used to rewrite, all of which it could only have made uncompilable. The search for an existing `super.finalize()` now matches syntactically as well as by type, because a call the recipe itself generates is not always attributed when the superclass is in the same compilation unit, and a later cycle appended it a second time. --- .../ObjectFinalizeCallsSuper.java | 76 +++++- .../ObjectFinalizeCallsSuperTest.java | 233 ++++++++++++++++++ 2 files changed, 300 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java index d11c99a97..1baded21c 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; import lombok.Getter; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; @@ -25,14 +26,20 @@ import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.DeclaresMethod; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.NameTree; +import org.openrewrite.java.tree.TypeUtils; +import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; public class ObjectFinalizeCallsSuper extends Recipe { private static final MethodMatcher FINALIZE_METHOD_MATCHER = new MethodMatcher("java.lang.Object finalize()", true); + private static final JavaType.FullyQualified THROWABLE = JavaType.ShallowClass.build("java.lang.Throwable"); @Getter final String displayName = "`finalize()` calls super"; @@ -50,15 +57,60 @@ public TreeVisitor getVisitor() { @Override public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { J.MethodDeclaration md = super.visitMethodDeclaration(method, ctx); - if (FINALIZE_METHOD_MATCHER.matches(md.getMethodType()) && !hasSuperFinalizeMethodInvocation(md)) { - //noinspection ConstantConditions - md = JavaTemplate.builder("super.finalize()") - .contextSensitive() - .build() - .apply(updateCursor(md), - md.getBody().getCoordinates().lastStatement()); + JavaType.Method methodType = md.getMethodType(); + J.Block body = md.getBody(); + if (methodType == null || body == null || !FINALIZE_METHOD_MATCHER.matches(methodType) || hasSuperFinalizeMethodInvocation(md)) { + return md; } - return md; + + // `Object#finalize()` is declared to throw `Throwable`. That is a checked exception in Java, but + // not in the other JVM languages this recipe runs on, so only a Java override has to declare it + // for the added call to compile. + List throwz = md.getThrows(); + boolean declaresThrowable = throwz != null && + throwz.stream().anyMatch(t -> TypeUtils.isOfClassType(t.getType(), "java.lang.Throwable")); + if (!declaresThrowable && getCursor().firstEnclosing(J.CompilationUnit.class) != null) { + // An override may only declare what the method it overrides declares. When the overridden + // `finalize()` declares `Throwable`, add it; when it declares nothing, the call needs no + // throws clause; anything else has no legal way to add the call without inventing + // exception handling, so leave those methods alone. + List superThrown = superFinalizeThrownExceptions(methodType); + if (superThrown == null) { + // Missing or unreliable type attribution. + return md; + } + if (!superThrown.isEmpty()) { + if (throwz != null || superThrown.stream().noneMatch(t -> TypeUtils.isOfClassType(t, "java.lang.Throwable"))) { + return md; + } + md = JavaTemplate.builder("Throwable") + .build() + .apply(updateCursor(md), md.getCoordinates().replaceThrows()); + JavaType.Method declaresThrowableType = methodType.withThrownExceptions(singletonList(THROWABLE)); + md = md.withMethodType(declaresThrowableType) + .withName(md.getName().withType(declaresThrowableType)); + } + } + + return JavaTemplate.builder("super.finalize()") + .contextSensitive() + .build() + .apply(updateCursor(md), body.getCoordinates().lastStatement()); + } + + private @Nullable List superFinalizeThrownExceptions(JavaType.Method finalizeMethod) { + for (JavaType.FullyQualified type = finalizeMethod.getDeclaringType().getSupertype(); type != null; type = type.getSupertype()) { + if ("java.lang.Object".equals(type.getFullyQualifiedName())) { + return singletonList(THROWABLE); + } + for (JavaType.Method superMethod : type.getMethods()) { + if ("finalize".equals(superMethod.getName()) && superMethod.getParameterTypes().isEmpty()) { + return superMethod.getThrownExceptions(); + } + } + } + // Missing or unreliable type attribution. + return null; } private boolean hasSuperFinalizeMethodInvocation(J.MethodDeclaration md) { @@ -67,7 +119,13 @@ private boolean hasSuperFinalizeMethodInvocation(J.MethodDeclaration md) { @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean exists) { J.MethodInvocation mi = super.visitMethodInvocation(method, exists); - if (FINALIZE_METHOD_MATCHER.matches(mi)) { + // A `super.finalize()` added by an earlier cycle is not always type attributed, so also + // match it syntactically to keep the recipe from adding the call a second time. + if (FINALIZE_METHOD_MATCHER.matches(mi) || + mi.getSelect() instanceof J.Identifier && + "super".equals(((J.Identifier) mi.getSelect()).getSimpleName()) && + "finalize".equals(mi.getSimpleName()) && + mi.getArguments().size() == 1 && mi.getArguments().get(0) instanceof J.Empty) { exists.set(Boolean.TRUE); } return mi; diff --git a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java index 71f460e4f..5654f227e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java @@ -19,6 +19,7 @@ import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; import static org.openrewrite.java.Assertions.java; @@ -79,4 +80,236 @@ protected void finalize() throws Throwable { ) ); } + + @Test + void addsThrowsThrowableWhenOverrideDeclaresNoThrows() { + rewriteRun( + //language=java + java( + """ + class F { + @Override + protected void finalize() { + cleanup(); + } + + void cleanup() { + } + } + """, + """ + class F { + @Override + protected void finalize() throws Throwable { + cleanup(); + super.finalize(); + } + + void cleanup() { + } + } + """ + ) + ); + } + + @Test + void addsThrowsThrowableToEmptyBody() { + rewriteRun( + //language=java + java( + """ + class F { + @Override + protected void finalize() { + } + } + """, + """ + class F { + @Override + protected void finalize() throws Throwable { + super.finalize(); + } + } + """ + ) + ); + } + + @Test + void addsThrowsThrowableRetainingComments() { + rewriteRun( + //language=java + java( + """ + class F { + Object o = new Object(); + + @Override + protected void finalize() { + // release the reference + o = null; + } + } + """, + """ + class F { + Object o = new Object(); + + @Override + protected void finalize() throws Throwable { + // release the reference + o = null; + super.finalize(); + } + } + """ + ) + ); + } + + @Test + void addsThrowsThrowableWhenSuperclassDeclaresThrowable() { + rewriteRun( + // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is + // declared in the same compilation unit + spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), + //language=java + java( + """ + class Parent { + @Override + protected void finalize() throws Throwable { + super.finalize(); + } + } + + class Child extends Parent { + @Override + protected void finalize() { + } + } + """, + """ + class Parent { + @Override + protected void finalize() throws Throwable { + super.finalize(); + } + } + + class Child extends Parent { + @Override + protected void finalize() throws Throwable { + super.finalize(); + } + } + """ + ) + ); + } + + @Test + void addsSuperFinalizeWithoutThrowsWhenSuperclassDeclaresNoThrows() { + rewriteRun( + // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is + // declared in the same compilation unit + spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), + //language=java + java( + """ + class Parent { + @Override + protected void finalize() { + try { + super.finalize(); + } catch (Throwable ignored) { + } + } + } + + class Child extends Parent { + @Override + protected void finalize() { + cleanup(); + } + + void cleanup() { + } + } + """, + """ + class Parent { + @Override + protected void finalize() { + try { + super.finalize(); + } catch (Throwable ignored) { + } + } + } + + class Child extends Parent { + @Override + protected void finalize() { + cleanup(); + super.finalize(); + } + + void cleanup() { + } + } + """ + ) + ); + } + + @Test + void doNotChangeWhenSuperclassNarrowsThrowsToAnotherException() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + + class Parent { + @Override + protected void finalize() throws IOException { + } + } + + class Child extends Parent { + @Override + protected void finalize() { + cleanup(); + } + + void cleanup() { + } + } + """ + ) + ); + } + + @Test + void doNotChangeWhenThrowsClauseDoesNotCoverThrowable() { + rewriteRun( + //language=java + java( + """ + class F { + @Override + protected void finalize() throws Exception { + cleanup(); + } + + void cleanup() { + } + } + """ + ) + ); + } } From a89235f32db515fed5b1ae7a9b9b1b45e723fbce Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 10:49:40 +0200 Subject: [PATCH 2/4] Review fixes: accept a throws clause that already covers the super's exceptions --- .../ObjectFinalizeCallsSuper.java | 24 ++++----- .../ObjectFinalizeCallsSuperTest.java | 51 +++++++++++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java index 1baded21c..e04050966 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java @@ -63,32 +63,30 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return md; } - // `Object#finalize()` is declared to throw `Throwable`. That is a checked exception in Java, but - // not in the other JVM languages this recipe runs on, so only a Java override has to declare it - // for the added call to compile. + // `Object#finalize()` is declared to throw `Throwable`, which is checked in Java but not in the + // other JVM languages this recipe runs on, so only a Java override has to declare it. List throwz = md.getThrows(); boolean declaresThrowable = throwz != null && throwz.stream().anyMatch(t -> TypeUtils.isOfClassType(t.getType(), "java.lang.Throwable")); if (!declaresThrowable && getCursor().firstEnclosing(J.CompilationUnit.class) != null) { - // An override may only declare what the method it overrides declares. When the overridden - // `finalize()` declares `Throwable`, add it; when it declares nothing, the call needs no - // throws clause; anything else has no legal way to add the call without inventing - // exception handling, so leave those methods alone. List superThrown = superFinalizeThrownExceptions(methodType); if (superThrown == null) { - // Missing or unreliable type attribution. return md; } - if (!superThrown.isEmpty()) { - if (throwz != null || superThrown.stream().noneMatch(t -> TypeUtils.isOfClassType(t, "java.lang.Throwable"))) { + // An override may only declare what the method it overrides declares, so a throws clause that + // does not already cover the super's has no legal way to take the added call. + if (throwz != null) { + if (superThrown.stream().anyMatch(thrown -> + throwz.stream().noneMatch(t -> TypeUtils.isAssignableTo(t.getType(), thrown)))) { + return md; + } + } else if (!superThrown.isEmpty()) { + if (superThrown.stream().noneMatch(t -> TypeUtils.isOfClassType(t, "java.lang.Throwable"))) { return md; } md = JavaTemplate.builder("Throwable") .build() .apply(updateCursor(md), md.getCoordinates().replaceThrows()); - JavaType.Method declaresThrowableType = methodType.withThrownExceptions(singletonList(THROWABLE)); - md = md.withMethodType(declaresThrowableType) - .withName(md.getName().withType(declaresThrowableType)); } } diff --git a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java index 5654f227e..789dd3d51 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java @@ -265,6 +265,57 @@ void cleanup() { ); } + @Test + void addsSuperFinalizeWhenThrowsClauseCoversSuperclassThrows() { + rewriteRun( + // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is + // declared in the same compilation unit + spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), + //language=java + java( + """ + import java.io.IOException; + + class Parent { + @Override + protected void finalize() throws IOException { + } + } + + class Child extends Parent { + @Override + protected void finalize() throws IOException { + cleanup(); + } + + void cleanup() { + } + } + """, + """ + import java.io.IOException; + + class Parent { + @Override + protected void finalize() throws IOException { + } + } + + class Child extends Parent { + @Override + protected void finalize() throws IOException { + cleanup(); + super.finalize(); + } + + void cleanup() { + } + } + """ + ) + ); + } + @Test void doNotChangeWhenSuperclassNarrowsThrowsToAnotherException() { rewriteRun( From 931833a2abfa41587e3392f428f17fb2d22fa2dd Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:26:53 +0200 Subject: [PATCH 3/4] Trim commentary --- .../staticanalysis/ObjectFinalizeCallsSuper.java | 12 +++++------- .../staticanalysis/ObjectFinalizeCallsSuperTest.java | 9 +++------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java index e04050966..8ab4e3536 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java @@ -63,8 +63,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return md; } - // `Object#finalize()` is declared to throw `Throwable`, which is checked in Java but not in the - // other JVM languages this recipe runs on, so only a Java override has to declare it. + // `Throwable` is checked in Java but not in the other JVM languages this recipe runs on List throwz = md.getThrows(); boolean declaresThrowable = throwz != null && throwz.stream().anyMatch(t -> TypeUtils.isOfClassType(t.getType(), "java.lang.Throwable")); @@ -73,8 +72,8 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex if (superThrown == null) { return md; } - // An override may only declare what the method it overrides declares, so a throws clause that - // does not already cover the super's has no legal way to take the added call. + // An override may only declare what it overrides declares, so a narrower throws clause has no + // legal way to take the added call if (throwz != null) { if (superThrown.stream().anyMatch(thrown -> throwz.stream().noneMatch(t -> TypeUtils.isAssignableTo(t.getType(), thrown)))) { @@ -107,7 +106,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex } } } - // Missing or unreliable type attribution. + // Missing or unreliable type attribution return null; } @@ -117,8 +116,7 @@ private boolean hasSuperFinalizeMethodInvocation(J.MethodDeclaration md) { @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean exists) { J.MethodInvocation mi = super.visitMethodInvocation(method, exists); - // A `super.finalize()` added by an earlier cycle is not always type attributed, so also - // match it syntactically to keep the recipe from adding the call a second time. + // A call added by an earlier cycle is not always attributed, so match it syntactically too if (FINALIZE_METHOD_MATCHER.matches(mi) || mi.getSelect() instanceof J.Identifier && "super".equals(((J.Identifier) mi.getSelect()).getSimpleName()) && diff --git a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java index 789dd3d51..fed16b725 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuperTest.java @@ -172,8 +172,7 @@ protected void finalize() throws Throwable { @Test void addsThrowsThrowableWhenSuperclassDeclaresThrowable() { rewriteRun( - // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is - // declared in the same compilation unit + // `JavaTemplate` leaves the generated `super` unattributed when the superclass is in the same file spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), //language=java java( @@ -213,8 +212,7 @@ protected void finalize() throws Throwable { @Test void addsSuperFinalizeWithoutThrowsWhenSuperclassDeclaresNoThrows() { rewriteRun( - // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is - // declared in the same compilation unit + // `JavaTemplate` leaves the generated `super` unattributed when the superclass is in the same file spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), //language=java java( @@ -268,8 +266,7 @@ void cleanup() { @Test void addsSuperFinalizeWhenThrowsClauseCoversSuperclassThrows() { rewriteRun( - // `JavaTemplate` does not attribute the `super` of the generated call when the superclass is - // declared in the same compilation unit + // `JavaTemplate` leaves the generated `super` unattributed when the superclass is in the same file spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), //language=java java( From 80a0a4c4f6887611344554b6181c7c44115b4426 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:04:52 +0200 Subject: [PATCH 4/4] Extract explaining helper for empty argument list --- .../staticanalysis/ObjectFinalizeCallsSuper.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java index 8ab4e3536..09b6fe1aa 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/ObjectFinalizeCallsSuper.java @@ -121,7 +121,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Atomi mi.getSelect() instanceof J.Identifier && "super".equals(((J.Identifier) mi.getSelect()).getSimpleName()) && "finalize".equals(mi.getSimpleName()) && - mi.getArguments().size() == 1 && mi.getArguments().get(0) instanceof J.Empty) { + hasNoArguments(mi)) { exists.set(Boolean.TRUE); } return mi; @@ -131,4 +131,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Atomi } }); } + + // An invocation with no arguments holds a single J.Empty element + private static boolean hasNoArguments(J.MethodInvocation mi) { + return mi.getArguments().size() == 1 && mi.getArguments().get(0) instanceof J.Empty; + } }