From f40a391916ed6c1f9e1130638a0bf19479e514dd Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 15 Apr 2022 11:47:25 +0200 Subject: [PATCH] Fix handling of reflection target name in TypeReference This commit adds a `getName` to `TypeReference` that provides a way to generate the reflection target name of a type. This typically handle primitives (omitting the `java.lang` packages) and arrays. Closes gh-28347 --- .../aot/generator/GeneratedTypeReference.java | 26 +++------ .../aot/hint/AbstractTypeReference.java | 50 +++++++++++++++++ .../aot/hint/ReflectionTypeReference.java | 27 +++------ .../aot/hint/ResourceHints.java | 14 +---- .../aot/hint/SimpleTypeReference.java | 56 +++++++++---------- .../aot/hint/TypeReference.java | 6 ++ .../aot/nativex/BasicJsonWriter.java | 18 +----- .../GeneratedTypeReferenceTests.java | 18 ++++++ .../hint/ReflectionTypeReferenceTests.java | 47 ++++++++++++++++ .../aot/hint/SimpleTypeReferenceTests.java | 46 +++++++++++++++ .../aot/hint/TypeReferenceTests.java | 3 + 11 files changed, 214 insertions(+), 97 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/aot/hint/ReflectionTypeReferenceTests.java diff --git a/spring-core/src/main/java/org/springframework/aot/generator/GeneratedTypeReference.java b/spring-core/src/main/java/org/springframework/aot/generator/GeneratedTypeReference.java index 9e8b9ed51e..54ae72a1f4 100644 --- a/spring-core/src/main/java/org/springframework/aot/generator/GeneratedTypeReference.java +++ b/spring-core/src/main/java/org/springframework/aot/generator/GeneratedTypeReference.java @@ -32,14 +32,14 @@ public final class GeneratedTypeReference extends AbstractTypeReference { private final ClassName className; - @Nullable - private final TypeReference enclosingType; - private GeneratedTypeReference(ClassName className) { + super(className.packageName(), className.simpleName(), safeCreate(className.enclosingClassName())); this.className = className; - this.enclosingType = (className.enclosingClassName() != null - ? new GeneratedTypeReference(className.enclosingClassName()) - : null); + } + + @Nullable + private static GeneratedTypeReference safeCreate(@Nullable ClassName className) { + return (className != null ? new GeneratedTypeReference(className) : null); } public static GeneratedTypeReference of(ClassName className) { @@ -53,18 +53,8 @@ public final class GeneratedTypeReference extends AbstractTypeReference { } @Override - public String getPackageName() { - return this.className.packageName(); - } - - @Override - public String getSimpleName() { - return this.className.simpleName(); - } - - @Override - public TypeReference getEnclosingType() { - return this.enclosingType; + protected boolean isPrimitive() { + return this.className.isPrimitive(); } } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/AbstractTypeReference.java b/spring-core/src/main/java/org/springframework/aot/hint/AbstractTypeReference.java index 830fae9e48..ddc26ebfc8 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/AbstractTypeReference.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/AbstractTypeReference.java @@ -18,6 +18,8 @@ package org.springframework.aot.hint; import java.util.Objects; +import org.springframework.lang.Nullable; + /** * Base {@link TypeReference} implementation that ensures consistent behaviour * for {@code equals()}, {@code hashCode()}, and {@code toString()} based on @@ -28,6 +30,54 @@ import java.util.Objects; */ public abstract class AbstractTypeReference implements TypeReference { + private final String packageName; + + private final String simpleName; + + @Nullable + private final TypeReference enclosingType; + + protected AbstractTypeReference(String packageName, String simpleName, @Nullable TypeReference enclosingType) { + this.packageName = packageName; + this.simpleName = simpleName; + this.enclosingType = enclosingType; + } + + @Override + public String getName() { + TypeReference enclosingType = getEnclosingType(); + String simpleName = getSimpleName(); + return (enclosingType != null + ? (enclosingType.getName() + '$' + simpleName) + : addPackageIfNecessary(simpleName)); + } + + @Override + public String getPackageName() { + return this.packageName; + } + + @Override + public String getSimpleName() { + return this.simpleName; + } + + @Nullable + @Override + public TypeReference getEnclosingType() { + return this.enclosingType; + } + + protected abstract boolean isPrimitive(); + + protected String addPackageIfNecessary(String part) { + if (this.packageName.isEmpty() || + this.packageName.equals("java.lang") && isPrimitive()) { + return part; + } + return this.packageName + '.' + part; + } + @Override public int hashCode() { return Objects.hash(getCanonicalName()); diff --git a/spring-core/src/main/java/org/springframework/aot/hint/ReflectionTypeReference.java b/spring-core/src/main/java/org/springframework/aot/hint/ReflectionTypeReference.java index ff215b0d97..14a859b991 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/ReflectionTypeReference.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/ReflectionTypeReference.java @@ -27,14 +27,14 @@ final class ReflectionTypeReference extends AbstractTypeReference { private final Class type; - @Nullable - private final TypeReference enclosing; - - private ReflectionTypeReference(Class type) { + super(type.getPackageName(), type.getSimpleName(), safeCreate(type.getEnclosingClass())); this.type = type; - this.enclosing = (type.getEnclosingClass() != null - ? TypeReference.of(type.getEnclosingClass()) : null); + } + + @Nullable + private static ReflectionTypeReference safeCreate(@Nullable Class type) { + return (type != null ? new ReflectionTypeReference(type) : null); } static ReflectionTypeReference of(Class type) { @@ -47,18 +47,9 @@ final class ReflectionTypeReference extends AbstractTypeReference { } @Override - public String getPackageName() { - return this.type.getPackageName(); - } - - @Override - public String getSimpleName() { - return this.type.getSimpleName(); - } - - @Override - public TypeReference getEnclosingType() { - return this.enclosing; + protected boolean isPrimitive() { + return this.type.isPrimitive() || + (this.type.isArray() && this.type.getComponentType().isPrimitive()); } } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/ResourceHints.java b/spring-core/src/main/java/org/springframework/aot/hint/ResourceHints.java index 48208fa278..76cf9355bd 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/ResourceHints.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/ResourceHints.java @@ -131,19 +131,7 @@ public class ResourceHints { } private String toIncludePattern(TypeReference type) { - StringBuilder names = new StringBuilder(); - buildName(type, names); - String candidate = type.getPackageName() + "." + names; - return candidate.replace(".", "/") + ".class"; - } - - private void buildName(@Nullable TypeReference type, StringBuilder sb) { - if (type == null) { - return; - } - String typeName = (type.getEnclosingType() != null) ? "$" + type.getSimpleName() : type.getSimpleName(); - sb.insert(0, typeName); - buildName(type.getEnclosingType(), sb); + return type.getName().replace(".", "/") + ".class"; } } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/SimpleTypeReference.java b/spring-core/src/main/java/org/springframework/aot/hint/SimpleTypeReference.java index 7105738d91..2e95d1627c 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/SimpleTypeReference.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/SimpleTypeReference.java @@ -16,6 +16,8 @@ package org.springframework.aot.hint; +import java.util.List; + import javax.lang.model.SourceVersion; import org.springframework.lang.Nullable; @@ -28,21 +30,14 @@ import org.springframework.util.Assert; */ final class SimpleTypeReference extends AbstractTypeReference { + private static final List PRIMITIVE_NAMES = List.of("boolean", "byte", + "short", "int", "long", "char", "float", "double", "void"); + @Nullable private String canonicalName; - private final String packageName; - - private final String simpleName; - - @Nullable - private final TypeReference enclosingType; - - SimpleTypeReference(String packageName, String simpleName, @Nullable TypeReference enclosingType) { - this.packageName = packageName; - this.simpleName = simpleName; - this.enclosingType = enclosingType; + super(packageName, simpleName, enclosingType); } static SimpleTypeReference of(String className) { @@ -63,7 +58,8 @@ final class SimpleTypeReference extends AbstractTypeReference { private static boolean isValidClassName(String className) { for (String s : className.split("\\.", -1)) { - if (!SourceVersion.isIdentifier(s)) { + String candidate = s.replace("[", "").replace("]", ""); + if (!SourceVersion.isIdentifier(candidate)) { return false; } } @@ -72,8 +68,13 @@ final class SimpleTypeReference extends AbstractTypeReference { private static SimpleTypeReference createTypeReference(String className) { int i = className.lastIndexOf('.'); - return (i != -1 ? new SimpleTypeReference(className.substring(0, i), className.substring(i + 1), null) - : new SimpleTypeReference("", className, null)); + if (i != -1) { + return new SimpleTypeReference(className.substring(0, i), className.substring(i + 1), null); + } + else { + String packageName = isPrimitive(className) ? "java.lang" : ""; + return new SimpleTypeReference(packageName, className, null); + } } @Override @@ -81,12 +82,20 @@ final class SimpleTypeReference extends AbstractTypeReference { if (this.canonicalName == null) { StringBuilder names = new StringBuilder(); buildName(this, names); - this.canonicalName = (this.packageName.isEmpty() - ? names.toString() : this.packageName + "." + names); + this.canonicalName = addPackageIfNecessary(names.toString()); } return this.canonicalName; } + @Override + protected boolean isPrimitive() { + return isPrimitive(getSimpleName()); + } + + private static boolean isPrimitive(String name) { + return PRIMITIVE_NAMES.stream().anyMatch(name::startsWith); + } + private static void buildName(@Nullable TypeReference type, StringBuilder sb) { if (type == null) { return; @@ -96,19 +105,4 @@ final class SimpleTypeReference extends AbstractTypeReference { buildName(type.getEnclosingType(), sb); } - @Override - public String getPackageName() { - return this.packageName; - } - - @Override - public String getSimpleName() { - return this.simpleName; - } - - @Override - public TypeReference getEnclosingType() { - return this.enclosingType; - } - } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/TypeReference.java b/spring-core/src/main/java/org/springframework/aot/hint/TypeReference.java index 39bb07d2f2..2fcf62fb8c 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/TypeReference.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/TypeReference.java @@ -27,6 +27,12 @@ import org.springframework.lang.Nullable; */ public interface TypeReference { + /** + * Return the fully qualified name of this type reference. + * @return the reflection target name + */ + String getName(); + /** * Return the {@linkplain Class#getCanonicalName() canonical name} of this * type reference. diff --git a/spring-core/src/main/java/org/springframework/aot/nativex/BasicJsonWriter.java b/spring-core/src/main/java/org/springframework/aot/nativex/BasicJsonWriter.java index 3f2a039ce6..a8d5d42554 100644 --- a/spring-core/src/main/java/org/springframework/aot/nativex/BasicJsonWriter.java +++ b/spring-core/src/main/java/org/springframework/aot/nativex/BasicJsonWriter.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.function.Consumer; import org.springframework.aot.hint.TypeReference; -import org.springframework.lang.Nullable; /** * Very basic json writer for the purposes of translating runtime hints to native @@ -133,7 +132,7 @@ class BasicJsonWriter { writeArray(list, false); } else if (value instanceof TypeReference typeReference) { - this.writer.print(quote(toName(typeReference))); + this.writer.print(quote(typeReference.getName())); } else if (value instanceof CharSequence string) { this.writer.print(quote(escape(string))); @@ -150,21 +149,6 @@ class BasicJsonWriter { return "\"" + name + "\""; } - private String toName(TypeReference typeReference) { - StringBuilder names = new StringBuilder(); - buildName(typeReference, names); - return typeReference.getPackageName() + "." + names; - } - - private void buildName(@Nullable TypeReference type, StringBuilder sb) { - if (type == null) { - return; - } - String typeName = (type.getEnclosingType() != null) ? "$" + type.getSimpleName() : type.getSimpleName(); - sb.insert(0, typeName); - buildName(type.getEnclosingType(), sb); - } - private static String escape(CharSequence input) { StringBuilder builder = new StringBuilder(); diff --git a/spring-core/src/test/java/org/springframework/aot/generator/GeneratedTypeReferenceTests.java b/spring-core/src/test/java/org/springframework/aot/generator/GeneratedTypeReferenceTests.java index 93b0e3247c..23d7646b84 100644 --- a/spring-core/src/test/java/org/springframework/aot/generator/GeneratedTypeReferenceTests.java +++ b/spring-core/src/test/java/org/springframework/aot/generator/GeneratedTypeReferenceTests.java @@ -16,7 +16,12 @@ package org.springframework.aot.generator; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.aot.hint.TypeReference; import org.springframework.javapoet.ClassName; @@ -30,6 +35,19 @@ import static org.assertj.core.api.Assertions.assertThat; */ class GeneratedTypeReferenceTests { + + @ParameterizedTest + @MethodSource("reflectionTargetNames") + void hasSuitableReflectionTargetName(TypeReference typeReference, String binaryName) { + assertThat(typeReference.getName()).isEqualTo(binaryName); + } + + static Stream reflectionTargetNames() { + return Stream.of( + Arguments.of(GeneratedTypeReference.of(ClassName.get("com.example", "Test")), "com.example.Test"), + Arguments.of(GeneratedTypeReference.of(ClassName.get("com.example", "Test", "Inner")), "com.example.Test$Inner")); + } + @Test void createWithClassName() { GeneratedTypeReference typeReference = GeneratedTypeReference.of( diff --git a/spring-core/src/test/java/org/springframework/aot/hint/ReflectionTypeReferenceTests.java b/spring-core/src/test/java/org/springframework/aot/hint/ReflectionTypeReferenceTests.java new file mode 100644 index 0000000000..f351f6af23 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/aot/hint/ReflectionTypeReferenceTests.java @@ -0,0 +1,47 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.aot.hint; + +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ReflectionTypeReference}. + * + * @author Stephane Nicoll + */ +class ReflectionTypeReferenceTests { + + @ParameterizedTest + @MethodSource("reflectionTargetNames") + void typeReferenceFromClasHasSuitableReflectionTargetName(TypeReference typeReference, String binaryName) { + assertThat(typeReference.getName()).isEqualTo(binaryName); + } + + static Stream reflectionTargetNames() { + return Stream.of(Arguments.of(ReflectionTypeReference.of(int.class), "int"), + Arguments.of(ReflectionTypeReference.of(int[].class), "int[]"), + Arguments.of(ReflectionTypeReference.of(Integer[].class), "java.lang.Integer[]"), + Arguments.of(ReflectionTypeReference.of(Object[].class), "java.lang.Object[]")); + } + +} diff --git a/spring-core/src/test/java/org/springframework/aot/hint/SimpleTypeReferenceTests.java b/spring-core/src/test/java/org/springframework/aot/hint/SimpleTypeReferenceTests.java index 6ce3fb68a3..cb43859d45 100644 --- a/spring-core/src/test/java/org/springframework/aot/hint/SimpleTypeReferenceTests.java +++ b/spring-core/src/test/java/org/springframework/aot/hint/SimpleTypeReferenceTests.java @@ -16,8 +16,12 @@ package org.springframework.aot.hint; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import static org.assertj.core.api.Assertions.assertThat; @@ -30,6 +34,48 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; */ class SimpleTypeReferenceTests { + + @ParameterizedTest + @MethodSource("primitivesAndPrimitivesArray") + void primitivesAreHandledProperly(TypeReference typeReference, String expectedName) { + assertThat(typeReference.getName()).isEqualTo(expectedName); + assertThat(typeReference.getCanonicalName()).isEqualTo(expectedName); + assertThat(typeReference.getPackageName()).isEqualTo("java.lang"); + } + + static Stream primitivesAndPrimitivesArray() { + return Stream.of( + Arguments.of(SimpleTypeReference.of("boolean"), "boolean"), + Arguments.of(SimpleTypeReference.of("byte"), "byte"), + Arguments.of(SimpleTypeReference.of("short"), "short"), + Arguments.of(SimpleTypeReference.of("int"), "int"), + Arguments.of(SimpleTypeReference.of("long"), "long"), + Arguments.of(SimpleTypeReference.of("char"), "char"), + Arguments.of(SimpleTypeReference.of("float"), "float"), + Arguments.of(SimpleTypeReference.of("double"), "double"), + Arguments.of(SimpleTypeReference.of("boolean[]"), "boolean[]"), + Arguments.of(SimpleTypeReference.of("byte[]"), "byte[]"), + Arguments.of(SimpleTypeReference.of("short[]"), "short[]"), + Arguments.of(SimpleTypeReference.of("int[]"), "int[]"), + Arguments.of(SimpleTypeReference.of("long[]"), "long[]"), + Arguments.of(SimpleTypeReference.of("char[]"), "char[]"), + Arguments.of(SimpleTypeReference.of("float[]"), "float[]"), + Arguments.of(SimpleTypeReference.of("double[]"), "double[]")); + } + + @ParameterizedTest + @MethodSource("arrays") + void arraysHaveSuitableReflectionTargetName(TypeReference typeReference, String expectedName) { + assertThat(typeReference.getName()).isEqualTo(expectedName); + } + + static Stream arrays() { + return Stream.of( + Arguments.of(SimpleTypeReference.of("java.lang.Object[]"), "java.lang.Object[]"), + Arguments.of(SimpleTypeReference.of("java.lang.Integer[]"), "java.lang.Integer[]"), + Arguments.of(SimpleTypeReference.of("com.example.Test[]"), "com.example.Test[]")); + } + @Test void typeReferenceInRootPackage() { TypeReference type = SimpleTypeReference.of("MyRootClass"); diff --git a/spring-core/src/test/java/org/springframework/aot/hint/TypeReferenceTests.java b/spring-core/src/test/java/org/springframework/aot/hint/TypeReferenceTests.java index 00bb853fcc..e1fd96c43b 100644 --- a/spring-core/src/test/java/org/springframework/aot/hint/TypeReferenceTests.java +++ b/spring-core/src/test/java/org/springframework/aot/hint/TypeReferenceTests.java @@ -30,6 +30,7 @@ class TypeReferenceTests { @Test void typeReferenceWithClassName() { TypeReference type = TypeReference.of("java.lang.String"); + assertThat(type.getName()).isEqualTo("java.lang.String"); assertThat(type.getCanonicalName()).isEqualTo("java.lang.String"); assertThat(type.getPackageName()).isEqualTo("java.lang"); assertThat(type.getSimpleName()).isEqualTo("String"); @@ -39,6 +40,7 @@ class TypeReferenceTests { @Test void typeReferenceWithInnerClassName() { TypeReference type = TypeReference.of("com.example.Example$Inner"); + assertThat(type.getName()).isEqualTo("com.example.Example$Inner"); assertThat(type.getCanonicalName()).isEqualTo("com.example.Example.Inner"); assertThat(type.getPackageName()).isEqualTo("com.example"); assertThat(type.getSimpleName()).isEqualTo("Inner"); @@ -53,6 +55,7 @@ class TypeReferenceTests { @Test void typeReferenceWithNestedInnerClassName() { TypeReference type = TypeReference.of("com.example.Example$Inner$Nested"); + assertThat(type.getName()).isEqualTo("com.example.Example$Inner$Nested"); assertThat(type.getCanonicalName()).isEqualTo("com.example.Example.Inner.Nested"); assertThat(type.getPackageName()).isEqualTo("com.example"); assertThat(type.getSimpleName()).isEqualTo("Nested");