From d588d8b6f4f4caadb925284da2c82799a9290d99 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 8 Jul 2022 12:06:50 +0200 Subject: [PATCH] Qute - fix origin of an expression used as a section param - there is no easy way to reliably identify the column of an expression used as a section param, therefore, we'll report the column of the containing section/block for the moment - related to #26479 --- .../src/main/java/io/quarkus/qute/Parser.java | 19 +++++++++++------- .../java/io/quarkus/qute/SectionBlock.java | 14 ++++++------- .../io/quarkus/qute/SectionHelperFactory.java | 2 ++ .../java/io/quarkus/qute/SectionNode.java | 10 ++++------ .../java/io/quarkus/qute/IfSectionTest.java | 19 ++++++++++++++++++ .../test/java/io/quarkus/qute/ParserTest.java | 2 +- .../java/io/quarkus/qute/SetSectionTest.java | 20 +++++++++++++++++++ 7 files changed, 64 insertions(+), 22 deletions(-) diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/Parser.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/Parser.java index 31f163442f107..0afaec1cddb8c 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/Parser.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/Parser.java @@ -1,6 +1,7 @@ package io.quarkus.qute; import io.quarkus.qute.Expression.Part; +import io.quarkus.qute.SectionHelperFactory.BlockInfo; import io.quarkus.qute.SectionHelperFactory.ParametersInfo; import io.quarkus.qute.SectionHelperFactory.ParserDelegate; import io.quarkus.qute.TemplateNode.Origin; @@ -31,7 +32,7 @@ /** * Simple non-reusable parser. */ -class Parser implements Function, ParserHelper, ParserDelegate, WithOrigin, ErrorInitializer { +class Parser implements ParserHelper, ParserDelegate, WithOrigin, ErrorInitializer { private static final Logger LOGGER = Logger.getLogger(Parser.class); static final String ROOT_HELPER_NAME = "$root"; @@ -414,7 +415,7 @@ private void flushTag() { } else { // Expression sectionStack.peek().currentBlock() - .addNode(new ExpressionNode(apply(content), engine)); + .addNode(new ExpressionNode(createExpression(content), engine)); } this.buffer = new StringBuilder(); } @@ -446,7 +447,7 @@ private void sectionStart(String content, String tag) { // => New section block SectionBlock.Builder block = SectionBlock.builder("" + sectionBlockIdx++, this, this) - .setOrigin(origin(0)).setLabel(sectionName); + .setOrigin(origin(tag.length() - 1)).setLabel(sectionName); lastSection.addBlock(block); processParams(tag, sectionName, iter, block); @@ -465,7 +466,7 @@ private void sectionStart(String content, String tag) { .build(); } SectionNode.Builder sectionNode = SectionNode - .builder(sectionName, origin(0), this, this) + .builder(sectionName, origin(tag.length() - 1), this, this) .setEngine(engine) .setHelperFactory(factory); @@ -578,7 +579,8 @@ private void parameterDeclaration(String content, String tag) { String typeInfo = Expressions.typeInfoFrom(value); currentScope.putBinding(key, typeInfo); sectionStack.peek().currentBlock().addNode( - new ParameterDeclarationNode(typeInfo, key, defaultValue != null ? apply(defaultValue) : null, origin(0))); + new ParameterDeclarationNode(typeInfo, key, defaultValue != null ? createExpression(defaultValue) : null, + origin(0))); // If a default value is set we add a synthetic {#let} section to define local variables with default values if (defaultValue != null) { @@ -988,8 +990,11 @@ static boolean isRightBracket(char character) { return character == ')'; } - @Override - public ExpressionImpl apply(String value) { + ExpressionImpl createSectionBlockExpression(BlockInfo block, String value) { + return parseExpression(expressionIdGenerator::incrementAndGet, value, scopeStack.peek(), block.getOrigin()); + } + + ExpressionImpl createExpression(String value) { return parseExpression(expressionIdGenerator::incrementAndGet, value, scopeStack.peek(), origin(value.length() + 1)); } diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionBlock.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionBlock.java index e73e3875a8883..ccbfeeffc8224 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionBlock.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionBlock.java @@ -8,7 +8,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; import java.util.function.Predicate; /** @@ -180,9 +179,9 @@ private void collapseGroup(List group, List finalNodes) } } - static SectionBlock.Builder builder(String id, Function expressionFunc, + static SectionBlock.Builder builder(String id, Parser parser, ErrorInitializer errorInitializer) { - return new Builder(id, expressionFunc, errorInitializer).setLabel(id); + return new Builder(id, parser, errorInitializer).setLabel(id); } static class Builder implements BlockInfo { @@ -193,14 +192,13 @@ static class Builder implements BlockInfo { private Map parameters; private final List nodes; private Map expressions; - private final Function expressionFun; + private final Parser parser; private final ErrorInitializer errorInitializer; - public Builder(String id, Function expressionFun, - ErrorInitializer errorInitializer) { + public Builder(String id, Parser parser, ErrorInitializer errorInitializer) { this.id = id; this.nodes = new ArrayList<>(); - this.expressionFun = expressionFun; + this.parser = parser; this.errorInitializer = errorInitializer; } @@ -229,7 +227,7 @@ SectionBlock.Builder addParameter(String name, String value) { @Override public Expression addExpression(String param, String value) { - Expression expression = expressionFun.apply(value); + Expression expression = parser.createSectionBlockExpression(this, value); if (expressions == null) { expressions = new LinkedHashMap<>(); } diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionHelperFactory.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionHelperFactory.java index 2af895f7dbcc3..78e902c149940 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionHelperFactory.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionHelperFactory.java @@ -140,6 +140,8 @@ default boolean hasParameter(String name) { * Parse and register an expression for the specified parameter. *

* A registered expression contributes to the {@link Template#getExpressions()}, i.e. can be validated at build time. + *

+ * The origin of the returned expression is the origin of the containing block. * * @param param * @param value diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionNode.java index 0467c432651b6..a00d786e357be 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionNode.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SectionNode.java @@ -7,7 +7,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletionStage; -import java.util.function.Function; import java.util.function.Predicate; import org.jboss.logging.Logger; @@ -18,9 +17,9 @@ class SectionNode implements TemplateNode { private static final Logger LOG = Logger.getLogger("io.quarkus.qute.nodeResolve"); - static Builder builder(String helperName, Origin origin, Function expressionFun, + static Builder builder(String helperName, Origin origin, Parser parser, ErrorInitializer errorFun) { - return new Builder(helperName, origin, expressionFun, errorFun); + return new Builder(helperName, origin, parser, errorFun); } final String name; @@ -110,15 +109,14 @@ static class Builder { private EngineImpl engine; private final ErrorInitializer errorInitializer; - Builder(String helperName, Origin origin, Function expressionFun, - ErrorInitializer errorInitializer) { + Builder(String helperName, Origin origin, Parser parser, ErrorInitializer errorInitializer) { this.helperName = helperName; this.origin = origin; this.blocks = new ArrayList<>(); this.errorInitializer = errorInitializer; // The main block is always present addBlock(SectionBlock - .builder(SectionHelperFactory.MAIN_BLOCK_NAME, expressionFun, errorInitializer) + .builder(SectionHelperFactory.MAIN_BLOCK_NAME, parser, errorInitializer) .setOrigin(origin)); } diff --git a/independent-projects/qute/core/src/test/java/io/quarkus/qute/IfSectionTest.java b/independent-projects/qute/core/src/test/java/io/quarkus/qute/IfSectionTest.java index 7d338ad397802..56964eb816fc6 100644 --- a/independent-projects/qute/core/src/test/java/io/quarkus/qute/IfSectionTest.java +++ b/independent-projects/qute/core/src/test/java/io/quarkus/qute/IfSectionTest.java @@ -240,6 +240,25 @@ public void testFromageCondition() { .data("user", "Stef", "target", new Target(ContentStatus.NEW)).render()); } + @Test + public void testParameterOrigin() { + Engine engine = Engine.builder().addDefaults().build(); + Template template = engine.parse(" {#if item.price > 1}{/if}"); + List expressions = template.getExpressions(); + assertEquals(2, expressions.size()); + for (Expression expression : expressions) { + if (expression.isLiteral()) { + assertEquals(1, expression.getLiteralValue().getNow(false)); + assertEquals(1, expression.getOrigin().getLine()); + assertEquals(3, expression.getOrigin().getLineCharacterStart()); + } else { + assertEquals("item.price", expression.toOriginalString()); + assertEquals(1, expression.getOrigin().getLine()); + assertEquals(3, expression.getOrigin().getLineCharacterStart()); + } + } + } + public static class Target { public ContentStatus status; diff --git a/independent-projects/qute/core/src/test/java/io/quarkus/qute/ParserTest.java b/independent-projects/qute/core/src/test/java/io/quarkus/qute/ParserTest.java index e15f797ed4a6e..c64079d8a6ac5 100644 --- a/independent-projects/qute/core/src/test/java/io/quarkus/qute/ParserTest.java +++ b/independent-projects/qute/core/src/test/java/io/quarkus/qute/ParserTest.java @@ -157,7 +157,7 @@ public void testLines() { + "{/}"); Origin fooItemsOrigin = find(template.getExpressions(), "foo.items").getOrigin(); assertEquals(6, fooItemsOrigin.getLine()); - assertEquals(14, fooItemsOrigin.getLineCharacterStart()); + assertEquals(1, fooItemsOrigin.getLineCharacterStart()); assertEquals(24, fooItemsOrigin.getLineCharacterEnd()); Origin itemNameOrigin = find(template.getExpressions(), "item.name").getOrigin(); assertEquals(8, itemNameOrigin.getLine()); diff --git a/independent-projects/qute/core/src/test/java/io/quarkus/qute/SetSectionTest.java b/independent-projects/qute/core/src/test/java/io/quarkus/qute/SetSectionTest.java index 60364077f6c9a..25511136b6c7c 100644 --- a/independent-projects/qute/core/src/test/java/io/quarkus/qute/SetSectionTest.java +++ b/independent-projects/qute/core/src/test/java/io/quarkus/qute/SetSectionTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.List; import org.junit.jupiter.api.Test; public class SetSectionTest { @@ -41,4 +42,23 @@ public void testDefaultValues() { .data("bar", "42", "baz", "no").render()); } + @Test + public void testParameterOrigin() { + Engine engine = Engine.builder().addDefaults().build(); + Template template = engine.parse(" {#let item=1 foo=bar}{/let}"); + List expressions = template.getExpressions(); + assertEquals(2, expressions.size()); + for (Expression expression : expressions) { + if (expression.isLiteral()) { + assertEquals(1, expression.getLiteralValue().getNow(false)); + assertEquals(1, expression.getOrigin().getLine()); + assertEquals(3, expression.getOrigin().getLineCharacterStart()); + } else { + assertEquals("bar", expression.toOriginalString()); + assertEquals(1, expression.getOrigin().getLine()); + assertEquals(3, expression.getOrigin().getLineCharacterStart()); + } + } + } + }