Skip to content

Commit

Permalink
Qute message bundles - ignore loop metadate during validation
Browse files Browse the repository at this point in the history 
- also ignore template globals
- follows up on #23734
  • Loading branch information
@mkouba
mkouba committed Mar 18, 2022
1 parent d0b2b43 commit e29000e
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 30 deletions.
40 changes: 26 additions & 14 deletions ...ions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import io.quarkus.qute.Expression;
import io.quarkus.qute.Expression.Part;
import io.quarkus.qute.Expressions;
import io.quarkus.qute.LoopSectionHelper;
import io.quarkus.qute.Namespaces;
import io.quarkus.qute.Resolver;
import io.quarkus.qute.deployment.QuteProcessor.LookupConfig;
Expand Down Expand Up @@ -309,11 +310,14 @@ void initBundleContext(MessageBundleRecorder recorder,
@BuildStep
void validateMessageBundleMethods(TemplatesAnalysisBuildItem templatesAnalysis,
List<MessageBundleMethodBuildItem> messageBundleMethods,
List<TemplateGlobalBuildItem> templateGlobals,
BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions) {

Map<String, MessageBundleMethodBuildItem> bundleMethods = messageBundleMethods.stream()
.filter(MessageBundleMethodBuildItem::isValidatable)
.collect(Collectors.toMap(MessageBundleMethodBuildItem::getTemplateId, Function.identity()));
Set<String> globals = templateGlobals.stream().map(TemplateGlobalBuildItem::getName)
.collect(Collectors.toUnmodifiableSet());

for (TemplateAnalysis analysis : templatesAnalysis.getAnalysis()) {
MessageBundleMethodBuildItem messageBundleMethod = bundleMethods.get(analysis.id);
Expand All @@ -323,7 +327,8 @@ void validateMessageBundleMethods(TemplatesAnalysisBuildItem templatesAnalysis,
Set<String> paramNames = IntStream.range(0, messageBundleMethod.getMethod().parameters().size())
.mapToObj(idx -> getParameterName(messageBundleMethod.getMethod(), idx)).collect(Collectors.toSet());
for (Expression expression : analysis.expressions) {
validateExpression(incorrectExpressions, messageBundleMethod, expression, paramNames, usedParamNames);
validateExpression(incorrectExpressions, messageBundleMethod, expression, paramNames, usedParamNames,
globals);
}
// Log a warning if a parameter is not used in the template
for (String paramName : paramNames) {
Expand All @@ -339,31 +344,38 @@ void validateMessageBundleMethods(TemplatesAnalysisBuildItem templatesAnalysis,

private void validateExpression(BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions,
MessageBundleMethodBuildItem messageBundleMethod, Expression expression, Set<String> paramNames,
Set<String> usedParamNames) {
Set<String> usedParamNames, Set<String> globals) {
if (expression.isLiteral()) {
return;
}
if (!expression.hasNamespace()) {
Expression.Part firstPart = expression.getParts().get(0);
String name = firstPart.getName();
// Skip expressions that have type info derived from a parent section, e.g "it<loop#3>" and "foo<set#3>"
if (firstPart.getTypeInfo() == null || (firstPart.getTypeInfo().startsWith("" + Expressions.TYPE_INFO_SEPARATOR)
&& !paramNames.contains(name))) {
// Expression has no type info or type info that does not match a method parameter
incorrectExpressions.produce(new IncorrectExpressionBuildItem(expression.toOriginalString(),
name + " is not a parameter of the message bundle method "
+ messageBundleMethod.getMethod().declaringClass().name() + "#"
+ messageBundleMethod.getMethod().name() + "()",
expression.getOrigin()));
} else {
usedParamNames.add(name);
String typeInfo = firstPart.getTypeInfo();
boolean isGlobal = globals.contains(name);
boolean isLoopMetadata = typeInfo != null && typeInfo.endsWith(LoopSectionHelper.Factory.HINT_METADATA);
// Type info derived from a parent section, e.g "it<loop#3>" and "foo<set#3>"
boolean hasDerivedTypeInfo = typeInfo != null && !typeInfo.startsWith("" + Expressions.TYPE_INFO_SEPARATOR);

if (!isGlobal && !isLoopMetadata && !hasDerivedTypeInfo) {
if (typeInfo == null || !paramNames.contains(name)) {
// Expression has no type info or type info that does not match a method parameter
// expressions that have
incorrectExpressions.produce(new IncorrectExpressionBuildItem(expression.toOriginalString(),
name + " is not a parameter of the message bundle method "
+ messageBundleMethod.getMethod().declaringClass().name() + "#"
+ messageBundleMethod.getMethod().name() + "()",
expression.getOrigin()));
} else {
usedParamNames.add(name);
}
}
}
// Inspect method params too
for (Part part : expression.getParts()) {
if (part.isVirtualMethod()) {
for (Expression param : part.asVirtualMethod().getParameters()) {
validateExpression(incorrectExpressions, messageBundleMethod, param, paramNames, usedParamNames);
validateExpression(incorrectExpressions, messageBundleMethod, param, paramNames, usedParamNames, globals);
}
}
}
Expand Down
14 changes: 11 additions & 3 deletions .../src/test/java/io/quarkus/qute/deployment/i18n/MessageBundleExpressionValidationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.qute.TemplateException;
import io.quarkus.qute.TemplateGlobal;
import io.quarkus.qute.i18n.Message;
import io.quarkus.qute.i18n.MessageBundle;
import io.quarkus.test.QuarkusUnitTest;
Expand All @@ -17,7 +18,7 @@ public class MessageBundleExpressionValidationTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(WrongBundle.class, Item.class)
.addClasses(WrongBundle.class, Item.class, MyGlobals.class)
.addAsResource(new StringAsset(
// foo is not a parameter of WrongBundle.hello()
"hello=Hallo {foo}!"),
Expand Down Expand Up @@ -51,10 +52,17 @@ public void testValidation() {
@MessageBundle
public interface WrongBundle {

// item has no "foo" property, "bar" and "baf" are not parameters
@Message("Hello {item.foo} {bar} {#each item.names}{it}{it.baz}{baf}{/each}")
// item has no "foo" property, "bar" and "baf" are not parameters, string has no "baz" property
@Message("Hello {item.foo} {bar} {#each item.names}{it}{it.baz}{it_hasNext}{baf}{/each}{level}")
String hello(Item item);

}

@TemplateGlobal
static class MyGlobals {

static int level = 5;

}

}
25 changes: 13 additions & 12 deletions independent-projects/qute/core/src/main/java/io/quarkus/qute/LoopSectionHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public static class Factory implements SectionHelperFactory<LoopSectionHelper> {
public static final String ITERATION_METADATA_PREFIX_NONE = "<none>";

public static final String HINT_ELEMENT = "<loop-element>";
public static final String HINT_METADATA = "<loop-metadata>";
public static final String HINT_PREFIX = "<loop#";
private static final String IN = "in";

Expand Down Expand Up @@ -202,16 +203,16 @@ public Scope initializeBlock(Scope previousScope, BlockInfo block) {
newScope.putBinding(alias, alias + HINT_PREFIX + iterableExpr.getGeneratedId() + ">");
// Put bindings for iteration metadata
String prefix = prefixValue(alias, metadataPrefix);
newScopeBinding(newScope, prefix, "count", Integer.class.getName());
newScopeBinding(newScope, prefix, "index", Integer.class.getName());
newScopeBinding(newScope, prefix, "indexParity", String.class.getName());
newScopeBinding(newScope, prefix, "hasNext", Boolean.class.getName());
newScopeBinding(newScope, prefix, "isLast", Boolean.class.getName());
newScopeBinding(newScope, prefix, "isFirst", Boolean.class.getName());
newScopeBinding(newScope, prefix, "odd", Boolean.class.getName());
newScopeBinding(newScope, prefix, "isOdd", Boolean.class.getName());
newScopeBinding(newScope, prefix, "even", Boolean.class.getName());
newScopeBinding(newScope, prefix, "isEven", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "count", Integer.class.getName());
putMetadataBinding(newScope, prefix, "index", Integer.class.getName());
putMetadataBinding(newScope, prefix, "indexParity", String.class.getName());
putMetadataBinding(newScope, prefix, "hasNext", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "isLast", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "isFirst", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "odd", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "isOdd", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "even", Boolean.class.getName());
putMetadataBinding(newScope, prefix, "isEven", Boolean.class.getName());
return newScope;
} else {
// Make sure we do not try to validate against the parent context
Expand All @@ -224,8 +225,8 @@ public Scope initializeBlock(Scope previousScope, BlockInfo block) {
}
}

private void newScopeBinding(Scope scope, String prefix, String name, String typeName) {
scope.putBinding(prefix != null ? prefix + name : name, Expressions.typeInfoFrom(typeName));
private void putMetadataBinding(Scope scope, String prefix, String name, String typeName) {
scope.putBinding(prefix != null ? prefix + name : name, Expressions.typeInfoFrom(typeName) + HINT_METADATA);
}

static String prefixValue(String alias, String metadataPrefix) {
Expand Down
7 changes: 6 additions & 1 deletion independent-projects/qute/core/src/test/java/io/quarkus/qute/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public void testTypeInfos() {
+ "{/for}"
+ "{#each labels}"
+ "{it.name}"
+ "{it_hasNext}"
+ "{/each}"
+ "{inject:bean.name}"
+ "{#each inject:bean.labels('foo')}"
Expand All @@ -89,7 +90,8 @@ public void testTypeInfos() {
+ "{foo.baz}"
+ "{/for}"
+ "{foo.call(labels,bar)}"
+ "{#when machine.status}{#is OK}..{#is NOK}{/when}");
+ "{#when machine.status}{#is OK}..{#is NOK}{/when}"
+ "{not_typesafe}");
List<Expression> expressions = template.getExpressions();

assertExpr(expressions, "foo.name", 2, "|org.acme.Foo|.name");
Expand Down Expand Up @@ -117,6 +119,9 @@ public void testTypeInfos() {

Expression machineStatusExpr = find(expressions, "machine.status");
assertExpr(expressions, "OK", 1, "OK<when#" + machineStatusExpr.getGeneratedId() + ">");

assertExpr(expressions, "it_hasNext", 1, "|java.lang.Boolean|<loop-metadata>");
assertExpr(expressions, "not_typesafe", 1, null);
}

@Test
Expand Down

0 comments on commit e29000e

Please sign in to comment.