From e24fd5061539ad34ca043f775bab6cb7b1fb6fab Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Sat, 5 Apr 2025 21:16:44 +0300 Subject: [PATCH 1/2] Handle generics with identical names in different positions Update `TypeUtils` to handle generics with identical names in different positions. See gh-45011 Signed-off-by: Dmytro Nosan --- .../configurationprocessor/TypeUtils.java | 43 ++++----- .../TypeUtilsTests.java | 91 ++++++++++++++----- .../generic/MixGenericNameProperties.java | 29 ++++++ 3 files changed, 114 insertions(+), 49 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/generic/MixGenericNameProperties.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java index d5096cb33dd..e150a3134b6 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the original author or authors. + * Copyright 2012-2025 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. @@ -23,7 +23,6 @@ import java.util.EnumMap; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -135,7 +134,7 @@ class TypeUtils { if (type == null) { return null; } - return type.accept(this.typeExtractor, createTypeDescriptor(element)); + return type.accept(this.typeExtractor, resolveTypeDescriptor(element)); } /** @@ -394,37 +393,29 @@ class TypeUtils { private final Map generics = new HashMap<>(); - Map getGenerics() { - return Collections.unmodifiableMap(this.generics); - } - TypeMirror resolveGeneric(TypeVariable typeVariable) { - return resolveGeneric(getParameterName(typeVariable)); - } - - TypeMirror resolveGeneric(String parameterName) { - return this.generics.entrySet() - .stream() - .filter((e) -> getParameterName(e.getKey()).equals(parameterName)) - .findFirst() - .map(Entry::getValue) - .orElse(null); + if (this.generics.containsKey(typeVariable)) { + TypeMirror resolvedType = this.generics.get(typeVariable); + // Unresolved -> + if (resolvedType == typeVariable) { + return resolvedType; + } + // -> -> + if (resolvedType instanceof TypeVariable) { + return resolveGeneric((TypeVariable) resolvedType); + } + // Resolved e.g. java.lang.String + return resolvedType; + } + return null; } private void registerIfNecessary(TypeMirror variable, TypeMirror resolution) { if (variable instanceof TypeVariable typeVariable) { - if (this.generics.keySet() - .stream() - .noneMatch((candidate) -> getParameterName(candidate).equals(getParameterName(typeVariable)))) { - this.generics.put(typeVariable, resolution); - } + this.generics.put(typeVariable, resolution); } } - private String getParameterName(TypeVariable typeVariable) { - return typeVariable.asElement().getSimpleName().toString(); - } - } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java index 92d2ae00ecb..c1f06d7cccd 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2025 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. @@ -17,16 +17,23 @@ package org.springframework.boot.configurationprocessor; import java.time.Duration; +import java.util.Map; import java.util.function.BiConsumer; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.ElementFilter; + import org.junit.jupiter.api.Test; -import org.springframework.boot.configurationprocessor.TypeUtils.TypeDescriptor; import org.springframework.boot.configurationprocessor.test.RoundEnvironmentTester; import org.springframework.boot.configurationprocessor.test.TestableAnnotationProcessor; import org.springframework.boot.configurationsample.generic.AbstractGenericProperties; import org.springframework.boot.configurationsample.generic.AbstractIntermediateGenericProperties; +import org.springframework.boot.configurationsample.generic.MixGenericNameProperties; import org.springframework.boot.configurationsample.generic.SimpleGenericProperties; +import org.springframework.boot.configurationsample.generic.UnresolvedGenericProperties; import org.springframework.core.test.tools.SourceFile; import org.springframework.core.test.tools.TestCompiler; @@ -41,40 +48,53 @@ import static org.assertj.core.api.Assertions.assertThat; class TypeUtilsTests { @Test - void resolveTypeDescriptorOnConcreteClass() { + void resolveTypeOnConcreteClass() { process(SimpleGenericProperties.class, (roundEnv, typeUtils) -> { - TypeDescriptor typeDescriptor = typeUtils - .resolveTypeDescriptor(roundEnv.getRootElement(SimpleGenericProperties.class)); - assertThat(typeDescriptor.getGenerics().keySet().stream().map(Object::toString)).containsOnly("A", "B", - "C"); - assertThat(typeDescriptor.resolveGeneric("A")).hasToString(String.class.getName()); - assertThat(typeDescriptor.resolveGeneric("B")).hasToString(Integer.class.getName()); - assertThat(typeDescriptor.resolveGeneric("C")).hasToString(Duration.class.getName()); + TypeElement typeElement = roundEnv.getRootElement(SimpleGenericProperties.class); + assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(String.class.getName()); + assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) + .hasToString(constructMapType(Integer.class, Duration.class)); }); } @Test - void resolveTypeDescriptorOnIntermediateClass() { + void resolveTypeOnIntermediateClass() { process(AbstractIntermediateGenericProperties.class, (roundEnv, typeUtils) -> { - TypeDescriptor typeDescriptor = typeUtils - .resolveTypeDescriptor(roundEnv.getRootElement(AbstractIntermediateGenericProperties.class)); - assertThat(typeDescriptor.getGenerics().keySet().stream().map(Object::toString)).containsOnly("A", "B", - "C"); - assertThat(typeDescriptor.resolveGeneric("A")).hasToString(String.class.getName()); - assertThat(typeDescriptor.resolveGeneric("B")).hasToString(Integer.class.getName()); - assertThat(typeDescriptor.resolveGeneric("C")).hasToString("C"); + TypeElement typeElement = roundEnv.getRootElement(AbstractIntermediateGenericProperties.class); + assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(String.class.getName()); + assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) + .hasToString(constructMapType(Integer.class, Object.class)); }); } @Test - void resolveTypeDescriptorWithOnlyGenerics() { + void resolveTypeWithOnlyGenerics() { process(AbstractGenericProperties.class, (roundEnv, typeUtils) -> { - TypeDescriptor typeDescriptor = typeUtils - .resolveTypeDescriptor(roundEnv.getRootElement(AbstractGenericProperties.class)); - assertThat(typeDescriptor.getGenerics().keySet().stream().map(Object::toString)).containsOnly("A", "B", - "C"); + TypeElement typeElement = roundEnv.getRootElement(AbstractGenericProperties.class); + assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(Object.class.getName()); + assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) + .hasToString(constructMapType(Object.class, Object.class)); + }); + } + + @Test + void resolveTypeWithUnresolvedGenericProperties() { + process(UnresolvedGenericProperties.class, (roundEnv, typeUtils) -> { + TypeElement typeElement = roundEnv.getRootElement(UnresolvedGenericProperties.class); + assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(String.class.getName()); + assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) + .hasToString(constructMapType(Number.class, Object.class)); + }); + } + @Test + void resolvedTypeMixGenericNamePropertiesProperties() { + process(MixGenericNameProperties.class, (roundEnv, typeUtils) -> { + TypeElement typeElement = roundEnv.getRootElement(MixGenericNameProperties.class); + assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(String.class.getName()); + assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) + .hasToString(constructMapType(Number.class, Object.class)); }); } @@ -87,4 +107,29 @@ class TypeUtilsTests { }); } + private String constructMapType(Class keyType, Class valueType) { + return "%s<%s,%s>".formatted(Map.class.getName(), keyType.getName(), valueType.getName()); + } + + private String getTypeOfField(TypeUtils typeUtils, TypeElement typeElement, String name) { + TypeMirror field = findField(typeUtils, typeElement, name); + if (field == null) { + throw new IllegalStateException("Unable to find field '" + name + "' in " + typeElement); + } + return typeUtils.getType(typeElement, field); + } + + private TypeMirror findField(TypeUtils typeUtils, TypeElement typeElement, String name) { + for (VariableElement variableElement : ElementFilter.fieldsIn(typeElement.getEnclosedElements())) { + if (variableElement.getSimpleName().contentEquals(name)) { + return variableElement.asType(); + } + } + TypeMirror superclass = typeElement.getSuperclass(); + if (superclass != null && !superclass.toString().equals(Object.class.getName())) { + return findField(typeUtils, (TypeElement) typeUtils.asElement(superclass), name); + } + return null; + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/generic/MixGenericNameProperties.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/generic/MixGenericNameProperties.java new file mode 100644 index 00000000000..a9ef550fdbf --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/generic/MixGenericNameProperties.java @@ -0,0 +1,29 @@ +/* + * Copyright 2012-2025 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.boot.configurationsample.generic; + +/** + * Properties with unresolved generic types that use identical generic parameter names but + * differ in their positions. + * + * @param mapping name type + * @param mapping value type + * @author Dmytro Nosan + */ +public class MixGenericNameProperties extends AbstractGenericProperties { + +} From 46b14de6e9510a59bd4823a8de0943ec78b77974 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 8 Apr 2025 13:31:42 -0700 Subject: [PATCH 2/2] Polish 'Handle generics with identical names in different positions' See gh-45011 --- .../configurationprocessor/TypeUtils.java | 36 ++++++++----------- .../TypeUtilsTests.java | 1 - 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java index e150a3134b6..f410788de09 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java @@ -46,6 +46,7 @@ import javax.lang.model.util.Types; * @author Stephane Nicoll * @author Phillip Webb * @author Pavel Anisimov + * @author Dmytro Nosan */ class TypeUtils { @@ -217,7 +218,7 @@ class TypeUtils { return WRAPPER_TO_PRIMITIVE.get(type.toString()); } - TypeDescriptor resolveTypeDescriptor(TypeElement element) { + private TypeDescriptor resolveTypeDescriptor(TypeElement element) { if (this.typeDescriptors.containsKey(element)) { return this.typeDescriptors.get(element); } @@ -318,22 +319,22 @@ class TypeUtils { } @Override - public String visitTypeVariable(TypeVariable t, TypeDescriptor descriptor) { - TypeMirror typeMirror = descriptor.resolveGeneric(t); - if (typeMirror != null) { - if (typeMirror instanceof TypeVariable typeVariable) { + public String visitTypeVariable(TypeVariable typeVariable, TypeDescriptor descriptor) { + TypeMirror resolvedGeneric = descriptor.resolveGeneric(typeVariable); + if (resolvedGeneric != null) { + if (resolvedGeneric instanceof TypeVariable resolveTypeVariable) { // Still unresolved, let's use the upper bound, checking first if // a cycle may exist - if (!hasCycle(typeVariable)) { - return visit(typeVariable.getUpperBound(), descriptor); + if (!hasCycle(resolveTypeVariable)) { + return visit(resolveTypeVariable.getUpperBound(), descriptor); } } else { - return visit(typeMirror, descriptor); + return visit(resolvedGeneric, descriptor); } } // Fallback to simple representation of the upper bound - return defaultAction(t.getUpperBound(), descriptor); + return defaultAction(typeVariable.getUpperBound(), descriptor); } private boolean hasCycle(TypeVariable variable) { @@ -394,20 +395,11 @@ class TypeUtils { private final Map generics = new HashMap<>(); TypeMirror resolveGeneric(TypeVariable typeVariable) { - if (this.generics.containsKey(typeVariable)) { - TypeMirror resolvedType = this.generics.get(typeVariable); - // Unresolved -> - if (resolvedType == typeVariable) { - return resolvedType; - } - // -> -> - if (resolvedType instanceof TypeVariable) { - return resolveGeneric((TypeVariable) resolvedType); - } - // Resolved e.g. java.lang.String - return resolvedType; + TypeMirror resolved = this.generics.get(typeVariable); + if (resolved != typeVariable && resolved instanceof TypeVariable resolvedTypeVariable) { + return resolveGeneric(resolvedTypeVariable); } - return null; + return resolved; } private void registerIfNecessary(TypeMirror variable, TypeMirror resolution) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java index c1f06d7cccd..0593b6cbcbb 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/TypeUtilsTests.java @@ -54,7 +54,6 @@ class TypeUtilsTests { assertThat(getTypeOfField(typeUtils, typeElement, "name")).hasToString(String.class.getName()); assertThat(getTypeOfField(typeUtils, typeElement, "mappings")) .hasToString(constructMapType(Integer.class, Duration.class)); - }); }