Merge pull request #13890 from mkouba/qute-param-assignability-fix-1-10

Qute type-safe templates: fix parameter assignability check
quarkusio · Dec 15, 2020 · b643ec5 · b643ec5
2 parents 76ad29c + 933dae5
commit b643ec5
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 15 deletions.
diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
@@ -513,9 +513,13 @@ static void validateNestedExpressions(ClassInfo rootClazz, Map<String, Match> re
             Info info = parts.next();
             if (match.clazz != null) {
                 // By default, we only consider properties
-                Set<String> membersUsed = implicitClassToMembersUsed.computeIfAbsent(match.clazz, c -> new HashSet<>());
+                Set<String> membersUsed = implicitClassToMembersUsed.get(match.clazz);
+                if (membersUsed == null) {
+                    membersUsed = new HashSet<>();
+                    implicitClassToMembersUsed.put(match.clazz, membersUsed);
+                }
                 AnnotationTarget member = null;
-                // First try to find java members
+                // First try to find a java member
                 if (info.isVirtualMethod()) {
                     member = findMethod(info.part.asVirtualMethod(), match.clazz, expression, index, templateIdToPathFun,
                             results);
@@ -528,9 +532,10 @@ static void validateNestedExpressions(ClassInfo rootClazz, Map<String, Match> re
                         membersUsed.add(member.kind() == Kind.FIELD ? member.asField().name() : member.asMethod().name());
                     }
                 }
-                // Java member not found - try extension methods
                 if (member == null) {
-                    member = findTemplateExtensionMethod(info, match.clazz, templateExtensionMethods, expression, index,
+                    // Then try to find an etension method
+                    member = findTemplateExtensionMethod(info, match.clazz, templateExtensionMethods, expression,
+                            index,
                             templateIdToPathFun, results);
                 }
 
@@ -1141,13 +1146,17 @@ private static AnnotationTarget findTemplateExtensionMethod(Info info, ClassInfo
                 // Name does not match
                 continue;
             }
+            List<Type> parameters = extensionMethod.getMethod().parameters();
+            int realParamSize = parameters.size() - (TemplateExtension.ANY.equals(extensionMethod.getMatchName()) ? 2 : 1);
+            if (realParamSize > 0 && !info.isVirtualMethod()) {
+                // If method accepts additional params the info must be a virtual method
+                continue;
+            }
             if (info.isVirtualMethod()) {
                 // For virtual method validate the number of params and attempt to validate the parameter types if available
                 VirtualMethodPart virtualMethod = info.part.asVirtualMethod();
                 boolean isVarArgs = ValueResolverGenerator.isVarArgs(extensionMethod.getMethod());
-                List<Type> parameters = extensionMethod.getMethod().parameters();
                 int lastParamIdx = parameters.size() - 1;
-                int realParamSize = parameters.size() - (TemplateExtension.ANY.equals(extensionMethod.getMatchName()) ? 2 : 1);
 
                 if (isVarArgs) {
                     // For varargs methods match the minimal number of params
@@ -1178,8 +1187,8 @@ private static AnnotationTarget findTemplateExtensionMethod(Info info, ClassInfo
                         } else {
                             paramType = parameters.get(idx);
                         }
-                        if (!Types.isAssignableFrom(result.type,
-                                paramType, index)) {
+                        if (!Types.isAssignableFrom(paramType,
+                                result.type, index)) {
                             matches = false;
                             break;
                         }
@@ -1288,8 +1297,8 @@ private static AnnotationTarget findMethod(VirtualMethodPart virtualMethod, Clas
                             } else {
                                 paramType = parameters.get(idx);
                             }
-                            if (!Types.isAssignableFrom(result.type,
-                                    paramType, index)) {
+                            if (!Types.isAssignableFrom(paramType,
+                                    result.type, index)) {
                                 matches = false;
                                 break;
                             }

diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TypeInfos.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TypeInfos.java
@@ -108,7 +108,7 @@ private static Type resolveType(String value) {
             String[] parts = value.substring(angleIdx + 1, value.length() - 1).split(",");
             Type[] arguments = new Type[parts.length];
             for (int i = 0; i < arguments.length; i++) {
-                arguments[i] = resolveType(parts[i]);
+                arguments[i] = resolveType(parts[i].trim());
             }
             return ParameterizedType.create(rawName, arguments, null);
         }

diff --git a/...e/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/ValidationSuccessTest.java b/...e/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/ValidationSuccessTest.java
@@ -2,6 +2,8 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import java.util.Collections;
+
 import javax.inject.Inject;
 
 import org.jboss.shrinkwrap.api.ShrinkWrap;
@@ -22,6 +24,7 @@ public class ValidationSuccessTest {
                     .addAsResource(new StringAsset("{@io.quarkus.qute.deployment.typesafe.Movie movie}"
                             + "{@java.lang.Long age}"
                             + "{@java.lang.String surname}"
+                            + "{@java.util.Map<String,String map}"
                             // Property found
                             + "{movie.name} "
                             // Built-in value resolvers
@@ -39,7 +42,9 @@ public class ValidationSuccessTest {
                             // Varargs extension method
                             + "{movie.toLong(1l,2l)} "
                             // Field access
-                            + "{#each movie.mainCharacters}{it.substring(1)}{/}"),
+                            + "{#each movie.mainCharacters}{it.substring(1)}{/} "
+                            // Method param assignability
+                            + "{map.get('foo')}"),
                             "templates/movie.html"));
 
     @Inject
@@ -48,8 +53,9 @@ public class ValidationSuccessTest {
     @Test
     public void testResult() {
         // Validation succeeded! Yay!
-        assertEquals("Jason Jason Mono 1 10 11 ok 43 3 ohn",
-                movie.data("movie", new Movie("John")).data("name", "Vasik").data("surname", "Hu").data("age", 10l).render());
+        assertEquals("Jason Jason Mono 1 10 11 ok 43 3 ohn bar",
+                movie.data("movie", new Movie("John"), "name", "Vasik", "surname", "Hu", "age", 10l, "map",
+                        Collections.singletonMap("foo", "bar")).render());
     }
 
 }
diff --git a/...nsions/qute/runtime/src/main/java/io/quarkus/qute/runtime/devmode/QuteErrorPageSetup.java b/...nsions/qute/runtime/src/main/java/io/quarkus/qute/runtime/devmode/QuteErrorPageSetup.java
@@ -154,7 +154,7 @@ String getProblemInfo(int index, Throwable problem, Template problemTemplate, Es
         } else {
             for (int j = 0; j < sourceLines.size(); j++) {
                 // [1,2,3]
-                realLines.add(j);
+                realLines.add(j + 1);
             }
         }