Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Qute validation - improve hierarchy indexing to fix assignability issues #30593

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Qute validation - improve hierarchy indexing to fix assignability issues #30593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...ions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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;
}

}