From ff782c779e2f81bc2be03627d28208c3e496698d Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 9 Aug 2024 09:27:21 +0200 Subject: [PATCH] Qute template records: fix the way the canonical constructor is found - also fix the problem with compact record constructor - resolves #42411 --- .../qute/deployment/QuteProcessor.java | 9 +- .../deployment/TemplateRecordEnhancer.java | 129 +++++++++--------- .../records/TemplateRecordTest.java | 26 +++- 3 files changed, 93 insertions(+), 71 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 index 51c396c94d29b..92fbfbdc1847e 100644 --- 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 @@ -434,9 +434,9 @@ && isNotLocatedByCustomTemplateLocator(locatorPatternsBuildItem.getLocationPatte if (!recordClass.isRecord()) { continue; } - Type[] componentTypes = recordClass.recordComponents().stream().map(RecordComponentInfo::type) - .toArray(Type[]::new); - MethodInfo canonicalConstructor = recordClass.method(MethodDescriptor.INIT, componentTypes); + MethodInfo canonicalConstructor = recordClass.method(MethodDescriptor.INIT, + recordClass.unsortedRecordComponents().stream().map(RecordComponentInfo::type) + .toArray(Type[]::new)); AnnotationInstance checkedTemplateAnnotation = recordClass.declaredAnnotation(Names.CHECKED_TEMPLATE); String fragmentId = getCheckedFragmentId(recordClass, checkedTemplateAnnotation); @@ -509,7 +509,8 @@ && isNotLocatedByCustomTemplateLocator(locatorPatternsBuildItem.getLocationPatte requireTypeSafeExpressions != null ? requireTypeSafeExpressions.asBoolean() : true)); transformers.produce(new BytecodeTransformerBuildItem(recordClass.name().toString(), new TemplateRecordEnhancer(recordInterface, recordClass, templatePath, fragmentId, - canonicalConstructor.parameters(), adaptors.get(recordInterfaceName)))); + canonicalConstructor.descriptor(), canonicalConstructor.parameters(), + adaptors.get(recordInterfaceName)))); } } diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateRecordEnhancer.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateRecordEnhancer.java index b9da1875790d7..9e29ea4a2b710 100644 --- a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateRecordEnhancer.java +++ b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TemplateRecordEnhancer.java @@ -34,15 +34,17 @@ class TemplateRecordEnhancer implements BiFunction parameters; private final CheckedTemplateAdapter adapter; TemplateRecordEnhancer(ClassInfo interfaceToImplement, ClassInfo recordClass, String templateId, String fragmentId, - List parameters, CheckedTemplateAdapter adapter) { + String canonicalConstructorDescriptor, List parameters, CheckedTemplateAdapter adapter) { this.interfaceToImplement = interfaceToImplement; this.recordClassName = recordClass.name().toString(); this.templateId = templateId; this.fragmentId = fragmentId; + this.canonicalConstructorDescriptor = canonicalConstructorDescriptor; this.parameters = parameters; this.adapter = adapter; } @@ -61,7 +63,7 @@ public TemplateRecordClassVisitor(ClassVisitor outputClassVisitor) { @Override public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { MethodVisitor ret = super.visitMethod(access, name, descriptor, signature, exceptions); - if (name.equals(MethodDescriptor.INIT)) { + if (name.equals(MethodDescriptor.INIT) && descriptor.equals(canonicalConstructorDescriptor)) { return new TemplateRecordConstructorVisitor(ret); } return ret; @@ -126,74 +128,69 @@ public TemplateRecordConstructorVisitor(MethodVisitor outputVisitor) { @Override public void visitInsn(int opcode) { - if (opcode != Opcodes.RETURN) { - super.visitInsn(opcode); - } - } + if (opcode == Opcodes.RETURN) { + visitVarInsn(Opcodes.ALOAD, 0); + + MethodDescriptor arcContainer = MethodDescriptor.ofMethod(Arc.class, "container", ArcContainer.class); + MethodDescriptor arcContainerInstance = MethodDescriptor.ofMethod(ArcContainer.class, "instance", + InstanceHandle.class, Class.class, Annotation[].class); + MethodDescriptor instanceHandleGet = MethodDescriptor.ofMethod(InstanceHandle.class, "get", Object.class); + MethodDescriptor templateProducerGetInjectableTemplate = MethodDescriptor.ofMethod(TemplateProducer.class, + "getInjectableTemplate", Template.class, String.class); + MethodDescriptor templateInstance = MethodDescriptor.ofMethod(Template.class, "instance", + TemplateInstance.class); + MethodDescriptor templateGetFragment = MethodDescriptor.ofMethod(Template.class, "getFragment", + Template.Fragment.class, String.class); + MethodDescriptor templateInstanceData = MethodDescriptor.ofMethod(TemplateInstance.class, "data", + TemplateInstance.class, String.class, Object.class); + + // Template template = Arc.container().instance(TemplateProducer.class).get().getInjectableTemplate("HelloResource/typedTemplate"); + visitMethodInsn(Opcodes.INVOKESTATIC, arcContainer.getDeclaringClass(), arcContainer.getName(), + arcContainer.getDescriptor(), + false); + visitLdcInsn(org.objectweb.asm.Type.getType(TemplateProducer.class)); + visitLdcInsn(0); + visitTypeInsn(Opcodes.ANEWARRAY, toInternalClassName(Annotation.class)); + invokeInterface(this, arcContainerInstance); + invokeInterface(this, instanceHandleGet); + visitTypeInsn(Opcodes.CHECKCAST, toInternalClassName(TemplateProducer.class)); + visitLdcInsn(templateId); + visitMethodInsn(Opcodes.INVOKEVIRTUAL, templateProducerGetInjectableTemplate.getDeclaringClass(), + templateProducerGetInjectableTemplate.getName(), + templateProducerGetInjectableTemplate.getDescriptor(), false); + if (fragmentId != null) { + // template = template.getFragment(id); + visitLdcInsn(fragmentId); + invokeInterface(this, templateGetFragment); + } + // templateInstance = template.instance(); + invokeInterface(this, templateInstance); - @Override - public void visitEnd() { - visitCode(); - visitVarInsn(Opcodes.ALOAD, 0); - - MethodDescriptor arcContainer = MethodDescriptor.ofMethod(Arc.class, "container", ArcContainer.class); - MethodDescriptor arcContainerInstance = MethodDescriptor.ofMethod(ArcContainer.class, "instance", - InstanceHandle.class, Class.class, Annotation[].class); - MethodDescriptor instanceHandleGet = MethodDescriptor.ofMethod(InstanceHandle.class, "get", Object.class); - MethodDescriptor templateProducerGetInjectableTemplate = MethodDescriptor.ofMethod(TemplateProducer.class, - "getInjectableTemplate", Template.class, String.class); - MethodDescriptor templateInstance = MethodDescriptor.ofMethod(Template.class, "instance", TemplateInstance.class); - MethodDescriptor templateGetFragment = MethodDescriptor.ofMethod(Template.class, "getFragment", - Template.Fragment.class, String.class); - MethodDescriptor templateInstanceData = MethodDescriptor.ofMethod(TemplateInstance.class, "data", - TemplateInstance.class, String.class, Object.class); - - // Template template = Arc.container().instance(TemplateProducer.class).get().getInjectableTemplate("HelloResource/typedTemplate"); - visitMethodInsn(Opcodes.INVOKESTATIC, arcContainer.getDeclaringClass(), arcContainer.getName(), - arcContainer.getDescriptor(), - false); - visitLdcInsn(org.objectweb.asm.Type.getType(TemplateProducer.class)); - visitLdcInsn(0); - visitTypeInsn(Opcodes.ANEWARRAY, toInternalClassName(Annotation.class)); - invokeInterface(this, arcContainerInstance); - invokeInterface(this, instanceHandleGet); - visitTypeInsn(Opcodes.CHECKCAST, toInternalClassName(TemplateProducer.class)); - visitLdcInsn(templateId); - visitMethodInsn(Opcodes.INVOKEVIRTUAL, templateProducerGetInjectableTemplate.getDeclaringClass(), - templateProducerGetInjectableTemplate.getName(), - templateProducerGetInjectableTemplate.getDescriptor(), false); - if (fragmentId != null) { - // template = template.getFragment(id); - visitLdcInsn(fragmentId); - invokeInterface(this, templateGetFragment); - } - // templateInstance = template.instance(); - invokeInterface(this, templateInstance); + if (adapter != null) { + adapter.convertTemplateInstance(this); + templateInstanceData = MethodDescriptor.ofMethod(adapter.templateInstanceBinaryName(), "data", + adapter.templateInstanceBinaryName(), String.class, Object.class); + } - if (adapter != null) { - adapter.convertTemplateInstance(this); - templateInstanceData = MethodDescriptor.ofMethod(adapter.templateInstanceBinaryName(), "data", - adapter.templateInstanceBinaryName(), String.class, Object.class); - } + int slot = 1; + for (int i = 0; i < parameters.size(); i++) { + // instance = instance.data("name", value); + Type paramType = parameters.get(i).type(); + visitLdcInsn(parameters.get(i).name()); + visitVarInsn(AsmUtil.getLoadOpcode(paramType), slot); + AsmUtil.boxIfRequired(this, paramType); + invokeInterface(this, templateInstanceData); + slot += AsmUtil.getParameterSize(paramType); + } - int slot = 1; - for (int i = 0; i < parameters.size(); i++) { - // instance = instance.data("name", value); - Type paramType = parameters.get(i).type(); - visitLdcInsn(parameters.get(i).name()); - visitVarInsn(AsmUtil.getLoadOpcode(paramType), slot); - AsmUtil.boxIfRequired(this, paramType); - invokeInterface(this, templateInstanceData); - slot += AsmUtil.getParameterSize(paramType); + FieldDescriptor quteTemplateInstanceDescriptor = FieldDescriptor.of(recordClassName, "qute$templateInstance", + interfaceToImplement.name().toString()); + visitFieldInsn(Opcodes.PUTFIELD, quteTemplateInstanceDescriptor.getDeclaringClass(), + quteTemplateInstanceDescriptor.getName(), quteTemplateInstanceDescriptor.getType()); + super.visitInsn(Opcodes.RETURN); + } else { + super.visitInsn(opcode); } - - FieldDescriptor quteTemplateInstanceDescriptor = FieldDescriptor.of(recordClassName, "qute$templateInstance", - interfaceToImplement.name().toString()); - visitFieldInsn(Opcodes.PUTFIELD, quteTemplateInstanceDescriptor.getDeclaringClass(), - quteTemplateInstanceDescriptor.getName(), quteTemplateInstanceDescriptor.getType()); - super.visitInsn(Opcodes.RETURN); - super.visitEnd(); - visitMaxs(-1, -1); } } diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/records/TemplateRecordTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/records/TemplateRecordTest.java index e5e9e13b8840a..d3f7e1546d14a 100644 --- a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/records/TemplateRecordTest.java +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/records/TemplateRecordTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -34,7 +35,9 @@ public class TemplateRecordTest { .addAsResource(new StringAsset("Hello {name}!"), "templates/hello_world.txt") .addAsResource(new StringAsset("Hello {#fragment id=name}{name}{/fragment}!"), - "templates/hello.txt")); + "templates/hello.txt") + .addAsResource(new StringAsset("{alpha}:{bravo}:{charlie}"), + "templates/TemplateRecordTest/multiParams.txt")); @Inject Engine engine; @@ -69,6 +72,9 @@ public void testTemplateRecords() throws InterruptedException, ExecutionExceptio assertEquals("Hello Lu!", new helloWorld("Lu").render()); assertEquals("Lu", new hello$name("Lu").render()); + + assertEquals("15:true:foo", new multiParams(true, 15, "foo").render()); + assertThrows(IllegalArgumentException.class, () -> new multiParams(false, 50, null)); } record HelloInt(int val) implements TemplateInstance { @@ -82,4 +88,22 @@ record helloWorld(String name) implements TemplateInstance { record hello$name(String name) implements TemplateInstance { } + record multiParams(boolean bravo, int alpha, String charlie) implements TemplateInstance { + + public multiParams { + if (alpha > 20) { + throw new IllegalArgumentException(); + } + } + + public multiParams(String delta) { + this(false, 1, delta); + } + + public multiParams(int alpha, boolean bravo, String charlie) { + this(bravo, alpha, charlie); + throw new IllegalArgumentException(""); + } + } + }