From 8f0dacf0c58ba7113692860249f771d22572dfd0 Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Wed, 7 May 2025 20:51:32 -0400 Subject: [PATCH 1/3] support nested wildcard lists - fix cases where a list of generic wildcards generates bad code - now generic types are registered with their main type as a key --- .../avaje/inject/generator/MethodReader.java | 22 ++++++++++++++++--- .../inject/generator/TypeExtendsReader.java | 18 +++++++++++---- .../io/avaje/inject/generator/TypeReader.java | 10 ++++++--- .../java/io/avaje/inject/generator/Util.java | 18 +++++++++------ .../org/example/coffee/CoffeeMakerTest.java | 3 ++- .../coffee/generic/MultiGenericConsumer.java | 17 ++++++++++++-- .../generic/MultiGenericConsumerTest.java | 7 +++--- 7 files changed, 72 insertions(+), 23 deletions(-) diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java index 92f6a29f8..5707cfc2a 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java @@ -563,8 +563,18 @@ void addDependsOnGeneric(Set set) { } void builderGetDependency(Append writer, String builderName) { - writer.append(builderName).append(".").append(utilType.getMethod(nullable, isBeanMap)); - if (!genericType.isGeneric() || genericType.param0().kind() == TypeKind.WILDCARD) { + + boolean wildcard = + genericType.isGeneric() + && genericType.componentTypes().stream().allMatch(g -> g.kind() == TypeKind.WILDCARD); + + var wildParam = wildcard ? String.format("<%s>", genericType.shortWithoutAnnotations()) : ""; + writer + .append(builderName) + .append(".") + .append(wildParam) + .append(utilType.getMethod(nullable, isBeanMap)); + if (!genericType.isGeneric() || wildcard) { writer.append(Util.shortName(genericType.mainType())).append(".class"); } else { writer.append("TYPE_").append(Util.shortName(genericType).replace(".", "_")); @@ -602,7 +612,13 @@ boolean isGenericParam() { } Dependency dependsOn() { - return new Dependency(paramType, named, utilType.isCollection()); + + boolean wildcard = + genericType.isGeneric() + && genericType.componentTypes().stream().allMatch(g -> g.kind() == TypeKind.WILDCARD); + + return new Dependency( + wildcard ? genericType.mainType() : paramType, named, utilType.isCollection()); } void addImports(ImportTypeMap importTypes) { diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java index b7e9df67e..2f93375f2 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java @@ -210,13 +210,20 @@ private void addSuperType(TypeElement element, TypeMirror mirror, boolean proxyB final String type = Util.unwrapProvider(fullName); if (proxyBean || isPublic(element)) { - final var genericType = !Objects.equals(fullName, type) ? UType.parse(mirror).param0() : UType.parse(mirror); + UType uType = UType.parse(mirror); + final var genericType = !Objects.equals(fullName, type) ? uType.param0() : uType; // check if any unknown generic types are in the parameters (T,T2, etc.) - final var knownType = genericType.componentTypes().stream() - .flatMap(g -> Stream.concat(Stream.of(g), g.componentTypes().stream())) - .noneMatch(g -> g.kind() == TypeKind.TYPEVAR); + final var knownType = + genericType.componentTypes().stream() + .flatMap(g -> Stream.concat(Stream.of(g), g.componentTypes().stream())) + .noneMatch(g -> g.kind() == TypeKind.TYPEVAR); extendsTypes.add(knownType ? Util.unwrapProvider(mirror) : genericType); + + if (uType.isGeneric()) { + extendsTypes.add(UType.parse(types().erasure(Util.stripProvider(mirror)))); + } + extendsInjection.read(element); } @@ -257,6 +264,9 @@ private void readInterfacesOf(TypeMirror anInterface) { } } interfaceTypes.add(rawUType); + if (rawUType.isGeneric()) { + interfaceTypes.add(UType.parse(types().erasure(Util.stripProvider(anInterface)))); + } for (final TypeMirror supertype : types().directSupertypes(anInterface)) { readInterfacesOf(supertype); } diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java index cffc9eba0..aa36927c3 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java @@ -74,9 +74,13 @@ List autoProvides() { return injectsTypes.stream().map(UType::full).collect(toList()); } return extendsReader.autoProvides().stream() - .filter(u -> u.componentTypes().stream().noneMatch(p -> p.kind() == TypeKind.TYPEVAR)) - .map(UType::full) - .collect(toList()); + .map( + u -> + u.componentTypes().stream().noneMatch(p -> p.kind() == TypeKind.TYPEVAR) + ? u.full() + : u.mainType()) + .distinct() + .collect(toList()); } String providesAspect() { diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/Util.java b/inject-generator/src/main/java/io/avaje/inject/generator/Util.java index de9b34a94..473ab3933 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/Util.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/Util.java @@ -50,7 +50,7 @@ private static boolean importJavaLangSubpackage(String type) { static String classOfMethod(String method) { final int pos = method.lastIndexOf('.'); - return (pos == -1) ? "" : method.substring(0, pos); + return pos == -1 ? "" : method.substring(0, pos); } static String shortMethod(String method) { @@ -92,7 +92,7 @@ static String nestedPackageOf(String cls) { return ""; } pos = cls.lastIndexOf('.', pos - 1); - return (pos == -1) ? "" : cls.substring(0, pos); + return pos == -1 ? "" : cls.substring(0, pos); } static String unwrapProvider(String maybeProvider) { @@ -104,11 +104,7 @@ static String unwrapProvider(String maybeProvider) { } static UType unwrapProvider(TypeMirror maybeProvider) { - if (isProvider(maybeProvider.toString())) { - return UType.parse(maybeProvider).param0(); - } else { - return UType.parse(maybeProvider); - } + return UType.parse(stripProvider(maybeProvider)); } static UType unwrapProvider(UType maybeProvider) { @@ -119,6 +115,14 @@ static UType unwrapProvider(UType maybeProvider) { } } + static TypeMirror stripProvider(TypeMirror maybeProvider) { + if (isProvider(maybeProvider.toString())) { + return ((DeclaredType) maybeProvider).getTypeArguments().get(0); + } else { + return maybeProvider; + } + } + static String initLower(String name) { final StringBuilder sb = new StringBuilder(name.length()); boolean upper = true; diff --git a/inject-test/src/test/java/org/example/coffee/CoffeeMakerTest.java b/inject-test/src/test/java/org/example/coffee/CoffeeMakerTest.java index 531f38726..f85e0f642 100644 --- a/inject-test/src/test/java/org/example/coffee/CoffeeMakerTest.java +++ b/inject-test/src/test/java/org/example/coffee/CoffeeMakerTest.java @@ -5,6 +5,7 @@ import org.example.coffee.core.DuperPump; import org.example.coffee.generic.HazRepo; import org.example.coffee.generic.HazRepo$DI; +import org.example.coffee.generic.Repository; import org.example.coffee.list.BSomei; import org.example.coffee.list.Somei; import org.example.coffee.provider.AProv; @@ -115,7 +116,7 @@ void beanScope_all_includesGenericInterfaces() { .findFirst().orElse(null); assertThat(hazRepo.keys()) - .containsExactly(name(HazRepo.class), name(HazRepo$DI.TYPE_RepositoryHazLong)); + .containsExactly(name(HazRepo.class), name(HazRepo$DI.TYPE_RepositoryHazLong), name(Repository.class)); } } diff --git a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java index c6e85a308..59ecfa3d8 100644 --- a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java +++ b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java @@ -1,5 +1,7 @@ package org.example.coffee.generic; +import java.util.List; + import org.example.coffee.grind.AMusher; import jakarta.inject.Singleton; @@ -16,18 +18,29 @@ class MultiGenericConsumer { private final AMusher aMusher; - MultiGenericConsumer(Repository hazRepo, AMusher aMusher, SomeGeneric stringProcessor) { + private List> list; + + MultiGenericConsumer( + Repository hazRepo, + AMusher aMusher, + SomeGeneric stringProcessor, + List> list) { this.hazRepo = hazRepo; this.aMusher = aMusher; this.stringProcessor = stringProcessor; + this.list = list; } String findAndDo(long id) { final Haz byId = hazRepo.findById(id); - return (byId == null) ? "not found" : "found " + stringProcessor.process("" + byId.id); + return byId == null ? "not found" : "found " + stringProcessor.process("" + byId.id); } String mushString() { return aMusher.toString(); } + + public List> list() { + return list; + } } diff --git a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumerTest.java b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumerTest.java index 33bafb5d7..42e602f63 100644 --- a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumerTest.java +++ b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumerTest.java @@ -1,9 +1,10 @@ package org.example.coffee.generic; -import io.avaje.inject.xtra.ApplicationScope; +import static org.assertj.core.api.Assertions.assertThat; + import org.junit.jupiter.api.Test; -import static org.assertj.core.api.Assertions.assertThat; +import io.avaje.inject.xtra.ApplicationScope; public class MultiGenericConsumerTest { @@ -14,6 +15,6 @@ public void find() { assertThat(bean.findAndDo(34L)).isEqualTo("found 34 stuff"); assertThat(bean.mushString()).isNotNull(); + assertThat(bean.list()).isNotEmpty(); } - } From 285b590b2680a1ba1b5c3635f14f92446b915eaa Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Fri, 9 May 2025 16:16:54 +1200 Subject: [PATCH 2/3] Extract isWildcard() helper method, restore format to simplify diff --- .../avaje/inject/generator/MethodReader.java | 29 ++++++++----------- .../inject/generator/TypeExtendsReader.java | 8 ++--- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java index 5707cfc2a..cc4119270 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java @@ -563,17 +563,13 @@ void addDependsOnGeneric(Set set) { } void builderGetDependency(Append writer, String builderName) { - - boolean wildcard = - genericType.isGeneric() - && genericType.componentTypes().stream().allMatch(g -> g.kind() == TypeKind.WILDCARD); - - var wildParam = wildcard ? String.format("<%s>", genericType.shortWithoutAnnotations()) : ""; + final boolean wildcard = isWildcard(); + final var wildParam = wildcard ? String.format("<%s>", genericType.shortWithoutAnnotations()) : ""; writer - .append(builderName) - .append(".") - .append(wildParam) - .append(utilType.getMethod(nullable, isBeanMap)); + .append(builderName) + .append(".") + .append(wildParam) + .append(utilType.getMethod(nullable, isBeanMap)); if (!genericType.isGeneric() || wildcard) { writer.append(Util.shortName(genericType.mainType())).append(".class"); } else { @@ -595,6 +591,11 @@ void builderGetDependency(Append writer, String builderName) { writer.append(")"); } + private boolean isWildcard() { + return genericType.isGeneric() + && genericType.componentTypes().stream().allMatch(g -> g.kind() == TypeKind.WILDCARD); + } + String simpleName() { return simpleName; } @@ -612,13 +613,7 @@ boolean isGenericParam() { } Dependency dependsOn() { - - boolean wildcard = - genericType.isGeneric() - && genericType.componentTypes().stream().allMatch(g -> g.kind() == TypeKind.WILDCARD); - - return new Dependency( - wildcard ? genericType.mainType() : paramType, named, utilType.isCollection()); + return new Dependency(isWildcard() ? genericType.mainType() : paramType, named, utilType.isCollection()); } void addImports(ImportTypeMap importTypes) { diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java index 2f93375f2..b18324a1a 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java @@ -213,13 +213,11 @@ private void addSuperType(TypeElement element, TypeMirror mirror, boolean proxyB UType uType = UType.parse(mirror); final var genericType = !Objects.equals(fullName, type) ? uType.param0() : uType; // check if any unknown generic types are in the parameters (T,T2, etc.) - final var knownType = - genericType.componentTypes().stream() - .flatMap(g -> Stream.concat(Stream.of(g), g.componentTypes().stream())) - .noneMatch(g -> g.kind() == TypeKind.TYPEVAR); + final var knownType = genericType.componentTypes().stream() + .flatMap(g -> Stream.concat(Stream.of(g), g.componentTypes().stream())) + .noneMatch(g -> g.kind() == TypeKind.TYPEVAR); extendsTypes.add(knownType ? Util.unwrapProvider(mirror) : genericType); - if (uType.isGeneric()) { extendsTypes.add(UType.parse(types().erasure(Util.stripProvider(mirror)))); } From 3414c01bf11b9ab285ea9738a16ec0bed821dd23 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Fri, 9 May 2025 16:35:02 +1200 Subject: [PATCH 3/3] Revert the change to TypeReader.autoProvides() such that its back to prior behaviour. TypeReader.autoProvides() excludes types that have a generic type parameter. --- .../io/avaje/inject/generator/TypeExtendsReader.java | 1 - .../java/io/avaje/inject/generator/TypeReader.java | 10 +++------- .../example/coffee/generic/MultiGenericConsumer.java | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java index b18324a1a..dbaf5b15b 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java @@ -221,7 +221,6 @@ private void addSuperType(TypeElement element, TypeMirror mirror, boolean proxyB if (uType.isGeneric()) { extendsTypes.add(UType.parse(types().erasure(Util.stripProvider(mirror)))); } - extendsInjection.read(element); } diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java index aa36927c3..cffc9eba0 100644 --- a/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java +++ b/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java @@ -74,13 +74,9 @@ List autoProvides() { return injectsTypes.stream().map(UType::full).collect(toList()); } return extendsReader.autoProvides().stream() - .map( - u -> - u.componentTypes().stream().noneMatch(p -> p.kind() == TypeKind.TYPEVAR) - ? u.full() - : u.mainType()) - .distinct() - .collect(toList()); + .filter(u -> u.componentTypes().stream().noneMatch(p -> p.kind() == TypeKind.TYPEVAR)) + .map(UType::full) + .collect(toList()); } String providesAspect() { diff --git a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java index 59ecfa3d8..0598c39be 100644 --- a/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java +++ b/inject-test/src/test/java/org/example/coffee/generic/MultiGenericConsumer.java @@ -40,7 +40,7 @@ String mushString() { return aMusher.toString(); } - public List> list() { + List> list() { return list; } }