Keep ArrayStoreException catches when adding TypeNotPresentException

[martinfrancois]

Suggested review order: 34 of 52 (Score: 2.5)
Review first: openrewrite/rewrite-static-analysis#967

What's changed?

ArrayStoreExceptionToTypeNotPresentException now adds TypeNotPresentException as an alternative on the existing catch instead of replacing ArrayStoreException, so both exceptions reach the same handler.

The recipe changes a catch (ArrayStoreException e) clause only when all three of the following hold.

First, a call to Class.getAnnotation(Class) occurs in the region the catch clauses of this try protect, its resources or its body. A call in a catch or a finally block does not count, nor one in the body of a lambda, an anonymous class or a local class declared inside the try, since such a body can run long after the try has finished. That also skips a lambda invoked on the spot and an anonymous class's instance initializer, whose bodies do run inside the protected region, which costs a migration but never performs a wrong one.

Second, no catch clause on this try, and none on an enclosing try whose protected region contains this try, already concerns itself with TypeNotPresentException: a clause catching RuntimeException, Exception or Throwable already handles it, and a clause catching TypeNotPresentException itself or a subclass of it would become unreachable.

Third, every use of the catch parameter sits on a deliberately short allow list of contexts that still compile, and keep their meaning, after the parameter's type widens. Per JLS 14.20 the type of a multi-catch parameter is the least upper bound of its alternatives, here RuntimeException, so javac reads the rewritten parameter as a RuntimeException. The allow list covers the common handler shapes: calls on the parameter whose resolved method is declared by a supertype of RuntimeException, except getClass, whose result type follows the receiver's static type; string concatenation and ==/!=; instanceof; throw; and arguments, initializers and assignments whose declared target type accepts every RuntimeException. Any other use leaves the clause untouched, which costs a migration but never performs a wrong one.

Where a class declared in the sources could shadow the simple name TypeNotPresentException, the recipe leaves the file unchanged. Finding those classes needs a ScanningRecipe, whose accumulator is keyed by the JavaProject marker of the file declaring the class, so such a class in one module never blocks another module's files; files carrying no marker share one entry. HasNoJakartaAnnotations here and Maven AddDependency in openrewrite/rewrite scope their accumulators the same way.

What's your motivation?

Recipe: org.openrewrite.java.migrate.ArrayStoreExceptionToTypeNotPresentException.

Before

try {
    type.getAnnotation(annotation);
    Object[] values = new String[1];
    values[0] = value;
} catch (ArrayStoreException e) {
    recover(e);
}

Actual after the recipe

try {
    type.getAnnotation(annotation);
    Object[] values = new String[1];
    values[0] = value;
} catch (TypeNotPresentException e) {
    recover(e);
}

Expected after the recipe

try {
    type.getAnnotation(annotation);
    Object[] values = new String[1];
    values[0] = value;
} catch (ArrayStoreException | TypeNotPresentException e) {
    recover(e);
}

The premise of this recipe, as part of Java8toJava11, is that Class.getAnnotation reports a missing annotation type as TypeNotPresentException from Java 11 onwards, so a handler written for ArrayStoreException alone stops catching that failure. It does not follow that ArrayStoreException can no longer occur: the protected code can still throw it for reasons unrelated to annotations, as the values[0] = value; assignment does.

After the recipe on main runs, nothing catches ArrayStoreException, even when the try body can still throw it. I ran the example on Java 21 with a printing recover: the original prints HANDLED java.lang.ArrayStoreException, while main's output lets that exception escape uncaught. Where the try already catches TypeNotPresentException, main's output does not compile: error: exception TypeNotPresentException has already been caught. Reproduced on 3.41.0 and current main.

Main also looks for Class.getAnnotation anywhere inside the try statement, including in finally, in sibling catch blocks and in lambdas the try does not protect, so it rewrites catch clauses that an exception from that call cannot reach.

Affected code in real projects

• wildfly/wildfly BusinessViewAnnotationProcessor: catches ArrayStoreException at three sites around getAnnotation(Remote.class), getAnnotation(Local.class) and iface.getAnnotation(annotation), citing JDK-7183985, to raise WildFly's "missing class in annotation" deployment error. On Java 11+ the raw TypeNotPresentException bypasses those handlers today; the multi-catch this recipe now emits routes it back to them.
• wildfly/wildfly ServletContainerInitializerDeploymentProcessor: the same guard around getAnnotation(HandlesTypes.class) while wiring ServletContainerInitializer services during deployment.
• apache/grails-core DomainClassArtefactHandler: deliberately swallows the failure around getAnnotation(Artefact.class) with an empty catch; the multi-catch restores that swallowing on Java 11+.
• oracle/graal NodeUtil: supporting evidence for keeping ArrayStoreException caught rather than replacing it, this Truffle code catches ArrayStoreException around a genuine array store in a file that also calls getAnnotation.

All three affected files are eligible under this implementation, and their two handler shapes, a throw that never references the parameter and an empty swallowing catch, are the requireAnnotation and isDomainClass scenarios in widenCatchWhenHandlerAcceptsRuntimeException.

Anything in particular you'd like reviewers to focus on?

One existing test changed: the @DocumentExample test, renamed from replaceCaughtException to alsoCatchTypeNotPresentException. Its input source is unchanged; in its expected output the two lines that read } catch (TypeNotPresentException e) { now read } catch (ArrayStoreException | TypeNotPresentException e) {. The generated examples.yml carries the same rename and the same two lines, and the generated recipes.csv row carries the recipe's new description.

Five limits:

• The second condition skips the most code of anything here: any sibling or enclosing clause that already handles TypeNotPresentException leaves the whole try statement alone.
• Non-Java sources are now skipped, because multi-catch is Java syntax. The recipe on main rewrote Kotlin too.
• The third condition is an allow list, not a proof: handlers that return the exception, bind it in a method reference, read e.getClass(), pass it to a generic method such as Objects.requireNonNull, or use it in a ternary keep their original catch. Each of those could be shown safe individually, but the analysis outgrew the migrations it enabled, so the recipe skips them.
• When the class that shadows the simple name is declared in another module, the recipe still writes the plain TypeNotPresentException, because JavaProject markers carry no dependency edges: a class in another module is treated like one from a compiled dependency, even when that module is on the compile classpath and its class really does shadow the name there. Where it does shadow, for instance by sitting in the same package as the file being rewritten, the emitted simple name resolves to that class, so the output can fail to compile, or compile and catch the wrong type. A single accumulator entry for the whole repository would avoid that, but only by skipping every module as soon as any module declares such a class.
• In the LST, the lossless semantic tree that OpenRewrite rewrites, the catch parameter is still attributed with the type ArrayStoreException after the rewrite, while parsing the printed output afresh attributes it with RuntimeException, the least upper bound named in the third condition above. I found no case where that difference matters.

Have you considered any alternatives or workarounds?

A broad clause beside the narrow one on the same try need not stop the migration: Java accepts a narrow catch clause that gains an alternative while a broader clause follows it. I skip the whole try statement anyway, because the broad clause already catches TypeNotPresentException, so migrating the narrow clause beside it gains nothing. Reversing that means dropping the three entries of the HANDLES_TYPE_NOT_PRESENT_EXCEPTION set this change introduces, java.lang.RuntimeException, java.lang.Exception and java.lang.Throwable, from the check on this try while keeping them in the check on an enclosing try, where widening would intercept exceptions the enclosing handler sees today, and updating the one scenario that covers this shape, existingBroaderCatch in retainWhenTypeNotPresentExceptionIsAlreadyHandled.

The multi-catch is built directly rather than with JavaTemplate: a catch parameter is not a template insertion point, since J.Try.Catch and J.MultiCatch expose no coordinates, and regenerating the parameter would drop the type expression's formatting and the parameter's modifiers and annotations. Splicing one name into the existing type expression follows CombineSemanticallyEqualCatchBlocks in rewrite-static-analysis.

An earlier revision of this branch proved widening safety through generic method type inference and Class<? extends T> result flows, and emitted the qualified java.lang.TypeNotPresentException where sources shadow the simple name instead of skipping the file. That version migrated more inputs but tripled the implementation. A missed migration is visible and recoverable by hand, while the proof machinery would need maintaining indefinitely, so this revision trades those rare migrations for the allow list and the skip.

Any additional context

Pre-existing tests changed: ArrayStoreExceptionToTypeNotPresentExceptionTest.java.replaceCaughtException (removed).

The focused ArrayStoreExceptionToTypeNotPresentExceptionTest class now reports 21 executions, up from 3 on main. One existing test was renamed, and the branch adds 18 net test methods. Without the code change in this pull request, 18 executions fail. Related scenarios share one source fixture with one method per scenario rather than one test per scenario: widenCatchWhenHandlerAcceptsRuntimeException holds 5 widening-safe handler shapes (including the WildFly and Grails shapes above), retainCatchWhoseHandlerUsesTheExceptionBeyondTheAllowList holds 14 conservatively skipped parameter uses, retainWhenGetAnnotationIsOutsideTheProtectedRegion holds 8 lookup placements the catches do not protect, and retainWhenTypeNotPresentExceptionIsAlreadyHandled holds 4 tries whose handling already covers it. The tests cover six shapes:

• the clause gains TypeNotPresentException and keeps ArrayStoreException, for example alsoCatchTypeNotPresentException;
• the try is skipped where it, or an enclosing try, already handles TypeNotPresentException, for example the existingCatch scenario of retainWhenTypeNotPresentExceptionIsAlreadyHandled, whose output on main names that exception in two clauses and does not compile;
• the clause is skipped where every Class.getAnnotation call sits outside the region its catch clauses protect, for example the inFinally scenario of retainWhenGetAnnotationIsOutsideTheProtectedRegion;
• the clause is skipped where a use of the catch parameter would stop compiling, or change meaning, once that parameter widens to RuntimeException, for example the passToNarrowerParameter scenario of retainCatchWhoseHandlerUsesTheExceptionBeyondTheAllowList; the uses that do tolerate the wider type are migrated, and covered too;
• the file is left unchanged where a class in the sources could shadow the simple name, for example retainWhenSamePackageClassShadowsSimpleName;
• a Kotlin source is left untouched, the second limitation above.

The full list is the test file in this change.

One of those 18 failing executions, alsoCatchTypeNotPresentException, is the pre-existing @DocumentExample test under its new name, so this change does alter an expectation that used to hold, as described above. retainArrayStoreExceptionWhenLookupIsNotTypeAttributed passes either way, as do the two pre-existing tests left untouched, retainOtherCaughtExceptions and retainArrayStoreExceptionWithoutClassGetAnnotation.

This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch