Repository: avaje/avaje-inject
Pull request #358 (2023-07-01 15:18:34)

Fix split packages from Component.Import
Before Imported beans would have the DI/Proxy classes generated in the same package as the target type, causing runtime errors on Jlink application images.

- Modifies Generation to add `di` to the end of imported packages
- adds Inject Generator tests (we have a bunch of black-box tests, but it's easier to debug compilation when you can set breakpoints)


Patch:
diff --git a/inject-generator/pom.xml b/inject-generator/pom.xml
index 8879b247..6da286a9 100644
--- a/inject-generator/pom.xml
+++ b/inject-generator/pom.xml
@@ -13,7 +13,7 @@
   <name>avaje inject generator</name>
   <description>annotation processor generating di as source code</description>
   <properties>
-    <avaje.prisms.version>1.8</avaje.prisms.version>
+    <avaje.prisms.version>1.10</avaje.prisms.version>
   </properties>
   <dependencies>
 
@@ -66,9 +66,7 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
-          <argLine>
-            --add-opens io.avaje.inject.generator/io.avaje.inject.generator=ALL-UNNAMED
-          </argLine>
+          <useModulePath>false</useModulePath>
         </configuration>
       </plugin>
     </plugins>
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java b/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java
index 7ddc366d..ca7db65c 100644
--- a/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java
+++ b/inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java
@@ -1,5 +1,5 @@
 package io.avaje.inject.generator;
-
+import static io.avaje.inject.generator.ProcessingContext.*;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -79,7 +79,7 @@ final class MetaData {
     if (Util.isVoid(type)) {
       return "void_" + Util.trimMethod(method);
     } else {
-      String trimType = Util.trimMethod(type);
+      final String trimType = Util.trimMethod(type);
       if (name != null) {
         return trimType + "_" + name;
       } else {
@@ -154,7 +154,17 @@ final class MetaData {
     if (hasMethod()) {
       importTypes.add(Util.classOfMethod(method));
     } else if (!generateProxy) {
-      importTypes.add(type + Constants.DI);
+      if (isImportedType(type)) {
+        String packageName;
+        if (element(type).getNestingKind().isNested()) {
+          packageName = Util.nestedPackageOf(type);
+        } else {
+          packageName = Util.packageOf(type);
+        }
+        importTypes.add(packageName + "." + shortType + Constants.DI);
+      } else {
+        importTypes.add(type + Constants.DI);
+      }
     }
   }
 
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/ProcessingContext.java b/inject-generator/src/main/java/io/avaje/inject/generator/ProcessingContext.java
index f11e4b97..da429382 100644
--- a/inject-generator/src/main/java/io/avaje/inject/generator/ProcessingContext.java
+++ b/inject-generator/src/main/java/io/avaje/inject/generator/ProcessingContext.java
@@ -42,6 +42,7 @@ final class ProcessingContext {
     private final Set<String> uniqueModuleNames = new HashSet<>();
     private final Set<String> providedTypes = new HashSet<>();
     private final Set<String> optionalTypes = new LinkedHashSet<>();
+    private final Set<String> importedTypes = new LinkedHashSet<>();
     private final Map<String, AspectImportPrism> aspectImportPrisms = new HashMap<>();
 
     public Ctx(ProcessingEnvironment processingEnv, Set<String> moduleFileProvided) {
@@ -52,12 +53,22 @@ final class ProcessingContext {
       ExternalProvider.registerModuleProvidedTypes(providedTypes);
       providedTypes.addAll(moduleFileProvided);
     }
+    public Ctx() {
+      this.messager = null;
+      this.filer = null;
+      this.elementUtils = null;
+      this.typeUtils = null;
+    }
   }
 
   public static void init(ProcessingEnvironment processingEnv, Set<String> moduleFileProvided) {
     CTX.set(new Ctx(processingEnv, moduleFileProvided));
   }
 
+  public static void testInit() {
+    CTX.set(new Ctx());
+  }
+
   /** Log an error message. */
   static void logError(Element e, String msg, Object... args) {
     CTX.get().messager.printMessage(Diagnostic.Kind.ERROR, String.format(msg, args), e);
@@ -180,6 +191,14 @@ final class ProcessingContext {
     CTX.get().aspectImportPrisms.putAll(importedMap);
   }
 
+  static void addImportedType(String importedMap) {
+    CTX.get().importedTypes.add(importedMap);
+  }
+
+  static boolean isImportedType(String type) {
+    return CTX.get().importedTypes.contains(type);
+  }
+
   static Optional<AspectImportPrism> getImportedAspect(String type) {
     return Optional.ofNullable(CTX.get().aspectImportPrisms.get(type));
   }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/Processor.java b/inject-generator/src/main/java/io/avaje/inject/generator/Processor.java
index a12b8247..9bd6b225 100644
--- a/inject-generator/src/main/java/io/avaje/inject/generator/Processor.java
+++ b/inject-generator/src/main/java/io/avaje/inject/generator/Processor.java
@@ -1,6 +1,7 @@
 package io.avaje.inject.generator;
 
 import static io.avaje.inject.generator.ProcessingContext.addImportedAspects;
+import static io.avaje.inject.generator.ProcessingContext.addImportedType;
 import static io.avaje.inject.generator.ProcessingContext.element;
 import static io.avaje.inject.generator.ProcessingContext.loadMetaInfCustom;
 import static io.avaje.inject.generator.ProcessingContext.loadMetaInfServices;
@@ -113,12 +114,16 @@ public final class Processor extends AbstractProcessor {
     readChangedBeans(roundEnv.getElementsAnnotatedWith(element(Constants.PROTOTYPE)), false);
 
     final var importedElements =
-      roundEnv.getElementsAnnotatedWith(element(ImportPrism.PRISM_TYPE)).stream()
-        .map(ImportPrism::getInstanceOn)
-        .flatMap(p -> p.value().stream())
-        .map(ProcessingContext::asElement)
-        .map(TypeElement.class::cast)
-        .collect(Collectors.toSet());
+        roundEnv.getElementsAnnotatedWith(element(ImportPrism.PRISM_TYPE)).stream()
+            .map(ImportPrism::getInstanceOn)
+            .flatMap(p -> p.value().stream())
+            .map(ProcessingContext::asElement)
+            .map(
+                e -> {
+                  addImportedType(e.getQualifiedName().toString());
+                  return e;
+                })
+            .collect(Collectors.toSet());
 
     readChangedBeans(importedElements, false);
 
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java b/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java
index 2062a9b8..c9c1937a 100644
--- a/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java
+++ b/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java
@@ -31,8 +31,8 @@ final class SimpleBeanWriter {
 
   SimpleBeanWriter(BeanReader beanReader) {
     this.beanReader = beanReader;
-    TypeElement origin = beanReader.beanType();
-    this.originName = origin.getQualifiedName().toString();
+    final TypeElement origin = beanReader.beanType();
+    final var originName = origin.getQualifiedName().toString();
     if (origin.getNestingKind().isNested()) {
       this.packageName = Util.nestedPackageOf(originName);
       this.shortName = Util.nestedShortName(originName);
@@ -42,6 +42,7 @@ final class SimpleBeanWriter {
     }
     this.suffix = beanReader.suffix();
     this.proxied = beanReader.isGenerateProxy();
+    this.originName = packageName + "." + shortName;
   }
 
   private Writer createFileWriter() throws IOException {
@@ -49,7 +50,7 @@ final class SimpleBeanWriter {
     if (beanReader.beanType().getNestingKind().isNested()) {
       originName = originName.replace(shortName, shortName.replace(".", "$"));
     }
-    JavaFileObject jfo = createWriter(originName + suffix);
+    final JavaFileObject jfo = createWriter(originName + suffix);
     return jfo.openWriter();
   }
 
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 6176aad9..82dcf50a 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
@@ -1,6 +1,7 @@
 package io.avaje.inject.generator;
 
 import java.util.Optional;
+import static io.avaje.inject.generator.ProcessingContext.isImportedType;
 
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
@@ -41,7 +42,7 @@ final class Util {
   }
 
   static String trimGenerics(String type) {
-    int i = type.indexOf('<');
+    final int i = type.indexOf('<');
     if (i == -1) {
       return type;
     }
@@ -51,7 +52,7 @@ final class Util {
 
   /** Trim off annotations from the raw type if present. */
   public static String trimAnnotations(String type) {
-    int pos = type.indexOf(".@");
+    final int pos = type.indexOf(".@");
     if (pos == -1) {
       return type;
     }
@@ -59,11 +60,11 @@ final class Util {
   }
 
   public static String sanitizeImports(String type) {
-    int pos = type.indexOf("@");
+    final int pos = type.indexOf("@");
     if (pos == -1) {
       return type.replace("[]", "");
     }
-    var start = pos == 0 ? type.substring(0, pos) : "";
+    final var start = pos == 0 ? type.substring(0, pos) : "";
     return start + type.substring(type.lastIndexOf(' ') + 1).replace("[]", "");
   }
 
@@ -73,12 +74,14 @@ final class Util {
       return "";
     }
     pos = cls.lastIndexOf('.', pos - 1);
-    return (pos == -1) ? "" : cls.substring(0, pos);
+    final var packageName = (pos == -1) ? "" : cls.substring(0, pos);
+    return isImportedType(cls) ? packageName + ".di" : packageName;
   }
 
   static String packageOf(String cls) {
-    int pos = cls.lastIndexOf('.');
-    return (pos == -1) ? "" : cls.substring(0, pos);
+    final int pos = cls.lastIndexOf('.');
+    final var packageName = (pos == -1) ? "" : cls.substring(0, pos);
+    return isImportedType(cls) ? packageName + ".di" : packageName;
   }
 
   static String unwrapProvider(String maybeProvider) {
@@ -90,9 +93,9 @@ final class Util {
   }
 
   static String initLower(String name) {
-    StringBuilder sb = new StringBuilder(name.length());
+    final StringBuilder sb = new StringBuilder(name.length());
     boolean upper = true;
-    for (char ch : name.toCharArray()) {
+    for (final char ch : name.toCharArray()) {
       if (upper && Character.isUpperCase(ch)) {
         sb.append(Character.toLowerCase(ch));
       } else {
@@ -114,7 +117,7 @@ final class Util {
   }
 
   static String shortName(String fullType) {
-    int p = fullType.lastIndexOf('.');
+    final int p = fullType.lastIndexOf('.');
     if (p == -1) {
       return fullType;
     } else {
@@ -135,7 +138,7 @@ final class Util {
   }
 
   static String extractList(String rawType) {
-    String listType = rawType.substring(15, rawType.length() - 1);
+    final String listType = rawType.substring(15, rawType.length() - 1);
     if (listType.startsWith("? extends")) {
       return listType.substring(10);
     }
@@ -143,7 +146,7 @@ final class Util {
   }
 
   static String extractSet(String rawType) {
-    String setType = rawType.substring(14, rawType.length() - 1);
+    final String setType = rawType.substring(14, rawType.length() - 1);
     if (setType.startsWith("? extends")) {
       return setType.substring(10);
     }
@@ -151,7 +154,7 @@ final class Util {
   }
 
   static String extractMap(String rawType) {
-    String valType = rawType.substring(31, rawType.length() - 1);
+    final String valType = rawType.substring(31, rawType.length() - 1);
     if (valType.startsWith("? extends")) {
       return valType.substring(10);
     }
@@ -209,13 +212,13 @@ final class Util {
    * Return the name via <code>@Named</code> or a Qualifier annotation.
    */
   public static String getNamed(Element p) {
-    NamedPrism named = NamedPrism.getInstanceOn(p);
+    final NamedPrism named = NamedPrism.getInstanceOn(p);
     if (named != null) {
       return named.value().toLowerCase();
     }
-    for (AnnotationMirror annotationMirror : p.getAnnotationMirrors()) {
-      DeclaredType annotationType = annotationMirror.getAnnotationType();
-      var hasQualifier = QualifierPrism.isPresent(annotationType.asElement());
+    for (final AnnotationMirror annotationMirror : p.getAnnotationMirrors()) {
+      final DeclaredType annotationType = annotationMirror.getAnnotationType();
+      final var hasQualifier = QualifierPrism.isPresent(annotationType.asElement());
       if (hasQualifier) {
         return Util.shortName(annotationType.toString()).toLowerCase();
       }
@@ -227,7 +230,7 @@ final class Util {
    * Return true if the element has a Nullable annotation.
    */
   public static boolean isNullable(Element p) {
-    for (AnnotationMirror mirror : p.getAnnotationMirrors()) {
+    for (final AnnotationMirror mirror : p.getAnnotationMirrors()) {
       if (NULLABLE.equals(shortName(mirror.getAnnotationType().toString()))) {
         return true;
       }
@@ -236,7 +239,7 @@ final class Util {
   }
 
   public static Optional<DeclaredType> getNullableAnnotation(Element p) {
-    for (AnnotationMirror mirror : p.getAnnotationMirrors()) {
+    for (final AnnotationMirror mirror : p.getAnnotationMirrors()) {
       if (NULLABLE.equals(shortName(mirror.getAnnotationType().toString()))) {
         return Optional.of(mirror.getAnnotationType());
       }


Test patch:
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/InjectProcessorTest.java b/inject-generator/src/test/java/io/avaje/inject/generator/InjectProcessorTest.java
new file mode 100644
index 00000000..f7a01a79
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/InjectProcessorTest.java
@@ -0,0 +1,65 @@
+package io.avaje.inject.generator;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Set;
+
+import javax.tools.JavaCompiler;
+import javax.tools.JavaCompiler.CompilationTask;
+import javax.tools.JavaFileObject;
+import javax.tools.JavaFileObject.Kind;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+import javax.tools.ToolProvider;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+class InjectProcessorTest {
+
+  @AfterEach
+  void deleteGeneratedFiles() throws IOException {
+    try {
+      Files.walk(Paths.get("io").toAbsolutePath())
+          .sorted(Comparator.reverseOrder())
+          .map(Path::toFile)
+          .forEach(File::delete);
+      Paths.get("io.avaje.inject.spi.Module").toAbsolutePath().toFile().delete();
+    } catch (final Exception e) {
+    }
+  }
+
+  @Test
+  void testGeneration() throws Exception {
+    final String source =
+        Paths.get("src/test/java/io/avaje/inject/generator/models/valid")
+            .toAbsolutePath()
+            .toString();
+
+    final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+    final StandardJavaFileManager manager = compiler.getStandardFileManager(null, null, null);
+
+    manager.setLocation(StandardLocation.SOURCE_PATH, Arrays.asList(new File(source)));
+
+    final Set<Kind> fileKinds = Collections.singleton(Kind.SOURCE);
+
+    final Iterable<JavaFileObject> files =
+        manager.list(StandardLocation.SOURCE_PATH, "", fileKinds, true);
+
+    final CompilationTask task =
+        compiler.getTask(
+            new PrintWriter(System.out), null, null, Arrays.asList("--release=11"), null, files);
+    task.setProcessors(Arrays.asList(new Processor()));
+
+    assertThat(task.call()).isTrue();
+  }
+}
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/UtilTest.java b/inject-generator/src/test/java/io/avaje/inject/generator/UtilTest.java
index 64673e90..34a3f074 100644
--- a/inject-generator/src/test/java/io/avaje/inject/generator/UtilTest.java
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/UtilTest.java
@@ -16,12 +16,39 @@ class UtilTest {
     assertEquals(Util.nestedShortName("com.example.foo.Bar"), "foo.Bar");
   }
 
+  @Test
+  void PackageOf() {
+    ProcessingContext.testInit();
+    assertEquals(Util.packageOf("com.example.Bar"), "com.example");
+    assertEquals(Util.packageOf("com.example.other.Bar"), "com.example.other");
+  }
+
+  @Test
+  void PackageOfImported() {
+    ProcessingContext.testInit();
+    ProcessingContext.addImportedType("com.example.Bar1");
+    ProcessingContext.addImportedType("com.example.other.Bar1");
+    assertEquals(Util.packageOf("com.example.Bar1"), "com.example.di");
+    assertEquals(Util.packageOf("com.example.other.Bar1"), "com.example.other.di");
+  }
+
+
   @Test
   void nestedPackageOf() {
+    ProcessingContext.testInit();
     assertEquals(Util.nestedPackageOf("com.example.Foo.Bar"), "com.example");
     assertEquals(Util.nestedPackageOf("com.example.other.foo.Bar"), "com.example.other");
   }
 
+  @Test
+  void nestedPackageOfImported() {
+    ProcessingContext.testInit();
+    ProcessingContext.addImportedType("com.example.Foo.Bar1");
+    ProcessingContext.addImportedType("com.example.other.foo.Bar1");
+    assertEquals(Util.nestedPackageOf("com.example.Foo.Bar1"), "com.example.di");
+    assertEquals(Util.nestedPackageOf("com.example.other.foo.Bar1"), "com.example.other.di");
+  }
+
   @Test
   void commonParent() {
     assertEquals(Util.commonParent(null, "org.b"), "org.b");
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TestClass.java b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TestClass.java
new file mode 100644
index 00000000..b257460f
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TestClass.java
@@ -0,0 +1,10 @@
+package io.avaje.inject.generator.models.valid;
+
+import io.avaje.inject.Component;
+import io.avaje.inject.generator.models.valid.imported.ImportedClass;
+import io.avaje.inject.generator.models.valid.imported.ImportedClassProxy;
+import jakarta.inject.Singleton;
+
+@Singleton
+@Component.Import({ImportedClass.class, ImportedClassProxy.class})
+public class TestClass {}
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/Timed.java b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/Timed.java
new file mode 100644
index 00000000..47fa32ba
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/Timed.java
@@ -0,0 +1,12 @@
+package io.avaje.inject.generator.models.valid;
+
+import io.avaje.inject.aop.Aspect;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Aspect(ordering = 2000)
+@Target({ElementType.METHOD, ElementType.TYPE})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Timed {}
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TimedAspect.java b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TimedAspect.java
new file mode 100644
index 00000000..e1365159
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/TimedAspect.java
@@ -0,0 +1,29 @@
+package io.avaje.inject.generator.models.valid;
+
+import io.avaje.inject.aop.AspectProvider;
+import io.avaje.inject.aop.Invocation;
+import io.avaje.inject.aop.MethodInterceptor;
+import jakarta.inject.Singleton;
+import java.lang.reflect.Method;
+
+@Singleton
+public class TimedAspect implements AspectProvider<Timed>, MethodInterceptor {
+
+  @Override
+  public MethodInterceptor interceptor(Method method, Timed aspectAnnotation) {
+    return this;
+  }
+
+  @Override
+  public void invoke(Invocation invocation) throws Throwable {
+
+    long start = System.nanoTime();
+    try {
+      invocation.invoke();
+    } finally {
+      long exeNanos = System.nanoTime() - start;
+      String fullName = invocation.method().getName();
+      System.out.println("executed " + fullName + " in " + exeNanos);
+    }
+  }
+}
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClass.java b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClass.java
new file mode 100644
index 00000000..c9ed33c7
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClass.java
@@ -0,0 +1,3 @@
+package io.avaje.inject.generator.models.valid.imported;
+
+public class ImportedClass {}
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClassProxy.java b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClassProxy.java
new file mode 100644
index 00000000..80743f3d
--- /dev/null
+++ b/inject-generator/src/test/java/io/avaje/inject/generator/models/valid/imported/ImportedClassProxy.java
@@ -0,0 +1,9 @@
+package io.avaje.inject.generator.models.valid.imported;
+
+import io.avaje.inject.generator.models.valid.Timed;
+
+public class ImportedClassProxy {
+
+  @Timed
+  public void somethin() throws RuntimeException {}
+}