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 quarkusio#26479
mkouba · Jul 8, 2022 · 8e51609 · 8e51609
1 parent b57ef54
commit 8e51609
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 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
@@ -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<String, Expression>, 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())).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()), 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
@@ -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<TextNode> group, List<TemplateNode> finalNodes)
         }
     }
 
-    static SectionBlock.Builder builder(String id, Function<String, Expression> 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<String, String> parameters;
         private final List<TemplateNode> nodes;
         private Map<String, Expression> expressions;
-        private final Function<String, Expression> expressionFun;
+        private final Parser parser;
         private final ErrorInitializer errorInitializer;
 
-        public Builder(String id, Function<String, Expression> 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
@@ -140,6 +140,8 @@ default boolean hasParameter(String name) {
          * Parse and register an expression for the specified parameter.
          * <p>
          * A registered expression contributes to the {@link Template#getExpressions()}, i.e. can be validated at build time.
+         * <p>
+         * 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
@@ -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<String, Expression> 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<String, Expression> 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
@@ -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<Expression> 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(2, expression.getOrigin().getLineCharacterStart());
+            } else {
+                assertEquals("item.price", expression.toOriginalString());
+                assertEquals(1, expression.getOrigin().getLine());
+                assertEquals(2, 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
@@ -157,7 +157,7 @@ public void testLines() {
                 + "{/}");
         Origin fooItemsOrigin = find(template.getExpressions(), "foo.items").getOrigin();
         assertEquals(6, fooItemsOrigin.getLine());
-        assertEquals(14, fooItemsOrigin.getLineCharacterStart());
+        assertEquals(0, 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
@@ -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<Expression> 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(2, expression.getOrigin().getLineCharacterStart());
+            } else {
+                assertEquals("bar", expression.toOriginalString());
+                assertEquals(1, expression.getOrigin().getLine());
+                assertEquals(2, expression.getOrigin().getLineCharacterStart());
+            }
+        }
+    }
+
 }