From 6f478e4c1b3cceb744b083bb1be337c4e5878b24 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:24:59 +0200 Subject: [PATCH 1/7] ArrayStoreExceptionToTypeNotPresentException: add the new type, keep the old The recipe replaced the `ArrayStoreException` catch with `TypeNotPresentException`, so a protected region that could still throw an array store, such as an assignment into an array, lost its handler. It also fired on any `Class.getAnnotation` call anywhere under the try, including one in a catch body, a finally block or a deferred lambda, none of which the catch protects, and it ignored the other catches, so a try that already caught `TypeNotPresentException` was rewritten into two catches of the same type and stopped compiling. `TypeNotPresentException` is now added as a multi-catch alternative and the `ArrayStoreException` catch is kept. The call has to sit in the protected region, the resources or the body, with deferred bodies left out, and a try is skipped when its own catches, or those of an enclosing try that protects it, already concern `TypeNotPresentException`. Per JLS 14.20 a multi-catch parameter is implicitly final and typed as the least upper bound of its alternatives, here `RuntimeException`, so every reference to it in the handler has to survive that widening. An allow-list of contexts decides; anything unrecognized leaves the catch untouched, so the recipe now declines some catches it used to rewrite. The recipe became a `ScanningRecipe` so it can emit the fully qualified name where a source declares its own `TypeNotPresentException`, and it visits Java sources only, multi-catch being Java-only syntax. That scanned state is keyed by the `JavaProject` marker, so a declaration in one module no longer qualifies the name in every other module of a multi-module build. The existing `@DocumentExample` test expects the multi-catch now, and the recipe description, `examples.yml` and `recipes.csv` follow it. --- ...oreExceptionToTypeNotPresentException.java | 903 ++++++++- .../resources/META-INF/rewrite/examples.yml | 6 +- .../resources/META-INF/rewrite/recipes.csv | 2 +- ...xceptionToTypeNotPresentExceptionTest.java | 1779 ++++++++++++++++- 4 files changed, 2669 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java index bbf1d8653c..724fa79ce1 100644 --- a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java +++ b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java @@ -16,47 +16,922 @@ package org.openrewrite.java.migrate; import lombok.Getter; +import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; -import org.openrewrite.Recipe; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; -import org.openrewrite.java.ChangeType; import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.search.FindMethods; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.search.UsesMethod; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Markers; -public class ArrayStoreExceptionToTypeNotPresentException extends Recipe { +import java.util.HashMap; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; +import static java.util.Collections.newSetFromMap; + +public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe { private static final String ARRAY_STORE_EXCEPTION = "java.lang.ArrayStoreException"; private static final String TYPE_NOT_PRESENT_EXCEPTION = "java.lang.TypeNotPresentException"; + private static final String TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME = "TypeNotPresentException"; + private static final MethodMatcher CLASS_GET_ANNOTATION = new MethodMatcher("java.lang.Class getAnnotation(java.lang.Class)"); + + /** + * A catch of any of these types already handles {@code TypeNotPresentException}. + */ + private static final Set HANDLES_TYPE_NOT_PRESENT_EXCEPTION = new HashSet<>(asList( + "java.lang.RuntimeException", "java.lang.Exception", "java.lang.Throwable")); + + /** + * The supertypes of {@code RuntimeException}, a closed set because {@code java.lang} can not be extended. + * A position declared with one of these types accepts every {@code RuntimeException}, so it keeps compiling + * and keeps accepting the same values when the catch parameter's type widens from + * {@code ArrayStoreException} to {@code RuntimeException}. Unresolved types are not in the set, so they + * conservatively block the widening. + */ + private static final Set SUPERTYPES_OF_RUNTIME_EXCEPTION = new HashSet<>(asList( + "java.lang.RuntimeException", "java.lang.Exception", "java.lang.Throwable", + "java.lang.Object", "java.io.Serializable")); + + /** + * Types that accept any {@code Class} value regardless of its type argument: raw {@code Class} itself, and + * the supertypes of {@code Class} a value is realistically declared with. Any supertype not listed here, + * such as {@code java.lang.constant.Constable} (Java 12+), conservatively blocks the widening, as do + * unresolved types. + */ + private static final Set ACCEPTS_ANY_CLASS = new HashSet<>(asList( + "java.lang.Class", "java.lang.Object", "java.io.Serializable", + "java.lang.reflect.Type", "java.lang.reflect.AnnotatedElement", "java.lang.reflect.GenericDeclaration")); @Getter final String displayName = "Catch `TypeNotPresentException` thrown by `Class.getAnnotation()`"; @Getter - final String description = "Replace catch blocks for `ArrayStoreException` around `Class.getAnnotation()` with `TypeNotPresentException` to ensure compatibility with Java 11+."; + final String description = "Also catch `TypeNotPresentException` where `ArrayStoreException` is caught around `Class.getAnnotation()` to ensure compatibility with Java 11+. " + + "The `ArrayStoreException` is retained as the protected code can still throw it for reasons unrelated to annotations."; + + /** + * Where the sources declare their own class named {@code TypeNotPresentException}, the spliced simple name + * would resolve to it instead of to {@code java.lang.TypeNotPresentException}, either failing to compile or, + * worse, silently catching the wrong type. The scanner records where such classes are declared so the + * visitor can emit the fully qualified name at the affected sites. + *

+ * The declarations are scoped per {@link JavaProject} marker: only a declaration in the same module can + * shadow the simple name at compile time, so one module's {@code TypeNotPresentException} does not qualify + * the name in the other modules of a multi-module repository. Sources without the marker share one scope. + */ + public static class Accumulator { + /** + * Per project, the packages declaring a top-level class named {@code TypeNotPresentException}, + * {@code ""} for the default package. + */ + private final Map<@Nullable JavaProject, Set> packagesByProject = new HashMap<>(); + + /** + * Per project, the classes declaring a nested class named {@code TypeNotPresentException}, which + * shadows through inheritance and through on-demand imports. + */ + private final Map<@Nullable JavaProject, Set> classesByProject = new HashMap<>(); + + void recordPackage(@Nullable JavaProject project, String packageName) { + packagesByProject.computeIfAbsent(project, key -> new HashSet<>()).add(packageName); + } + + void recordClass(@Nullable JavaProject project, String className) { + classesByProject.computeIfAbsent(project, key -> new HashSet<>()).add(className); + } + + Set packagesDeclaringTypeNotPresentException(@Nullable JavaProject project) { + return packagesByProject.getOrDefault(project, emptySet()); + } + + Set classesDeclaringTypeNotPresentException(@Nullable JavaProject project) { + return classesByProject.getOrDefault(project, emptySet()); + } + } + + @Override + public Accumulator getInitialValue(ExecutionContext ctx) { + return new Accumulator(); + } + + @Override + public TreeVisitor getScanner(Accumulator acc) { + return new JavaIsoVisitor() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(classDecl.getSimpleName())) { + JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class); + JavaProject project = javaProject(sourceFile); + JavaType.FullyQualified owner = classDecl.getType() == null ? null : classDecl.getType().getOwningClass(); + if (owner != null) { + acc.recordClass(project, owner.getFullyQualifiedName()); + } else if (sourceFile != null) { + acc.recordPackage(project, packageName(sourceFile)); + } + } + return super.visitClassDeclaration(classDecl, ctx); + } + }; + } @Override - public TreeVisitor getVisitor() { - String classGetAnnotationPattern = "java.lang.Class getAnnotation(java.lang.Class)"; - return Preconditions.check(new UsesMethod<>(classGetAnnotationPattern), new JavaIsoVisitor() { + public TreeVisitor getVisitor(Accumulator acc) { + return Preconditions.check(new UsesMethod<>(CLASS_GET_ANNOTATION), new JavaIsoVisitor() { @Override public J.Try visitTry(J.Try tryStatement, ExecutionContext ctx) { J.Try try_ = super.visitTry(tryStatement, ctx); - if (FindMethods.find(try_, classGetAnnotationPattern).isEmpty()) { + JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class); + if (!(sourceFile instanceof J.CompilationUnit)) { + // A multi-catch is Java-only syntax, so other JVM languages are left alone return try_; } + if (anyCatchConcernsTypeNotPresentException(try_) || !protectedRegionCallsGetAnnotation(try_) || + anyEnclosingCatchConcernsTypeNotPresentException(getCursor())) { + return try_; + } + Cursor tryCursor = getCursor(); + boolean qualify = typeNotPresentExceptionSimpleNameIsShadowed((J.CompilationUnit) sourceFile, tryCursor, + acc, javaProject(sourceFile)); return try_.withCatches(ListUtils.map(try_.getCatches(), catch_ -> { - if (TypeUtils.isOfClassType(catch_.getParameter().getType(), ARRAY_STORE_EXCEPTION)) { - return (J.Try.Catch) new ChangeType(ARRAY_STORE_EXCEPTION, TYPE_NOT_PRESENT_EXCEPTION, true) - .getVisitor().visit(catch_, ctx); + if (TypeUtils.isOfClassType(catch_.getParameter().getType(), ARRAY_STORE_EXCEPTION) && + allParameterReferencesSurviveWidening(catch_, tryCursor)) { + return alsoCatchTypeNotPresentException(catch_, qualify); } return catch_; })); } }); } + + /** + * Only the resources and the body of a try are protected by its catches. A call in a catch or in the finally + * block runs outside that region, and so do the method bodies of a lambda, anonymous class or local class + * created inside the try. The instance initializers of such a class do run inside the protected region, but + * are left out as well; that only costs a migration that is not applied. + */ + private static boolean protectedRegionCallsGetAnnotation(J.Try try_) { + AtomicBoolean found = new AtomicBoolean(false); + JavaIsoVisitor scanner = new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) { + if (CLASS_GET_ANNOTATION.matches(method)) { + found.set(true); + return method; + } + return super.visitMethodInvocation(method, found); + } + + @Override + public J.Lambda visitLambda(J.Lambda lambda, AtomicBoolean found) { + return lambda; + } + + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, AtomicBoolean found) { + return classDecl; + } + + @Override + public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { + if (newClass.getBody() != null) { + // Constructor arguments are evaluated here, and so are the anonymous class's instance + // initializers; only its method bodies are deferred. The whole body is left out anyway, + // which only costs a migration that is not applied + for (Expression argument : newClass.getArguments()) { + visit(argument, found); + } + return newClass; + } + return super.visitNewClass(newClass, found); + } + }; + if (try_.getResources() != null) { + for (J.Try.Resource resource : try_.getResources()) { + scanner.visit(resource, found); + } + } + scanner.visit(try_.getBody(), found); + return found.get(); + } + + private static boolean anyCatchConcernsTypeNotPresentException(J.Try try_) { + for (J.Try.Catch catch_ : try_.getCatches()) { + TypeTree typeExpression = catch_.getParameter().getTree().getTypeExpression(); + if (typeExpression instanceof J.MultiCatch) { + for (NameTree alternative : ((J.MultiCatch) typeExpression).getAlternatives()) { + if (concernsTypeNotPresentException(alternative.getType())) { + return true; + } + } + } else if (typeExpression != null && concernsTypeNotPresentException(typeExpression.getType())) { + return true; + } + } + return false; + } + + /** + * A catch of an enclosing try whose protected region contains this try is reached by every + * {@code TypeNotPresentException} this try does not catch. Widening a catch here would intercept those + * exceptions before the enclosing handler sees them, silently rerouting them, so any enclosing try that + * concerns itself with {@code TypeNotPresentException} blocks the widening. Only enclosing tries whose + * body or resources contain this try count: from a catch or finally block the enclosing catches are no + * longer reachable. The walk deliberately does not stop at lambda or class boundaries, whose bodies may + * run inside the enclosing protected region; that errs towards not widening. + */ + private static boolean anyEnclosingCatchConcernsTypeNotPresentException(Cursor tryCursor) { + J child = tryCursor.getValue(); + for (Cursor cursor = tryCursor.getParent(); cursor != null; cursor = cursor.getParent()) { + Object value = cursor.getValue(); + if (value instanceof J.Try) { + J.Try enclosing = (J.Try) value; + boolean inProtectedRegion = child == enclosing.getBody() || + enclosing.getResources() != null && enclosing.getResources().contains(child); + if (inProtectedRegion && anyCatchConcernsTypeNotPresentException(enclosing)) { + return true; + } + } + if (value instanceof J) { + child = (J) value; + } + } + return false; + } + + /** + * A catch of a supertype of {@code TypeNotPresentException} already handles it, and a catch of + * {@code TypeNotPresentException} itself or of a subclass would become unreachable if it were added elsewhere. + */ + private static boolean concernsTypeNotPresentException(@Nullable JavaType type) { + JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); + return fullyQualified != null && + (HANDLES_TYPE_NOT_PRESENT_EXCEPTION.contains(fullyQualified.getFullyQualifiedName()) || + TypeUtils.isAssignableTo(TYPE_NOT_PRESENT_EXCEPTION, fullyQualified)); + } + + /** + * Per JLS 14.20 a multi-catch parameter is implicitly final and its type is the least upper bound of the + * alternatives, here {@code RuntimeException}. Widening therefore breaks any handler that assigns to the + * parameter or uses it where the narrower {@code ArrayStoreException} type is required. Rather than + * enumerating the ways a handler can depend on the narrower type, every reference to the parameter must + * occur in a context that provably tolerates the wider type; any reference in an unrecognized context + * means the catch is left untouched. + */ + private static boolean allParameterReferencesSurviveWidening(J.Try.Catch catch_, Cursor tryCursor) { + List variables = catch_.getParameter().getTree().getVariables(); + if (variables.size() != 1) { + return false; + } + String parameterName = variables.get(0).getSimpleName(); + JavaType.Variable parameterType = variables.get(0).getVariableType(); + AtomicBoolean unsafe = new AtomicBoolean(false); + new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean unsafe) { + if (referencesParameter(identifier, getCursor(), parameterName, parameterType) && + !widenedReferenceIsSafe(getCursor())) { + unsafe.set(true); + } + return identifier; + } + }.visit(catch_, unsafe, tryCursor); + return !unsafe.get(); + } + + /** + * Whether this identifier is a use of the catch parameter. Identifiers that are provably something else, a + * method or member name, a declaration or a label, are skipped. When variable attribution is missing the + * identifier can not be told apart from the parameter, so it is conservatively treated as a use. + */ + private static boolean referencesParameter(J.Identifier identifier, Cursor cursor, String parameterName, + JavaType.@Nullable Variable parameterType) { + if (!parameterName.equals(identifier.getSimpleName())) { + return false; + } + J parent = cursor.getParentTreeCursor().getValue(); + if (parent instanceof J.MethodInvocation && ((J.MethodInvocation) parent).getName() == identifier || + parent instanceof J.FieldAccess && ((J.FieldAccess) parent).getName() == identifier || + parent instanceof J.MemberReference && ((J.MemberReference) parent).getReference() == identifier || + parent instanceof J.VariableDeclarations.NamedVariable && ((J.VariableDeclarations.NamedVariable) parent).getName() == identifier || + parent instanceof J.Label || parent instanceof J.Break || parent instanceof J.Continue) { + return false; + } + JavaType.Variable fieldType = identifier.getFieldType(); + if (fieldType != null && parameterType != null) { + return fieldType == parameterType || + fieldType.getName().equals(parameterType.getName()) && + TypeUtils.isOfType(fieldType.getType(), parameterType.getType()); + } + return true; + } + + /** + * Whether an expression whose static type the widening changes from {@code ArrayStoreException} to + * {@code RuntimeException} keeps compiling, and keeps the same meaning, in its enclosing context. This is + * an allow-list: only contexts that provably tolerate the wider type are accepted, everything else fails + * safe. In particular an expression-bodied lambda, a switch, or any unforeseen context blocks the widening. + */ + private static boolean widenedReferenceIsSafe(Cursor cursor) { + J expression = cursor.getValue(); + Cursor parentCursor = cursor.getParentTreeCursor(); + J parent = parentCursor.getValue(); + if (parent instanceof J.Parentheses) { + // The parenthesized expression widens with its content + return widenedReferenceIsSafe(parentCursor); + } + if (parent instanceof J.Ternary) { + // The reference can only be a result branch, and the conditional's own type widens with it + J.Ternary ternary = (J.Ternary) parent; + return (expression == ternary.getTruePart() || expression == ternary.getFalsePart()) && + widenedReferenceIsSafe(parentCursor); + } + if (parent instanceof J.Binary || parent instanceof J.InstanceOf || parent instanceof J.Throw || + parent instanceof J.Assert) { + // The reference operations valid on an ArrayStoreException, string concatenation, == and !=, + // a type test, throwing (RuntimeException is unchecked) and an assert message, all remain valid + // and unchanged in behavior for the values the original handler could receive + return true; + } + if (parent instanceof J.AssignmentOperation) { + // Of the compound assignments only String's += compiles with an exception operand, and + // concatenation tolerates any RuntimeException; the parameter as the assigned variable fails safe + return expression == ((J.AssignmentOperation) parent).getAssignment(); + } + if (parent instanceof J.ControlParentheses) { + // Of the statements that parenthesize a bare expression, only a synchronized monitor keeps its + // meaning with a widened operand, any object being a valid monitor; a pattern switch selector + // is deliberately excluded + return parentCursor.getParentTreeCursor().getValue() instanceof J.Synchronized; + } + if (parent instanceof J.TypeCast) { + // The cast's own type does not change, but a cast to a type narrower than RuntimeException would + // throw ClassCastException for the TypeNotPresentException values the widened handler receives + return expression == ((J.TypeCast) parent).getExpression() && + acceptsAnyRuntimeException(((J.TypeCast) parent).getType()); + } + if (parent instanceof J.MethodInvocation) { + J.MethodInvocation invocation = (J.MethodInvocation) parent; + if (expression == invocation.getSelect()) { + return invokedMethodRemainsAvailable(invocation.getMethodType()) && + (!resultTypeDependsOnReceiverType(invocation.getMethodType()) || + widenedResultIsSafe(parentCursor)); + } + int argumentIndex = invocation.getArguments().indexOf(expression); + return argumentIndex >= 0 && argumentRemainsCompatible(invocation.getMethodType(), argumentIndex, parentCursor); + } + if (parent instanceof J.NewClass) { + int argumentIndex = ((J.NewClass) parent).getArguments().indexOf(expression); + return argumentIndex >= 0 && argumentRemainsCompatible(((J.NewClass) parent).getMethodType(), argumentIndex, parentCursor); + } + if (parent instanceof J.MemberReference) { + // The reference's result type would have to be checked against the functional interface's method, + // which is not reliably recoverable here, so a receiver-dependent result fails safe + J.MemberReference reference = (J.MemberReference) parent; + return expression == reference.getContaining() && + invokedMethodRemainsAvailable(reference.getMethodType()) && + !resultTypeDependsOnReceiverType(reference.getMethodType()); + } + if (parent instanceof J.VariableDeclarations.NamedVariable) { + // Covers an explicit declared type; `var` infers the narrower type and is rejected here + J.VariableDeclarations.NamedVariable variable = (J.VariableDeclarations.NamedVariable) parent; + return expression == variable.getInitializer() && acceptsAnyRuntimeException(variable.getType()); + } + if (parent instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) parent; + if (expression == assignment.getVariable()) { + // A multi-catch parameter is implicitly final + return false; + } + return acceptsAnyRuntimeException(assignment.getVariable().getType()); + } + if (parent instanceof J.Return) { + JavaType returnType = enclosingMethodReturnType(parentCursor); + return returnType != null && acceptsAnyRuntimeException(returnType); + } + if (parent instanceof J.NewArray) { + J.NewArray newArray = (J.NewArray) parent; + JavaType type = newArray.getType(); + return newArray.getInitializer() != null && newArray.getInitializer().contains(expression) && + type instanceof JavaType.Array && acceptsAnyRuntimeException(((JavaType.Array) type).getElemType()); + } + return isStatementPosition(parent); + } + + /** + * A parent that holds the expression as a statement discards its value, so the expression keeps compiling + * no matter how its type widens. Only reachable by recursion, since a bare identifier is not a statement. + * The unbraced forms, {@code if (flag) Objects.requireNonNull(e);}, discard the value exactly like a + * block does. A switch's arrow case is deliberately absent: there the expression may be the switch's own + * value. + */ + private static boolean isStatementPosition(J parent) { + return parent instanceof J.Block || parent instanceof J.If || parent instanceof J.If.Else || + parent instanceof J.Label || parent instanceof J.WhileLoop || parent instanceof J.DoWhileLoop || + parent instanceof J.ForLoop || parent instanceof J.ForEachLoop; + } + + /** + * A method resolved against the parameter's receiver remains available when its declaring type is a + * supertype of {@code RuntimeException}; {@code ArrayStoreException} declares no methods of its own, so + * this holds for every resolvable call, and an unresolved one blocks the widening. + */ + private static boolean invokedMethodRemainsAvailable(JavaType.@Nullable Method methodType) { + return methodType != null && acceptsAnyRuntimeException(methodType.getDeclaringType()); + } + + /** + * Whether the invocation's own result type widens along with its receiver. Per JLS 4.3.2 the type of + * {@code e.getClass()} is {@code Class} where |E| is the erasure of the receiver's STATIC + * type, so widening the receiver silently changes the result from + * {@code Class} to {@code Class}. + * {@code getClass()} is the only receiver-polymorphic member in {@code java.lang} and the parser + * attributes it with its declared signature, so it is recognized by name and arity; a resolved signature + * that mentions {@code ArrayStoreException} or a type variable is treated the same way, which also covers + * a future member whose site-specific attribution exposes the dependence. + */ + private static boolean resultTypeDependsOnReceiverType(JavaType.Method methodType) { + return "getClass".equals(methodType.getName()) && methodType.getParameterTypes().isEmpty() || + involvesReceiverTypeArgument(methodType.getReturnType(), newIdentitySet()); + } + + /** + * Whether the context of an invocation whose result type widens from + * {@code Class} to {@code Class} tolerates the + * wider result. Mirrors {@link #widenedReferenceIsSafe} with the acceptance test adjusted to the class + * type; everything unrecognized fails safe. + */ + private static boolean widenedResultIsSafe(Cursor cursor) { + J expression = cursor.getValue(); + Cursor parentCursor = cursor.getParentTreeCursor(); + J parent = parentCursor.getValue(); + if (parent instanceof J.Parentheses) { + return widenedResultIsSafe(parentCursor); + } + if (parent instanceof J.Ternary) { + J.Ternary ternary = (J.Ternary) parent; + return (expression == ternary.getTruePart() || expression == ternary.getFalsePart()) && + widenedResultIsSafe(parentCursor); + } + if (parent instanceof J.Binary) { + // Widening the wildcard's bound never breaks string concatenation, and it never removes the + // cast-compatibility that == and != require: every type castable to Class is also castable to Class + J.Binary.Type operator = ((J.Binary) parent).getOperator(); + return operator == J.Binary.Type.Addition || operator == J.Binary.Type.Equal || + operator == J.Binary.Type.NotEqual; + } + if (parent instanceof J.MethodInvocation) { + J.MethodInvocation invocation = (J.MethodInvocation) parent; + if (expression == invocation.getSelect()) { + // A chained call is a member of Class, so it stays available; it remains valid exactly when + // nothing in its resolved signature involves the receiver's type argument, as with getName(). + // A signature that does, as with cast() or getDeclaredConstructor(), fails safe + JavaType.Method methodType = invocation.getMethodType(); + if (methodType == null || involvesReceiverTypeArgument(methodType.getReturnType(), newIdentitySet())) { + return false; + } + for (JavaType parameterType : methodType.getParameterTypes()) { + if (involvesReceiverTypeArgument(parameterType, newIdentitySet())) { + return false; + } + } + return true; + } + int argumentIndex = invocation.getArguments().indexOf(expression); + if (argumentIndex < 0 || invocation.getMethodType() == null) { + return false; + } + JavaType parameterType = parameterType(invocation.getMethodType(), argumentIndex); + return parameterType != null && acceptsWidenedClassResult(parameterType); + } + if (parent instanceof J.VariableDeclarations.NamedVariable) { + J.VariableDeclarations.NamedVariable variable = (J.VariableDeclarations.NamedVariable) parent; + return expression == variable.getInitializer() && acceptsWidenedClassResult(variable.getType()); + } + if (parent instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) parent; + return expression != assignment.getVariable() && acceptsWidenedClassResult(assignment.getVariable().getType()); + } + if (parent instanceof J.Return) { + JavaType returnType = enclosingMethodReturnType(parentCursor); + return returnType != null && acceptsWidenedClassResult(returnType); + } + return isStatementPosition(parent); + } + + /** + * Whether the type mentions {@code ArrayStoreException}, a type variable, a wildcard or an unresolved + * type anywhere in its structure, in which case it can not be relied on to survive the widening. + */ + private static boolean involvesReceiverTypeArgument(@Nullable JavaType type, Set visited) { + if (type == null || type instanceof JavaType.Unknown) { + return true; + } + if (!visited.add(type)) { + return false; + } + if (type instanceof JavaType.GenericTypeVariable) { + return true; + } + JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); + if (fullyQualified != null && ARRAY_STORE_EXCEPTION.equals(fullyQualified.getFullyQualifiedName())) { + return true; + } + if (type instanceof JavaType.Parameterized) { + for (JavaType typeParameter : ((JavaType.Parameterized) type).getTypeParameters()) { + if (involvesReceiverTypeArgument(typeParameter, visited)) { + return true; + } + } + } else if (type instanceof JavaType.Array) { + return involvesReceiverTypeArgument(((JavaType.Array) type).getElemType(), visited); + } else if (type instanceof JavaType.Intersection) { + for (JavaType bound : ((JavaType.Intersection) type).getBounds()) { + if (involvesReceiverTypeArgument(bound, visited)) { + return true; + } + } + } + return false; + } + + /** + * Whether a position declared with this type accepts a {@code Class}: raw + * {@code Class} or a supertype of it, {@code Class}, or {@code Class} of a covariant wildcard whose + * every bound accepts any {@code RuntimeException}. + */ + private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { + if (type instanceof JavaType.Parameterized) { + JavaType.Parameterized parameterized = (JavaType.Parameterized) type; + if (!"java.lang.Class".equals(parameterized.getType().getFullyQualifiedName()) || + parameterized.getTypeParameters().size() != 1) { + return false; + } + JavaType argument = parameterized.getTypeParameters().get(0); + if (!(argument instanceof JavaType.GenericTypeVariable) || + !"?".equals(((JavaType.GenericTypeVariable) argument).getName())) { + return false; + } + JavaType.GenericTypeVariable wildcard = (JavaType.GenericTypeVariable) argument; + if (wildcard.getBounds().isEmpty()) { + return true; + } + if (wildcard.getVariance() != JavaType.GenericTypeVariable.Variance.COVARIANT) { + return false; + } + for (JavaType bound : wildcard.getBounds()) { + if (!acceptsAnyRuntimeException(bound)) { + return false; + } + } + return true; + } + JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); + return fullyQualified != null && ACCEPTS_ANY_CLASS.contains(fullyQualified.getFullyQualifiedName()); + } + + /** + * The return type of the method declaration enclosing this return statement, or null from a lambda, whose + * functional interface's return type is not reliably recoverable here. + */ + private static @Nullable JavaType enclosingMethodReturnType(Cursor returnCursor) { + for (Cursor cursor = returnCursor.getParent(); cursor != null; cursor = cursor.getParent()) { + Object enclosing = cursor.getValue(); + if (enclosing instanceof J.Lambda) { + return null; + } + if (enclosing instanceof J.MethodDeclaration) { + JavaType.Method methodType = ((J.MethodDeclaration) enclosing).getMethodType(); + return methodType == null ? null : methodType.getReturnType(); + } + } + return null; + } + + private static boolean argumentRemainsCompatible(JavaType.@Nullable Method methodType, int argumentIndex, + Cursor invocationCursor) { + if (methodType == null) { + // Unresolved: the parameter's requirements are unknowable, so leave the catch alone + return false; + } + JavaType parameterType = parameterType(methodType, argumentIndex); + if (parameterType == null) { + return false; + } + if (acceptsAnyRuntimeException(parameterType)) { + return true; + } + return inferredTypeParameterAcceptsWidening(methodType, argumentIndex, invocationCursor); + } + + private static @Nullable JavaType parameterType(JavaType.Method methodType, int argumentIndex) { + List parameterTypes = methodType.getParameterTypes(); + if (parameterTypes.isEmpty()) { + return null; + } + int parameterIndex = Math.min(argumentIndex, parameterTypes.size() - 1); + JavaType parameterType = parameterTypes.get(parameterIndex); + if (methodType.hasFlags(Flag.Varargs) && parameterIndex == parameterTypes.size() - 1 && + parameterType instanceof JavaType.Array) { + // The reference is never an array, so in the variable arity position it is passed as an element + return ((JavaType.Array) parameterType).getElemType(); + } + return parameterType; + } + + /** + * The resolved method type reports the inferred argument type: {@code Objects.requireNonNull(e)} reports + * its parameter as {@code ArrayStoreException} although the declaration is {@code T requireNonNull(T)} + * and would simply re-infer {@code T = RuntimeException} after the widening. Consult the declaration: + * widening is safe when the parameter is a type variable of the method itself (a class type variable is + * fixed by the receiver and can not re-infer), every bound accepts any {@code RuntimeException}, no other + * parameter constrains the same variable, and a result whose type mentions the variable is itself only + * used where the widened type is acceptable. + */ + private static boolean inferredTypeParameterAcceptsWidening(JavaType.Method methodType, int argumentIndex, + Cursor invocationCursor) { + J call = invocationCursor.getValue(); + if (!(call instanceof J.MethodInvocation) || ((J.MethodInvocation) call).getTypeParameters() != null) { + // Explicit type arguments do not re-infer, and constructor inference is driven by the class type + return false; + } + JavaType.Method declared = declaredMethod(methodType); + if (declared == null) { + return false; + } + JavaType declaredParameter = parameterType(declared, argumentIndex); + if (!(declaredParameter instanceof JavaType.GenericTypeVariable)) { + return false; + } + JavaType.GenericTypeVariable typeVariable = (JavaType.GenericTypeVariable) declaredParameter; + if (declaredByClass(typeVariable.getName(), declared.getDeclaringType())) { + return false; + } + for (JavaType bound : typeVariable.getBounds()) { + if (!acceptsAnyRuntimeException(bound)) { + return false; + } + } + List declaredParameterTypes = declared.getParameterTypes(); + int parameterIndex = Math.min(argumentIndex, declaredParameterTypes.size() - 1); + for (int i = 0; i < declaredParameterTypes.size(); i++) { + if (i != parameterIndex && mentionsTypeVariable(declaredParameterTypes.get(i), typeVariable.getName(), newIdentitySet())) { + return false; + } + } + if (mentionsTypeVariable(declared.getReturnType(), typeVariable.getName(), newIdentitySet())) { + // The call's own type widens with the parameter, so its context must be safe as well + return widenedReferenceIsSafe(invocationCursor); + } + return true; + } + + /** + * The single declaration matching the resolved method by name and arity, or null when it can not be + * identified unambiguously. + */ + private static JavaType.@Nullable Method declaredMethod(JavaType.Method methodType) { + JavaType.Method declared = null; + for (JavaType.Method candidate : methodType.getDeclaringType().getMethods()) { + if (candidate.getName().equals(methodType.getName()) && + candidate.getParameterTypes().size() == methodType.getParameterTypes().size()) { + if (declared != null) { + return null; + } + declared = candidate; + } + } + return declared; + } + + /** + * Whether the declaring class or one of its owning classes declares a type variable of this name. A method + * reusing such a name declares its own variable, so this errs towards attributing the variable to the + * class, which only blocks a widening that may have been safe. + */ + private static boolean declaredByClass(String typeVariableName, JavaType.@Nullable FullyQualified declaringType) { + for (JavaType.FullyQualified type = declaringType; type != null; type = type.getOwningClass()) { + JavaType.FullyQualified unwrapped = type instanceof JavaType.Parameterized ? ((JavaType.Parameterized) type).getType() : type; + for (JavaType typeParameter : unwrapped.getTypeParameters()) { + if (typeParameter instanceof JavaType.GenericTypeVariable && + typeVariableName.equals(((JavaType.GenericTypeVariable) typeParameter).getName())) { + return true; + } + } + } + return false; + } + + private static boolean mentionsTypeVariable(@Nullable JavaType type, String typeVariableName, Set visited) { + if (type == null || !visited.add(type)) { + return false; + } + if (type instanceof JavaType.GenericTypeVariable) { + if (typeVariableName.equals(((JavaType.GenericTypeVariable) type).getName())) { + return true; + } + for (JavaType bound : ((JavaType.GenericTypeVariable) type).getBounds()) { + if (mentionsTypeVariable(bound, typeVariableName, visited)) { + return true; + } + } + } else if (type instanceof JavaType.Array) { + return mentionsTypeVariable(((JavaType.Array) type).getElemType(), typeVariableName, visited); + } else if (type instanceof JavaType.Parameterized) { + for (JavaType typeParameter : ((JavaType.Parameterized) type).getTypeParameters()) { + if (mentionsTypeVariable(typeParameter, typeVariableName, visited)) { + return true; + } + } + } else if (type instanceof JavaType.Intersection) { + for (JavaType bound : ((JavaType.Intersection) type).getBounds()) { + if (mentionsTypeVariable(bound, typeVariableName, visited)) { + return true; + } + } + } + return false; + } + + private static Set newIdentitySet() { + return newSetFromMap(new IdentityHashMap<>()); + } + + private static boolean acceptsAnyRuntimeException(@Nullable JavaType type) { + JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); + return fullyQualified != null && SUPERTYPES_OF_RUNTIME_EXCEPTION.contains(fullyQualified.getFullyQualifiedName()); + } + + /** + * Whether the simple name {@code TypeNotPresentException} at this try would resolve to anything other + * than {@code java.lang.TypeNotPresentException}: a class or type parameter of that name declared in this + * file, a single-type import of another such class, a top-level class of that name in this file's package + * or reachable through an on-demand import, a nested class of that name inherited from a supertype of an + * enclosing class, or any other such type already referenced in this file. A shadowing class that exists + * only as a compiled dependency, never as a source in this run and never referenced in this file, is not + * visible here; the simple name is emitted for it. A class declared in a different {@link JavaProject} is + * treated the same way: it can only shadow here by being on this module's compile classpath, which the + * markers do not reveal, so it is handled like any other compiled dependency. + */ + private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.CompilationUnit cu, Cursor tryCursor, + Accumulator acc, @Nullable JavaProject project) { + Set declaringPackages = acc.packagesDeclaringTypeNotPresentException(project); + Set declaringClasses = acc.classesDeclaringTypeNotPresentException(project); + if (declaringPackages.contains(packageName(cu))) { + return true; + } + for (J.Import import_ : cu.getImports()) { + String simpleName = import_.getQualid().getSimpleName(); + if ("*".equals(simpleName)) { + String imported = qualifierName(import_.getQualid().getTarget()); + if (imported != null && + (declaringPackages.contains(imported) || declaringClasses.contains(imported))) { + return true; + } + } else if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(simpleName)) { + JavaType.FullyQualified imported = TypeUtils.asFullyQualified(import_.getQualid().getType()); + if (imported == null || !TYPE_NOT_PRESENT_EXCEPTION.equals(imported.getFullyQualifiedName())) { + return true; + } + } + } + for (JavaType type : cu.getTypesInUse().getTypesInUse()) { + JavaType.FullyQualified used = TypeUtils.asFullyQualified(type); + if (used != null && isForeignTypeNotPresentException(used.getFullyQualifiedName())) { + return true; + } + } + for (Cursor cursor = tryCursor; cursor != null; cursor = cursor.getParent()) { + Object enclosing = cursor.getValue(); + JavaType.FullyQualified enclosingType = null; + if (enclosing instanceof J.ClassDeclaration) { + enclosingType = ((J.ClassDeclaration) enclosing).getType(); + } else if (enclosing instanceof J.NewClass && ((J.NewClass) enclosing).getBody() != null) { + TypeTree clazz = ((J.NewClass) enclosing).getClazz(); + enclosingType = clazz == null ? null : TypeUtils.asFullyQualified(clazz.getType()); + } + if (anySupertypeDeclaresTypeNotPresentException(enclosingType, declaringClasses, new HashSet<>())) { + return true; + } + } + return declaresTypeNotPresentException(cu); + } + + /** + * The {@link JavaProject} marker of this source file, or null where the build did not attach one, as in a + * single-module parse; every unmarked source then shares the null scope. + */ + private static @Nullable JavaProject javaProject(@Nullable JavaSourceFile sourceFile) { + return sourceFile == null ? null : sourceFile.getMarkers().findFirst(JavaProject.class).orElse(null); + } + + private static String packageName(JavaSourceFile sourceFile) { + return sourceFile.getPackageDeclaration() == null ? "" : sourceFile.getPackageDeclaration().getPackageName(); + } + + private static @Nullable String qualifierName(Expression expression) { + if (expression instanceof J.Identifier) { + return ((J.Identifier) expression).getSimpleName(); + } + if (expression instanceof J.FieldAccess) { + String target = qualifierName(((J.FieldAccess) expression).getTarget()); + return target == null ? null : target + "." + ((J.FieldAccess) expression).getSimpleName(); + } + return null; + } + + private static boolean isForeignTypeNotPresentException(String fullyQualifiedName) { + return !TYPE_NOT_PRESENT_EXCEPTION.equals(fullyQualifiedName) && + (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(fullyQualifiedName) || + fullyQualifiedName.endsWith("." + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME) || + fullyQualifiedName.endsWith("$" + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME)); + } + + private static boolean anySupertypeDeclaresTypeNotPresentException(JavaType.@Nullable FullyQualified type, + Set declaringClasses, Set visited) { + for (JavaType.FullyQualified enclosing = type; enclosing != null; enclosing = enclosing.getSupertype()) { + if (!visited.add(enclosing.getFullyQualifiedName())) { + return false; + } + if (declaringClasses.contains(enclosing.getFullyQualifiedName())) { + return true; + } + for (JavaType.FullyQualified interface_ : enclosing.getInterfaces()) { + if (anySupertypeDeclaresTypeNotPresentException(interface_, declaringClasses, visited)) { + return true; + } + } + } + return false; + } + + private static boolean declaresTypeNotPresentException(J.CompilationUnit cu) { + AtomicBoolean found = new AtomicBoolean(false); + new JavaIsoVisitor() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, AtomicBoolean found) { + if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(classDecl.getSimpleName())) { + found.set(true); + return classDecl; + } + return super.visitClassDeclaration(classDecl, found); + } + + @Override + public J.TypeParameter visitTypeParameter(J.TypeParameter typeParameter, AtomicBoolean found) { + if (typeParameter.getName() instanceof J.Identifier && + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(((J.Identifier) typeParameter.getName()).getSimpleName())) { + found.set(true); + return typeParameter; + } + return super.visitTypeParameter(typeParameter, found); + } + }.visit(cu, found); + return found.get(); + } + + /** + * The multi-catch is assembled directly rather than through {@code JavaTemplate}: a catch parameter is not + * a template insertion point ({@code J.Try.Catch} and {@code J.MultiCatch} have no coordinates), and + * regenerating the whole catch from a template would discard the original type expression as written along + * with the parameter's modifiers and annotations. Keeping the existing type expression as the first + * alternative and splicing in the one new name preserves all of that, as + * {@code CombineSemanticallyEqualCatchBlocks} does upstream. + */ + private static J.Try.Catch alsoCatchTypeNotPresentException(J.Try.Catch catch_, boolean qualify) { + J.VariableDeclarations parameter = catch_.getParameter().getTree(); + TypeTree typeExpression = parameter.getTypeExpression(); + if (typeExpression == null) { + return catch_; + } + TypeTree typeNotPresentException; + if (qualify) { + TypeTree qualified = TypeTree.build(TYPE_NOT_PRESENT_EXCEPTION); + qualified = qualified.withType(JavaType.ShallowClass.build(TYPE_NOT_PRESENT_EXCEPTION)); + typeNotPresentException = qualified.withPrefix(Space.SINGLE_SPACE); + } else { + typeNotPresentException = new J.Identifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, + emptyList(), TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME, JavaType.ShallowClass.build(TYPE_NOT_PRESENT_EXCEPTION), null); + } + J.MultiCatch multiCatch = new J.MultiCatch(Tree.randomId(), typeExpression.getPrefix(), Markers.EMPTY, asList( + JRightPadded.build(typeExpression.withPrefix(Space.EMPTY)).withAfter(Space.SINGLE_SPACE), + JRightPadded.build(typeNotPresentException))); + return catch_.withParameter(catch_.getParameter().withTree(parameter.withTypeExpression(multiCatch))); + } } diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 1dfc38ed48..6c3c527436 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -347,7 +347,7 @@ examples: type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.java.migrate.ArrayStoreExceptionToTypeNotPresentException examples: -- description: '`ArrayStoreExceptionToTypeNotPresentExceptionTest#replaceCaughtException`' +- description: '`ArrayStoreExceptionToTypeNotPresentExceptionTest#alsoCatchTypeNotPresentException`' sources: - before: | import java.lang.annotation.*; @@ -377,12 +377,12 @@ examples: try { Object o = "test"; o.getClass().getAnnotation(Override.class); - } catch (TypeNotPresentException e) { + } catch (ArrayStoreException | TypeNotPresentException e) { System.out.println("Caught Exception"); } try { Object.class.getAnnotation(Override.class); - } catch (TypeNotPresentException e) { + } catch (ArrayStoreException | TypeNotPresentException e) { System.out.println("Caught ArrayStoreException"); } } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index e51cb061b2..f76e523f07 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -18,7 +18,7 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.A maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.AddStaticVariableOnProducerSessionBean,Adds `static` modifier to `@Produces` fields that are in session beans,"Ensures that the fields annotated with `@Produces` which is inside the session bean (`@Stateless`, `@Stateful`, or `@Singleton`) are declared `static`.",1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.AddSuppressionForIllegalReflectionWarningsPlugin,Add maven jar plugin to suppress illegal reflection warnings,Adds a maven jar plugin that's configured to suppress Illegal Reflection Warnings.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""version"",""type"":""String"",""displayName"":""Version"",""description"":""An exact version number, or node-style semver selector used to select the version number."",""example"":""29.X""}]", maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.AddSurefireFailsafeArgLine,Add `argLine` to surefire and failsafe plugins,"Adds the specified arguments to the `argLine` configuration of the Maven Surefire and Failsafe plugins, merging with any existing argLine value without duplicating arguments. The `@{argLine}` [late property reference](https://maven.apache.org/surefire/maven-surefire-plugin/faq.html) is prepended so that an agent injected by another plugin during the build, such as the JaCoCo coverage agent from `jacoco-maven-plugin:prepare-agent`, is preserved rather than overwritten. It is not added when the existing `argLine` already references the `argLine` property.",1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""argLine"",""type"":""String"",""displayName"":""Arg line"",""description"":""The arguments to add to the surefire and failsafe plugin `argLine` configuration. Individual arguments are space-separated. Arguments already present in the existing argLine are not duplicated."",""example"":""--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED"",""required"":true}]", -maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.ArrayStoreExceptionToTypeNotPresentException,Catch `TypeNotPresentException` thrown by `Class.getAnnotation()`,Replace catch blocks for `ArrayStoreException` around `Class.getAnnotation()` with `TypeNotPresentException` to ensure compatibility with Java 11+.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.ArrayStoreExceptionToTypeNotPresentException,Catch `TypeNotPresentException` thrown by `Class.getAnnotation()`,Also catch `TypeNotPresentException` where `ArrayStoreException` is caught around `Class.getAnnotation()` to ensure compatibility with Java 11+. The `ArrayStoreException` is retained as the protected code can still throw it for reasons unrelated to annotations.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.BeanDiscovery,Behavior change to bean discovery in modules with `beans.xml` file with no version specified,Alters beans with missing version attribute to include this attribute as well as the bean-discovery-mode="all" attribute to maintain an explicit bean archive.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.BeansXmlNamespace,Change `beans.xml` `schemaLocation` to match XML namespace,Set the `schemaLocation` that corresponds to the `xmlns` set in `beans.xml` files.,1,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.BounceCastleFromJdk15OntoJdk18On,Migrate Bouncy Castle to `jdk18on`,This recipe will upgrade Bouncy Castle dependencies from `-jdk15on` or `-jdk15to18` to `-jdk18on`.,15,,,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index 59b8b9fc29..7b25c104c1 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -17,10 +17,14 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.marker.JavaProject; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; +import static org.openrewrite.Tree.randomId; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.kotlin.Assertions.kotlin; class ArrayStoreExceptionToTypeNotPresentExceptionTest implements RewriteTest { @@ -31,7 +35,7 @@ public void defaults(RecipeSpec spec) { @DocumentExample @Test - void replaceCaughtException() { + void alsoCatchTypeNotPresentException() { rewriteRun( //language=java java( @@ -64,12 +68,12 @@ public void testMethod() { try { Object o = "test"; o.getClass().getAnnotation(Override.class); - } catch (TypeNotPresentException e) { + } catch (ArrayStoreException | TypeNotPresentException e) { System.out.println("Caught Exception"); } try { Object.class.getAnnotation(Override.class); - } catch (TypeNotPresentException e) { + } catch (ArrayStoreException | TypeNotPresentException e) { System.out.println("Caught ArrayStoreException"); } } @@ -119,4 +123,1773 @@ public void testMethod() { ) ); } + + @Test + void retainArrayStoreExceptionWhenBodyCanStillThrowIt() { + rewriteRun( + //language=java + java( + """ + import java.lang.annotation.Annotation; + + class Example { + void inspect(Class type, Class annotation, Object value) { + try { + type.getAnnotation(annotation); + Object[] values = new String[1]; + values[0] = value; + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + import java.lang.annotation.Annotation; + + class Example { + void inspect(Class type, Class annotation, Object value) { + try { + type.getAnnotation(annotation); + Object[] values = new String[1]; + values[0] = value; + } catch (ArrayStoreException | TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupInTryWithResourcesInitializer() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try (Resource resource = new Resource(type.getAnnotation(Override.class))) { + resource.use(); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + + static class Resource implements AutoCloseable { + Resource(Object annotation) { + } + + void use() { + } + + @Override + public void close() { + } + } + } + """, + """ + class Example { + void inspect(Class type) { + try (Resource resource = new Resource(type.getAnnotation(Override.class))) { + resource.use(); + } catch (ArrayStoreException | TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + + static class Resource implements AutoCloseable { + Resource(Object annotation) { + } + + void use() { + } + + @Override + public void close() { + } + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInFinally() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + } catch (ArrayStoreException e) { + recover(e); + } finally { + type.getAnnotation(Override.class); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInSiblingCatch() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + } catch (ArrayStoreException e) { + recover(e); + } catch (IllegalArgumentException e) { + type.getAnnotation(Override.class); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInDeferredLambda() { + rewriteRun( + //language=java + java( + """ + class Example { + Runnable inspectLater(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + return () -> type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + return () -> { + }; + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInMethodReference() { + rewriteRun( + //language=java + java( + """ + import java.util.function.Function; + + class Example { + Function, Override> inspectLater(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + return type::getAnnotation; + } catch (ArrayStoreException e) { + recover(e); + return null; + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * Whether a lambda created inside the try is invoked before the try completes can not be decided locally, so + * the conservative choice is to leave the handler alone rather than widen it on a lookup that may never run + * inside the protected region. + */ + @Test + void lookupOnlyInImmediatelyInvokedLambda() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + + class Example { + void inspect(List> types, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + types.forEach(type -> type.getAnnotation(Override.class)); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInAnonymousClass() { + rewriteRun( + //language=java + java( + """ + class Example { + Runnable inspectLater(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + return new Runnable() { + @Override + public void run() { + type.getAnnotation(Override.class); + } + }; + } catch (ArrayStoreException e) { + recover(e); + return null; + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void lookupOnlyInLocalClass() { + rewriteRun( + //language=java + java( + """ + class Example { + Runnable inspectLater(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + class Inspector implements Runnable { + @Override + public void run() { + type.getAnnotation(Override.class); + } + } + return new Inspector(); + } catch (ArrayStoreException e) { + recover(e); + return null; + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * Unlike a method body, an anonymous class's instance initializers run at the {@code new}, inside the + * protected region, so this lookup can throw into the enclosing catch. The recipe leaves the whole + * anonymous class body out regardless, which only costs a migration that is not applied. + */ + @Test + void lookupOnlyInAnonymousClassInstanceInitializer() { + rewriteRun( + //language=java + java( + """ + import java.lang.annotation.Annotation; + + class Example { + Runnable inspectLater(Class type, Object value) { + try { + Object[] values = new String[1]; + values[0] = value; + return new Runnable() { + final Annotation a = type.getAnnotation(Override.class); + + @Override + public void run() { + } + }; + } catch (ArrayStoreException e) { + recover(e); + return null; + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void existingTypeNotPresentExceptionCatch() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } catch (TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void existingMultiCatch() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | IllegalStateException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void existingBroaderCatchAlreadyHandlesTypeNotPresentException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } catch (RuntimeException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * A catch of a subclass of `TypeNotPresentException` would become unreachable if the earlier handler were + * widened. + */ + @Test + void existingTypeNotPresentExceptionSubclassCatch() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } catch (MissingType e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + + static class MissingType extends TypeNotPresentException { + MissingType() { + super("Missing", null); + } + } + } + """ + ) + ); + } + + /** + * After widening, the parameter's static type is `RuntimeException`, the least upper bound of the + * multi-catch alternatives, which any `Throwable` method, string concatenation and rethrow tolerate. + */ + @Test + void alsoCatchWhenHandlerLogsAndRethrowsTheException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + System.out.println("failed: " + e.getMessage()); + e.printStackTrace(); + throw e; + } + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + System.out.println("failed: " + e.getMessage()); + e.printStackTrace(); + throw e; + } + } + } + """ + ) + ); + } + + @Test + void alsoCatchWhenHandlerWrapsTheException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + throw new IllegalStateException("wrap", e); + } + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + throw new IllegalStateException("wrap", e); + } + } + } + """ + ) + ); + } + + @Test + void alsoCatchWhenHandlerAssignsTheExceptionToABroaderVariable() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + RuntimeException cause = e; + RuntimeException chosen = flag ? e : null; + recover(cause); + recover(chosen); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + RuntimeException cause = e; + RuntimeException chosen = flag ? e : null; + recover(cause); + recover(chosen); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void alsoCatchWhenHandlerReturnsTheExceptionAsABroaderType() { + rewriteRun( + //language=java + java( + """ + class Example { + RuntimeException inspect(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e; + } + } + } + """, + """ + class Example { + RuntimeException inspect(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException | TypeNotPresentException e) { + return e; + } + } + } + """ + ) + ); + } + + @Test + void alsoCatchWhenHandlerUsesAMethodReferenceOnTheException() { + rewriteRun( + //language=java + java( + """ + class Example { + Runnable printer(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e::printStackTrace; + } + } + } + """, + """ + class Example { + Runnable printer(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException | TypeNotPresentException e) { + return e::printStackTrace; + } + } + } + """ + ) + ); + } + + /** + * `Objects.requireNonNull` reports its inferred parameter type as `ArrayStoreException`, but the + * declaration is ` T requireNonNull(T)` and simply re-infers `T = RuntimeException` after widening. + */ + @Test + void alsoCatchWhenHandlerChecksTheExceptionWithRequireNonNull() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Objects.requireNonNull(e); + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + import java.util.Objects; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + Objects.requireNonNull(e); + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * A multi-catch parameter has the least upper bound of its alternatives as its type, here + * `RuntimeException`, so a parameter declared as `ArrayStoreException...` no longer accepts it. + */ + @Test + void retainCatchThatPassesTheExceptionToAVarargsParameter() { + rewriteRun( + //language=java + java( + """ + class Example { + void log(String message, ArrayStoreException... exceptions) { + } + + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + log("failed", e); + } + } + } + """ + ) + ); + } + + @Test + void retainCatchThatPassesTheExceptionToANarrowerParameter() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(ArrayStoreException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatAssignsTheExceptionToANarrowerVariable() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + ArrayStoreException copy = e; + recover(copy); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatUsesTheExceptionInATernaryInitializer() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + ArrayStoreException copy = flag ? e : null; + recover(copy); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatPassesATernaryToANarrowerParameter() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(flag ? e : null); + } + } + + void recover(ArrayStoreException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatStoresTheExceptionInAnArrayInitializer() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + ArrayStoreException[] all = {e}; + recover(all[0]); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatReturnsTheExceptionFromAnExpressionLambda() { + rewriteRun( + //language=java + java( + """ + import java.util.function.Supplier; + + class Example { + Supplier inspect(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return () -> e; + } + } + } + """ + ) + ); + } + + @Test + void retainCatchThatReturnsTheExceptionAsTheNarrowerType() { + rewriteRun( + //language=java + java( + """ + class Example { + ArrayStoreException inspect(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e; + } + } + } + """ + ) + ); + } + + /** + * The cast itself would still compile, but it would throw `ClassCastException` for the + * `TypeNotPresentException` values the widened handler newly receives. + */ + @Test + void retainCatchThatCastsTheExceptionToTheNarrowerType() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Object narrowed = (ArrayStoreException) e; + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * When the method receiving the parameter can not be resolved, its requirements are unknowable, so the + * catch is conservatively left alone. + */ + @Test + void retainCatchThatPassesTheExceptionToAnUnresolvableMethod() { + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()), + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Unknown.log(e); + } + } + } + """ + ) + ); + } + + /** + * A multi-catch parameter is implicitly final, so widening this handler would not compile. + */ + @Test + void retainCatchThatReassignsTheException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + e = new ArrayStoreException("wrapped"); + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * Only the caught exception is implicitly final; an unrelated variable that happens to share its name is not. + */ + @Test + void alsoCatchWhenAnUnrelatedSameNamedVariableIsAssigned() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + run(new Runnable() { + String e; + + @Override + public void run() { + e = "unrelated"; + } + }); + } + } + + void recover(RuntimeException e) { + } + + void run(Runnable runnable) { + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + recover(e); + run(new Runnable() { + String e; + + @Override + public void run() { + e = "unrelated"; + } + }); + } + } + + void recover(RuntimeException e) { + } + + void run(Runnable runnable) { + } + } + """ + ) + ); + } + + /** + * Kotlin has no multi-catch, so Kotlin sources are left alone. + */ + @Test + void kotlinFileNotChanged() { + rewriteRun( + //language=kotlin + kotlin( + """ + class Example { + fun inspect(type: Class<*>) { + try { + type.getAnnotation(Override::class.java) + } catch (e: ArrayStoreException) { + recover(e) + } + } + + fun recover(e: RuntimeException) { + } + } + """ + ) + ); + } + + @Test + void retainArrayStoreExceptionWhenLookupIsNotTypeAttributed() { + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()), + //language=java + java( + """ + class Example { + void inspect(Unknown type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * Per JLS 4.3.2 the type of {@code e.getClass()} is {@code Class} where {@code |E|} is the + * erasure of the receiver's static type, so widening the receiver would change this initializer's type + * from {@code Class} to {@code Class}, which no + * longer compiles. + */ + @Test + void retainCatchThatReadsTheExceptionClassAsTheNarrowerClassType() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Class narrow = e.getClass(); + Class viaTernary = flag ? (e.getClass()) : null; + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatPassesTheExceptionClassToANarrowerClassParameter() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + report(e.getClass()); + } + } + + void report(Class failure) { + } + } + """ + ) + ); + } + + @Test + void retainCatchThatBindsTheExceptionClassMethodReference() { + rewriteRun( + //language=java + java( + """ + import java.util.function.Supplier; + + class Example { + Supplier> inspect(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e::getClass; + } + } + } + """ + ) + ); + } + + /** + * {@code Class.cast()} returns the class's own type argument, so after widening it would return + * {@code RuntimeException} rather than {@code ArrayStoreException}. + */ + @Test + void retainCatchThatCastsThroughTheExceptionClass() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type, Object value) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + ArrayStoreException narrowed = e.getClass().cast(value); + recover(narrowed); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * {@code Objects.requireNonNull(e)} re-infers the parameter's own type, so {@code getClass()} on its + * result depends on the widening just as it does on the parameter directly. + */ + @Test + void retainCatchThatReadsTheLaunderedExceptionClass() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Class narrow = Objects.requireNonNull(e).getClass(); + } + } + } + """ + ) + ); + } + + /** + * {@code Class} and signatures like {@code getName()} that do not involve the class's type argument + * tolerate the widened {@code Class}, so common logging keeps migrating. + */ + @Test + void alsoCatchWhenHandlerReadsTheExceptionClassGenerically() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Class wide = e.getClass(); + String name = e.getClass().getName(); + System.out.println("caught " + name + wide); + } + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + Class wide = e.getClass(); + String name = e.getClass().getName(); + System.out.println("caught " + name + wide); + } + } + } + """ + ) + ); + } + + /** + * Every {@code TypeNotPresentException} the inner try does not catch reaches the enclosing handler, so + * widening the inner catch would steal the exception from it. + */ + @Test + void retainWhenEnclosingTryHandlesTypeNotPresentException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } catch (TypeNotPresentException e) { + handleMissingType(e); + } + } + + void recover(RuntimeException e) { + } + + void handleMissingType(TypeNotPresentException e) { + } + } + """ + ) + ); + } + + /** + * From inside a catch block the enclosing try's catches are no longer reachable, so an inner try there is + * not stealing from them and still migrates. + */ + @Test + void alsoCatchInsideHandlerOfEnclosingTry() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getTypeParameters(); + } catch (TypeNotPresentException missing) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getTypeParameters(); + } catch (TypeNotPresentException missing) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + recover(e); + } + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * The nested class shadows the simple name, and because it extends {@code RuntimeException} the simple + * name would even compile while binding the catch to the wrong type; the fully qualified name is emitted. + */ + @Test + void alsoCatchFullyQualifiedWhenNestedClassShadowsSimpleName() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + + static class TypeNotPresentException extends RuntimeException { + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + + static class TypeNotPresentException extends RuntimeException { + } + } + """ + ) + ); + } + + @Test + void alsoCatchFullyQualifiedWhenImportShadowsSimpleName() { + rewriteRun( + //language=java + java( + """ + package shadow; + + public class TypeNotPresentException { + } + """ + ), + //language=java + java( + """ + package example; + + import shadow.TypeNotPresentException; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + package example; + + import shadow.TypeNotPresentException; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * A same-package class shadows the simple name even when this file never references it, which is why the + * scanner records every source-declared class of this name. + */ + @Test + void alsoCatchFullyQualifiedWhenSamePackageClassShadowsSimpleName() { + rewriteRun( + //language=java + java( + """ + package example; + + class TypeNotPresentException { + } + """ + ), + //language=java + java( + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * The scanner's record of declared {@code TypeNotPresentException} classes is scoped per + * {@code JavaProject} marker: a class declared in one module of a multi-module repository is not on + * another module's horizon, so there the simple name is emitted. + */ + @Test + void alsoCatchSimpleNameWhenShadowingClassIsDeclaredInAnotherJavaProject() { + var moduleA = new JavaProject(randomId(), "module-a", null); + var moduleB = new JavaProject(randomId(), "module-b", null); + rewriteRun( + //language=java + java( + """ + package example; + + class TypeNotPresentException { + } + """, + spec -> spec.markers(moduleA) + ), + //language=java + java( + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + spec -> spec.markers(moduleB) + ) + ); + } + + /** + * Within one {@code JavaProject} the declaration still shadows: the marked sibling source qualifies the + * name exactly as an unmarked one does. + */ + @Test + void alsoCatchFullyQualifiedWhenShadowingClassIsDeclaredInTheSameJavaProject() { + var module = new JavaProject(randomId(), "module-a", null); + rewriteRun( + //language=java + java( + """ + package example; + + class TypeNotPresentException { + } + """, + spec -> spec.markers(module) + ), + //language=java + java( + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + package example; + + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + spec -> spec.markers(module) + ) + ); + } + + /** + * A nested class inherited from a supertype shadows the simple name inside the subclass; it would compile + * while binding the catch to the inherited type, so the fully qualified name is emitted. + */ + @Test + void alsoCatchFullyQualifiedWhenInheritedNestedClassShadowsSimpleName() { + rewriteRun( + //language=java + java( + """ + package example; + + public class Base { + public static class TypeNotPresentException extends RuntimeException { + } + } + """ + ), + //language=java + java( + """ + package example; + + class Example extends Base { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + package example; + + class Example extends Base { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * {@code message += e} concatenates like {@code message = message + e}, which tolerates any + * {@code RuntimeException}. + */ + @Test + void alsoCatchWhenHandlerAppendsTheExceptionToAMessage() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + String message = "failed: "; + message += e; + System.out.println(message); + } + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + String message = "failed: "; + message += e; + System.out.println(message); + } + } + } + """ + ) + ); + } + + @Test + void alsoCatchWhenHandlerSynchronizesOnTheException() { + rewriteRun( + //language=java + java( + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + synchronized (e) { + recover(e); + } + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + class Example { + void inspect(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + synchronized (e) { + recover(e); + } + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } + + /** + * An unbraced statement discards the call's value exactly like a braced one, so migration must not depend + * on brace style. + */ + @Test + void alsoCatchWhenHandlerChecksTheExceptionInAnUnbracedIf() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + if (flag) Objects.requireNonNull(e); + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """, + """ + import java.util.Objects; + + class Example { + void inspect(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException | TypeNotPresentException e) { + if (flag) Objects.requireNonNull(e); + recover(e); + } + } + + void recover(RuntimeException e) { + } + } + """ + ) + ); + } } From b8af5f911105e80fd1cd21fbf5eecbe989a355f5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:34:28 +0200 Subject: [PATCH 2/7] Trim commentary --- ...oreExceptionToTypeNotPresentException.java | 203 +++++++----------- ...xceptionToTypeNotPresentExceptionTest.java | 30 ++- 2 files changed, 92 insertions(+), 141 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java index 724fa79ce1..b787f88edd 100644 --- a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java +++ b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java @@ -58,21 +58,17 @@ public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe "java.lang.RuntimeException", "java.lang.Exception", "java.lang.Throwable")); /** - * The supertypes of {@code RuntimeException}, a closed set because {@code java.lang} can not be extended. - * A position declared with one of these types accepts every {@code RuntimeException}, so it keeps compiling - * and keeps accepting the same values when the catch parameter's type widens from - * {@code ArrayStoreException} to {@code RuntimeException}. Unresolved types are not in the set, so they - * conservatively block the widening. + * The supertypes of {@code RuntimeException}, a closed set since {@code java.lang} cannot be extended. A + * position declared with one of these accepts every {@code RuntimeException}, so it survives the widening. + * Unresolved types are absent and so block it. */ private static final Set SUPERTYPES_OF_RUNTIME_EXCEPTION = new HashSet<>(asList( "java.lang.RuntimeException", "java.lang.Exception", "java.lang.Throwable", "java.lang.Object", "java.io.Serializable")); /** - * Types that accept any {@code Class} value regardless of its type argument: raw {@code Class} itself, and - * the supertypes of {@code Class} a value is realistically declared with. Any supertype not listed here, - * such as {@code java.lang.constant.Constable} (Java 12+), conservatively blocks the widening, as do - * unresolved types. + * Types accepting any {@code Class} whatever its type argument. Anything unlisted, such as + * {@code java.lang.constant.Constable} (Java 12+), blocks the widening, as do unresolved types. */ private static final Set ACCEPTS_ANY_CLASS = new HashSet<>(asList( "java.lang.Class", "java.lang.Object", "java.io.Serializable", @@ -86,25 +82,21 @@ public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe "The `ArrayStoreException` is retained as the protected code can still throw it for reasons unrelated to annotations."; /** - * Where the sources declare their own class named {@code TypeNotPresentException}, the spliced simple name - * would resolve to it instead of to {@code java.lang.TypeNotPresentException}, either failing to compile or, - * worse, silently catching the wrong type. The scanner records where such classes are declared so the - * visitor can emit the fully qualified name at the affected sites. - *

- * The declarations are scoped per {@link JavaProject} marker: only a declaration in the same module can - * shadow the simple name at compile time, so one module's {@code TypeNotPresentException} does not qualify - * the name in the other modules of a multi-module repository. Sources without the marker share one scope. + * Where the sources declare their own {@code TypeNotPresentException}, the spliced simple name would resolve + * to it instead, either failing to compile or silently catching the wrong type, so the scanner records those + * declarations and the visitor qualifies the name there. Scoped per {@link JavaProject} marker, since only a + * same-module declaration can shadow; unmarked sources share one scope. */ public static class Accumulator { /** - * Per project, the packages declaring a top-level class named {@code TypeNotPresentException}, - * {@code ""} for the default package. + * Per project, the packages declaring a top-level {@code TypeNotPresentException}, {@code ""} for the + * default package. */ private final Map<@Nullable JavaProject, Set> packagesByProject = new HashMap<>(); /** - * Per project, the classes declaring a nested class named {@code TypeNotPresentException}, which - * shadows through inheritance and through on-demand imports. + * Per project, the classes declaring a nested {@code TypeNotPresentException}, which shadows through + * inheritance and on-demand imports. */ private final Map<@Nullable JavaProject, Set> classesByProject = new HashMap<>(); @@ -180,10 +172,9 @@ public J.Try visitTry(J.Try tryStatement, ExecutionContext ctx) { } /** - * Only the resources and the body of a try are protected by its catches. A call in a catch or in the finally - * block runs outside that region, and so do the method bodies of a lambda, anonymous class or local class - * created inside the try. The instance initializers of such a class do run inside the protected region, but - * are left out as well; that only costs a migration that is not applied. + * Only a try's resources and body are protected by its catches. A catch or finally block runs outside that + * region, as do the method bodies of a lambda or class created inside it. Such a class's instance + * initializers do run inside, but are left out too, which only costs a migration that is not applied. */ private static boolean protectedRegionCallsGetAnnotation(J.Try try_) { AtomicBoolean found = new AtomicBoolean(false); @@ -210,9 +201,8 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, At @Override public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) { if (newClass.getBody() != null) { - // Constructor arguments are evaluated here, and so are the anonymous class's instance - // initializers; only its method bodies are deferred. The whole body is left out anyway, - // which only costs a migration that is not applied + // Constructor arguments and instance initializers are evaluated here, only method bodies are + // deferred; the whole body is left out anyway, costing only a migration that is not applied for (Expression argument : newClass.getArguments()) { visit(argument, found); } @@ -247,13 +237,10 @@ private static boolean anyCatchConcernsTypeNotPresentException(J.Try try_) { } /** - * A catch of an enclosing try whose protected region contains this try is reached by every - * {@code TypeNotPresentException} this try does not catch. Widening a catch here would intercept those - * exceptions before the enclosing handler sees them, silently rerouting them, so any enclosing try that - * concerns itself with {@code TypeNotPresentException} blocks the widening. Only enclosing tries whose - * body or resources contain this try count: from a catch or finally block the enclosing catches are no - * longer reachable. The walk deliberately does not stop at lambda or class boundaries, whose bodies may - * run inside the enclosing protected region; that errs towards not widening. + * An enclosing try whose protected region contains this one sees every {@code TypeNotPresentException} this + * try does not catch, so widening here would silently reroute them and any such enclosing try blocks it. + * Only enclosing tries containing this one in their body or resources count. The walk does not stop at + * lambda or class boundaries, whose bodies may run inside the enclosing region, erring towards not widening. */ private static boolean anyEnclosingCatchConcernsTypeNotPresentException(Cursor tryCursor) { J child = tryCursor.getValue(); @@ -275,8 +262,7 @@ private static boolean anyEnclosingCatchConcernsTypeNotPresentException(Cursor t } /** - * A catch of a supertype of {@code TypeNotPresentException} already handles it, and a catch of - * {@code TypeNotPresentException} itself or of a subclass would become unreachable if it were added elsewhere. + * A supertype catch already handles it, and a catch of it or a subclass would become unreachable. */ private static boolean concernsTypeNotPresentException(@Nullable JavaType type) { JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); @@ -286,12 +272,10 @@ private static boolean concernsTypeNotPresentException(@Nullable JavaType type) } /** - * Per JLS 14.20 a multi-catch parameter is implicitly final and its type is the least upper bound of the - * alternatives, here {@code RuntimeException}. Widening therefore breaks any handler that assigns to the - * parameter or uses it where the narrower {@code ArrayStoreException} type is required. Rather than - * enumerating the ways a handler can depend on the narrower type, every reference to the parameter must - * occur in a context that provably tolerates the wider type; any reference in an unrecognized context - * means the catch is left untouched. + * Per JLS 14.20 a multi-catch parameter is implicitly final and typed as the least upper bound of the + * alternatives, here {@code RuntimeException}. Rather than enumerate the ways a handler can depend on the + * narrower type, every reference must sit in a context that provably tolerates the wider one; anything + * unrecognized leaves the catch untouched. */ private static boolean allParameterReferencesSurviveWidening(J.Try.Catch catch_, Cursor tryCursor) { List variables = catch_.getParameter().getTree().getVariables(); @@ -315,9 +299,8 @@ public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean unsaf } /** - * Whether this identifier is a use of the catch parameter. Identifiers that are provably something else, a - * method or member name, a declaration or a label, are skipped. When variable attribution is missing the - * identifier can not be told apart from the parameter, so it is conservatively treated as a use. + * Whether this identifier uses the catch parameter. Provably-something-else identifiers, such as method or + * member names, declarations and labels, are skipped; an unattributed one counts as a use. */ private static boolean referencesParameter(J.Identifier identifier, Cursor cursor, String parameterName, JavaType.@Nullable Variable parameterType) { @@ -342,10 +325,8 @@ private static boolean referencesParameter(J.Identifier identifier, Cursor curso } /** - * Whether an expression whose static type the widening changes from {@code ArrayStoreException} to - * {@code RuntimeException} keeps compiling, and keeps the same meaning, in its enclosing context. This is - * an allow-list: only contexts that provably tolerate the wider type are accepted, everything else fails - * safe. In particular an expression-bodied lambda, a switch, or any unforeseen context blocks the widening. + * Whether an expression the widening retypes keeps compiling, and keeps its meaning, in its context. An + * allow list: an expression-bodied lambda, a switch, or anything unforeseen blocks the widening. */ private static boolean widenedReferenceIsSafe(Cursor cursor) { J expression = cursor.getValue(); @@ -363,25 +344,22 @@ private static boolean widenedReferenceIsSafe(Cursor cursor) { } if (parent instanceof J.Binary || parent instanceof J.InstanceOf || parent instanceof J.Throw || parent instanceof J.Assert) { - // The reference operations valid on an ArrayStoreException, string concatenation, == and !=, - // a type test, throwing (RuntimeException is unchecked) and an assert message, all remain valid - // and unchanged in behavior for the values the original handler could receive + // Concatenation, `==`/`!=`, a type test, throwing (`RuntimeException` is unchecked) and an assert + // message all stay valid and unchanged for the values the original handler could receive return true; } if (parent instanceof J.AssignmentOperation) { - // Of the compound assignments only String's += compiles with an exception operand, and - // concatenation tolerates any RuntimeException; the parameter as the assigned variable fails safe + // Only `String +=` compiles with an exception operand, and concatenation tolerates any + // `RuntimeException`; the parameter as the assigned variable fails safe return expression == ((J.AssignmentOperation) parent).getAssignment(); } if (parent instanceof J.ControlParentheses) { - // Of the statements that parenthesize a bare expression, only a synchronized monitor keeps its - // meaning with a widened operand, any object being a valid monitor; a pattern switch selector - // is deliberately excluded + // Of the statements parenthesizing a bare expression only a synchronized monitor keeps its meaning, + // any object being a valid monitor; a pattern switch selector is deliberately excluded return parentCursor.getParentTreeCursor().getValue() instanceof J.Synchronized; } if (parent instanceof J.TypeCast) { - // The cast's own type does not change, but a cast to a type narrower than RuntimeException would - // throw ClassCastException for the TypeNotPresentException values the widened handler receives + // The cast's own type is unchanged, but one narrower than `RuntimeException` would now throw return expression == ((J.TypeCast) parent).getExpression() && acceptsAnyRuntimeException(((J.TypeCast) parent).getType()); } @@ -400,8 +378,8 @@ private static boolean widenedReferenceIsSafe(Cursor cursor) { return argumentIndex >= 0 && argumentRemainsCompatible(((J.NewClass) parent).getMethodType(), argumentIndex, parentCursor); } if (parent instanceof J.MemberReference) { - // The reference's result type would have to be checked against the functional interface's method, - // which is not reliably recoverable here, so a receiver-dependent result fails safe + // Checking the result against the functional interface's method is not reliable here, so a + // receiver-dependent result fails safe J.MemberReference reference = (J.MemberReference) parent; return expression == reference.getContaining() && invokedMethodRemainsAvailable(reference.getMethodType()) && @@ -434,11 +412,9 @@ private static boolean widenedReferenceIsSafe(Cursor cursor) { } /** - * A parent that holds the expression as a statement discards its value, so the expression keeps compiling - * no matter how its type widens. Only reachable by recursion, since a bare identifier is not a statement. - * The unbraced forms, {@code if (flag) Objects.requireNonNull(e);}, discard the value exactly like a - * block does. A switch's arrow case is deliberately absent: there the expression may be the switch's own - * value. + * A parent holding the expression as a statement discards its value, so it compiles however its type + * widens. Unbraced forms discard it exactly as a block does. A switch's arrow case is deliberately absent, + * since there the expression may be the switch's own value. */ private static boolean isStatementPosition(J parent) { return parent instanceof J.Block || parent instanceof J.If || parent instanceof J.If.Else || @@ -447,23 +423,19 @@ private static boolean isStatementPosition(J parent) { } /** - * A method resolved against the parameter's receiver remains available when its declaring type is a - * supertype of {@code RuntimeException}; {@code ArrayStoreException} declares no methods of its own, so - * this holds for every resolvable call, and an unresolved one blocks the widening. + * A method stays available when its declaring type is a supertype of {@code RuntimeException}, which holds + * for every resolvable call since {@code ArrayStoreException} declares none of its own. */ private static boolean invokedMethodRemainsAvailable(JavaType.@Nullable Method methodType) { return methodType != null && acceptsAnyRuntimeException(methodType.getDeclaringType()); } /** - * Whether the invocation's own result type widens along with its receiver. Per JLS 4.3.2 the type of - * {@code e.getClass()} is {@code Class} where |E| is the erasure of the receiver's STATIC - * type, so widening the receiver silently changes the result from - * {@code Class} to {@code Class}. - * {@code getClass()} is the only receiver-polymorphic member in {@code java.lang} and the parser - * attributes it with its declared signature, so it is recognized by name and arity; a resolved signature - * that mentions {@code ArrayStoreException} or a type variable is treated the same way, which also covers - * a future member whose site-specific attribution exposes the dependence. + * Whether the invocation's result type widens with its receiver. Per JLS 4.3.2 {@code e.getClass()} is + * {@code Class} over the receiver's *static* type, so widening it silently changes the + * result. {@code getClass()} is the only receiver-polymorphic member in {@code java.lang} and is + * recognized by name and arity; a signature mentioning {@code ArrayStoreException} or a type variable + * counts too, covering any future member whose attribution exposes the same dependence. */ private static boolean resultTypeDependsOnReceiverType(JavaType.Method methodType) { return "getClass".equals(methodType.getName()) && methodType.getParameterTypes().isEmpty() || @@ -471,10 +443,8 @@ private static boolean resultTypeDependsOnReceiverType(JavaType.Method methodTyp } /** - * Whether the context of an invocation whose result type widens from - * {@code Class} to {@code Class} tolerates the - * wider result. Mirrors {@link #widenedReferenceIsSafe} with the acceptance test adjusted to the class - * type; everything unrecognized fails safe. + * Whether the context tolerates a result widened to {@code Class}. Mirrors + * {@link #widenedReferenceIsSafe} with the acceptance test adjusted to the class type. */ private static boolean widenedResultIsSafe(Cursor cursor) { J expression = cursor.getValue(); @@ -489,9 +459,8 @@ private static boolean widenedResultIsSafe(Cursor cursor) { widenedResultIsSafe(parentCursor); } if (parent instanceof J.Binary) { - // Widening the wildcard's bound never breaks string concatenation, and it never removes the - // cast-compatibility that == and != require: every type castable to Class is also castable to Class + // Widening the wildcard's bound breaks neither concatenation nor the cast-compatibility `==` and + // `!=` require, every such type staying castable J.Binary.Type operator = ((J.Binary) parent).getOperator(); return operator == J.Binary.Type.Addition || operator == J.Binary.Type.Equal || operator == J.Binary.Type.NotEqual; @@ -499,9 +468,8 @@ private static boolean widenedResultIsSafe(Cursor cursor) { if (parent instanceof J.MethodInvocation) { J.MethodInvocation invocation = (J.MethodInvocation) parent; if (expression == invocation.getSelect()) { - // A chained call is a member of Class, so it stays available; it remains valid exactly when - // nothing in its resolved signature involves the receiver's type argument, as with getName(). - // A signature that does, as with cast() or getDeclaredConstructor(), fails safe + // A chained call is a member of `Class` and so stays available, valid exactly when its resolved + // signature avoids the receiver's type argument; `cast()` and the like fail safe JavaType.Method methodType = invocation.getMethodType(); if (methodType == null || involvesReceiverTypeArgument(methodType.getReturnType(), newIdentitySet())) { return false; @@ -536,8 +504,8 @@ private static boolean widenedResultIsSafe(Cursor cursor) { } /** - * Whether the type mentions {@code ArrayStoreException}, a type variable, a wildcard or an unresolved - * type anywhere in its structure, in which case it can not be relied on to survive the widening. + * Whether the type mentions {@code ArrayStoreException}, a type variable, a wildcard or an unresolved type + * anywhere, in which case it cannot be relied on to survive the widening. */ private static boolean involvesReceiverTypeArgument(@Nullable JavaType type, Set visited) { if (type == null || type instanceof JavaType.Unknown) { @@ -572,9 +540,8 @@ private static boolean involvesReceiverTypeArgument(@Nullable JavaType type, Set } /** - * Whether a position declared with this type accepts a {@code Class}: raw - * {@code Class} or a supertype of it, {@code Class}, or {@code Class} of a covariant wildcard whose - * every bound accepts any {@code RuntimeException}. + * Whether a position of this type accepts a {@code Class}: raw {@code Class} or + * a supertype, {@code Class}, or a covariant wildcard whose bounds all accept any {@code RuntimeException}. */ private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { if (type instanceof JavaType.Parameterized) { @@ -607,8 +574,8 @@ private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { } /** - * The return type of the method declaration enclosing this return statement, or null from a lambda, whose - * functional interface's return type is not reliably recoverable here. + * The enclosing method declaration's return type, or null from a lambda, whose functional interface's + * return type is not reliably recoverable here. */ private static @Nullable JavaType enclosingMethodReturnType(Cursor returnCursor) { for (Cursor cursor = returnCursor.getParent(); cursor != null; cursor = cursor.getParent()) { @@ -656,13 +623,11 @@ private static boolean argumentRemainsCompatible(JavaType.@Nullable Method metho } /** - * The resolved method type reports the inferred argument type: {@code Objects.requireNonNull(e)} reports - * its parameter as {@code ArrayStoreException} although the declaration is {@code T requireNonNull(T)} - * and would simply re-infer {@code T = RuntimeException} after the widening. Consult the declaration: - * widening is safe when the parameter is a type variable of the method itself (a class type variable is - * fixed by the receiver and can not re-infer), every bound accepts any {@code RuntimeException}, no other - * parameter constrains the same variable, and a result whose type mentions the variable is itself only - * used where the widened type is acceptable. + * The resolved method type reports the *inferred* argument type, so {@code Objects.requireNonNull(e)} looks + * like it takes an {@code ArrayStoreException} although {@code T requireNonNull(T)} would simply + * re-infer. Consult the declaration instead: safe when the parameter is a type variable of the method + * itself (a class variable is fixed by the receiver), its bounds all accept any {@code RuntimeException}, + * no other parameter constrains it, and a result mentioning it is itself used safely. */ private static boolean inferredTypeParameterAcceptsWidening(JavaType.Method methodType, int argumentIndex, Cursor invocationCursor) { @@ -703,8 +668,7 @@ private static boolean inferredTypeParameterAcceptsWidening(JavaType.Method meth } /** - * The single declaration matching the resolved method by name and arity, or null when it can not be - * identified unambiguously. + * The single declaration matching the resolved method by name and arity, or null when ambiguous. */ private static JavaType.@Nullable Method declaredMethod(JavaType.Method methodType) { JavaType.Method declared = null; @@ -721,9 +685,8 @@ private static boolean inferredTypeParameterAcceptsWidening(JavaType.Method meth } /** - * Whether the declaring class or one of its owning classes declares a type variable of this name. A method - * reusing such a name declares its own variable, so this errs towards attributing the variable to the - * class, which only blocks a widening that may have been safe. + * Whether the declaring class or an owner declares a type variable of this name. A method reusing the name + * declares its own, so this errs towards the class and only blocks a widening that may have been safe. */ private static boolean declaredByClass(String typeVariableName, JavaType.@Nullable FullyQualified declaringType) { for (JavaType.FullyQualified type = declaringType; type != null; type = type.getOwningClass()) { @@ -779,15 +742,11 @@ private static boolean acceptsAnyRuntimeException(@Nullable JavaType type) { } /** - * Whether the simple name {@code TypeNotPresentException} at this try would resolve to anything other - * than {@code java.lang.TypeNotPresentException}: a class or type parameter of that name declared in this - * file, a single-type import of another such class, a top-level class of that name in this file's package - * or reachable through an on-demand import, a nested class of that name inherited from a supertype of an - * enclosing class, or any other such type already referenced in this file. A shadowing class that exists - * only as a compiled dependency, never as a source in this run and never referenced in this file, is not - * visible here; the simple name is emitted for it. A class declared in a different {@link JavaProject} is - * treated the same way: it can only shadow here by being on this module's compile classpath, which the - * markers do not reveal, so it is handled like any other compiled dependency. + * Whether the simple name would resolve to anything but {@code java.lang.TypeNotPresentException}: a class + * or type parameter of that name in this file, a single-type import, a top-level class in this package or + * reachable through an on-demand import, a nested class inherited from an enclosing class's supertype, or + * any such type already referenced here. A shadowing class visible only as a compiled dependency, or + * declared in another {@link JavaProject}, cannot be seen from here, so the simple name is emitted for it. */ private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.CompilationUnit cu, Cursor tryCursor, Accumulator acc, @Nullable JavaProject project) { @@ -834,8 +793,8 @@ private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.Compilation } /** - * The {@link JavaProject} marker of this source file, or null where the build did not attach one, as in a - * single-module parse; every unmarked source then shares the null scope. + * The {@link JavaProject} marker of this source file, or null where the build attached none, as in a + * single-module parse; unmarked sources share the null scope. */ private static @Nullable JavaProject javaProject(@Nullable JavaSourceFile sourceFile) { return sourceFile == null ? null : sourceFile.getMarkers().findFirst(JavaProject.class).orElse(null); @@ -907,11 +866,9 @@ public J.TypeParameter visitTypeParameter(J.TypeParameter typeParameter, AtomicB } /** - * The multi-catch is assembled directly rather than through {@code JavaTemplate}: a catch parameter is not - * a template insertion point ({@code J.Try.Catch} and {@code J.MultiCatch} have no coordinates), and - * regenerating the whole catch from a template would discard the original type expression as written along - * with the parameter's modifiers and annotations. Keeping the existing type expression as the first - * alternative and splicing in the one new name preserves all of that, as + * Assembled directly rather than through {@code JavaTemplate}: a catch parameter is not an insertion point, + * and regenerating the catch would discard the type expression as written along with the parameter's + * modifiers and annotations. Keeping it as the first alternative preserves all of that, as * {@code CombineSemanticallyEqualCatchBlocks} does upstream. */ private static J.Try.Catch alsoCatchTypeNotPresentException(J.Try.Catch catch_, boolean qualify) { diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index 7b25c104c1..ff12e63d21 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -338,9 +338,8 @@ void recover(RuntimeException e) { } /** - * Whether a lambda created inside the try is invoked before the try completes can not be decided locally, so - * the conservative choice is to leave the handler alone rather than widen it on a lookup that may never run - * inside the protected region. + * Whether a lambda created inside the try runs before the try completes cannot be decided locally, so the + * handler is left alone. */ @Test void lookupOnlyInImmediatelyInvokedLambda() { @@ -433,9 +432,8 @@ void recover(RuntimeException e) { } /** - * Unlike a method body, an anonymous class's instance initializers run at the {@code new}, inside the - * protected region, so this lookup can throw into the enclosing catch. The recipe leaves the whole - * anonymous class body out regardless, which only costs a migration that is not applied. + * Instance initializers run at the {@code new}, inside the protected region, so this lookup can throw into + * the enclosing catch. The whole anonymous body is left out regardless, costing only a missed migration. */ @Test void lookupOnlyInAnonymousClassInstanceInitializer() { @@ -545,8 +543,7 @@ void recover(RuntimeException e) { } /** - * A catch of a subclass of `TypeNotPresentException` would become unreachable if the earlier handler were - * widened. + * A catch of a `TypeNotPresentException` subclass would become unreachable if the earlier handler widened. */ @Test void existingTypeNotPresentExceptionSubclassCatch() { @@ -580,8 +577,8 @@ static class MissingType extends TypeNotPresentException { } /** - * After widening, the parameter's static type is `RuntimeException`, the least upper bound of the - * multi-catch alternatives, which any `Throwable` method, string concatenation and rethrow tolerate. + * The widened parameter is typed `RuntimeException`, which `Throwable` methods, concatenation and rethrow + * all tolerate. */ @Test void alsoCatchWhenHandlerLogsAndRethrowsTheException() { @@ -759,8 +756,8 @@ Runnable printer(Class type) { } /** - * `Objects.requireNonNull` reports its inferred parameter type as `ArrayStoreException`, but the - * declaration is ` T requireNonNull(T)` and simply re-infers `T = RuntimeException` after widening. + * `Objects.requireNonNull` reports an inferred `ArrayStoreException` parameter, but ` T requireNonNull(T)` + * simply re-infers after widening. */ @Test void alsoCatchWhenHandlerChecksTheExceptionWithRequireNonNull() { @@ -806,8 +803,7 @@ void recover(RuntimeException e) { } /** - * A multi-catch parameter has the least upper bound of its alternatives as its type, here - * `RuntimeException`, so a parameter declared as `ArrayStoreException...` no longer accepts it. + * The widened parameter is typed `RuntimeException`, which an `ArrayStoreException...` parameter rejects. */ @Test void retainCatchThatPassesTheExceptionToAVarargsParameter() { @@ -995,8 +991,7 @@ ArrayStoreException inspect(Class type) { } /** - * The cast itself would still compile, but it would throw `ClassCastException` for the - * `TypeNotPresentException` values the widened handler newly receives. + * The cast still compiles, but throws for the `TypeNotPresentException` values the widened handler receives. */ @Test void retainCatchThatCastsTheExceptionToTheNarrowerType() { @@ -1023,8 +1018,7 @@ void recover(RuntimeException e) { } /** - * When the method receiving the parameter can not be resolved, its requirements are unknowable, so the - * catch is conservatively left alone. + * An unresolvable receiving method has unknowable requirements, so the catch is left alone. */ @Test void retainCatchThatPassesTheExceptionToAnUnresolvableMethod() { From f0fae3c8b1bce54581231dd382e8ffa3d385f2e4 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 15:16:19 +0200 Subject: [PATCH 3/7] Consolidate catch-handler tests --- ...xceptionToTypeNotPresentExceptionTest.java | 393 ++++-------------- 1 file changed, 71 insertions(+), 322 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index ff12e63d21..dba433eaae 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -576,235 +576,110 @@ static class MissingType extends TypeNotPresentException { ); } - /** - * The widened parameter is typed `RuntimeException`, which `Throwable` methods, concatenation and rethrow - * all tolerate. - */ @Test - void alsoCatchWhenHandlerLogsAndRethrowsTheException() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - System.out.println("failed: " + e.getMessage()); - e.printStackTrace(); - throw e; - } - } - } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - System.out.println("failed: " + e.getMessage()); - e.printStackTrace(); - throw e; - } - } - } - """ - ) - ); - } + void widenCatchWhenHandlerAcceptsRuntimeException() { + //language=java + var source = """ + import java.util.Objects; - @Test - void alsoCatchWhenHandlerWrapsTheException() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - throw new IllegalStateException("wrap", e); - } + class Example { + void logAndRethrow(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + System.out.println("failed: " + e.getMessage()); + e.printStackTrace(); + throw e; } } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - throw new IllegalStateException("wrap", e); - } - } - } - """ - ) - ); - } - @Test - void alsoCatchWhenHandlerAssignsTheExceptionToABroaderVariable() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - RuntimeException cause = e; - RuntimeException chosen = flag ? e : null; - recover(cause); - recover(chosen); - } - } - - void recover(RuntimeException e) { + void wrap(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + throw new IllegalStateException("wrap", e); } } - """, - """ - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - RuntimeException cause = e; - RuntimeException chosen = flag ? e : null; - recover(cause); - recover(chosen); - } - } - void recover(RuntimeException e) { + void assign(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + RuntimeException cause = e; + RuntimeException chosen = flag ? e : null; + recover(cause); + recover(chosen); } } - """ - ) - ); - } - @Test - void alsoCatchWhenHandlerReturnsTheExceptionAsABroaderType() { - rewriteRun( - //language=java - java( - """ - class Example { - RuntimeException inspect(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (ArrayStoreException e) { - return e; - } + RuntimeException returnBroaderType(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (%1$s e) { + return e; } } - """, - """ - class Example { - RuntimeException inspect(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (ArrayStoreException | TypeNotPresentException e) { - return e; - } + + Runnable methodReference(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (%1$s e) { + return e::printStackTrace; } } - """ - ) - ); - } - @Test - void alsoCatchWhenHandlerUsesAMethodReferenceOnTheException() { - rewriteRun( - //language=java - java( - """ - class Example { - Runnable printer(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (ArrayStoreException e) { - return e::printStackTrace; - } + void genericInference(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + Objects.requireNonNull(e); + recover(e); } } - """, - """ - class Example { - Runnable printer(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (ArrayStoreException | TypeNotPresentException e) { - return e::printStackTrace; - } + + void appendToMessage(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + String message = "failed: "; + message += e; + System.out.println(message); } } - """ - ) - ); - } - /** - * `Objects.requireNonNull` reports an inferred `ArrayStoreException` parameter, but ` T requireNonNull(T)` - * simply re-infers after widening. - */ - @Test - void alsoCatchWhenHandlerChecksTheExceptionWithRequireNonNull() { - rewriteRun( - //language=java - java( - """ - import java.util.Objects; - - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - Objects.requireNonNull(e); + void synchronize(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + synchronized (e) { recover(e); } } - - void recover(RuntimeException e) { - } } - """, - """ - import java.util.Objects; - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - Objects.requireNonNull(e); - recover(e); - } + void unbracedIf(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + if (flag) Objects.requireNonNull(e); + recover(e); } + } - void recover(RuntimeException e) { - } + void recover(RuntimeException e) { } - """ + } + """; + rewriteRun( + java( + source.formatted("ArrayStoreException"), + source.formatted("ArrayStoreException | TypeNotPresentException") ) ); } - /** - * The widened parameter is typed `RuntimeException`, which an `ArrayStoreException...` parameter rejects. - */ @Test void retainCatchThatPassesTheExceptionToAVarargsParameter() { rewriteRun( @@ -1760,130 +1635,4 @@ void recover(RuntimeException e) { ); } - /** - * {@code message += e} concatenates like {@code message = message + e}, which tolerates any - * {@code RuntimeException}. - */ - @Test - void alsoCatchWhenHandlerAppendsTheExceptionToAMessage() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - String message = "failed: "; - message += e; - System.out.println(message); - } - } - } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - String message = "failed: "; - message += e; - System.out.println(message); - } - } - } - """ - ) - ); - } - - @Test - void alsoCatchWhenHandlerSynchronizesOnTheException() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - synchronized (e) { - recover(e); - } - } - } - - void recover(RuntimeException e) { - } - } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - synchronized (e) { - recover(e); - } - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - /** - * An unbraced statement discards the call's value exactly like a braced one, so migration must not depend - * on brace style. - */ - @Test - void alsoCatchWhenHandlerChecksTheExceptionInAnUnbracedIf() { - rewriteRun( - //language=java - java( - """ - import java.util.Objects; - - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - if (flag) Objects.requireNonNull(e); - recover(e); - } - } - - void recover(RuntimeException e) { - } - } - """, - """ - import java.util.Objects; - - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - if (flag) Objects.requireNonNull(e); - recover(e); - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } } From 369ad3bf4fd9b6f052b132d5884384c968e2ad01 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:08:18 +0200 Subject: [PATCH 4/7] Extract isWildcard explaining method for the GenericTypeVariable wildcard check --- .../ArrayStoreExceptionToTypeNotPresentException.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java index b787f88edd..07af0989ee 100644 --- a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java +++ b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java @@ -551,8 +551,7 @@ private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { return false; } JavaType argument = parameterized.getTypeParameters().get(0); - if (!(argument instanceof JavaType.GenericTypeVariable) || - !"?".equals(((JavaType.GenericTypeVariable) argument).getName())) { + if (!isWildcard(argument)) { return false; } JavaType.GenericTypeVariable wildcard = (JavaType.GenericTypeVariable) argument; @@ -573,6 +572,14 @@ private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { return fullyQualified != null && ACCEPTS_ANY_CLASS.contains(fullyQualified.getFullyQualifiedName()); } + /** + * OpenRewrite models a wildcard type argument as a {@link JavaType.GenericTypeVariable} literally named {@code "?"}. + */ + private static boolean isWildcard(JavaType argument) { + return argument instanceof JavaType.GenericTypeVariable && + "?".equals(((JavaType.GenericTypeVariable) argument).getName()); + } + /** * The enclosing method declaration's return type, or null from a lambda, whose functional interface's * return type is not reliably recoverable here. From ec8ba0fa5fb17a717386ff687221026aae34221f Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:28:35 +0200 Subject: [PATCH 5/7] Replace the widening proof and qualified-name emission with a conservative allow list The catch-parameter check now recognizes only the common handler shapes: calls declared by Throwable except getClass, concatenation and comparison, instanceof, throw, and arguments, initializers and assignments whose declared type accepts every RuntimeException. Sources where the TypeNotPresentException simple name could be shadowed are skipped instead of receiving a fully qualified name. Both changes only cost migrations that are not applied. --- ...oreExceptionToTypeNotPresentException.java | 539 ++---------------- ...xceptionToTypeNotPresentExceptionTest.java | 380 +++--------- 2 files changed, 138 insertions(+), 781 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java index 07af0989ee..df6530e9b5 100644 --- a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java +++ b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java @@ -31,18 +31,13 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; -import java.util.HashMap; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static java.util.Collections.emptySet; -import static java.util.Collections.newSetFromMap; public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe { @@ -66,14 +61,6 @@ public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe "java.lang.RuntimeException", "java.lang.Exception", "java.lang.Throwable", "java.lang.Object", "java.io.Serializable")); - /** - * Types accepting any {@code Class} whatever its type argument. Anything unlisted, such as - * {@code java.lang.constant.Constable} (Java 12+), blocks the widening, as do unresolved types. - */ - private static final Set ACCEPTS_ANY_CLASS = new HashSet<>(asList( - "java.lang.Class", "java.lang.Object", "java.io.Serializable", - "java.lang.reflect.Type", "java.lang.reflect.AnnotatedElement", "java.lang.reflect.GenericDeclaration")); - @Getter final String displayName = "Catch `TypeNotPresentException` thrown by `Class.getAnnotation()`"; @@ -82,38 +69,21 @@ public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe "The `ArrayStoreException` is retained as the protected code can still throw it for reasons unrelated to annotations."; /** - * Where the sources declare their own {@code TypeNotPresentException}, the spliced simple name would resolve - * to it instead, either failing to compile or silently catching the wrong type, so the scanner records those - * declarations and the visitor qualifies the name there. Scoped per {@link JavaProject} marker, since only a - * same-module declaration can shadow; unmarked sources share one scope. + * Where the sources declare their own {@code TypeNotPresentException}, the spliced simple name could + * resolve to it instead, either failing to compile or silently catching the wrong type, so the scanner + * records those declarations and the visitor leaves the affected sources unchanged. Scoped per + * {@link JavaProject} marker, since only a same-module declaration can shadow; unmarked sources share + * one scope. */ public static class Accumulator { - /** - * Per project, the packages declaring a top-level {@code TypeNotPresentException}, {@code ""} for the - * default package. - */ - private final Map<@Nullable JavaProject, Set> packagesByProject = new HashMap<>(); - - /** - * Per project, the classes declaring a nested {@code TypeNotPresentException}, which shadows through - * inheritance and on-demand imports. - */ - private final Map<@Nullable JavaProject, Set> classesByProject = new HashMap<>(); - - void recordPackage(@Nullable JavaProject project, String packageName) { - packagesByProject.computeIfAbsent(project, key -> new HashSet<>()).add(packageName); - } - - void recordClass(@Nullable JavaProject project, String className) { - classesByProject.computeIfAbsent(project, key -> new HashSet<>()).add(className); - } + private final Set<@Nullable JavaProject> declaringProjects = new HashSet<>(); - Set packagesDeclaringTypeNotPresentException(@Nullable JavaProject project) { - return packagesByProject.getOrDefault(project, emptySet()); + void recordDeclaration(@Nullable JavaProject project) { + declaringProjects.add(project); } - Set classesDeclaringTypeNotPresentException(@Nullable JavaProject project) { - return classesByProject.getOrDefault(project, emptySet()); + boolean declaresTypeNotPresentException(@Nullable JavaProject project) { + return declaringProjects.contains(project); } } @@ -128,14 +98,7 @@ public TreeVisitor getScanner(Accumulator acc) { @Override public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(classDecl.getSimpleName())) { - JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class); - JavaProject project = javaProject(sourceFile); - JavaType.FullyQualified owner = classDecl.getType() == null ? null : classDecl.getType().getOwningClass(); - if (owner != null) { - acc.recordClass(project, owner.getFullyQualifiedName()); - } else if (sourceFile != null) { - acc.recordPackage(project, packageName(sourceFile)); - } + acc.recordDeclaration(javaProject(getCursor().firstEnclosing(JavaSourceFile.class))); } return super.visitClassDeclaration(classDecl, ctx); } @@ -154,16 +117,15 @@ public J.Try visitTry(J.Try tryStatement, ExecutionContext ctx) { return try_; } if (anyCatchConcernsTypeNotPresentException(try_) || !protectedRegionCallsGetAnnotation(try_) || - anyEnclosingCatchConcernsTypeNotPresentException(getCursor())) { + anyEnclosingCatchConcernsTypeNotPresentException(getCursor()) || + typeNotPresentExceptionSimpleNameIsShadowed((J.CompilationUnit) sourceFile, acc, javaProject(sourceFile))) { return try_; } Cursor tryCursor = getCursor(); - boolean qualify = typeNotPresentExceptionSimpleNameIsShadowed((J.CompilationUnit) sourceFile, tryCursor, - acc, javaProject(sourceFile)); return try_.withCatches(ListUtils.map(try_.getCatches(), catch_ -> { if (TypeUtils.isOfClassType(catch_.getParameter().getType(), ARRAY_STORE_EXCEPTION) && allParameterReferencesSurviveWidening(catch_, tryCursor)) { - return alsoCatchTypeNotPresentException(catch_, qualify); + return alsoCatchTypeNotPresentException(catch_); } return catch_; })); @@ -273,9 +235,9 @@ private static boolean concernsTypeNotPresentException(@Nullable JavaType type) /** * Per JLS 14.20 a multi-catch parameter is implicitly final and typed as the least upper bound of the - * alternatives, here {@code RuntimeException}. Rather than enumerate the ways a handler can depend on the - * narrower type, every reference must sit in a context that provably tolerates the wider one; anything - * unrecognized leaves the catch untouched. + * alternatives, here {@code RuntimeException}. Rather than prove every way a handler can depend on the + * narrower type safe, only a short allow list of common contexts is recognized; anything else leaves the + * catch untouched, which only costs a migration that is not applied. */ private static boolean allParameterReferencesSurviveWidening(J.Try.Catch catch_, Cursor tryCursor) { List variables = catch_.getParameter().getTree().getVariables(); @@ -325,8 +287,9 @@ private static boolean referencesParameter(J.Identifier identifier, Cursor curso } /** - * Whether an expression the widening retypes keeps compiling, and keeps its meaning, in its context. An - * allow list: an expression-bodied lambda, a switch, or anything unforeseen blocks the widening. + * Whether an expression the widening retypes keeps compiling, and keeps its meaning, in its context. A + * deliberately short allow list covering the common handler shapes; anything unrecognized blocks the + * widening. */ private static boolean widenedReferenceIsSafe(Cursor cursor) { J expression = cursor.getValue(); @@ -336,54 +299,24 @@ private static boolean widenedReferenceIsSafe(Cursor cursor) { // The parenthesized expression widens with its content return widenedReferenceIsSafe(parentCursor); } - if (parent instanceof J.Ternary) { - // The reference can only be a result branch, and the conditional's own type widens with it - J.Ternary ternary = (J.Ternary) parent; - return (expression == ternary.getTruePart() || expression == ternary.getFalsePart()) && - widenedReferenceIsSafe(parentCursor); - } - if (parent instanceof J.Binary || parent instanceof J.InstanceOf || parent instanceof J.Throw || - parent instanceof J.Assert) { - // Concatenation, `==`/`!=`, a type test, throwing (`RuntimeException` is unchecked) and an assert - // message all stay valid and unchanged for the values the original handler could receive + if (parent instanceof J.Binary || parent instanceof J.InstanceOf || parent instanceof J.Throw) { + // Concatenation, `==`/`!=`, a type test and throwing (`RuntimeException` is unchecked) all stay + // valid and unchanged for the values the original handler could receive return true; } - if (parent instanceof J.AssignmentOperation) { - // Only `String +=` compiles with an exception operand, and concatenation tolerates any - // `RuntimeException`; the parameter as the assigned variable fails safe - return expression == ((J.AssignmentOperation) parent).getAssignment(); - } - if (parent instanceof J.ControlParentheses) { - // Of the statements parenthesizing a bare expression only a synchronized monitor keeps its meaning, - // any object being a valid monitor; a pattern switch selector is deliberately excluded - return parentCursor.getParentTreeCursor().getValue() instanceof J.Synchronized; - } - if (parent instanceof J.TypeCast) { - // The cast's own type is unchanged, but one narrower than `RuntimeException` would now throw - return expression == ((J.TypeCast) parent).getExpression() && - acceptsAnyRuntimeException(((J.TypeCast) parent).getType()); - } if (parent instanceof J.MethodInvocation) { J.MethodInvocation invocation = (J.MethodInvocation) parent; if (expression == invocation.getSelect()) { + // Per JLS 4.3.2 `e.getClass()` is `Class` over the receiver's *static* type, so + // its result widens with the receiver and blocks the widening return invokedMethodRemainsAvailable(invocation.getMethodType()) && - (!resultTypeDependsOnReceiverType(invocation.getMethodType()) || - widenedResultIsSafe(parentCursor)); + !"getClass".equals(invocation.getSimpleName()); } - int argumentIndex = invocation.getArguments().indexOf(expression); - return argumentIndex >= 0 && argumentRemainsCompatible(invocation.getMethodType(), argumentIndex, parentCursor); + return argumentRemainsCompatible(invocation.getMethodType(), invocation.getArguments().indexOf(expression)); } if (parent instanceof J.NewClass) { - int argumentIndex = ((J.NewClass) parent).getArguments().indexOf(expression); - return argumentIndex >= 0 && argumentRemainsCompatible(((J.NewClass) parent).getMethodType(), argumentIndex, parentCursor); - } - if (parent instanceof J.MemberReference) { - // Checking the result against the functional interface's method is not reliable here, so a - // receiver-dependent result fails safe - J.MemberReference reference = (J.MemberReference) parent; - return expression == reference.getContaining() && - invokedMethodRemainsAvailable(reference.getMethodType()) && - !resultTypeDependsOnReceiverType(reference.getMethodType()); + return argumentRemainsCompatible(((J.NewClass) parent).getMethodType(), + ((J.NewClass) parent).getArguments().indexOf(expression)); } if (parent instanceof J.VariableDeclarations.NamedVariable) { // Covers an explicit declared type; `var` infers the narrower type and is rejected here @@ -391,35 +324,11 @@ private static boolean widenedReferenceIsSafe(Cursor cursor) { return expression == variable.getInitializer() && acceptsAnyRuntimeException(variable.getType()); } if (parent instanceof J.Assignment) { + // The parameter as the assigned variable fails: a multi-catch parameter is implicitly final J.Assignment assignment = (J.Assignment) parent; - if (expression == assignment.getVariable()) { - // A multi-catch parameter is implicitly final - return false; - } - return acceptsAnyRuntimeException(assignment.getVariable().getType()); - } - if (parent instanceof J.Return) { - JavaType returnType = enclosingMethodReturnType(parentCursor); - return returnType != null && acceptsAnyRuntimeException(returnType); - } - if (parent instanceof J.NewArray) { - J.NewArray newArray = (J.NewArray) parent; - JavaType type = newArray.getType(); - return newArray.getInitializer() != null && newArray.getInitializer().contains(expression) && - type instanceof JavaType.Array && acceptsAnyRuntimeException(((JavaType.Array) type).getElemType()); + return expression != assignment.getVariable() && acceptsAnyRuntimeException(assignment.getVariable().getType()); } - return isStatementPosition(parent); - } - - /** - * A parent holding the expression as a statement discards its value, so it compiles however its type - * widens. Unbraced forms discard it exactly as a block does. A switch's arrow case is deliberately absent, - * since there the expression may be the switch's own value. - */ - private static boolean isStatementPosition(J parent) { - return parent instanceof J.Block || parent instanceof J.If || parent instanceof J.If.Else || - parent instanceof J.Label || parent instanceof J.WhileLoop || parent instanceof J.DoWhileLoop || - parent instanceof J.ForLoop || parent instanceof J.ForEachLoop; + return false; } /** @@ -430,188 +339,15 @@ private static boolean invokedMethodRemainsAvailable(JavaType.@Nullable Method m return methodType != null && acceptsAnyRuntimeException(methodType.getDeclaringType()); } - /** - * Whether the invocation's result type widens with its receiver. Per JLS 4.3.2 {@code e.getClass()} is - * {@code Class} over the receiver's *static* type, so widening it silently changes the - * result. {@code getClass()} is the only receiver-polymorphic member in {@code java.lang} and is - * recognized by name and arity; a signature mentioning {@code ArrayStoreException} or a type variable - * counts too, covering any future member whose attribution exposes the same dependence. - */ - private static boolean resultTypeDependsOnReceiverType(JavaType.Method methodType) { - return "getClass".equals(methodType.getName()) && methodType.getParameterTypes().isEmpty() || - involvesReceiverTypeArgument(methodType.getReturnType(), newIdentitySet()); - } - - /** - * Whether the context tolerates a result widened to {@code Class}. Mirrors - * {@link #widenedReferenceIsSafe} with the acceptance test adjusted to the class type. - */ - private static boolean widenedResultIsSafe(Cursor cursor) { - J expression = cursor.getValue(); - Cursor parentCursor = cursor.getParentTreeCursor(); - J parent = parentCursor.getValue(); - if (parent instanceof J.Parentheses) { - return widenedResultIsSafe(parentCursor); - } - if (parent instanceof J.Ternary) { - J.Ternary ternary = (J.Ternary) parent; - return (expression == ternary.getTruePart() || expression == ternary.getFalsePart()) && - widenedResultIsSafe(parentCursor); - } - if (parent instanceof J.Binary) { - // Widening the wildcard's bound breaks neither concatenation nor the cast-compatibility `==` and - // `!=` require, every such type staying castable - J.Binary.Type operator = ((J.Binary) parent).getOperator(); - return operator == J.Binary.Type.Addition || operator == J.Binary.Type.Equal || - operator == J.Binary.Type.NotEqual; - } - if (parent instanceof J.MethodInvocation) { - J.MethodInvocation invocation = (J.MethodInvocation) parent; - if (expression == invocation.getSelect()) { - // A chained call is a member of `Class` and so stays available, valid exactly when its resolved - // signature avoids the receiver's type argument; `cast()` and the like fail safe - JavaType.Method methodType = invocation.getMethodType(); - if (methodType == null || involvesReceiverTypeArgument(methodType.getReturnType(), newIdentitySet())) { - return false; - } - for (JavaType parameterType : methodType.getParameterTypes()) { - if (involvesReceiverTypeArgument(parameterType, newIdentitySet())) { - return false; - } - } - return true; - } - int argumentIndex = invocation.getArguments().indexOf(expression); - if (argumentIndex < 0 || invocation.getMethodType() == null) { - return false; - } - JavaType parameterType = parameterType(invocation.getMethodType(), argumentIndex); - return parameterType != null && acceptsWidenedClassResult(parameterType); - } - if (parent instanceof J.VariableDeclarations.NamedVariable) { - J.VariableDeclarations.NamedVariable variable = (J.VariableDeclarations.NamedVariable) parent; - return expression == variable.getInitializer() && acceptsWidenedClassResult(variable.getType()); - } - if (parent instanceof J.Assignment) { - J.Assignment assignment = (J.Assignment) parent; - return expression != assignment.getVariable() && acceptsWidenedClassResult(assignment.getVariable().getType()); - } - if (parent instanceof J.Return) { - JavaType returnType = enclosingMethodReturnType(parentCursor); - return returnType != null && acceptsWidenedClassResult(returnType); - } - return isStatementPosition(parent); - } - - /** - * Whether the type mentions {@code ArrayStoreException}, a type variable, a wildcard or an unresolved type - * anywhere, in which case it cannot be relied on to survive the widening. - */ - private static boolean involvesReceiverTypeArgument(@Nullable JavaType type, Set visited) { - if (type == null || type instanceof JavaType.Unknown) { - return true; - } - if (!visited.add(type)) { - return false; - } - if (type instanceof JavaType.GenericTypeVariable) { - return true; - } - JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); - if (fullyQualified != null && ARRAY_STORE_EXCEPTION.equals(fullyQualified.getFullyQualifiedName())) { - return true; - } - if (type instanceof JavaType.Parameterized) { - for (JavaType typeParameter : ((JavaType.Parameterized) type).getTypeParameters()) { - if (involvesReceiverTypeArgument(typeParameter, visited)) { - return true; - } - } - } else if (type instanceof JavaType.Array) { - return involvesReceiverTypeArgument(((JavaType.Array) type).getElemType(), visited); - } else if (type instanceof JavaType.Intersection) { - for (JavaType bound : ((JavaType.Intersection) type).getBounds()) { - if (involvesReceiverTypeArgument(bound, visited)) { - return true; - } - } - } - return false; - } - - /** - * Whether a position of this type accepts a {@code Class}: raw {@code Class} or - * a supertype, {@code Class}, or a covariant wildcard whose bounds all accept any {@code RuntimeException}. - */ - private static boolean acceptsWidenedClassResult(@Nullable JavaType type) { - if (type instanceof JavaType.Parameterized) { - JavaType.Parameterized parameterized = (JavaType.Parameterized) type; - if (!"java.lang.Class".equals(parameterized.getType().getFullyQualifiedName()) || - parameterized.getTypeParameters().size() != 1) { - return false; - } - JavaType argument = parameterized.getTypeParameters().get(0); - if (!isWildcard(argument)) { - return false; - } - JavaType.GenericTypeVariable wildcard = (JavaType.GenericTypeVariable) argument; - if (wildcard.getBounds().isEmpty()) { - return true; - } - if (wildcard.getVariance() != JavaType.GenericTypeVariable.Variance.COVARIANT) { - return false; - } - for (JavaType bound : wildcard.getBounds()) { - if (!acceptsAnyRuntimeException(bound)) { - return false; - } - } - return true; - } - JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); - return fullyQualified != null && ACCEPTS_ANY_CLASS.contains(fullyQualified.getFullyQualifiedName()); - } - - /** - * OpenRewrite models a wildcard type argument as a {@link JavaType.GenericTypeVariable} literally named {@code "?"}. - */ - private static boolean isWildcard(JavaType argument) { - return argument instanceof JavaType.GenericTypeVariable && - "?".equals(((JavaType.GenericTypeVariable) argument).getName()); - } - - /** - * The enclosing method declaration's return type, or null from a lambda, whose functional interface's - * return type is not reliably recoverable here. - */ - private static @Nullable JavaType enclosingMethodReturnType(Cursor returnCursor) { - for (Cursor cursor = returnCursor.getParent(); cursor != null; cursor = cursor.getParent()) { - Object enclosing = cursor.getValue(); - if (enclosing instanceof J.Lambda) { - return null; - } - if (enclosing instanceof J.MethodDeclaration) { - JavaType.Method methodType = ((J.MethodDeclaration) enclosing).getMethodType(); - return methodType == null ? null : methodType.getReturnType(); - } - } - return null; - } - - private static boolean argumentRemainsCompatible(JavaType.@Nullable Method methodType, int argumentIndex, - Cursor invocationCursor) { - if (methodType == null) { + private static boolean argumentRemainsCompatible(JavaType.@Nullable Method methodType, int argumentIndex) { + if (methodType == null || argumentIndex < 0) { // Unresolved: the parameter's requirements are unknowable, so leave the catch alone return false; } + // The resolved type reports the *inferred* argument type, so a generic parameter such as + // ` T requireNonNull(T)` resolves to `ArrayStoreException` and is rejected with it JavaType parameterType = parameterType(methodType, argumentIndex); - if (parameterType == null) { - return false; - } - if (acceptsAnyRuntimeException(parameterType)) { - return true; - } - return inferredTypeParameterAcceptsWidening(methodType, argumentIndex, invocationCursor); + return parameterType != null && acceptsAnyRuntimeException(parameterType); } private static @Nullable JavaType parameterType(JavaType.Method methodType, int argumentIndex) { @@ -629,148 +365,26 @@ private static boolean argumentRemainsCompatible(JavaType.@Nullable Method metho return parameterType; } - /** - * The resolved method type reports the *inferred* argument type, so {@code Objects.requireNonNull(e)} looks - * like it takes an {@code ArrayStoreException} although {@code T requireNonNull(T)} would simply - * re-infer. Consult the declaration instead: safe when the parameter is a type variable of the method - * itself (a class variable is fixed by the receiver), its bounds all accept any {@code RuntimeException}, - * no other parameter constrains it, and a result mentioning it is itself used safely. - */ - private static boolean inferredTypeParameterAcceptsWidening(JavaType.Method methodType, int argumentIndex, - Cursor invocationCursor) { - J call = invocationCursor.getValue(); - if (!(call instanceof J.MethodInvocation) || ((J.MethodInvocation) call).getTypeParameters() != null) { - // Explicit type arguments do not re-infer, and constructor inference is driven by the class type - return false; - } - JavaType.Method declared = declaredMethod(methodType); - if (declared == null) { - return false; - } - JavaType declaredParameter = parameterType(declared, argumentIndex); - if (!(declaredParameter instanceof JavaType.GenericTypeVariable)) { - return false; - } - JavaType.GenericTypeVariable typeVariable = (JavaType.GenericTypeVariable) declaredParameter; - if (declaredByClass(typeVariable.getName(), declared.getDeclaringType())) { - return false; - } - for (JavaType bound : typeVariable.getBounds()) { - if (!acceptsAnyRuntimeException(bound)) { - return false; - } - } - List declaredParameterTypes = declared.getParameterTypes(); - int parameterIndex = Math.min(argumentIndex, declaredParameterTypes.size() - 1); - for (int i = 0; i < declaredParameterTypes.size(); i++) { - if (i != parameterIndex && mentionsTypeVariable(declaredParameterTypes.get(i), typeVariable.getName(), newIdentitySet())) { - return false; - } - } - if (mentionsTypeVariable(declared.getReturnType(), typeVariable.getName(), newIdentitySet())) { - // The call's own type widens with the parameter, so its context must be safe as well - return widenedReferenceIsSafe(invocationCursor); - } - return true; - } - - /** - * The single declaration matching the resolved method by name and arity, or null when ambiguous. - */ - private static JavaType.@Nullable Method declaredMethod(JavaType.Method methodType) { - JavaType.Method declared = null; - for (JavaType.Method candidate : methodType.getDeclaringType().getMethods()) { - if (candidate.getName().equals(methodType.getName()) && - candidate.getParameterTypes().size() == methodType.getParameterTypes().size()) { - if (declared != null) { - return null; - } - declared = candidate; - } - } - return declared; - } - - /** - * Whether the declaring class or an owner declares a type variable of this name. A method reusing the name - * declares its own, so this errs towards the class and only blocks a widening that may have been safe. - */ - private static boolean declaredByClass(String typeVariableName, JavaType.@Nullable FullyQualified declaringType) { - for (JavaType.FullyQualified type = declaringType; type != null; type = type.getOwningClass()) { - JavaType.FullyQualified unwrapped = type instanceof JavaType.Parameterized ? ((JavaType.Parameterized) type).getType() : type; - for (JavaType typeParameter : unwrapped.getTypeParameters()) { - if (typeParameter instanceof JavaType.GenericTypeVariable && - typeVariableName.equals(((JavaType.GenericTypeVariable) typeParameter).getName())) { - return true; - } - } - } - return false; - } - - private static boolean mentionsTypeVariable(@Nullable JavaType type, String typeVariableName, Set visited) { - if (type == null || !visited.add(type)) { - return false; - } - if (type instanceof JavaType.GenericTypeVariable) { - if (typeVariableName.equals(((JavaType.GenericTypeVariable) type).getName())) { - return true; - } - for (JavaType bound : ((JavaType.GenericTypeVariable) type).getBounds()) { - if (mentionsTypeVariable(bound, typeVariableName, visited)) { - return true; - } - } - } else if (type instanceof JavaType.Array) { - return mentionsTypeVariable(((JavaType.Array) type).getElemType(), typeVariableName, visited); - } else if (type instanceof JavaType.Parameterized) { - for (JavaType typeParameter : ((JavaType.Parameterized) type).getTypeParameters()) { - if (mentionsTypeVariable(typeParameter, typeVariableName, visited)) { - return true; - } - } - } else if (type instanceof JavaType.Intersection) { - for (JavaType bound : ((JavaType.Intersection) type).getBounds()) { - if (mentionsTypeVariable(bound, typeVariableName, visited)) { - return true; - } - } - } - return false; - } - - private static Set newIdentitySet() { - return newSetFromMap(new IdentityHashMap<>()); - } - private static boolean acceptsAnyRuntimeException(@Nullable JavaType type) { JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type); return fullyQualified != null && SUPERTYPES_OF_RUNTIME_EXCEPTION.contains(fullyQualified.getFullyQualifiedName()); } /** - * Whether the simple name would resolve to anything but {@code java.lang.TypeNotPresentException}: a class - * or type parameter of that name in this file, a single-type import, a top-level class in this package or - * reachable through an on-demand import, a nested class inherited from an enclosing class's supertype, or - * any such type already referenced here. A shadowing class visible only as a compiled dependency, or - * declared in another {@link JavaProject}, cannot be seen from here, so the simple name is emitted for it. + * Whether the simple name could resolve to anything but {@code java.lang.TypeNotPresentException}: a class + * of that name declared anywhere in this file's {@link JavaProject}, a type parameter of that name in this + * file, a single-type import of another such class, or any such type already referenced here. Such sources + * are left unchanged; emitting a qualified name instead was judged not worth the machinery. A shadowing + * class visible only as a compiled dependency, or declared in another project, cannot be seen from here + * and is not detected. */ - private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.CompilationUnit cu, Cursor tryCursor, - Accumulator acc, @Nullable JavaProject project) { - Set declaringPackages = acc.packagesDeclaringTypeNotPresentException(project); - Set declaringClasses = acc.classesDeclaringTypeNotPresentException(project); - if (declaringPackages.contains(packageName(cu))) { + private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.CompilationUnit cu, Accumulator acc, + @Nullable JavaProject project) { + if (acc.declaresTypeNotPresentException(project)) { return true; } for (J.Import import_ : cu.getImports()) { - String simpleName = import_.getQualid().getSimpleName(); - if ("*".equals(simpleName)) { - String imported = qualifierName(import_.getQualid().getTarget()); - if (imported != null && - (declaringPackages.contains(imported) || declaringClasses.contains(imported))) { - return true; - } - } else if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(simpleName)) { + if (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(import_.getQualid().getSimpleName())) { JavaType.FullyQualified imported = TypeUtils.asFullyQualified(import_.getQualid().getType()); if (imported == null || !TYPE_NOT_PRESENT_EXCEPTION.equals(imported.getFullyQualifiedName())) { return true; @@ -783,19 +397,6 @@ private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.Compilation return true; } } - for (Cursor cursor = tryCursor; cursor != null; cursor = cursor.getParent()) { - Object enclosing = cursor.getValue(); - JavaType.FullyQualified enclosingType = null; - if (enclosing instanceof J.ClassDeclaration) { - enclosingType = ((J.ClassDeclaration) enclosing).getType(); - } else if (enclosing instanceof J.NewClass && ((J.NewClass) enclosing).getBody() != null) { - TypeTree clazz = ((J.NewClass) enclosing).getClazz(); - enclosingType = clazz == null ? null : TypeUtils.asFullyQualified(clazz.getType()); - } - if (anySupertypeDeclaresTypeNotPresentException(enclosingType, declaringClasses, new HashSet<>())) { - return true; - } - } return declaresTypeNotPresentException(cu); } @@ -807,21 +408,6 @@ private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.Compilation return sourceFile == null ? null : sourceFile.getMarkers().findFirst(JavaProject.class).orElse(null); } - private static String packageName(JavaSourceFile sourceFile) { - return sourceFile.getPackageDeclaration() == null ? "" : sourceFile.getPackageDeclaration().getPackageName(); - } - - private static @Nullable String qualifierName(Expression expression) { - if (expression instanceof J.Identifier) { - return ((J.Identifier) expression).getSimpleName(); - } - if (expression instanceof J.FieldAccess) { - String target = qualifierName(((J.FieldAccess) expression).getTarget()); - return target == null ? null : target + "." + ((J.FieldAccess) expression).getSimpleName(); - } - return null; - } - private static boolean isForeignTypeNotPresentException(String fullyQualifiedName) { return !TYPE_NOT_PRESENT_EXCEPTION.equals(fullyQualifiedName) && (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(fullyQualifiedName) || @@ -829,24 +415,6 @@ private static boolean isForeignTypeNotPresentException(String fullyQualifiedNam fullyQualifiedName.endsWith("$" + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME)); } - private static boolean anySupertypeDeclaresTypeNotPresentException(JavaType.@Nullable FullyQualified type, - Set declaringClasses, Set visited) { - for (JavaType.FullyQualified enclosing = type; enclosing != null; enclosing = enclosing.getSupertype()) { - if (!visited.add(enclosing.getFullyQualifiedName())) { - return false; - } - if (declaringClasses.contains(enclosing.getFullyQualifiedName())) { - return true; - } - for (JavaType.FullyQualified interface_ : enclosing.getInterfaces()) { - if (anySupertypeDeclaresTypeNotPresentException(interface_, declaringClasses, visited)) { - return true; - } - } - } - return false; - } - private static boolean declaresTypeNotPresentException(J.CompilationUnit cu) { AtomicBoolean found = new AtomicBoolean(false); new JavaIsoVisitor() { @@ -878,21 +446,14 @@ public J.TypeParameter visitTypeParameter(J.TypeParameter typeParameter, AtomicB * modifiers and annotations. Keeping it as the first alternative preserves all of that, as * {@code CombineSemanticallyEqualCatchBlocks} does upstream. */ - private static J.Try.Catch alsoCatchTypeNotPresentException(J.Try.Catch catch_, boolean qualify) { + private static J.Try.Catch alsoCatchTypeNotPresentException(J.Try.Catch catch_) { J.VariableDeclarations parameter = catch_.getParameter().getTree(); TypeTree typeExpression = parameter.getTypeExpression(); if (typeExpression == null) { return catch_; } - TypeTree typeNotPresentException; - if (qualify) { - TypeTree qualified = TypeTree.build(TYPE_NOT_PRESENT_EXCEPTION); - qualified = qualified.withType(JavaType.ShallowClass.build(TYPE_NOT_PRESENT_EXCEPTION)); - typeNotPresentException = qualified.withPrefix(Space.SINGLE_SPACE); - } else { - typeNotPresentException = new J.Identifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, - emptyList(), TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME, JavaType.ShallowClass.build(TYPE_NOT_PRESENT_EXCEPTION), null); - } + TypeTree typeNotPresentException = new J.Identifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, + emptyList(), TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME, JavaType.ShallowClass.build(TYPE_NOT_PRESENT_EXCEPTION), null); J.MultiCatch multiCatch = new J.MultiCatch(Tree.randomId(), typeExpression.getPrefix(), Markers.EMPTY, asList( JRightPadded.build(typeExpression.withPrefix(Space.EMPTY)).withAfter(Space.SINGLE_SPACE), JRightPadded.build(typeNotPresentException))); diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index dba433eaae..b534390aef 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -580,8 +580,6 @@ static class MissingType extends TypeNotPresentException { void widenCatchWhenHandlerAcceptsRuntimeException() { //language=java var source = """ - import java.util.Objects; - class Example { void logAndRethrow(Class type) { try { @@ -601,81 +599,101 @@ void wrap(Class type) { } } - void assign(Class type, boolean flag) { + void assign(Class type) { try { type.getAnnotation(Override.class); } catch (%1$s e) { RuntimeException cause = e; - RuntimeException chosen = flag ? e : null; + cause = e; recover(cause); - recover(chosen); + recover(e); } } - RuntimeException returnBroaderType(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (%1$s e) { - return e; - } + void recover(RuntimeException e) { } + } + """; + rewriteRun( + java( + source.formatted("ArrayStoreException"), + source.formatted("ArrayStoreException | TypeNotPresentException") + ) + ); + } - Runnable methodReference(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (%1$s e) { - return e::printStackTrace; + /** + * The allow list deliberately stops at the common handler shapes; any use of the parameter it does not + * recognize retains the catch, which only costs a migration that is not applied. + */ + @Test + void retainCatchWhoseHandlerUsesTheExceptionBeyondTheAllowList() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Example { + RuntimeException returnBroaderType(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e; + } } - } - void genericInference(Class type) { - try { - type.getAnnotation(Override.class); - } catch (%1$s e) { - Objects.requireNonNull(e); - recover(e); + Runnable methodReference(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return e::printStackTrace; + } } - } - void appendToMessage(Class type) { - try { - type.getAnnotation(Override.class); - } catch (%1$s e) { - String message = "failed: "; - message += e; - System.out.println(message); + void genericInference(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + Objects.requireNonNull(e); + } } - } - void synchronize(Class type) { - try { - type.getAnnotation(Override.class); - } catch (%1$s e) { - synchronized (e) { - recover(e); + void ternary(Class type, boolean flag) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + RuntimeException chosen = flag ? e : null; + recover(chosen); } } - } - void unbracedIf(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (%1$s e) { - if (flag) Objects.requireNonNull(e); - recover(e); + void appendToMessage(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + String message = "failed: "; + message += e; + System.out.println(message); + } } - } - void recover(RuntimeException e) { + void synchronize(Class type) { + try { + type.getAnnotation(Override.class); + } catch (ArrayStoreException e) { + synchronized (e) { + recover(e); + } + } + } + + void recover(RuntimeException e) { + } } - } - """; - rewriteRun( - java( - source.formatted("ArrayStoreException"), - source.formatted("ArrayStoreException | TypeNotPresentException") + """ ) ); } @@ -1057,23 +1075,23 @@ void recover(RuntimeException e) { /** * Per JLS 4.3.2 the type of {@code e.getClass()} is {@code Class} where {@code |E|} is the - * erasure of the receiver's static type, so widening the receiver would change this initializer's type - * from {@code Class} to {@code Class}, which no - * longer compiles. + * erasure of the receiver's static type, so its result widens with the receiver. Proving a particular use + * of that result safe is not worth the machinery, so any read of the class retains the catch, even one a + * wider {@code Class} would tolerate. */ @Test - void retainCatchThatReadsTheExceptionClassAsTheNarrowerClassType() { + void retainCatchThatReadsTheExceptionClass() { rewriteRun( //language=java java( """ class Example { - void inspect(Class type, boolean flag) { + void inspect(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { Class narrow = e.getClass(); - Class viaTernary = flag ? (e.getClass()) : null; + Class wide = e.getClass(); recover(e); } } @@ -1086,145 +1104,6 @@ void recover(RuntimeException e) { ); } - @Test - void retainCatchThatPassesTheExceptionClassToANarrowerClassParameter() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - report(e.getClass()); - } - } - - void report(Class failure) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatBindsTheExceptionClassMethodReference() { - rewriteRun( - //language=java - java( - """ - import java.util.function.Supplier; - - class Example { - Supplier> inspect(Class type) { - try { - type.getAnnotation(Override.class); - return null; - } catch (ArrayStoreException e) { - return e::getClass; - } - } - } - """ - ) - ); - } - - /** - * {@code Class.cast()} returns the class's own type argument, so after widening it would return - * {@code RuntimeException} rather than {@code ArrayStoreException}. - */ - @Test - void retainCatchThatCastsThroughTheExceptionClass() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type, Object value) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - ArrayStoreException narrowed = e.getClass().cast(value); - recover(narrowed); - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - /** - * {@code Objects.requireNonNull(e)} re-infers the parameter's own type, so {@code getClass()} on its - * result depends on the widening just as it does on the parameter directly. - */ - @Test - void retainCatchThatReadsTheLaunderedExceptionClass() { - rewriteRun( - //language=java - java( - """ - import java.util.Objects; - - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - Class narrow = Objects.requireNonNull(e).getClass(); - } - } - } - """ - ) - ); - } - - /** - * {@code Class} and signatures like {@code getName()} that do not involve the class's type argument - * tolerate the widened {@code Class}, so common logging keeps migrating. - */ - @Test - void alsoCatchWhenHandlerReadsTheExceptionClassGenerically() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - Class wide = e.getClass(); - String name = e.getClass().getName(); - System.out.println("caught " + name + wide); - } - } - } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | TypeNotPresentException e) { - Class wide = e.getClass(); - String name = e.getClass().getName(); - System.out.println("caught " + name + wide); - } - } - } - """ - ) - ); - } - /** * Every {@code TypeNotPresentException} the inner try does not catch reaches the enclosing handler, so * widening the inner catch would steal the exception from it. @@ -1310,10 +1189,10 @@ void recover(RuntimeException e) { /** * The nested class shadows the simple name, and because it extends {@code RuntimeException} the simple - * name would even compile while binding the catch to the wrong type; the fully qualified name is emitted. + * name would even compile while binding the catch to the wrong type; the file is left unchanged. */ @Test - void alsoCatchFullyQualifiedWhenNestedClassShadowsSimpleName() { + void retainWhenNestedClassShadowsSimpleName() { rewriteRun( //language=java java( @@ -1333,30 +1212,13 @@ void recover(RuntimeException e) { static class TypeNotPresentException extends RuntimeException { } } - """, - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { - recover(e); - } - } - - void recover(RuntimeException e) { - } - - static class TypeNotPresentException extends RuntimeException { - } - } """ ) ); } @Test - void alsoCatchFullyQualifiedWhenImportShadowsSimpleName() { + void retainWhenImportShadowsSimpleName() { rewriteRun( //language=java java( @@ -1386,24 +1248,6 @@ void inspect(Class type) { void recover(RuntimeException e) { } } - """, - """ - package example; - - import shadow.TypeNotPresentException; - - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { - recover(e); - } - } - - void recover(RuntimeException e) { - } - } """ ) ); @@ -1414,7 +1258,7 @@ void recover(RuntimeException e) { * scanner records every source-declared class of this name. */ @Test - void alsoCatchFullyQualifiedWhenSamePackageClassShadowsSimpleName() { + void retainWhenSamePackageClassShadowsSimpleName() { rewriteRun( //language=java java( @@ -1442,22 +1286,6 @@ void inspect(Class type) { void recover(RuntimeException e) { } } - """, - """ - package example; - - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { - recover(e); - } - } - - void recover(RuntimeException e) { - } - } """ ) ); @@ -1523,11 +1351,11 @@ void recover(RuntimeException e) { } /** - * Within one {@code JavaProject} the declaration still shadows: the marked sibling source qualifies the - * name exactly as an unmarked one does. + * Within one {@code JavaProject} the declaration still shadows: the marked sibling source is left + * unchanged exactly as an unmarked one is. */ @Test - void alsoCatchFullyQualifiedWhenShadowingClassIsDeclaredInTheSameJavaProject() { + void retainWhenShadowingClassIsDeclaredInTheSameJavaProject() { var module = new JavaProject(randomId(), "module-a", null); rewriteRun( //language=java @@ -1558,22 +1386,6 @@ void recover(RuntimeException e) { } } """, - """ - package example; - - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { - recover(e); - } - } - - void recover(RuntimeException e) { - } - } - """, spec -> spec.markers(module) ) ); @@ -1581,10 +1393,10 @@ void recover(RuntimeException e) { /** * A nested class inherited from a supertype shadows the simple name inside the subclass; it would compile - * while binding the catch to the inherited type, so the fully qualified name is emitted. + * while binding the catch to the inherited type, so the file is left unchanged. */ @Test - void alsoCatchFullyQualifiedWhenInheritedNestedClassShadowsSimpleName() { + void retainWhenInheritedNestedClassShadowsSimpleName() { rewriteRun( //language=java java( @@ -1614,22 +1426,6 @@ void inspect(Class type) { void recover(RuntimeException e) { } } - """, - """ - package example; - - class Example extends Base { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException | java.lang.TypeNotPresentException e) { - recover(e); - } - } - - void recover(RuntimeException e) { - } - } """ ) ); From 33c544f14d758410c085383f771ba052eabcb85b Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 23:43:53 +0200 Subject: [PATCH 6/7] Name the separators and document the format hole --- .../ArrayStoreExceptionToTypeNotPresentException.java | 6 ++++-- .../ArrayStoreExceptionToTypeNotPresentExceptionTest.java | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java index df6530e9b5..3c79e15b75 100644 --- a/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java +++ b/src/main/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentException.java @@ -44,6 +44,8 @@ public class ArrayStoreExceptionToTypeNotPresentException extends ScanningRecipe private static final String ARRAY_STORE_EXCEPTION = "java.lang.ArrayStoreException"; private static final String TYPE_NOT_PRESENT_EXCEPTION = "java.lang.TypeNotPresentException"; private static final String TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME = "TypeNotPresentException"; + private static final String PACKAGE_SEPARATOR = "."; + private static final String NESTED_TYPE_SEPARATOR = "$"; private static final MethodMatcher CLASS_GET_ANNOTATION = new MethodMatcher("java.lang.Class getAnnotation(java.lang.Class)"); /** @@ -411,8 +413,8 @@ private static boolean typeNotPresentExceptionSimpleNameIsShadowed(J.Compilation private static boolean isForeignTypeNotPresentException(String fullyQualifiedName) { return !TYPE_NOT_PRESENT_EXCEPTION.equals(fullyQualifiedName) && (TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME.equals(fullyQualifiedName) || - fullyQualifiedName.endsWith("." + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME) || - fullyQualifiedName.endsWith("$" + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME)); + fullyQualifiedName.endsWith(PACKAGE_SEPARATOR + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME) || + fullyQualifiedName.endsWith(NESTED_TYPE_SEPARATOR + TYPE_NOT_PRESENT_EXCEPTION_SIMPLE_NAME)); } private static boolean declaresTypeNotPresentException(J.CompilationUnit cu) { diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index b534390aef..71fc88764e 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -578,6 +578,7 @@ static class MissingType extends TypeNotPresentException { @Test void widenCatchWhenHandlerAcceptsRuntimeException() { + // %1$s fills every catch site with the single formatted argument //language=java var source = """ class Example { From e6d0ac5b040f6cc5c2375a9a6ece7a3a6f8af1a9 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 17 Aug 2026 02:14:08 +0200 Subject: [PATCH 7/7] Consolidate scenario tests into shared fixtures and cover real consumer handler shapes One fixture each for calls outside the protected region, for tries that already handle TypeNotPresentException, and for parameter uses beyond the allow list, plus two positive handler shapes mirroring WildFly's BusinessViewAnnotationProcessor and Grails' DomainClassArtefactHandler. --- ...xceptionToTypeNotPresentExceptionTest.java | 488 +++--------------- 1 file changed, 85 insertions(+), 403 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java index 71fc88764e..99f8fed2ed 100644 --- a/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/ArrayStoreExceptionToTypeNotPresentExceptionTest.java @@ -230,14 +230,24 @@ public void close() { ); } + /** + * Only the try's resources and body are protected by its catches: a call in a finally block, a sibling + * catch, or a body that runs after the try never throws into these handlers. The immediately invoked + * lambda and the instance initializer do run inside the protected region, but the bodies of lambdas and + * classes created in the try are conservatively skipped as a whole, costing only a missed migration. + */ @Test - void lookupOnlyInFinally() { + void retainWhenGetAnnotationIsOutsideTheProtectedRegion() { rewriteRun( //language=java java( """ + import java.lang.annotation.Annotation; + import java.util.List; + import java.util.function.Function; + class Example { - void inspect(Class type, Object value) { + void inFinally(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -248,22 +258,7 @@ void inspect(Class type, Object value) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void lookupOnlyInSiblingCatch() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type, Object value) { + void inSiblingCatch(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -274,22 +269,7 @@ void inspect(Class type, Object value) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void lookupOnlyInDeferredLambda() { - rewriteRun( - //language=java - java( - """ - class Example { - Runnable inspectLater(Class type, Object value) { + Runnable inDeferredLambda(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -301,24 +281,7 @@ Runnable inspectLater(Class type, Object value) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void lookupOnlyInMethodReference() { - rewriteRun( - //language=java - java( - """ - import java.util.function.Function; - - class Example { - Function, Override> inspectLater(Class type, Object value) { + Function, Override> inMethodReference(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -329,28 +292,7 @@ Function, Override> inspectLater(Class type, Object value) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - /** - * Whether a lambda created inside the try runs before the try completes cannot be decided locally, so the - * handler is left alone. - */ - @Test - void lookupOnlyInImmediatelyInvokedLambda() { - rewriteRun( - //language=java - java( - """ - import java.util.List; - - class Example { - void inspect(List> types, Object value) { + void inImmediatelyInvokedLambda(List> types, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -360,22 +302,7 @@ void inspect(List> types, Object value) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void lookupOnlyInAnonymousClass() { - rewriteRun( - //language=java - java( - """ - class Example { - Runnable inspectLater(Class type, Object value) { + Runnable inAnonymousClass(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -391,22 +318,7 @@ public void run() { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void lookupOnlyInLocalClass() { - rewriteRun( - //language=java - java( - """ - class Example { - Runnable inspectLater(Class type, Object value) { + Runnable inLocalClass(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -423,28 +335,7 @@ public void run() { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - /** - * Instance initializers run at the {@code new}, inside the protected region, so this lookup can throw into - * the enclosing catch. The whole anonymous body is left out regardless, costing only a missed migration. - */ - @Test - void lookupOnlyInAnonymousClassInstanceInitializer() { - rewriteRun( - //language=java - java( - """ - import java.lang.annotation.Annotation; - - class Example { - Runnable inspectLater(Class type, Object value) { + Runnable inAnonymousClassInstanceInitializer(Class type, Object value) { try { Object[] values = new String[1]; values[0] = value; @@ -469,14 +360,19 @@ void recover(RuntimeException e) { ); } + /** + * A sibling catch of {@code TypeNotPresentException} or a supertype already handles it, a catch of a + * subclass would become unreachable, and a multi-catch parameter is typed as the least upper bound + * rather than {@code ArrayStoreException}, so none of these tries change. + */ @Test - void existingTypeNotPresentExceptionCatch() { + void retainWhenTypeNotPresentExceptionIsAlreadyHandled() { rewriteRun( //language=java java( """ class Example { - void inspect(Class type) { + void existingCatch(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { @@ -486,22 +382,7 @@ void inspect(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void existingMultiCatch() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void existingMultiCatch(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException | IllegalStateException e) { @@ -509,22 +390,7 @@ void inspect(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void existingBroaderCatchAlreadyHandlesTypeNotPresentException() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void existingBroaderCatch(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { @@ -534,25 +400,7 @@ void inspect(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - /** - * A catch of a `TypeNotPresentException` subclass would become unreachable if the earlier handler widened. - */ - @Test - void existingTypeNotPresentExceptionSubclassCatch() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void existingSubclassCatch(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { @@ -576,6 +424,11 @@ static class MissingType extends TypeNotPresentException { ); } + /** + * The last two handlers mirror real consumers: WildFly's {@code BusinessViewAnnotationProcessor} throws + * its own deployment error without referencing the parameter, and Grails' {@code DomainClassArtefactHandler} + * swallows the failure with an empty catch. + */ @Test void widenCatchWhenHandlerAcceptsRuntimeException() { // %1$s fills every catch site with the single formatted argument @@ -611,6 +464,24 @@ void assign(Class type) { } } + void requireAnnotation(Class type) { + try { + type.getAnnotation(Override.class); + } catch (%1$s e) { + throw new IllegalStateException("missing class in annotation on " + type.getName()); + } + } + + boolean isDomainClass(Class type) { + Deprecated artefact = null; + try { + artefact = type.getAnnotation(Deprecated.class); + } catch (%1$s e) { + // a reference to a class that no longer exists + } + return artefact != null; + } + void recover(RuntimeException e) { } } @@ -634,6 +505,7 @@ void retainCatchWhoseHandlerUsesTheExceptionBeyondTheAllowList() { java( """ import java.util.Objects; + import java.util.function.Supplier; class Example { RuntimeException returnBroaderType(Class type) { @@ -654,6 +526,15 @@ Runnable methodReference(Class type) { } } + Supplier expressionLambda(Class type) { + try { + type.getAnnotation(Override.class); + return null; + } catch (ArrayStoreException e) { + return () -> e; + } + } + void genericInference(Class type) { try { type.getAnnotation(Override.class); @@ -691,68 +572,23 @@ void synchronize(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatPassesTheExceptionToAVarargsParameter() { - rewriteRun( - //language=java - java( - """ - class Example { - void log(String message, ArrayStoreException... exceptions) { - } - - void inspect(Class type) { + void passToVarargs(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { log("failed", e); } } - } - """ - ) - ); - } - @Test - void retainCatchThatPassesTheExceptionToANarrowerParameter() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void passToNarrowerParameter(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { - recover(e); + recoverNarrow(e); } } - void recover(ArrayStoreException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatAssignsTheExceptionToANarrowerVariable() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void assignToNarrowerVariable(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { @@ -761,69 +597,7 @@ void inspect(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatUsesTheExceptionInATernaryInitializer() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - ArrayStoreException copy = flag ? e : null; - recover(copy); - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatPassesATernaryToANarrowerParameter() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type, boolean flag) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - recover(flag ? e : null); - } - } - - void recover(ArrayStoreException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatStoresTheExceptionInAnArrayInitializer() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void storeInArrayInitializer(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { @@ -832,79 +606,45 @@ void inspect(Class type) { } } - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - - @Test - void retainCatchThatReturnsTheExceptionFromAnExpressionLambda() { - rewriteRun( - //language=java - java( - """ - import java.util.function.Supplier; - - class Example { - Supplier inspect(Class type) { + void castToNarrowerType(Class type) { try { type.getAnnotation(Override.class); - return null; } catch (ArrayStoreException e) { - return () -> e; + // The cast still compiles, but would throw for the TypeNotPresentException values a widened handler receives + Object narrowed = (ArrayStoreException) e; + recover(e); } } - } - """ - ) - ); - } - @Test - void retainCatchThatReturnsTheExceptionAsTheNarrowerType() { - rewriteRun( - //language=java - java( - """ - class Example { - ArrayStoreException inspect(Class type) { + void reassignParameter(Class type) { try { type.getAnnotation(Override.class); - return null; } catch (ArrayStoreException e) { - return e; + // A multi-catch parameter is implicitly final, so this would not compile widened + e = new ArrayStoreException("wrapped"); + recover(e); } } - } - """ - ) - ); - } - /** - * The cast still compiles, but throws for the `TypeNotPresentException` values the widened handler receives. - */ - @Test - void retainCatchThatCastsTheExceptionToTheNarrowerType() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { + void readTheExceptionClass(Class type) { try { type.getAnnotation(Override.class); } catch (ArrayStoreException e) { - Object narrowed = (ArrayStoreException) e; + // Per JLS 4.3.2 `e.getClass()` is typed over the receiver's static type, so it widens with it + Class narrow = e.getClass(); + Class wide = e.getClass(); recover(e); } } + void log(String message, ArrayStoreException... exceptions) { + } + void recover(RuntimeException e) { } + + void recoverNarrow(ArrayStoreException e) { + } } """ ) @@ -935,33 +675,6 @@ void inspect(Class type) { ); } - /** - * A multi-catch parameter is implicitly final, so widening this handler would not compile. - */ - @Test - void retainCatchThatReassignsTheException() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - e = new ArrayStoreException("wrapped"); - recover(e); - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - /** * Only the caught exception is implicitly final; an unrelated variable that happens to share its name is not. */ @@ -1074,37 +787,6 @@ void recover(RuntimeException e) { ); } - /** - * Per JLS 4.3.2 the type of {@code e.getClass()} is {@code Class} where {@code |E|} is the - * erasure of the receiver's static type, so its result widens with the receiver. Proving a particular use - * of that result safe is not worth the machinery, so any read of the class retains the catch, even one a - * wider {@code Class} would tolerate. - */ - @Test - void retainCatchThatReadsTheExceptionClass() { - rewriteRun( - //language=java - java( - """ - class Example { - void inspect(Class type) { - try { - type.getAnnotation(Override.class); - } catch (ArrayStoreException e) { - Class narrow = e.getClass(); - Class wide = e.getClass(); - recover(e); - } - } - - void recover(RuntimeException e) { - } - } - """ - ) - ); - } - /** * Every {@code TypeNotPresentException} the inner try does not catch reaches the enclosing handler, so * widening the inner catch would steal the exception from it.