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.

Sign up for GitHub

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 message bundles - ignore loop metadata during validation #24403

Merged
merged 1 commit into from
Mar 20, 2022
Merged

Qute message bundles - ignore loop metadata during validation #24403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 26 additions & 14 deletions ...ions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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