Skip to content

Commit 6062363

Browse files
committed
Use canonical names in error messages in annotation processing
Prior to this commit, we invoked `Class.getName()` when building error messages during annotation processing, resulting in exceptions like the following which use binary names for nested types and arrays. Attribute 'chars' in annotation org.springframework.core.annotation.AnnotationUtilsTests$CharsContainer should be compatible with [C but a [I value was returned This commit switches to canonical names in error messages in annotation processing, resulting in improved such errors messages such as the following. Attribute 'chars' in annotation org.springframework.core.annotation.AnnotationUtilsTests.CharsContainer should be compatible with char[] but a int[] value was returned In addition, this commit introduces a new getCanonicalName(Class) method in ClassUtils, which has effectively been extracted from the following classes where this functionality was previously duplicated. - AttributeMethods - SynthesizedMergedAnnotationInvocationHandler - TypeDescriptor - DefaultRetryPolicy - ReflectiveIndexAccessor Closes gh-36607
1 parent 31d9fe5 commit 6062363

21 files changed

Lines changed: 143 additions & 113 deletions

spring-core-test/src/main/java/org/springframework/aot/agent/MethodReference.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import org.jspecify.annotations.Nullable;
2222

23+
import org.springframework.util.ClassUtils;
24+
2325
/**
2426
* Reference to a Java method, identified by its owner class and the method name.
2527
*
@@ -43,7 +45,7 @@ private MethodReference(String className, String methodName) {
4345
}
4446

4547
public static MethodReference of(Class<?> klass, String methodName) {
46-
return new MethodReference(klass.getCanonicalName(), methodName);
48+
return new MethodReference(ClassUtils.getCanonicalName(klass), methodName);
4749
}
4850

4951
/**

spring-core-test/src/main/java/org/springframework/aot/agent/RecordedInvocation.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.aot.hint.RuntimeHints;
2626
import org.springframework.aot.hint.TypeReference;
2727
import org.springframework.util.Assert;
28+
import org.springframework.util.ClassUtils;
2829

2930
/**
3031
* Record of an invocation of a method relevant to {@link org.springframework.aot.hint.RuntimeHints}.
@@ -181,7 +182,7 @@ public String toString() {
181182
else {
182183
Class<?> instanceType = (getInstance() instanceof Class<?> clazz) ? clazz : getInstance().getClass();
183184
return "<%s> invocation of <%s> on type <%s> with arguments %s".formatted(
184-
getHintType().hintClassName(), getMethodReference(), instanceType.getCanonicalName(), getArguments());
185+
getHintType().hintClassName(), getMethodReference(), ClassUtils.getCanonicalName(instanceType), getArguments());
185186
}
186187
}
187188

spring-core/src/main/java/org/springframework/core/annotation/AbstractMergedAnnotation.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.jspecify.annotations.Nullable;
2525

2626
import org.springframework.util.Assert;
27+
import org.springframework.util.ClassUtils;
2728

2829
/**
2930
* Abstract base class for {@link MergedAnnotation} implementations.
@@ -215,7 +216,7 @@ private <T> T getRequiredAttributeValue(String attributeName, Class<T> type) {
215216
T value = getAttributeValue(attributeName, type);
216217
if (value == null) {
217218
throw new NoSuchElementException("No attribute named '" + attributeName +
218-
"' present in merged annotation " + getType().getName());
219+
"' present in merged annotation " + ClassUtils.getCanonicalName(getType()));
219220
}
220221
return value;
221222
}

spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.jspecify.annotations.Nullable;
2626

2727
import org.springframework.util.Assert;
28+
import org.springframework.util.ClassUtils;
2829
import org.springframework.util.StringUtils;
2930

3031
/**
@@ -126,7 +127,7 @@ public AnnotationAttributes(Class<? extends Annotation> annotationType) {
126127
AnnotationAttributes(Class<? extends Annotation> annotationType, boolean validated) {
127128
Assert.notNull(annotationType, "'annotationType' must not be null");
128129
this.annotationType = annotationType;
129-
this.displayName = annotationType.getName();
130+
this.displayName = ClassUtils.getCanonicalName(annotationType);
130131
this.validated = validated;
131132
}
132133

spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.jspecify.annotations.Nullable;
3333

3434
import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.MirrorSet;
35+
import org.springframework.util.ClassUtils;
3536
import org.springframework.util.ObjectUtils;
3637
import org.springframework.util.StringUtils;
3738

@@ -649,7 +650,7 @@ <A> int resolve(@Nullable Object source, @Nullable A annotation, ValueExtractor
649650
throw new AnnotationConfigurationException(String.format(
650651
"Different @AliasFor mirror values for annotation [%s]%s; attribute '%s' " +
651652
"and its alias '%s' are declared with values of [%s] and [%s].",
652-
getAnnotationType().getName(), on,
653+
ClassUtils.getCanonicalName(getAnnotationType()), on,
653654
attributes.get(result).getName(),
654655
attribute.getName(),
655656
ObjectUtils.nullSafeToString(lastValue),

spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.jspecify.annotations.Nullable;
2929

3030
import org.springframework.lang.Contract;
31+
import org.springframework.util.ClassUtils;
3132
import org.springframework.util.ConcurrentReferenceHashMap;
3233

3334
/**
@@ -124,7 +125,8 @@ private void addIfPossible(Deque<AnnotationTypeMapping> queue, @Nullable Annotat
124125
AnnotationUtils.rethrowAnnotationConfigurationException(ex);
125126
if (failureLogger.isEnabled()) {
126127
failureLogger.log("Failed to introspect " + (meta ? "meta-annotation @" : "annotation @") +
127-
annotationType.getName(), (source != null ? source.getAnnotationType() : null), ex);
128+
ClassUtils.getCanonicalName(annotationType),
129+
(source != null ? ClassUtils.getCanonicalName(source.getAnnotationType()) : null), ex);
128130
}
129131
}
130132
}

spring-core/src/main/java/org/springframework/core/annotation/AttributeMethods.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.jspecify.annotations.Nullable;
2727

2828
import org.springframework.util.Assert;
29+
import org.springframework.util.ClassUtils;
2930
import org.springframework.util.ConcurrentReferenceHashMap;
3031
import org.springframework.util.ReflectionUtils;
3132

@@ -143,8 +144,9 @@ void validate(Annotation annotation) {
143144
throw ex;
144145
}
145146
catch (Throwable ex) {
146-
throw new IllegalStateException("Could not obtain annotation attribute value for " +
147-
get(i).getName() + " declared on @" + getName(annotation.annotationType()), ex);
147+
throw new IllegalStateException(
148+
"Could not obtain annotation attribute value for " + get(i).getName() +
149+
" declared on @" + ClassUtils.getCanonicalName(annotation.annotationType()), ex);
148150
}
149151
}
150152
}
@@ -305,13 +307,8 @@ static String describe(@Nullable Class<?> annotationType, @Nullable String attri
305307
if (attributeName == null) {
306308
return "(none)";
307309
}
308-
String in = (annotationType != null ? " in annotation [" + annotationType.getName() + "]" : "");
310+
String in = (annotationType != null ? " in annotation [" + ClassUtils.getCanonicalName(annotationType) + "]" : "");
309311
return "attribute '" + attributeName + "'" + in;
310312
}
311313

312-
private static String getName(Class<?> clazz) {
313-
String canonicalName = clazz.getCanonicalName();
314-
return (canonicalName != null ? canonicalName : clazz.getName());
315-
}
316-
317314
}

spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.springframework.lang.Contract;
2828
import org.springframework.util.Assert;
29+
import org.springframework.util.ClassUtils;
2930
import org.springframework.util.ConcurrentReferenceHashMap;
3031
import org.springframework.util.ObjectUtils;
3132

@@ -312,7 +313,7 @@ private static class ExplicitRepeatableContainer extends RepeatableContainers {
312313
if (returnType.componentType() != repeatable) {
313314
throw new AnnotationConfigurationException(
314315
"Container type [%s] must declare a 'value' attribute for an array of type [%s]"
315-
.formatted(container.getName(), repeatable.getName()));
316+
.formatted(ClassUtils.getCanonicalName(container), ClassUtils.getCanonicalName(repeatable)));
316317
}
317318
}
318319
catch (AnnotationConfigurationException ex) {
@@ -321,7 +322,7 @@ private static class ExplicitRepeatableContainer extends RepeatableContainers {
321322
catch (Throwable ex) {
322323
throw new AnnotationConfigurationException(
323324
"Invalid declaration of container type [%s] for repeatable annotation [%s]"
324-
.formatted(container.getName(), repeatable.getName()), ex);
325+
.formatted(ClassUtils.getCanonicalName(container), ClassUtils.getCanonicalName(repeatable)), ex);
325326
}
326327
this.repeatable = repeatable;
327328
this.container = container;
@@ -331,7 +332,7 @@ private static class ExplicitRepeatableContainer extends RepeatableContainers {
331332
private Class<? extends Annotation> deduceContainer(Class<? extends Annotation> repeatable) {
332333
Repeatable annotation = repeatable.getAnnotation(Repeatable.class);
333334
Assert.notNull(annotation, () -> "Annotation type must be a repeatable annotation: " +
334-
"failed to resolve container type for " + repeatable.getName());
335+
"failed to resolve container type for " + ClassUtils.getCanonicalName(repeatable));
335336
return annotation.value();
336337
}
337338

spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ private Integer computeHashCode() {
136136
private String annotationToString() {
137137
String string = this.string;
138138
if (string == null) {
139-
StringBuilder builder = new StringBuilder("@").append(getName(this.type)).append('(');
139+
StringBuilder builder = new StringBuilder("@").append(ClassUtils.getCanonicalName(this.type)).append('(');
140140
if (this.attributes.size() == 1 && this.attributes.get(0).getName().equals(MergedAnnotation.VALUE)) {
141141
// Don't prepend "value=" for an annotation that only declares a "value" attribute.
142142
builder.append(toString(getAttributeValue(this.attributes.get(0))));
@@ -208,7 +208,7 @@ private String toString(Object value) {
208208
return e.name();
209209
}
210210
if (type == Class.class) {
211-
return getName((Class<?>) value) + ".class";
211+
return ClassUtils.getCanonicalName((Class<?>) value) + ".class";
212212
}
213213
return String.valueOf(value);
214214
}
@@ -218,7 +218,7 @@ private Object getAttributeValue(Method method) {
218218
Class<?> type = ClassUtils.resolvePrimitiveIfNecessary(method.getReturnType());
219219
return this.annotation.getValue(attributeName, type).orElseThrow(
220220
() -> new NoSuchElementException("No value found for attribute named '" + attributeName +
221-
"' in merged annotation " + getName(this.annotation.getType())));
221+
"' in merged annotation " + ClassUtils.getCanonicalName(this.annotation.getType())));
222222
});
223223

224224
// Clone non-empty arrays so that users cannot alter the contents of values in our cache.
@@ -272,9 +272,4 @@ static <A extends Annotation> A createProxy(MergedAnnotation<A> annotation, Clas
272272
return (A) Proxy.newProxyInstance(classLoader, interfaces, handler);
273273
}
274274

275-
private static String getName(Class<?> clazz) {
276-
String canonicalName = clazz.getCanonicalName();
277-
return (canonicalName != null ? canonicalName : clazz.getName());
278-
}
279-
280275
}

spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ else if (value instanceof MergedAnnotation<?>[] annotations &&
490490
}
491491
if (!type.isInstance(value)) {
492492
throw new IllegalArgumentException("Unable to adapt value of type " +
493-
value.getClass().getName() + " to " + type.getName());
493+
ClassUtils.getCanonicalName(value.getClass()) + " to " + ClassUtils.getCanonicalName(type));
494494
}
495495
return (T) value;
496496
}
@@ -525,8 +525,8 @@ private Object adaptForAttribute(Method attribute, Object value) {
525525
}
526526
if (!attributeType.isInstance(value)) {
527527
throw new IllegalStateException("Attribute '" + attribute.getName() +
528-
"' in annotation " + getType().getName() + " should be compatible with " +
529-
attributeType.getName() + " but a " + value.getClass().getName() +
528+
"' in annotation " + ClassUtils.getCanonicalName(getType()) + " should be compatible with " +
529+
ClassUtils.getCanonicalName(attributeType) + " but a " + ClassUtils.getCanonicalName(value.getClass()) +
530530
" value was returned");
531531
}
532532
return value;
@@ -583,7 +583,7 @@ private int getAttributeIndex(String attributeName, boolean required) {
583583
int attributeIndex = (isFiltered(attributeName) ? -1 : this.mapping.getAttributes().indexOf(attributeName));
584584
if (attributeIndex == -1 && required) {
585585
throw new NoSuchElementException("No attribute named '" + attributeName +
586-
"' present in merged annotation " + getType().getName());
586+
"' present in merged annotation " + ClassUtils.getCanonicalName(getType()));
587587
}
588588
return attributeIndex;
589589
}
@@ -660,9 +660,10 @@ static <A extends Annotation> MergedAnnotation<A> of(
660660
catch (Exception ex) {
661661
AnnotationUtils.rethrowAnnotationConfigurationException(ex);
662662
if (logger.isEnabled()) {
663-
String type = mapping.getAnnotationType().getName();
663+
String type = ClassUtils.getCanonicalName(mapping.getAnnotationType());
664664
String item = (mapping.getDistance() == 0 ? "annotation " + type :
665-
"meta-annotation " + type + " from " + mapping.getRoot().getAnnotationType().getName());
665+
"meta-annotation " + type + " from " +
666+
ClassUtils.getCanonicalName(mapping.getRoot().getAnnotationType()));
666667
logger.log("Failed to introspect " + item, source, ex);
667668
}
668669
return null;

0 commit comments

Comments
 (0)