From 5cc625e4d69c50500d07e044fbbf0824773b8820 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis <geoand@gmail.com> Date: Thu, 22 Apr 2021 16:20:29 +0300 Subject: [PATCH] Ensure that a non-indexed leaf doesn't break generic type resolution This can happen if say some interface extends a JDK interface and there is no reason the type resolution should fail because of it --- .../quarkus/deployment/util/JandexUtil.java | 57 ++++++++++++++----- .../deployment/util/JandexUtilTest.java | 15 +++++ 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java b/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java index f85928fd3a0b8..18a9f9e103c7a 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -65,14 +66,27 @@ private JandexUtil() { * then the result will contain a single element of class ClassType whose name() would return a DotName for String */ public static List<Type> resolveTypeParameters(DotName input, DotName target, IndexView index) { - final ClassInfo inputClassInfo = fetchFromIndex(input, index); + final ClassInfo inputClassInfo; + try { + inputClassInfo = fetchFromIndex(input, index); + } catch (Exception e) { + // keep compatibility with what clients already expect + throw new IllegalArgumentException(e); + } Type startingType = getType(inputClassInfo, index); + Set<DotName> unindexedClasses = new LinkedHashSet<>(); final List<Type> result = findParametersRecursively(startingType, target, - new HashSet<>(), index); + new HashSet<>(), index, unindexedClasses); // null means not found if (result == null) { - return Collections.emptyList(); + if (unindexedClasses.isEmpty()) { + // no un-indexed classes means that there were no problems traversing the class and interface hierarchies + return Collections.emptyList(); + } + throw new IllegalArgumentException( + "The following classes were not part of the index and could be the reason that the captured generic type of '" + + target + "' could not be determined: " + unindexedClasses); } return result; @@ -98,7 +112,7 @@ private static Type getType(ClassInfo inputClassInfo, IndexView index) { * generics when found, on the way down. Returns null if not found. */ private static List<Type> findParametersRecursively(Type type, DotName target, - Set<DotName> visitedTypes, IndexView index) { + Set<DotName> visitedTypes, IndexView index, Set<DotName> unindexedClasses) { DotName name = type.name(); // cache results first if (!visitedTypes.add(name)) { @@ -123,18 +137,26 @@ private static List<Type> findParametersRecursively(Type type, DotName target, // superclasses first Type superClassType = inputClassInfo.superClassType(); - List<Type> superResult = findParametersRecursively(superClassType, target, visitedTypes, index); - if (superResult != null) { - // map any returned type parameters to our type arguments on the way down - return mapTypeArguments(superClassType, superResult, index); + try { + List<Type> superResult = findParametersRecursively(superClassType, target, visitedTypes, index, unindexedClasses); + if (superResult != null) { + // map any returned type parameters to our type arguments on the way down + return mapTypeArguments(superClassType, superResult, index); + } + } catch (ClassNotIndexedException e) { + unindexedClasses.add(e.dotName); } // interfaces second for (Type interfaceType : inputClassInfo.interfaceTypes()) { - List<Type> ret = findParametersRecursively(interfaceType, target, visitedTypes, index); - if (ret != null) { - // map any returned type parameters to our type arguments on the way down - return mapTypeArguments(interfaceType, ret, index); + try { + List<Type> ret = findParametersRecursively(interfaceType, target, visitedTypes, index, unindexedClasses); + if (ret != null) { + // map any returned type parameters to our type arguments on the way down + return mapTypeArguments(interfaceType, ret, index); + } + } catch (ClassNotIndexedException e) { + unindexedClasses.add(e.dotName); } } @@ -260,7 +282,7 @@ private static Type mapGenerics(Type type, Map<String, Type> mapping) { private static ClassInfo fetchFromIndex(DotName dotName, IndexView index) { final ClassInfo classInfo = index.getClassByName(dotName); if (classInfo == null) { - throw new IllegalArgumentException("Class " + dotName + " was not found in the index"); + throw new ClassNotIndexedException(dotName); } return classInfo; } @@ -344,4 +366,13 @@ public static String getBoxedTypeName(Type type) { } return type.toString(); } + + private static class ClassNotIndexedException extends RuntimeException { + + private final DotName dotName; + + public ClassNotIndexedException(DotName dotName) { + this.dotName = dotName; + } + } } diff --git a/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java b/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java index d7dd632f46ed7..e2b8a47b0130c 100644 --- a/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java +++ b/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java @@ -1,6 +1,7 @@ package io.quarkus.deployment.util; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.io.InputStream; @@ -154,6 +155,20 @@ public void testErasedGenerics() { checkRepoArg(index, ErasedRepo2.class, Repo.class, A.class); } + @Test + public void testNonProblematicUnindexed() { + final Index index = index(Single.class, SingleFromInterfaceAndSuperClass.class); + checkRepoArg(index, SingleFromInterfaceAndSuperClass.class, Single.class, String.class); + } + + @Test + public void testProblematicUnindexed() { + final Index index = index(Single.class, AbstractSingleImpl.class, ExtendsAbstractSingleImpl.class); + assertThatThrownBy(() -> { + JandexUtil.resolveTypeParameters(name(ExtendsAbstractSingleImpl.class), name(Single.class), index); + }).isInstanceOf(IllegalArgumentException.class); + } + public interface Single<T> { }