From 97582fd0a1f786c2cde12ba2af8196e423d47d87 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 9 Feb 2022 15:56:17 +0100 Subject: [PATCH 1/3] Update Eclipse template to @since 5.3.16 --- src/eclipse/org.eclipse.jdt.ui.prefs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eclipse/org.eclipse.jdt.ui.prefs b/src/eclipse/org.eclipse.jdt.ui.prefs index 1685154bbab..ad75f7c7f2d 100644 --- a/src/eclipse/org.eclipse.jdt.ui.prefs +++ b/src/eclipse/org.eclipse.jdt.ui.prefs @@ -63,4 +63,4 @@ org.eclipse.jdt.ui.keywordthis=false org.eclipse.jdt.ui.ondemandthreshold=9999 org.eclipse.jdt.ui.overrideannotation=true org.eclipse.jdt.ui.staticondemandthreshold=9999 -org.eclipse.jdt.ui.text.custom_code_templates= +org.eclipse.jdt.ui.text.custom_code_templates= From ce87285be5d4be7ab9d981273acac8a8624883dc Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 10 Feb 2022 14:16:34 +0100 Subject: [PATCH 2/3] Use canonical names for types in synthesized annotation toString My proposal for the same change in the JDK is currently targeted for JDK 19. - https://bugs.openjdk.java.net/browse/JDK-8281462 - https://bugs.openjdk.java.net/browse/JDK-8281568 - https://github.com/openjdk/jdk/pull/7418 See gh-28015 --- ...izedMergedAnnotationInvocationHandler.java | 9 +++++++-- .../annotation/MergedAnnotationsTests.java | 20 +++++++++++++------ .../test/context/BootstrapUtilsTests.java | 6 +++--- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java index 6838dc37797..8cbfcc6f5a4 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java @@ -177,7 +177,7 @@ final class SynthesizedMergedAnnotationInvocationHandler i private String annotationToString() { String string = this.string; if (string == null) { - StringBuilder builder = new StringBuilder("@").append(this.type.getName()).append('('); + StringBuilder builder = new StringBuilder("@").append(getName(this.type)).append('('); for (int i = 0; i < this.attributes.size(); i++) { Method attribute = this.attributes.get(i); if (i > 0) { @@ -202,7 +202,7 @@ final class SynthesizedMergedAnnotationInvocationHandler i return ((Enum) value).name(); } if (value instanceof Class) { - return ((Class) value).getName() + ".class"; + return getName((Class) value) + ".class"; } if (value.getClass().isArray()) { StringBuilder builder = new StringBuilder("{"); @@ -277,6 +277,11 @@ final class SynthesizedMergedAnnotationInvocationHandler i return (A) Proxy.newProxyInstance(classLoader, interfaces, handler); } + private static String getName(Class clazz) { + String canonicalName = clazz.getCanonicalName(); + return (canonicalName != null ? canonicalName : clazz.getName()); + } + private static boolean isVisible(ClassLoader classLoader, Class interfaceClass) { if (classLoader == interfaceClass.getClassLoader()) { diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index 0b92547863e..a6ff5a0f07a 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -1887,15 +1887,22 @@ class MergedAnnotationsTests { // 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") + .contains("value={\"/test\"}", "path={\"/test\"}", "name=\"bar\"") .endsWith(")"); if (webMapping instanceof SynthesizedAnnotation) { - assertThat(string).as("Spring uses Enum#name()").contains("method={GET, POST}"); + assertThat(string).as("Spring formatting") + .startsWith("@org.springframework.core.annotation.MergedAnnotationsTests.RequestMapping(") + .contains("method={GET, POST}", + "clazz=org.springframework.core.annotation.MergedAnnotationsTests.RequestMethod.class", + "classes={org.springframework.core.annotation.MergedAnnotationsTests.RequestMethod.class}"); } else { - assertThat(string).as("JDK uses Enum#toString()").contains("method={method: get, method: post}"); + assertThat(string).as("JDK 9-18 formatting") + .startsWith("@org.springframework.core.annotation.MergedAnnotationsTests$RequestMapping(") + .contains("method={method: get, method: post}", + "clazz=org.springframework.core.annotation.MergedAnnotationsTests$RequestMethod.class", + "classes={org.springframework.core.annotation.MergedAnnotationsTests$RequestMethod.class}"); } } @@ -2989,8 +2996,9 @@ class MergedAnnotationsTests { RequestMethod[] method() default {}; - // clazz is only used for testing annotation toString() implementations - Class clazz() default Object.class; + Class clazz() default RequestMethod.class; + + Class[] classes() default {RequestMethod.class}; } @Retention(RetentionPolicy.RUNTIME) diff --git a/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java index 7ed75786f8f..5c409d88044 100644 --- a/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.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. @@ -72,8 +72,8 @@ class BootstrapUtilsTests { assertThatIllegalStateException().isThrownBy(() -> resolveTestContextBootstrapper(bootstrapContext)) .withMessageContaining("Configuration error: found multiple declarations of @BootstrapWith") - .withMessageContaining(FooBootstrapper.class.getName()) - .withMessageContaining(BarBootstrapper.class.getName()); + .withMessageContaining(FooBootstrapper.class.getCanonicalName()) + .withMessageContaining(BarBootstrapper.class.getCanonicalName()); } @Test From 2fd39839f83e8923277cb89bcff0b5dc5a18abfc Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 10 Feb 2022 16:58:36 +0100 Subject: [PATCH 3/3] Improve toString() for synthesized annotations Although the initial report in gh-28015 only covered inconsistencies for arrays and strings in the toString() implementations for annotations between the JDK (after Java 9) and Spring, it has since come to our attention that there was further room for improvement. This commit therefore addresses the following in toString() output for synthesized annotations. - characters are now wrapped in single quotes. - bytes are now properly formatted as "(byte) 0x##". - long, float, and double values are now appended with "L", "f", and "d", respectively. The use of lowercase for "f" and "d" is solely to align with the choice made by the JDK team. However, this commit does not address the following issues which we may choose to address at a later point in time. - non-ASCII, non-visible, and non-printable characters within a character or String literal are not escaped. - formatting for float and double values does not take into account whether a value is not a number (NaN) or infinite. Closes gh-28015 --- ...izedMergedAnnotationInvocationHandler.java | 28 +++++++++++ .../annotation/MergedAnnotationsTests.java | 50 +++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java index 8cbfcc6f5a4..5d3f9a40949 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java @@ -194,10 +194,38 @@ final class SynthesizedMergedAnnotationInvocationHandler i return string; } + /** + * This method currently does not address the following issues which we may + * choose to address at a later point in time. + * + *
    + *
  • non-ASCII, non-visible, and non-printable characters within a character + * or String literal are not escaped.
    • + *
  • formatting for float and double values does not take into account whether + * a value is not a number (NaN) or infinite.
    • + *
+ * @param value the attribute value to format + * @return the formatted string representation + */ private String toString(Object value) { if (value instanceof String) { return '"' + value.toString() + '"'; } + if (value instanceof Character) { + return '\'' + value.toString() + '\''; + } + if (value instanceof Byte) { + return String.format("(byte) 0x%02X", value); + } + if (value instanceof Long) { + return Long.toString(((Long) value)) + 'L'; + } + if (value instanceof Float) { + return Float.toString(((Float) value)) + 'f'; + } + if (value instanceof Double) { + return Double.toString(((Double) value)) + 'd'; + } if (value instanceof Enum) { return ((Enum) value).name(); } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index a6ff5a0f07a..acd8fc9abaf 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -1887,7 +1887,7 @@ class MergedAnnotationsTests { // Formatting common to Spring and JDK 9+ assertThat(string) - .contains("value={\"/test\"}", "path={\"/test\"}", "name=\"bar\"") + .contains("value={\"/test\"}", "path={\"/test\"}", "name=\"bar\"", "ch='X'", "chars={'X'}") .endsWith(")"); if (webMapping instanceof SynthesizedAnnotation) { @@ -1895,14 +1895,36 @@ class MergedAnnotationsTests { .startsWith("@org.springframework.core.annotation.MergedAnnotationsTests.RequestMapping(") .contains("method={GET, POST}", "clazz=org.springframework.core.annotation.MergedAnnotationsTests.RequestMethod.class", - "classes={org.springframework.core.annotation.MergedAnnotationsTests.RequestMethod.class}"); + "classes={int[][].class, org.springframework.core.annotation.MergedAnnotationsTests.RequestMethod[].class}", + "byteValue=(byte) 0xFF", "bytes={(byte) 0xFF}", + "shortValue=9876", "shorts={9876}", + "longValue=42L", "longs={42L}", + "floatValue=3.14f", "floats={3.14f}", + "doubleValue=99.999d", "doubles={99.999d}" + ); } else { assertThat(string).as("JDK 9-18 formatting") .startsWith("@org.springframework.core.annotation.MergedAnnotationsTests$RequestMapping(") .contains("method={method: get, method: post}", "clazz=org.springframework.core.annotation.MergedAnnotationsTests$RequestMethod.class", - "classes={org.springframework.core.annotation.MergedAnnotationsTests$RequestMethod.class}"); + "classes={int[][].class, org.springframework.core.annotation.MergedAnnotationsTests$RequestMethod[].class}", + "shortValue=9876", "shorts={9876}", + "floatValue=3.14f", "floats={3.14f}", + "doubleValue=99.999", "doubles={99.999}" + ); + if (JRE.currentVersion().ordinal() < JRE.JAVA_14.ordinal()) { + assertThat(string).as("JDK 9-13 formatting") + .contains("longValue=42", "longs={42}", + "byteValue=-1", "bytes={-1}" + ); + } + else { + assertThat(string).as("JDK 14+ formatting") + .contains("longValue=42L", "longs={42L}", + "byteValue=(byte)0xff", "bytes={(byte)0xff}" + ); + } } } @@ -2996,9 +3018,29 @@ class MergedAnnotationsTests { RequestMethod[] method() default {}; + // --------------------------------------------------------------------- + // All remaining attributes declare default values that are used solely + // for the purpose of testing the toString() implementations for annotations. Class clazz() default RequestMethod.class; + Class[] classes() default {int[][].class, RequestMethod[].class}; - Class[] classes() default {RequestMethod.class}; + char ch() default 'X'; + char[] chars() default {'X'}; + + byte byteValue() default (byte) 0xFF; + byte[] bytes() default {(byte) 0xFF}; + + short shortValue() default 9876; + short[] shorts() default {9876}; + + long longValue() default 42L; + long[] longs() default {42L}; + + float floatValue() default 3.14F; + float[] floats() default {3.14F}; + + double doubleValue() default 99.999D; + double[] doubles() default {99.999D}; } @Retention(RetentionPolicy.RUNTIME)