From b01fdb01408499bf9c73de1e927017b4870ffe26 Mon Sep 17 00:00:00 2001 From: Junseo Bae Date: Wed, 1 Apr 2026 00:54:39 +0900 Subject: [PATCH 1/2] Fix ClassFileMethodMetadata return type names for primitives and arrays Return correct names for primitive and array types Introduce resolveTypeName helper for ClassDesc handling Add ClassFileMethodMetadataTests (java24Test) Extend AbstractMethodMetadataTests with void return case See gh-36577 Signed-off-by: Junseo Bae --- .../classreading/ClassFileMethodMetadata.java | 14 +++- .../type/AbstractMethodMetadataTests.java | 12 +++ .../ClassFileMethodMetadataTests.java | 81 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java diff --git a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java index bc61f08101a7..76d36f4b3d81 100644 --- a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java +++ b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java @@ -143,7 +143,7 @@ static ClassFileMethodMetadata of(MethodModel methodModel, ClassLoader classLoad AccessFlags flags = methodModel.flags(); String declaringClassName = methodModel.parent().map(parent -> ClassUtils.convertResourcePathToClassName(parent.thisClass().name().stringValue())).orElse(null); ClassDesc returnType = methodModel.methodTypeSymbol().returnType(); - String returnTypeName = returnType.packageName() + "." + returnType.displayName(); + String returnTypeName = resolveTypeName(returnType); Source source = new Source(declaringClassName, flags, methodName, methodModel.methodTypeSymbol()); MergedAnnotations annotations = methodModel.elementStream() .filter(element -> element instanceof RuntimeVisibleAnnotationsAttribute) @@ -154,6 +154,18 @@ static ClassFileMethodMetadata of(MethodModel methodModel, ClassLoader classLoad } + private static String resolveTypeName(ClassDesc type) { + if (type.isPrimitive()) { + return type.displayName(); + } + if (type.isArray()) { + return resolveTypeName(type.componentType()) + "[]"; + } + String packageName = type.packageName(); + return (packageName.isEmpty() ? type.displayName() : packageName + "." + type.displayName()); + } + + /** * {@link MergedAnnotation} source. * @param declaringClassName the name of the declaring class diff --git a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java index 4d52c7f44940..26c14b3159b9 100644 --- a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java +++ b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java @@ -102,6 +102,11 @@ void getReturnTypeReturnsReturnType() { String.class.getName()); } + @Test + void getReturnTypeReturnsVoidForVoidReturnType() { + assertThat(getTagged(WithVoidMethod.class).getReturnTypeName()).isEqualTo("void"); + } + @Test void isAbstractWhenAbstractReturnsTrue() { assertThat(getTagged(WithAbstractMethod.class).isAbstract()).isTrue(); @@ -217,6 +222,13 @@ public String test() { } + public static class WithVoidMethod { + + @Tag + public void test() {} + + } + public static class WithMethodWithOneArgument { @Tag diff --git a/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java b/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java new file mode 100644 index 000000000000..23b970190548 --- /dev/null +++ b/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java @@ -0,0 +1,81 @@ +package org.springframework.core.type.classreading; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.type.MethodMetadata; + +import static org.assertj.core.api.Assertions.assertThat; + +class ClassFileMethodMetadataTests { + + @Test + void getReturnTypeReturnsVoidForVoidReturnType() throws Exception { + MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) + .getMetadataReader(WithVoidMethod.class.getName()) + .getAnnotationMetadata() + .getAnnotatedMethods(Tag.class.getName()) + .iterator().next(); + + assertThat(metadata.getReturnTypeName()).isEqualTo("void"); + } + + @Test + void getReturnTypeReturnsPrimitiveForPrimitiveReturnType() throws Exception { + MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) + .getMetadataReader(WithIntMethod.class.getName()) + .getAnnotationMetadata() + .getAnnotatedMethods(Tag.class.getName()) + .iterator().next(); + + assertThat(metadata.getReturnTypeName()).isEqualTo("int"); + } + + @Test + void getReturnTypeReturnsReferenceTypeForReferenceReturnType() throws Exception { + MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) + .getMetadataReader(WithStringMethod.class.getName()) + .getAnnotationMetadata() + .getAnnotatedMethods(Tag.class.getName()) + .iterator().next(); + + assertThat(metadata.getReturnTypeName()).isEqualTo(String.class.getName()); + } + + @Test + void getReturnTypeReturnsArrayTypeForArrayReturnType() throws Exception { + MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) + .getMetadataReader(WithStringArrayMethod.class.getName()) + .getAnnotationMetadata() + .getAnnotatedMethods(Tag.class.getName()) + .iterator().next(); + + assertThat(metadata.getReturnTypeName()).isEqualTo("java.lang.String[]"); + } + + @Retention(RetentionPolicy.RUNTIME) + @interface Tag {} + + public static class WithVoidMethod { + @Tag + public void test() {} + } + + public static class WithIntMethod { + @Tag + public int test() { return 0; } + } + + public static class WithStringMethod { + @Tag + public String test() { return ""; } + } + + public static class WithStringArrayMethod { + @Tag + public String[] test() { return new String[0]; } + } + +} From 40ef6f68726521752ff030490eb9d658c97ee265 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 2 Apr 2026 10:13:17 +0200 Subject: [PATCH 2/2] Fix method and annotation metadata for ClassFile variant Prior to this commit, the ClassFile variant for annotation and method metadata would report incorrect metadata for: * the method return type names in case of primitives and array types * `toString` values for methods * `equals` and `hashcode` information for methods This commit expands the test suite and ensures that the ASM and ClassFile variants are aligned. Fixes gh-36577 Signed-off-by: Brian Clozel --- .../ClassFileAnnotationDelegate.java | 13 +-- .../ClassFileAnnotationMetadata.java | 12 +++ .../classreading/ClassFileMethodMetadata.java | 35 ++++---- .../type/AbstractMethodMetadataTests.java | 55 ++++++++++++- ...s.java => DefaultMethodMetadataTests.java} | 17 ++-- .../ClassFileMethodMetadataTests.java | 81 ------------------- 6 files changed, 97 insertions(+), 116 deletions(-) rename spring-core/src/test/java/org/springframework/core/type/classreading/{SimpleMethodMetadataTests.java => DefaultMethodMetadataTests.java} (71%) delete mode 100644 spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java diff --git a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationDelegate.java b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationDelegate.java index edc25f6937c3..810636236da3 100644 --- a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationDelegate.java +++ b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationDelegate.java @@ -20,7 +20,6 @@ import java.lang.classfile.AnnotationElement; import java.lang.classfile.AnnotationValue; import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; -import java.lang.constant.ClassDesc; import java.lang.reflect.Array; import java.util.Collections; import java.util.LinkedHashMap; @@ -63,7 +62,7 @@ static MergedAnnotations createMergedAnnotations( private static @Nullable MergedAnnotation createMergedAnnotation( String className, Annotation annotation, @Nullable ClassLoader classLoader) { - String typeName = fromTypeDescriptor(annotation.className().stringValue()); + String typeName = ClassFileAnnotationMetadata.resolveTypeName(annotation.classSymbol()); if (AnnotationFilter.PLAIN.matches(typeName)) { return null; } @@ -97,7 +96,7 @@ static MergedAnnotations createMergedAnnotations( return createMergedAnnotation(className, annotationValue.annotation(), classLoader); } case AnnotationValue.OfClass classValue -> { - return fromTypeDescriptor(classValue.className().stringValue()); + return ClassFileAnnotationMetadata.resolveTypeName(classValue.classSymbol()); } case AnnotationValue.OfEnum enumValue -> { return parseEnum(enumValue, classLoader); @@ -108,12 +107,6 @@ static MergedAnnotations createMergedAnnotations( } } - private static String fromTypeDescriptor(String descriptor) { - ClassDesc classDesc = ClassDesc.ofDescriptor(descriptor); - return (classDesc.isPrimitive() ? classDesc.displayName() : - classDesc.packageName() + "." + classDesc.displayName()); - } - private static Object parseArrayValue(String className, @Nullable ClassLoader classLoader, AnnotationValue.OfArray arrayValue) { if (arrayValue.values().isEmpty()) { return new Object[0]; @@ -145,7 +138,7 @@ private static > Enum parseEnum(AnnotationValue.OfEnum enum } private static Class loadEnumClass(AnnotationValue.OfEnum enumValue, @Nullable ClassLoader classLoader) { - String className = fromTypeDescriptor(enumValue.className().stringValue()); + String className = ClassFileAnnotationMetadata.resolveTypeName(enumValue.classSymbol()); return ClassUtils.resolveClassName(className, classLoader); } diff --git a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationMetadata.java b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationMetadata.java index 99c5bab22b02..e205385ff18d 100644 --- a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationMetadata.java +++ b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationMetadata.java @@ -26,6 +26,7 @@ import java.lang.classfile.attribute.NestHostAttribute; import java.lang.classfile.attribute.RuntimeVisibleAnnotationsAttribute; import java.lang.classfile.constantpool.ClassEntry; +import java.lang.constant.ClassDesc; import java.lang.reflect.AccessFlag; import java.util.Collections; import java.util.LinkedHashSet; @@ -221,6 +222,17 @@ static ClassFileAnnotationMetadata of(ClassModel classModel, @Nullable ClassLoad return builder.build(); } + static String resolveTypeName(ClassDesc type) { + if (type.isPrimitive()) { + return type.displayName(); + } + if (type.isArray()) { + return resolveTypeName(type.componentType()) + "[]"; + } + String packageName = type.packageName(); + return (packageName.isEmpty() ? type.displayName() : packageName + "." + type.displayName()); + } + static class Builder { diff --git a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java index 76d36f4b3d81..902774906f93 100644 --- a/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java +++ b/spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java @@ -24,6 +24,7 @@ import java.lang.reflect.AccessFlag; import java.util.Collections; import java.util.Locale; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -143,7 +144,7 @@ static ClassFileMethodMetadata of(MethodModel methodModel, ClassLoader classLoad AccessFlags flags = methodModel.flags(); String declaringClassName = methodModel.parent().map(parent -> ClassUtils.convertResourcePathToClassName(parent.thisClass().name().stringValue())).orElse(null); ClassDesc returnType = methodModel.methodTypeSymbol().returnType(); - String returnTypeName = resolveTypeName(returnType); + String returnTypeName = ClassFileAnnotationMetadata.resolveTypeName(returnType); Source source = new Source(declaringClassName, flags, methodName, methodModel.methodTypeSymbol()); MergedAnnotations annotations = methodModel.elementStream() .filter(element -> element instanceof RuntimeVisibleAnnotationsAttribute) @@ -154,18 +155,6 @@ static ClassFileMethodMetadata of(MethodModel methodModel, ClassLoader classLoad } - private static String resolveTypeName(ClassDesc type) { - if (type.isPrimitive()) { - return type.displayName(); - } - if (type.isArray()) { - return resolveTypeName(type.componentType()) + "[]"; - } - String packageName = type.packageName(); - return (packageName.isEmpty() ? type.displayName() : packageName + "." + type.displayName()); - } - - /** * {@link MergedAnnotation} source. * @param declaringClassName the name of the declaring class @@ -175,6 +164,22 @@ private static String resolveTypeName(ClassDesc type) { */ record Source(@Nullable String declaringClassName, AccessFlags flags, String methodName, MethodTypeDesc descriptor) { + @Override + public boolean equals(Object o) { + if (!(o instanceof Source source)) { + return false; + } + return Objects.equals(this.flags.flagsMask(), source.flags.flagsMask()) && + Objects.equals(this.methodName, source.methodName) && + Objects.equals(this.declaringClassName, source.declaringClassName) && + Objects.equals(this.descriptor.descriptorString(), source.descriptor.descriptorString()); + } + + @Override + public int hashCode() { + return Objects.hash(this.declaringClassName, this.flags.flagsMask(), this.methodName, this.descriptor.descriptorString()); + } + @Override public String toString() { StringBuilder builder = new StringBuilder(); @@ -182,9 +187,7 @@ public String toString() { builder.append(flag.name().toLowerCase(Locale.ROOT)); builder.append(' '); }); - builder.append(this.descriptor.returnType().packageName()); - builder.append("."); - builder.append(this.descriptor.returnType().displayName()); + builder.append(ClassFileAnnotationMetadata.resolveTypeName(this.descriptor.returnType())); builder.append(' '); builder.append(this.declaringClassName); builder.append('.'); diff --git a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java index 26c14b3159b9..a1fe7ead4c24 100644 --- a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java +++ b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java @@ -28,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * Base class for {@link MethodMetadata} tests. @@ -76,13 +77,19 @@ void verifyHashCode() { @Test void verifyToString() { assertThat(getTagged(WithMethod.class).toString()) - .endsWith(WithMethod.class.getName() + ".test()"); + .isEqualTo("public java.lang.String " + WithMethod.class.getName() + ".test()"); assertThat(getTagged(WithMethodWithOneArgument.class).toString()) - .endsWith(WithMethodWithOneArgument.class.getName() + ".test(java.lang.String)"); + .isEqualTo("public java.lang.String " + WithMethodWithOneArgument.class.getName() + ".test(java.lang.String)"); assertThat(getTagged(WithMethodWithTwoArguments.class).toString()) - .endsWith(WithMethodWithTwoArguments.class.getName() + ".test(java.lang.String,java.lang.Integer)"); + .isEqualTo("public java.lang.String " + WithMethodWithTwoArguments.class.getName() + ".test(java.lang.String,java.lang.Integer)"); + + assertThat(getTagged(WithPrimitiveArrayMethod.class).toString()) + .isEqualTo("public int[] " + WithPrimitiveArrayMethod.class.getName() + ".test()"); + + assertThat(getTagged(WithStringArrayMethod.class).toString()) + .isEqualTo("public java.lang.String[] " + WithStringArrayMethod.class.getName() + ".test()"); } @Test @@ -107,6 +114,34 @@ void getReturnTypeReturnsVoidForVoidReturnType() { assertThat(getTagged(WithVoidMethod.class).getReturnTypeName()).isEqualTo("void"); } + @Test + void getReturnTypeReturnsPrimitiveArrayForPrimitiveArrayReturnTypeForStandardReflection() { + MethodMetadata methodMetadata = getTagged(WithPrimitiveArrayMethod.class); + assumeTrue(methodMetadata instanceof StandardMethodMetadata, "skipped for ASM and ClassFile"); + assertThat(methodMetadata.getReturnTypeName()).isEqualTo("[I"); + } + + @Test + void getReturnTypeReturnsPrimitiveArrayForPrimitiveArrayReturnType() { + MethodMetadata methodMetadata = getTagged(WithPrimitiveArrayMethod.class); + assumeTrue(!(methodMetadata instanceof StandardMethodMetadata), "skipped for standard reflection"); + assertThat(methodMetadata.getReturnTypeName()).isEqualTo("int[]"); + } + + @Test + void getReturnTypeReturnsStringArrayForStringArrayReturnTypeForStandardReflection() { + MethodMetadata methodMetadata = getTagged(WithStringArrayMethod.class); + assumeTrue(methodMetadata instanceof StandardMethodMetadata, "skipped for ASM and ClassFile"); + assertThat(methodMetadata.getReturnTypeName()).isEqualTo("[Ljava.lang.String;"); + } + + @Test + void getReturnTypeReturnsStringArrayForStringArrayReturnType() { + MethodMetadata methodMetadata = getTagged(WithStringArrayMethod.class); + assumeTrue(!(methodMetadata instanceof StandardMethodMetadata), "skipped for standard reflection"); + assertThat(methodMetadata.getReturnTypeName()).isEqualTo("java.lang.String[]"); + } + @Test void isAbstractWhenAbstractReturnsTrue() { assertThat(getTagged(WithAbstractMethod.class).isAbstract()).isTrue(); @@ -229,6 +264,20 @@ public void test() {} } + public static class WithPrimitiveArrayMethod { + + @Tag + public int[] test() { return new int[0];} + + } + + public static class WithStringArrayMethod { + + @Tag + public String[] test() { return new String[0];} + + } + public static class WithMethodWithOneArgument { @Tag diff --git a/spring-core/src/test/java/org/springframework/core/type/classreading/SimpleMethodMetadataTests.java b/spring-core/src/test/java/org/springframework/core/type/classreading/DefaultMethodMetadataTests.java similarity index 71% rename from spring-core/src/test/java/org/springframework/core/type/classreading/SimpleMethodMetadataTests.java rename to spring-core/src/test/java/org/springframework/core/type/classreading/DefaultMethodMetadataTests.java index 2e75b49c3cdf..5b539cfd6ff1 100644 --- a/spring-core/src/test/java/org/springframework/core/type/classreading/SimpleMethodMetadataTests.java +++ b/spring-core/src/test/java/org/springframework/core/type/classreading/DefaultMethodMetadataTests.java @@ -16,25 +16,30 @@ package org.springframework.core.type.classreading; +import java.io.IOException; + import org.springframework.core.type.AbstractMethodMetadataTests; import org.springframework.core.type.AnnotationMetadata; + /** * Tests for {@link SimpleMethodMetadata} and - * {@link SimpleMethodMetadataReadingVisitor}. + * {@link SimpleMethodMetadataReadingVisitor} on Java < 24, + * and for the ClassFile API variant on Java >= 24. * * @author Phillip Webb + * @author Brian Clozel */ -class SimpleMethodMetadataTests extends AbstractMethodMetadataTests { +class DefaultMethodMetadataTests extends AbstractMethodMetadataTests { + @Override protected AnnotationMetadata get(Class source) { try { - return new SimpleMetadataReaderFactory( - source.getClassLoader()).getMetadataReader( - source.getName()).getAnnotationMetadata(); + return MetadataReaderFactory.create(source.getClassLoader()) + .getMetadataReader(source.getName()).getAnnotationMetadata(); } - catch (Exception ex) { + catch (IOException ex) { throw new IllegalStateException(ex); } } diff --git a/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java b/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java deleted file mode 100644 index 23b970190548..000000000000 --- a/spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java +++ /dev/null @@ -1,81 +0,0 @@ -package org.springframework.core.type.classreading; - -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -import org.junit.jupiter.api.Test; - -import org.springframework.core.type.MethodMetadata; - -import static org.assertj.core.api.Assertions.assertThat; - -class ClassFileMethodMetadataTests { - - @Test - void getReturnTypeReturnsVoidForVoidReturnType() throws Exception { - MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) - .getMetadataReader(WithVoidMethod.class.getName()) - .getAnnotationMetadata() - .getAnnotatedMethods(Tag.class.getName()) - .iterator().next(); - - assertThat(metadata.getReturnTypeName()).isEqualTo("void"); - } - - @Test - void getReturnTypeReturnsPrimitiveForPrimitiveReturnType() throws Exception { - MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) - .getMetadataReader(WithIntMethod.class.getName()) - .getAnnotationMetadata() - .getAnnotatedMethods(Tag.class.getName()) - .iterator().next(); - - assertThat(metadata.getReturnTypeName()).isEqualTo("int"); - } - - @Test - void getReturnTypeReturnsReferenceTypeForReferenceReturnType() throws Exception { - MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) - .getMetadataReader(WithStringMethod.class.getName()) - .getAnnotationMetadata() - .getAnnotatedMethods(Tag.class.getName()) - .iterator().next(); - - assertThat(metadata.getReturnTypeName()).isEqualTo(String.class.getName()); - } - - @Test - void getReturnTypeReturnsArrayTypeForArrayReturnType() throws Exception { - MethodMetadata metadata = new ClassFileMetadataReaderFactory(getClass().getClassLoader()) - .getMetadataReader(WithStringArrayMethod.class.getName()) - .getAnnotationMetadata() - .getAnnotatedMethods(Tag.class.getName()) - .iterator().next(); - - assertThat(metadata.getReturnTypeName()).isEqualTo("java.lang.String[]"); - } - - @Retention(RetentionPolicy.RUNTIME) - @interface Tag {} - - public static class WithVoidMethod { - @Tag - public void test() {} - } - - public static class WithIntMethod { - @Tag - public int test() { return 0; } - } - - public static class WithStringMethod { - @Tag - public String test() { return ""; } - } - - public static class WithStringArrayMethod { - @Tag - public String[] test() { return new String[0]; } - } - -}