Qute checked templates - require type-safe expressions by default

- related to #14534
quarkusio · Apr 13, 2021 · cb56ccc · cb56ccc
1 parent f32102d
commit cb56ccc
Show file tree

Hide file tree

Showing 8 changed files with 226 additions and 44 deletions.
diff --git a/docs/src/main/asciidoc/qute-reference.adoc b/docs/src/main/asciidoc/qute-reference.adoc
@@ -1148,7 +1148,7 @@ Note that sections can override names that would otherwise match a parameter dec
 
 [[typesafe_templates]]
 === Type-safe Templates
-You can also declare your templates in your Java code.
+You can also define type-safe templates in your Java code.
 If using <<resteasy_integration,templates in JAX-RS resources>>, you can rely on the following convention:
 
 - Organise your template files in the `/src/main/resources/templates` directory, by grouping them into one directory per resource class. So, if
@@ -1193,6 +1193,9 @@ public class ItemResource {
 <1> Declare a method that gives us a `TemplateInstance` for `templates/ItemResource/item.html` and declare its `Item item` parameter so we can validate the template.
 <2> The `item` parameter is automatically turned into a <<typesafe_expressions,parameter declaration>> and so all expressions that reference this name will be validated.
 <3> Make the `Item` object accessible in the template.
+
+TIP: By default, the templates defined in a class annotated with `@CheckedTemplate` can only contain type-safe expressions, i.e. expressions that can be validated at build time. You can use `@CheckedTemplate(requireTypeSafeExpressions = false)` to relax this requirement.
+
 
 You can also declare a top-level Java class annotated with `@CheckedTemplate`:
 

diff --git a/...ns/qute/deployment/src/main/java/io/quarkus/qute/deployment/CheckedTemplateBuildItem.java b/...ns/qute/deployment/src/main/java/io/quarkus/qute/deployment/CheckedTemplateBuildItem.java
@@ -5,17 +5,26 @@
 import org.jboss.jandex.MethodInfo;
 
 import io.quarkus.builder.item.MultiBuildItem;
+import io.quarkus.qute.CheckedTemplate;
 
+/**
+ * Represents a method of a class annotated with {@link CheckedTemplate}.
+ */
 public final class CheckedTemplateBuildItem extends MultiBuildItem {
 
+    // A template path, potentially incomplete
     public final String templateId;
+
     public final Map<String, String> bindings;
     public final MethodInfo method;
+    public final boolean requireTypeSafeExpressions;
 
-    public CheckedTemplateBuildItem(String templateId, Map<String, String> bindings, MethodInfo method) {
+    public CheckedTemplateBuildItem(String templateId, Map<String, String> bindings, MethodInfo method,
+            boolean requireTypeSafeExpressions) {
         this.templateId = templateId;
         this.bindings = bindings;
         this.method = method;
+        this.requireTypeSafeExpressions = requireTypeSafeExpressions;
     }
 
 }
diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
@@ -140,6 +140,9 @@ public class QuteProcessor {
 
     private static final Logger LOGGER = Logger.getLogger(QuteProcessor.class);
 
+    private static final String CHECKED_TEMPLATE_REQUIRE_TYPE_SAFE = "requireTypeSafeExpressions";
+    private static final String CHECKED_TEMPLATE_BASE_PATH = "basePath";
+
     private static final Function<FieldInfo, String> GETTER_FUN = new Function<FieldInfo, String>() {
         @Override
         public String apply(FieldInfo field) {
@@ -217,7 +220,7 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
             BuildProducer<BytecodeTransformerBuildItem> transformers,
             List<TemplatePathBuildItem> templatePaths,
             List<CheckedTemplateAdapterBuildItem> templateAdaptorBuildItems,
-            QuteConfig config) {
+            TemplateFilePathsBuildItem filePaths) {
         List<CheckedTemplateBuildItem> ret = new ArrayList<>();
 
         Map<DotName, CheckedTemplateAdapter> adaptors = new HashMap<>();
@@ -248,20 +251,6 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
         checkedTemplateAnnotations.addAll(index.getIndex().getAnnotations(Names.CHECKED_TEMPLATE_OLD));
         checkedTemplateAnnotations.addAll(index.getIndex().getAnnotations(Names.CHECKED_TEMPLATE));
 
-        // Build a set of file paths for validation
-        Set<String> filePaths = new HashSet<String>();
-        for (TemplatePathBuildItem templatePath : templatePaths) {
-            String path = templatePath.getPath();
-            filePaths.add(path);
-            // Also add version without suffix from the path
-            // For example for "items.html" also add "items"
-            for (String suffix : config.suffixes) {
-                if (path.endsWith(suffix)) {
-                    filePaths.add(path.substring(0, path.length() - (suffix.length() + 1)));
-                }
-            }
-        }
-
         for (AnnotationInstance annotation : checkedTemplateAnnotations) {
             if (annotation.target().kind() != Kind.CLASS) {
                 continue;
@@ -290,7 +279,7 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
                 }
 
                 StringBuilder templatePathBuilder = new StringBuilder();
-                AnnotationValue basePathValue = annotation.value("basePath");
+                AnnotationValue basePathValue = annotation.value(CHECKED_TEMPLATE_BASE_PATH);
                 if (basePathValue != null && !basePathValue.asString().equals(CheckedTemplate.DEFAULTED)) {
                     templatePathBuilder.append(basePathValue.asString());
                 } else if (classInfo.enclosingClass() != null) {
@@ -311,7 +300,7 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
                 }
                 if (!filePaths.contains(templatePath)) {
                     List<String> startsWith = new ArrayList<>();
-                    for (String filePath : filePaths) {
+                    for (String filePath : filePaths.getFilePaths()) {
                         if (filePath.startsWith(templatePath)) {
                             startsWith.add(filePath);
                         }
@@ -340,7 +329,9 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
                     bindings.put(name, JandexUtil.getBoxedTypeName(type));
                     parameterNames.add(name);
                 }
-                ret.add(new CheckedTemplateBuildItem(templatePath, bindings, methodInfo));
+                AnnotationValue requireTypeSafeExpressions = annotation.value(CHECKED_TEMPLATE_REQUIRE_TYPE_SAFE);
+                ret.add(new CheckedTemplateBuildItem(templatePath, bindings, methodInfo,
+                        requireTypeSafeExpressions != null ? requireTypeSafeExpressions.asBoolean() : true));
                 enhancer.implement(methodInfo, templatePath, parameterNames, adaptor);
             }
             transformers.produce(new BytecodeTransformerBuildItem(classInfo.name().toString(),
@@ -352,7 +343,8 @@ List<CheckedTemplateBuildItem> collectCheckedTemplates(BeanArchiveIndexBuildItem
 
     @BuildStep
     TemplatesAnalysisBuildItem analyzeTemplates(List<TemplatePathBuildItem> templatePaths,
-            List<CheckedTemplateBuildItem> checkedTemplates, List<MessageBundleMethodBuildItem> messageBundleMethods) {
+            TemplateFilePathsBuildItem filePaths, List<CheckedTemplateBuildItem> checkedTemplates,
+            List<MessageBundleMethodBuildItem> messageBundleMethods, QuteConfig config) {
         long start = System.currentTimeMillis();
         List<TemplateAnalysis> analysis = new ArrayList<>();
 
@@ -411,28 +403,54 @@ public Optional<Variant> getVariant() {
                 }
                 return Optional.empty();
             }
-        }).addParserHook(new ParserHook() {
+        });
+
+        Map<String, MessageBundleMethodBuildItem> messageBundleMethodsMap;
+        if (messageBundleMethods.isEmpty()) {
+            messageBundleMethodsMap = Collections.emptyMap();
+        } else {
+            messageBundleMethodsMap = new HashMap<>();
+            for (MessageBundleMethodBuildItem messageBundleMethod : messageBundleMethods) {
+                messageBundleMethodsMap.put(messageBundleMethod.getTemplateId(), messageBundleMethod);
+            }
+        }
+
+        builder.addParserHook(new ParserHook() {
 
             @Override
             public void beforeParsing(ParserHelper parserHelper) {
-                for (CheckedTemplateBuildItem checkedTemplate : checkedTemplates) {
-                    // FIXME: check for dot/extension?
-                    if (parserHelper.getTemplateId().startsWith(checkedTemplate.templateId)) {
-                        for (Entry<String, String> entry : checkedTemplate.bindings.entrySet()) {
-                            parserHelper.addParameter(entry.getKey(), entry.getValue());
+                // The template id may be the full path, e.g. "items.html" or a path without the suffic, e.g. "items"
+                String templateId = parserHelper.getTemplateId();
+
+                if (filePaths.contains(templateId)) {
+                    // It's a file-based template
+                    // We need to find out whether the parsed template represents a checked template
+                    String path = templateId;
+                    for (String suffix : config.suffixes) {
+                        if (path.endsWith(suffix)) {
+                            // Remove the suffix 
+                            path = path.substring(0, path.length() - (suffix.length() + 1));
+                            break;
                         }
                     }
-                }
-                // Add params to message bundle templates
-                for (MessageBundleMethodBuildItem messageBundleMethod : messageBundleMethods) {
-                    if (parserHelper.getTemplateId().equals(messageBundleMethod.getTemplateId())) {
-                        MethodInfo method = messageBundleMethod.getMethod();
-                        for (ListIterator<Type> it = method.parameters().listIterator(); it.hasNext();) {
-                            Type paramType = it.next();
-                            parserHelper.addParameter(method.parameterName(it.previousIndex()),
-                                    JandexUtil.getBoxedTypeName(paramType));
+                    for (CheckedTemplateBuildItem checkedTemplate : checkedTemplates) {
+                        if (checkedTemplate.templateId.equals(path)) {
+                            for (Entry<String, String> entry : checkedTemplate.bindings.entrySet()) {
+                                parserHelper.addParameter(entry.getKey(), entry.getValue());
+                            }
+                            break;
                         }
-                        break;
+                    }
+                }
+
+                // If needed add params to message bundle templates
+                MessageBundleMethodBuildItem messageBundleMethod = messageBundleMethodsMap.get(templateId);
+                if (messageBundleMethod != null) {
+                    MethodInfo method = messageBundleMethod.getMethod();
+                    for (ListIterator<Type> it = method.parameters().listIterator(); it.hasNext();) {
+                        Type paramType = it.next();
+                        parserHelper.addParameter(method.parameterName(it.previousIndex()),
+                                JandexUtil.getBoxedTypeName(paramType));
                     }
                 }
             }
@@ -462,12 +480,15 @@ public void beforeParsing(ParserHelper parserHelper) {
     }
 
     @BuildStep
-    void validateExpressions(TemplatesAnalysisBuildItem templatesAnalysis, BeanArchiveIndexBuildItem beanArchiveIndex,
+    void validateExpressions(TemplatesAnalysisBuildItem templatesAnalysis,
+            BeanArchiveIndexBuildItem beanArchiveIndex,
             List<TemplateExtensionMethodBuildItem> templateExtensionMethods,
             List<TypeCheckExcludeBuildItem> excludes,
             BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions,
             BuildProducer<ImplicitValueResolverBuildItem> implicitClasses,
-            BeanDiscoveryFinishedBuildItem beanDiscovery) {
+            BeanDiscoveryFinishedBuildItem beanDiscovery,
+            List<CheckedTemplateBuildItem> checkedTemplates,
+            QuteConfig config) {
 
         IndexView index = beanArchiveIndex.getIndex();
         Function<String, String> templateIdToPathFun = new Function<String, String>() {
@@ -487,6 +508,23 @@ public String apply(String id) {
         Map<DotName, Set<String>> implicitClassToMembersUsed = new HashMap<>();
 
         for (TemplateAnalysis templateAnalysis : templatesAnalysis.getAnalysis()) {
+
+            // Try to find the checked template
+            String path = templateAnalysis.path;
+            for (String suffix : config.suffixes) {
+                if (path.endsWith(suffix)) {
+                    path = path.substring(0, path.length() - (suffix.length() + 1));
+                    break;
+                }
+            }
+            CheckedTemplateBuildItem checkedTemplate = null;
+            for (CheckedTemplateBuildItem item : checkedTemplates) {
+                if (item.templateId.equals(path)) {
+                    checkedTemplate = item;
+                    break;
+                }
+            }
+
             // Maps an expression generated id to the last match of an expression (i.e. the type of the last part)
             Map<Integer, Match> generatedIdsToMatches = new HashMap<>();
 
@@ -504,6 +542,18 @@ public String apply(String id) {
                         continue;
                     }
                 } else {
+                    if (checkedTemplate != null && checkedTemplate.requireTypeSafeExpressions && !expression.hasTypeInfo()) {
+                        incorrectExpressions.produce(new IncorrectExpressionBuildItem(expression.toOriginalString(),
+                                "Only type-safe expressions are allowed in the checked template defined via: "
+                                        + checkedTemplate.method.declaringClass().name() + "."
+                                        + checkedTemplate.method.name()
+                                        + "(); an expression must be based on a checked template parameter "
+                                        + checkedTemplate.bindings.keySet()
+                                        + ", or bound via a param declaration, or the requirement must be relaxed via @CheckedTemplate(requireTypeSafeExpressions = false)",
+                                expression.getOrigin()));
+                        continue;
+                    }
+
                     generatedIdsToMatches.put(expression.getGeneratedId(),
                             validateNestedExpressions(templateAnalysis, null, new HashMap<>(), templateExtensionMethods,
                                     excludes,
@@ -1034,10 +1084,7 @@ void collectTemplates(ApplicationArchivesBuildItem applicationArchivesBuildItem,
     }
 
     @BuildStep
-    void validateTemplateInjectionPoints(QuteConfig config, List<TemplatePathBuildItem> templatePaths,
-            ValidationPhaseBuildItem validationPhase,
-            BuildProducer<ValidationErrorBuildItem> validationErrors) {
-
+    TemplateFilePathsBuildItem collectTemplateFilePaths(QuteConfig config, List<TemplatePathBuildItem> templatePaths) {
         Set<String> filePaths = new HashSet<String>();
         for (TemplatePathBuildItem templatePath : templatePaths) {
             String path = templatePath.getPath();
@@ -1050,6 +1097,13 @@ void validateTemplateInjectionPoints(QuteConfig config, List<TemplatePathBuildIt
                 }
             }
         }
+        return new TemplateFilePathsBuildItem(filePaths);
+    }
+
+    @BuildStep
+    void validateTemplateInjectionPoints(TemplateFilePathsBuildItem filePaths, List<TemplatePathBuildItem> templatePaths,
+            ValidationPhaseBuildItem validationPhase,
+            BuildProducer<ValidationErrorBuildItem> validationErrors) {
 
         for (InjectionPointInfo injectionPoint : validationPhase.getContext().getInjectionPoints()) {
             if (injectionPoint.getRequiredType().name().equals(Names.TEMPLATE)) {

diff --git a/.../qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateFilePathsBuildItem.java b/.../qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateFilePathsBuildItem.java
@@ -0,0 +1,29 @@
+package io.quarkus.qute.deployment;
+
+import java.util.Set;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.quarkus.qute.runtime.QuteConfig;
+
+/**
+ * Holds all template file paths, including the versions without suffixes configured via {@link QuteConfig#suffixes}.
+ * <p>
+ * For example, for the template {@code items.html} the set will contain {@code items.html} and {@code items}.
+ */
+public final class TemplateFilePathsBuildItem extends SimpleBuildItem {
+
+    private final Set<String> filePaths;
+
+    public TemplateFilePathsBuildItem(Set<String> filePaths) {
+        this.filePaths = filePaths;
+    }
+
+    public Set<String> getFilePaths() {
+        return filePaths;
+    }
+
+    public boolean contains(String path) {
+        return filePaths.contains(path);
+    }
+
+}
diff --git a/.../qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java b/.../qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java
@@ -21,12 +21,20 @@ public List<TemplateAnalysis> getAnalysis() {
         return analysis;
     }
 
+    /**
+     * Analysis of a particular template found in the given path.
+     */
     static class TemplateAnalysis {
 
-        // Path or other user-defined id; may be null
+        // A user-defined id; may be null
         public final String id;
+
+        // The id generated by the parser
         public final String generatedId;
+
         public final List<Expression> expressions;
+
+        // File path, e.g. hello.html or ItemResource/items.html
         public final String path;
 
         public TemplateAnalysis(String id, String generatedId, List<Expression> expressions, String path) {

diff --git a/...est/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateDoNotRequireTypeSafeTest.java b/...est/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateDoNotRequireTypeSafeTest.java
@@ -0,0 +1,36 @@
+package io.quarkus.qute.deployment.typesafe;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+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 CheckedTemplateDoNotRequireTypeSafeTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(Templates.class)
+                    .addAsResource(new StringAsset("Hello {name}!{any}"),
+                            "templates/CheckedTemplateDoNotRequireTypeSafeTest/hola.txt"));
+
+    @Test
+    public void testValidation() {
+        assertEquals("Hello Ondrej!!", Templates.hola("Ondrej").data("any", "!").render());
+    }
+
+    @CheckedTemplate(requireTypeSafeExpressions = false)
+    static class Templates {
+
+        static native TemplateInstance hola(String name);
+
+    }
+
+}