Merge pull request #23725 from mkouba/issue-23677

Qute type-safe expressions - ignore wildcards in parameter declarations

docs/src/main/asciidoc/qute-reference.adoc

@@ -1297,6 +1297,11 @@ For example, if there is an expression `item.name` where `item` maps to `org.acm
 An optional _parameter declaration_ is used to bind a Java type to expressions whose first part matches the parameter name.
 Parameter declarations are specified directly in a template.
 
+A Java type should be always identified with a _fully qualified name_ unless it's a JDK type from the `java.lang` package - in this case, the package name is optional.
+Parameterized types are supported, however wildcards are always ignored - only the upper/lower bound is taken into account.
+For example, the parameter declaration `{@java.util.List<? extends org.acme.Foo> list}` is recognized as `{@java.util.List<org.acme.Foo> list}`.
+Type variables are not handled in a special way and should never be used.
+
 .Parameter Declaration Example
 [source,html]
 ----
@@ -1343,6 +1348,19 @@ Note that sections can override names that would otherwise match a parameter dec
 ----
 <1> Validated against `org.acme.Foo`.
 <2> Not validated - `foo` is overridden in the loop section.
+
+.More Parameter Declarations Examples 
+[source]
+----
+{@int pages} <1>
+{@java.util.List<String> strings} <2>
+{@java.util.Map<String,? extends Number> numbers} <3>
+{@java.util.Optional<?> param} <4>
+----
+<1> A primitive type.
+<2> `String` is replaced with `java.lang.String`: `{@java.util.List<java.lang.String> strings}`
+<3> The wildcard is ignored and the upper bound is used instead: `{@java.util.Map<String,Number>}`
+<4> The wildcard is ignored and the `java.lang.Object` is used instead: `{@java.util.Optional<java.lang.Object>}`
 
 [[typesafe_templates]]
 === Type-safe Templates

extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java

@@ -619,6 +619,8 @@ static Match validateNestedExpressions(QuteConfig config, TemplateAnalysis templ
             List<TemplateExtensionMethodBuildItem> regularExtensionMethods,
             Map<String, List<TemplateExtensionMethodBuildItem>> namespaceExtensionMethods) {
 
+        LOGGER.debugf("Validate %s from %s", expression, expression.getOrigin());
+
         // Validate the parameters of nested virtual methods
         for (Expression.Part part : expression.getParts()) {
             if (part.isVirtualMethod()) {

extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TypeInfos.java

@@ -62,20 +62,26 @@ static Info create(String typeInfo, Expression.Part part, IndexView index, Funct
                 // TODO make the parsing logic more robust 
                 ClassInfo rawClass;
                 Type resolvedType;
-                int idx = classStr.indexOf(ARRAY_DIM);
-                if (idx > 0) {
+                int arrayDimIdx = classStr.indexOf(ARRAY_DIM);
+                if (arrayDimIdx > 0) {
                     // int[], java.lang.String[][], etc.
-                    String componentTypeStr = classStr.substring(0, idx);
+                    String componentTypeStr = classStr.substring(0, arrayDimIdx);
                     Type componentType = decodePrimitive(componentTypeStr);
                     if (componentType == null) {
                         componentType = resolveType(componentTypeStr);
                     }
-                    String[] dimensions = classStr.substring(idx, classStr.length()).split("\\]");
+                    String[] dimensions = classStr.substring(arrayDimIdx, classStr.length()).split("\\]");
                     rawClass = null;
                     resolvedType = ArrayType.create(componentType, dimensions.length);
                 } else {
-                    rawClass = getClassInfo(classStr, index, templateIdToPathFun, expressionOrigin);
-                    resolvedType = resolveType(classStr);
+                    Type primitiveType = decodePrimitive(classStr);
+                    if (primitiveType != null) {
+                        resolvedType = primitiveType;
+                        rawClass = null;
+                    } else {
+                        rawClass = getClassInfo(classStr, index, templateIdToPathFun, expressionOrigin);
+                        resolvedType = resolveType(classStr);
+                    }
                 }
                 return new TypeInfo(typeInfo, part, helperHint(typeInfo.substring(endIdx, typeInfo.length())), resolvedType,
                         rawClass);

extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java

@@ -1,5 +1,6 @@
 package io.quarkus.qute.deployment;
 
+import java.lang.reflect.Modifier;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -154,6 +155,7 @@ static boolean isAssignableFrom(DotName class1, DotName class2, IndexView index)
         for (ClassInfo implementor : implementors) {
             assignables.add(implementor.name());
         }
+        assignables.addAll(getAllInterfacesExtending(class1, index));
         return assignables.contains(class2);
     }
 
@@ -187,4 +189,17 @@ 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())) {
+                continue;
+            }
+            if (clazz.interfaceNames().contains(target)) {
+                ret.add(clazz.name());
+            }
+        }
+        return ret;
+    }
+
 }

extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplatePrimitiveTypeTest.java

@@ -0,0 +1,34 @@
+package io.quarkus.qute.deployment.typesafe;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.qute.CheckedTemplate;
+import io.quarkus.qute.TemplateInstance;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class CheckedTemplatePrimitiveTypeTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(Templates.class)
+                    .addAsResource(new StringAsset("Hello {val}!"),
+                            "templates/CheckedTemplatePrimitiveTypeTest/integers.txt"));
+
+    @Test
+    public void testPrimitiveParamBinding() {
+        assertEquals("Hello 1!", Templates.integers(1).render());
+    }
+
+    @CheckedTemplate
+    public static class Templates {
+
+        static native TemplateInstance integers(int val);
+
+    }
+
+}

extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/MovieExtensions.java

@@ -21,4 +21,8 @@ static boolean negate(boolean val) {
         return !val;
     }
 
+    static int toInt(Movie movie, int val) {
+        return val;
+    }
+
 }

extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/ValidationSuccessTest.java

@@ -22,7 +22,8 @@ public class ValidationSuccessTest {
                     .addAsResource(new StringAsset("{@io.quarkus.qute.deployment.typesafe.Movie movie}"
                             + "{@java.lang.Long age}"
                             + "{@java.lang.String surname}"
-                            + "{@java.util.Map<String,String map}"
+                            + "{@java.util.Map<String,String> map}"
+                            + "{@int cislo}"
                             // Property found
                             + "{movie.name} "
                             // Built-in value resolvers
@@ -41,6 +42,8 @@ public class ValidationSuccessTest {
                             + "{movie.toNumber(surname)} "
                             // Varargs extension method
                             + "{movie.toLong(1l,2l)} "
+                            // Primitive type in param declaration
+                            + "{movie.toInt(cislo)} "
                             // Field access
                             + "{#each movie.mainCharacters}{it.substring(1)}{/} "
                             // Method param assignability
@@ -53,9 +56,9 @@ public class ValidationSuccessTest {
     @Test
     public void testResult() {
         // Validation succeeded! Yay!
-        assertEquals("Jason Jason Mono Stereo true 1 10 11 ok 43 3 ohn bar",
+        assertEquals("Jason Jason Mono Stereo true 1 10 11 ok 43 3 1 ohn bar",
                 movie.data("movie", new Movie("John"), "name", "Vasik", "surname", "Hu", "age", 10l, "map",
-                        Collections.singletonMap("foo", "bar")).render());
+                        Collections.singletonMap("foo", "bar")).data("cislo", 1).render());
     }
 
 }

extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/EngineProducer.java

@@ -84,7 +84,6 @@ public EngineProducer(QuteContext context, QuteConfig config, QuteRuntimeConfig
         EngineBuilder builder = Engine.builder();
 
         // We don't register the map resolver because of param declaration validation
-        // See DefaultTemplateExtensions
         builder.addValueResolver(ValueResolvers.thisResolver());
         builder.addValueResolver(ValueResolvers.orResolver());
         builder.addValueResolver(ValueResolvers.trueResolver());

extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/extensions/CollectionTemplateExtensions.java

@@ -1,5 +1,7 @@
 package io.quarkus.qute.runtime.extensions;
 
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
@@ -64,4 +66,10 @@ static <T> List<T> takeLast(List<T> list, int n) {
         return list.subList(list.size() - n, list.size());
     }
 
+    // This extension method has higher priority than ValueResolvers.orEmpty() 
+    // and makes it possible to validate expressions derived from {list.orEmpty} 
+    static <T> Collection<T> orEmpty(Collection<T> iterable) {
+        return iterable != null ? iterable : Collections.emptyList();
+    }
+
 }

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanArchives.java

@@ -96,7 +96,16 @@ public IndexWrapper(IndexView index, ClassLoader deploymentClassLoader,
 
         @Override
         public Collection<ClassInfo> getKnownClasses() {
-            return index.getKnownClasses();
+            if (additionalClasses.isEmpty()) {
+                return index.getKnownClasses();
+            }
+            List<ClassInfo> all = new ArrayList<>(index.getKnownClasses());
+            for (Optional<ClassInfo> additional : additionalClasses.values()) {
+                if (additional.isPresent()) {
+                    all.add(additional.get());
+                }
+            }
+            return all;
         }
 
         @Override

independent-projects/qute/core/src/main/java/io/quarkus/qute/Scope.java

@@ -23,16 +23,18 @@ public Scope(Scope parentScope) {
     }
 
     public void putBinding(String binding, String type) {
-        if (bindings == null)
+        if (bindings == null) {
             bindings = new HashMap<>();
-        bindings.put(binding, type);
+        }
+        bindings.put(binding, sanitizeType(type));
     }
 
     public String getBinding(String binding) {
         // we can contain null types to override outer scopes
         if (bindings != null
-                && bindings.containsKey(binding))
+                && bindings.containsKey(binding)) {
             return bindings.get(binding);
+        }
         return parentScope != null ? parentScope.getBinding(binding) : null;
     }
 
@@ -60,4 +62,23 @@ public void setLastPartHint(String lastPartHint) {
         this.lastPartHint = lastPartHint;
     }
 
+    private String sanitizeType(String type) {
+        if (type != null && type.contains("?")) {
+            String upperBound = " extends ";
+            String lowerBound = " super ";
+            // ignore wildcards
+            if (type.contains(upperBound)) {
+                // upper bound
+                type = type.replace("?", "").replace(upperBound, "").trim();
+            } else if (type.contains(lowerBound)) {
+                // lower bound
+                type = type.replace("?", "").replace(lowerBound, "").trim();
+            } else {
+                // no bounds
+                type = type.replace("?", "java.lang.Object");
+            }
+        }
+        return type;
+    }
+
 }

independent-projects/qute/core/src/test/java/io/quarkus/qute/ScopeTest.java

@@ -0,0 +1,27 @@
+package io.quarkus.qute;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+public class ScopeTest {
+
+    @Test
+    public void testSanitizeType() {
+        assertBinding("List<? extends Foo>", "List<Foo>");
+        assertBinding("List<? super Foo>", "List<Foo>");
+        assertBinding("List< ? super  Foo>", "List<Foo>");
+        assertBinding("List<?>", "List<java.lang.Object>");
+        assertBinding("Map<String, ?>", "Map<String,java.lang.Object>");
+        assertBinding("Map<String, ? extends Foo>", "Map<String,Foo>");
+        assertBinding("Map<String, ? extends List<Foo>>", "Map<String,List<Foo>>");
+        assertBinding("Map<? extend java.lang.Object, ?>", "Map<java.lang.Object,java.lang.Object>");
+    }
+
+    private void assertBinding(String type, String sanitizedType) {
+        Scope scope = new Scope(null);
+        scope.putBinding("foo", sanitizedType);
+        assertEquals(sanitizedType, scope.getBinding("foo"));
+    }
+
+}