From 7c3fa487a8740665e72de223540b53224e9568fd Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Tue, 13 Apr 2021 12:10:55 +0200 Subject: [PATCH] Qute checked templates - require type-safe expressions by default - related to #14534 --- docs/src/main/asciidoc/qute-reference.adoc | 5 +- .../deployment/CheckedTemplateBuildItem.java | 11 +- .../qute/deployment/QuteProcessor.java | 136 ++++++++++++------ .../TemplateFilePathsBuildItem.java | 29 ++++ .../TemplatesAnalysisBuildItem.java | 10 +- ...eckedTemplateDoNotRequireTypeSafeTest.java | 36 +++++ .../CheckedTemplateRequireTypeSafeTest.java | 38 +++++ .../java/io/quarkus/qute/CheckedTemplate.java | 5 + 8 files changed, 226 insertions(+), 44 deletions(-) create mode 100644 extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateFilePathsBuildItem.java create mode 100644 extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateDoNotRequireTypeSafeTest.java create mode 100644 extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateRequireTypeSafeTest.java diff --git a/docs/src/main/asciidoc/qute-reference.adoc b/docs/src/main/asciidoc/qute-reference.adoc index bd7886bc1cbea..4819ca686ff90 100644 --- 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 <>, 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 <> 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/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/CheckedTemplateBuildItem.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/CheckedTemplateBuildItem.java index ee042871046c8..c2d3cfebb2a44 100644 --- a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/CheckedTemplateBuildItem.java +++ b/extensions/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 bindings; public final MethodInfo method; + public final boolean requireTypeSafeExpressions; - public CheckedTemplateBuildItem(String templateId, Map bindings, MethodInfo method) { + public CheckedTemplateBuildItem(String templateId, Map 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 index 1c83a483c451c..f08f9e8b26137 100644 --- 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 @@ -139,6 +139,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 GETTER_FUN = new Function() { @Override public String apply(FieldInfo field) { @@ -216,7 +219,7 @@ List collectCheckedTemplates(BeanArchiveIndexBuildItem BuildProducer transformers, List templatePaths, List templateAdaptorBuildItems, - QuteConfig config) { + TemplateFilePathsBuildItem filePaths) { List ret = new ArrayList<>(); Map adaptors = new HashMap<>(); @@ -246,20 +249,6 @@ List collectCheckedTemplates(BeanArchiveIndexBuildItem Set checkedTemplateAnnotations = new HashSet<>(); checkedTemplateAnnotations.addAll(index.getIndex().getAnnotations(Names.CHECKED_TEMPLATE)); - // Build a set of file paths for validation - Set filePaths = new HashSet(); - 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; @@ -288,7 +277,7 @@ List 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) { @@ -309,7 +298,7 @@ List collectCheckedTemplates(BeanArchiveIndexBuildItem } if (!filePaths.contains(templatePath)) { List startsWith = new ArrayList<>(); - for (String filePath : filePaths) { + for (String filePath : filePaths.getFilePaths()) { if (filePath.startsWith(templatePath)) { startsWith.add(filePath); } @@ -338,7 +327,9 @@ List 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(), @@ -350,7 +341,8 @@ List collectCheckedTemplates(BeanArchiveIndexBuildItem @BuildStep TemplatesAnalysisBuildItem analyzeTemplates(List templatePaths, - List checkedTemplates, List messageBundleMethods) { + TemplateFilePathsBuildItem filePaths, List checkedTemplates, + List messageBundleMethods, QuteConfig config) { long start = System.currentTimeMillis(); List analysis = new ArrayList<>(); @@ -409,28 +401,54 @@ public Optional getVariant() { } return Optional.empty(); } - }).addParserHook(new ParserHook() { + }); + + Map 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 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 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 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 it = method.parameters().listIterator(); it.hasNext();) { + Type paramType = it.next(); + parserHelper.addParameter(method.parameterName(it.previousIndex()), + JandexUtil.getBoxedTypeName(paramType)); } } } @@ -460,12 +478,15 @@ public void beforeParsing(ParserHelper parserHelper) { } @BuildStep - void validateExpressions(TemplatesAnalysisBuildItem templatesAnalysis, BeanArchiveIndexBuildItem beanArchiveIndex, + void validateExpressions(TemplatesAnalysisBuildItem templatesAnalysis, + BeanArchiveIndexBuildItem beanArchiveIndex, List templateExtensionMethods, List excludes, BuildProducer incorrectExpressions, BuildProducer implicitClasses, - BeanDiscoveryFinishedBuildItem beanDiscovery) { + BeanDiscoveryFinishedBuildItem beanDiscovery, + List checkedTemplates, + QuteConfig config) { IndexView index = beanArchiveIndex.getIndex(); Function templateIdToPathFun = new Function() { @@ -485,6 +506,23 @@ public String apply(String id) { Map> 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 generatedIdsToMatches = new HashMap<>(); @@ -502,6 +540,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, @@ -1032,10 +1082,7 @@ void collectTemplates(ApplicationArchivesBuildItem applicationArchivesBuildItem, } @BuildStep - void validateTemplateInjectionPoints(QuteConfig config, List templatePaths, - ValidationPhaseBuildItem validationPhase, - BuildProducer validationErrors) { - + TemplateFilePathsBuildItem collectTemplateFilePaths(QuteConfig config, List templatePaths) { Set filePaths = new HashSet(); for (TemplatePathBuildItem templatePath : templatePaths) { String path = templatePath.getPath(); @@ -1048,6 +1095,13 @@ void validateTemplateInjectionPoints(QuteConfig config, List templatePaths, + ValidationPhaseBuildItem validationPhase, + BuildProducer validationErrors) { for (InjectionPointInfo injectionPoint : validationPhase.getContext().getInjectionPoints()) { if (injectionPoint.getRequiredType().name().equals(Names.TEMPLATE)) { diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateFilePathsBuildItem.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateFilePathsBuildItem.java new file mode 100644 index 0000000000000..9febc0fc82c33 --- /dev/null +++ b/extensions/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}. + *

+ * 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 filePaths; + + public TemplateFilePathsBuildItem(Set filePaths) { + this.filePaths = filePaths; + } + + public Set getFilePaths() { + return filePaths; + } + + public boolean contains(String path) { + return filePaths.contains(path); + } + +} diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java index c844c5ec1f91b..277f8b26557e6 100644 --- a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java +++ b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplatesAnalysisBuildItem.java @@ -21,12 +21,20 @@ public List 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 expressions; + + // File path, e.g. hello.html or ItemResource/items.html public final String path; public TemplateAnalysis(String id, String generatedId, List expressions, String path) { diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateDoNotRequireTypeSafeTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateDoNotRequireTypeSafeTest.java new file mode 100644 index 0000000000000..7d611526a330e --- /dev/null +++ b/extensions/qute/deployment/src/test/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); + + } + +} diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateRequireTypeSafeTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateRequireTypeSafeTest.java new file mode 100644 index 0000000000000..18598de27ffdc --- /dev/null +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/CheckedTemplateRequireTypeSafeTest.java @@ -0,0 +1,38 @@ +package io.quarkus.qute.deployment.typesafe; + +import static org.junit.jupiter.api.Assertions.fail; + +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.TemplateException; +import io.quarkus.qute.TemplateInstance; +import io.quarkus.test.QuarkusUnitTest; + +public class CheckedTemplateRequireTypeSafeTest { + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(Templates.class) + .addAsResource(new StringAsset("Hello {name}! {any}"), + "templates/CheckedTemplateRequireTypeSafeTest/hola.txt")) + .setExpectedException(TemplateException.class); + + @Test + public void testValidation() { + fail(); + } + + @CheckedTemplate // requireTypeSafeExpressions=true by default + static class Templates { + + static native TemplateInstance hola(String name); + + } + +} diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/CheckedTemplate.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/CheckedTemplate.java index b253cc3b9ad13..9e1327451c088 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/CheckedTemplate.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/CheckedTemplate.java @@ -82,4 +82,9 @@ */ String basePath() default DEFAULTED; + /** + * If set to true then the defined templates can only contain type-safe expressions. + */ + boolean requireTypeSafeExpressions() default true; + }