Ensure toString() for synthesized annotations is source code compatible

Since the introduction of synthesized annotation support in Spring
Framework 4.2 (a.k.a., merged annotations), the toString()
implementation attempted to align with the formatting used by the JDK
itself. However, Class annotation attributes were formatted using
Class#getName in Spring; whereas, the JDK used Class#toString up until
JDK 9.

In addition, JDK 9 introduced new formatting for toString() for
annotations, apparently intended to align with the syntax used in the
source code declaration of the annotation. However, JDK 9+ formats enum
annotation attributes using Enum#toString instead of Enum#name, which
can lead to issues if toString() is overridden in an enum.

This commit updates the formatting used for synthesized annotations by
ensuring that toString() generates a string that is compatible with the
syntax of the originating source code, going beyond the changes made in
JDK 9 by using Enum#name instead of Enum#toString.

Closes gh-28015

Author: sbrannen

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -195,18 +195,24 @@ private String annotationToString() {
 	}
 
 	private String toString(Object value) {
+		if (value instanceof String) {
+			return '"' + value.toString() + '"';
+		}
+		if (value instanceof Enum) {
+			return ((Enum<?>) value).name();
+		}
 		if (value instanceof Class) {
-			return ((Class<?>) value).getName();
+			return ((Class<?>) value).getName() + ".class";
 		}
 		if (value.getClass().isArray()) {
-			StringBuilder builder = new StringBuilder("[");
+			StringBuilder builder = new StringBuilder("{");
 			for (int i = 0; i < Array.getLength(value); i++) {
 				if (i > 0) {
 					builder.append(", ");
 				}
 				builder.append(toString(Array.get(value, i)));
 			}
-			builder.append(']');
+			builder.append('}');
 			return builder.toString();
 		}
 		return String.valueOf(value);

spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -38,6 +38,7 @@
 import javax.annotation.Resource;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.JRE;
 
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.MergedAnnotation.Adapt;
@@ -1861,20 +1862,41 @@ void toStringForSynthesizedAnnotations() throws Exception {
 		Method methodWithPath = WebController.class.getMethod("handleMappedWithPathAttribute");
 		RequestMapping webMappingWithAliases = methodWithPath.getAnnotation(RequestMapping.class);
 		assertThat(webMappingWithAliases).isNotNull();
+
 		Method methodWithPathAndValue = WebController.class.getMethod("handleMappedWithSamePathAndValueAttributes");
 		RequestMapping webMappingWithPathAndValue = methodWithPathAndValue.getAnnotation(RequestMapping.class);
 		assertThat(methodWithPathAndValue).isNotNull();
+
 		RequestMapping synthesizedWebMapping1 = MergedAnnotation.from(webMappingWithAliases).synthesize();
 		RequestMapping synthesizedWebMapping2 = MergedAnnotation.from(webMappingWithPathAndValue).synthesize();
+
 		assertThat(webMappingWithAliases.toString()).isNotEqualTo(synthesizedWebMapping1.toString());
+
+		if (JRE.currentVersion().ordinal() > JRE.JAVA_8.ordinal()) {
+			// The unsynthesized annotation for handleMappedWithSamePathAndValueAttributes()
+			// should produce the same toString() results as synthesized annotations for
+			// handleMappedWithPathAttribute() on Java 9 or higher
+			assertToStringForWebMappingWithPathAndValue(webMappingWithPathAndValue);
+		}
 		assertToStringForWebMappingWithPathAndValue(synthesizedWebMapping1);
 		assertToStringForWebMappingWithPathAndValue(synthesizedWebMapping2);
 	}
 
 	private void assertToStringForWebMappingWithPathAndValue(RequestMapping webMapping) {
-		String prefix = "@" + RequestMapping.class.getName() + "(";
-		assertThat(webMapping.toString()).startsWith(prefix).contains("value=[/test]",
-				"path=[/test]", "name=bar", "method=", "[GET, POST]").endsWith(")");
+		String string = webMapping.toString();
+
+		// Formatting common to Spring and JDK 9+
+		assertThat(string)
+			.startsWith("@" + RequestMapping.class.getName() + "(")
+			.contains("value={\"/test\"}", "path={\"/test\"}", "name=\"bar\"", "clazz=java.lang.Object.class")
+			.endsWith(")");
+
+		if (webMapping instanceof SynthesizedAnnotation) {
+			assertThat(string).as("Spring uses Enum#name()").contains("method={GET, POST}");
+		}
+		else {
+			assertThat(string).as("JDK uses Enum#toString()").contains("method={method: get, method: post}");
+		}
 	}
 
 	@Test
@@ -2941,7 +2963,17 @@ static class SubSubMyRepeatableWithAdditionalLocalDeclarationsClass
 	}
 
 	enum RequestMethod {
-		GET, POST
+		GET,
+
+		POST;
+
+		/**
+		 * custom override to verify annotation toString() implementations.
+		 */
+		@Override
+		public String toString() {
+			return "method: " + name().toLowerCase();
+		}
 	}
 
 	@Retention(RetentionPolicy.RUNTIME)
@@ -2956,6 +2988,9 @@ enum RequestMethod {
 		String[] path() default "";
 
 		RequestMethod[] method() default {};
+
+		// clazz is only used for testing annotation toString() implementations
+		Class<?> clazz() default Object.class;
 	}
 
 	@Retention(RetentionPolicy.RUNTIME)