Skip to content

Commit

Permalink
Qute validation - improve hierarchy indexing to fix assignability issues
Browse files Browse the repository at this point in the history 
- related to #redhat-developer-demos/quarkus-petclinic/issues/7
  • Loading branch information
@mkouba
mkouba committed Jan 25, 2023
1 parent 3affaf2 commit d576fa2
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 60 deletions.
6 changes: 4 additions & 2 deletions ...ions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import io.quarkus.qute.deployment.QuteProcessor.Match;
import io.quarkus.qute.deployment.TemplatesAnalysisBuildItem.TemplateAnalysis;
import io.quarkus.qute.deployment.Types.AssignableInfo;
import io.quarkus.qute.deployment.Types.HierarchyIndexer;
import io.quarkus.qute.generator.Descriptors;
import io.quarkus.qute.generator.ValueResolverGenerator;
import io.quarkus.qute.i18n.Localized;
Expand Down Expand Up @@ -428,6 +429,7 @@ public String apply(String id) {

LookupConfig lookupConfig = new QuteProcessor.FixedLookupConfig(index, QuteProcessor.initDefaultMembersFilter(), false);
Map<DotName, AssignableInfo> assignableCache = new HashMap<>();
HierarchyIndexer hierarchyIndexer = new HierarchyIndexer(index);

// bundle name -> (key -> method)
Map<String, Map<String, MethodInfo>> bundleMethodsMap = new HashMap<>();
Expand Down Expand Up @@ -546,8 +548,8 @@ public String apply(String id) {
results, excludes, incorrectExpressions, expression, index,
implicitClassToMembersUsed, templateIdToPathFun, generatedIdsToMatches,
extensionMethodExcludes, checkedTemplate, lookupConfig, namedBeans,
namespaceTemplateData,
regularExtensionMethods, namespaceExtensionMethods, assignableCache);
namespaceTemplateData, regularExtensionMethods, namespaceExtensionMethods,
assignableCache, hierarchyIndexer);
Match match = results.get(param.toOriginalString());
if (match != null && !match.isEmpty() && !Types.isAssignableFrom(match.type(),
methodParams.get(idx), index, assignableCache)) {
Expand Down
30 changes: 17 additions & 13 deletions extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
import io.quarkus.qute.deployment.TypeCheckExcludeBuildItem.TypeCheck;
import io.quarkus.qute.deployment.TypeInfos.Info;
import io.quarkus.qute.deployment.Types.AssignableInfo;
import io.quarkus.qute.deployment.Types.HierarchyIndexer;
import io.quarkus.qute.generator.ExtensionMethodGenerator;
import io.quarkus.qute.generator.ExtensionMethodGenerator.NamespaceResolverCreator;
import io.quarkus.qute.generator.ExtensionMethodGenerator.NamespaceResolverCreator.ResolveCreator;
Expand Down Expand Up @@ -856,6 +857,7 @@ public String apply(String id) {

LookupConfig lookupConfig = new FixedLookupConfig(index, initDefaultMembersFilter(), false);
Map<DotName, AssignableInfo> assignableCache = new HashMap<>();
HierarchyIndexer hierarchyIndexer = new HierarchyIndexer(index);
int expressionsValidated = 0;

for (TemplateAnalysis templateAnalysis : templatesAnalysis.getAnalysis()) {
Expand Down Expand Up @@ -884,7 +886,7 @@ public String apply(String id) {
incorrectExpressions, expression, index, implicitClassToMembersUsed, templateIdToPathFun,
generatedIdsToMatches, extensionMethodExcludes,
checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData, regularExtensionMethods,
namespaceExtensionMethods, assignableCache);
namespaceExtensionMethods, assignableCache, hierarchyIndexer);
generatedIdsToMatches.put(expression.getGeneratedId(), match);
}

Expand All @@ -894,7 +896,7 @@ public String apply(String id) {
if (defaultValue != null) {
Match match;
if (defaultValue.isLiteral()) {
match = new Match(index, assignableCache);
match = new Match(hierarchyIndexer, assignableCache);
setMatchValues(match, defaultValue, generatedIdsToMatches, index);
} else {
match = generatedIdsToMatches.get(defaultValue.getGeneratedId());
Expand Down Expand Up @@ -1001,7 +1003,7 @@ static Match validateNestedExpressions(QuteConfig config, TemplateAnalysis templ
Map<String, TemplateDataBuildItem> namespaceTemplateData,
List<TemplateExtensionMethodBuildItem> regularExtensionMethods,
Map<String, List<TemplateExtensionMethodBuildItem>> namespaceExtensionMethods,
Map<DotName, AssignableInfo> assignableCache) {
Map<DotName, AssignableInfo> assignableCache, HierarchyIndexer hierarchyIndexer) {

LOGGER.debugf("Validate %s from %s", expression, expression.getOrigin());

Expand All @@ -1017,13 +1019,14 @@ static Match validateNestedExpressions(QuteConfig config, TemplateAnalysis templ
validateNestedExpressions(config, templateAnalysis, null, results, excludes,
incorrectExpressions, param, index, implicitClassToMembersUsed, templateIdToPathFun,
generatedIdsToMatches, extensionMethodExcludes, checkedTemplate, lookupConfig, namedBeans,
namespaceTemplateData, regularExtensionMethods, namespaceExtensionMethods, assignableCache);
namespaceTemplateData, regularExtensionMethods, namespaceExtensionMethods, assignableCache,
hierarchyIndexer);
}
}
}
}

Match match = new Match(index, assignableCache);
Match match = new Match(hierarchyIndexer, assignableCache);

String namespace = expression.getNamespace();
TypeInfos.TypeInfo dataNamespaceExpTypeInfo = null;
Expand Down Expand Up @@ -2365,14 +2368,14 @@ static Type extractMatchType(Set<Type> closure, DotName matchName, Function<Type

static class Match {

private final IndexView index;
private final HierarchyIndexer indexer;
private final Map<DotName, AssignableInfo> assignableCache;

private ClassInfo clazz;
private Type type;

Match(IndexView index, Map<DotName, AssignableInfo> assignableCache) {
this.index = index;
Match(HierarchyIndexer indexer, Map<DotName, AssignableInfo> assignableCache) {
this.indexer = indexer;
this.assignableCache = assignableCache;
}

Expand Down Expand Up @@ -2428,19 +2431,20 @@ boolean isEmpty() {
void autoExtractType() {
if (clazz != null) {
// Make sure that hierarchy of the matching class is indexed
Types.indexHierarchy(clazz, index);
boolean hasCompletionStage = Types.isAssignableFrom(Names.COMPLETION_STAGE, clazz.name(), index,
indexer.indexHierarchy(clazz);
boolean hasCompletionStage = Types.isAssignableFrom(Names.COMPLETION_STAGE, clazz.name(), indexer.index,
assignableCache);
boolean hasUni = hasCompletionStage ? false
: Types.isAssignableFrom(Names.UNI, clazz.name(), index, assignableCache);
: Types.isAssignableFrom(Names.UNI, clazz.name(), indexer.index, assignableCache);
if (hasCompletionStage || hasUni) {
Set<Type> closure = Types.getTypeClosure(clazz, Types.buildResolvedMap(
getParameterizedTypeArguments(), getTypeParameters(), new HashMap<>(), index), index);
getParameterizedTypeArguments(), getTypeParameters(), new HashMap<>(), indexer.index),
indexer.index);
// CompletionStage<List<Item>> => List<Item>
// Uni<List<String>> => List<String>
this.type = extractMatchType(closure, hasCompletionStage ? Names.COMPLETION_STAGE : Names.UNI,
FIRST_PARAM_TYPE_EXTRACT_FUN);
this.clazz = index.getClassByName(type.name());
this.clazz = indexer.index.getClassByName(type.name());
}
}
}
Expand Down
120 changes: 75 additions & 45 deletions extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package io.quarkus.qute.deployment;

import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
Expand All @@ -17,13 +18,16 @@
import org.jboss.jandex.Type;
import org.jboss.jandex.Type.Kind;
import org.jboss.jandex.TypeVariable;
import org.jboss.logging.Logger;

import io.quarkus.arc.processor.DotNames;

public final class Types {

static final String JAVA_LANG_PREFIX = "java.lang.";

private static final Logger LOG = Logger.getLogger(Types.class);

static Set<Type> getTypeClosure(ClassInfo classInfo, Map<TypeVariable, Type> resolvedTypeParameters,
IndexView index) {
Set<Type> types = new HashSet<>();
Expand Down Expand Up @@ -138,58 +142,99 @@ static boolean isAssignableFrom(Type type1, Type type2, IndexView index, Map<Dot

static class AssignableInfo {

static AssignableInfo from(ClassInfo classInfo, IndexView index) {
if (classInfo.isInterface()) {
return new AssignableInfo(null, toNames(index.getAllKnownImplementors(classInfo.name())),
toNames(index.getAllKnownSubinterfaces(classInfo.name())));
} else {
return new AssignableInfo(toNames(index.getAllKnownSubclasses(classInfo.name())), null, null);
}
}

private static Set<DotName> toNames(Collection<ClassInfo> classes) {
return classes.stream().map(ClassInfo::name).collect(Collectors.toSet());
}

final Set<DotName> subclasses;
final Set<DotName> implementors;
final Set<DotName> extendingInterfaces;
final Set<DotName> subInterfaces;

public AssignableInfo(Collection<ClassInfo> subclasses, Collection<ClassInfo> implementors,
Set<DotName> extendingInterfaces) {
this.subclasses = new HashSet<>();
for (ClassInfo subclass : subclasses) {
this.subclasses.add(subclass.name());
}
this.implementors = new HashSet<>();
for (ClassInfo implementor : implementors) {
this.implementors.add(implementor.name());
}
this.extendingInterfaces = extendingInterfaces;
AssignableInfo(Set<DotName> subclasses, Set<DotName> implementors, Set<DotName> subInterfaces) {
this.subclasses = subclasses;
this.implementors = implementors;
this.subInterfaces = subInterfaces;
}

boolean isAssignableFrom(DotName clazz) {
return subclasses.contains(clazz) || implementors.contains(clazz) || extendingInterfaces.contains(clazz);
if (subclasses != null && subclasses.contains(clazz)) {
return true;
}
if (implementors != null && implementors.contains(clazz)) {
return true;
}
return subInterfaces != null && subInterfaces.contains(clazz);
}

}

static boolean isAssignableFrom(DotName class1, DotName class2, IndexView index,
static boolean isAssignableFrom(DotName className1, DotName className2, IndexView index,
Map<DotName, AssignableInfo> assignableCache) {
// java.lang.Object is assignable from any type
if (class1.equals(DotNames.OBJECT)) {
if (className1.equals(DotNames.OBJECT)) {
return true;
}
// type1 is the same as type2
if (class1.equals(class2)) {
if (className1.equals(className2)) {
return true;
}
AssignableInfo assignableInfo = assignableCache.get(class1);
ClassInfo class1 = index.getClassByName(className1);
AssignableInfo assignableInfo = assignableCache.get(className1);
if (assignableInfo == null) {
assignableInfo = new AssignableInfo(index.getAllKnownSubclasses(class1), index.getAllKnownImplementors(class1),
getAllInterfacesExtending(class1, index));
assignableCache.put(class1, assignableInfo);
// No cached info
assignableInfo = AssignableInfo.from(class1, index);
assignableCache.put(className1, assignableInfo);
return assignableInfo.isAssignableFrom(className2);
} else {
if (assignableInfo.isAssignableFrom(className2)) {
return true;
}
// Cached info does not match - try to update the assignable info (a computing index is used)
assignableInfo = AssignableInfo.from(class1, index);
if (assignableInfo.isAssignableFrom(className2)) {
// Update the cache
assignableCache.put(className1, assignableInfo);
return true;
}
}
return assignableInfo.isAssignableFrom(class2);
return false;
}

static void indexHierarchy(ClassInfo classInfo, IndexView index) {
// Interfaces
for (DotName interfaceName : classInfo.interfaceNames()) {
index.getClassByName(interfaceName);
// This class is not thread-safe
static class HierarchyIndexer {

final IndexView index;
final Set<DotName> processed;

public HierarchyIndexer(IndexView index) {
this.index = Objects.requireNonNull(index);
this.processed = new HashSet<>();
}
// Superclass
DotName superName = classInfo.superName();
if (superName != null && !superName.equals(DotNames.OBJECT)) {
indexHierarchy(index.getClassByName(superName), index);

void indexHierarchy(ClassInfo classInfo) {
if (classInfo != null && processed.add(classInfo.name())) {
LOG.debugf("Index hierarchy of: %s", classInfo);
// Interfaces
for (DotName interfaceName : classInfo.interfaceNames()) {
indexHierarchy(index.getClassByName(interfaceName));
}
// Superclass
DotName superName = classInfo.superName();
if (superName != null && !superName.equals(DotNames.OBJECT)) {
indexHierarchy(index.getClassByName(superName));
}
}
}

}

static Type box(Type type) {
Expand Down Expand Up @@ -222,19 +267,4 @@ static Type box(Primitive primitive) {
}
}

private static Set<DotName> getAllInterfacesExtending(DotName target, IndexView index) {
Set<DotName> ret = new HashSet<>();
for (ClassInfo clazz : index.getKnownClasses()) {
if (!Modifier.isInterface(clazz.flags())
|| clazz.isAnnotation()
|| clazz.isEnum()) {
continue;
}
if (clazz.interfaceNames().contains(target)) {
ret.add(clazz.name());
}
}
return ret;
}

}

0 comments on commit d576fa2

Please sign in to comment.