spring/spring-framework
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-framework.git synced 2026-09-17 16:39:29 +00:00
Code Issues Projects Releases Wiki Activity

Fix method and annotation metadata for ClassFile variant

Browse Source 
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 <brian.clozel@broadcom.com>
This commit is contained in:
Brian Clozel
2026-04-02 10:18:13 +02:00
parent b01fdb0140
commit 40ef6f6872
6 changed files with 97 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationDelegate.java
+3 -10
View File
@@ -20,7 +20,6 @@ import java.lang.classfile.Annotation;
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 @@ abstract class ClassFileAnnotationDelegate {
private static <A extends java.lang.annotation.Annotation> @Nullable MergedAnnotation<A> 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 @@ abstract class ClassFileAnnotationDelegate {
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 @@ abstract class ClassFileAnnotationDelegate {
}
}
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 @@ abstract class ClassFileAnnotationDelegate {
}
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);
}
spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileAnnotationMetadata.java
+12
View File
@@ -26,6 +26,7 @@ import java.lang.classfile.attribute.InnerClassesAttribute;
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 @@ final class ClassFileAnnotationMetadata implements AnnotationMetadata {
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 {
spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMethodMetadata.java
+19 -16
View File
@@ -24,6 +24,7 @@ import java.lang.constant.MethodTypeDesc;
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 @@ final class ClassFileMethodMetadata implements MethodMetadata {
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 @@ final class ClassFileMethodMetadata implements MethodMetadata {
}
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 @@ final class ClassFileMethodMetadata implements MethodMetadata {
*/
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 @@ final class ClassFileMethodMetadata implements MethodMetadata {
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('.');
spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java
+52 -3
View File
@@ -28,6 +28,7 @@ import org.springframework.util.MultiValueMap;
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 @@ public abstract class AbstractMethodMetadataTests {
@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 @@ public abstract class AbstractMethodMetadataTests {
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 abstract class AbstractMethodMetadataTests {
}
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
spring-core/src/test/java/org/springframework/core/type/classreading/SimpleMethodMetadataTests.java → spring-core/src/test/java/org/springframework/core/type/classreading/DefaultMethodMetadataTests.java
+11 -6
View File
@@ -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);
}
}
spring-core/src/test/java24/org/springframework/core/type/classreading/ClassFileMethodMetadataTests.java
-81
View File
@@ -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]; }
}
}