Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qute - fix origin of an expression used as a section param #26622

Merged
merged 1 commit into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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<>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(3, expression.getOrigin().getLineCharacterStart());
} else {
assertEquals("bar", expression.toOriginalString());
assertEquals(1, expression.getOrigin().getLine());
assertEquals(3, expression.getOrigin().getLineCharacterStart());
}
}
}

}