Validate Qualifiers at compile time (#645)

* validate qualifiers at compile time

* remove transitive aop and events from inject test

* fix tests

* fix optional types

* final test

* fix event publishers not getting detected

* refactor reading external modules

* Revert "refactor reading external modules"

This reverts commit 96c3a94.

* Add Util.addQualifierSuffix() helper methods

* Format changes

* Move and adjust the test to QualifierCircularDependency etc

---------

Co-authored-by: Rob Bygrave <[email protected]>

Author: SentryMan

blackbox-test-inject/src/main/java/org/example/myapp/assist/droid/Radio.java

@@ -1,6 +1,8 @@
 package org.example.myapp.assist.droid;
 
+import jakarta.inject.Named;
 import jakarta.inject.Singleton;
 
 @Singleton
+@Named("red")
 public class Radio {}

blackbox-test-inject/src/main/java/org/example/myapp/other/QualifierCircularDependency.java

@@ -0,0 +1,26 @@
+package org.example.myapp.other;
+
+import io.avaje.inject.Bean;
+import io.avaje.inject.Factory;
+import jakarta.inject.Named;
+
+@Factory
+public class QualifierCircularDependency {
+
+  @Bean
+  @Named("parent") // for clarity, not necessary
+  public String parent(@Named("child") String child) {
+    return "parent-"+ child;
+  }
+
+  @Bean
+  @Named("child")
+  public String child() {
+    return "child";
+  }
+
+//  @Bean compilation rightly fails when this is uncommented
+//  public String child1(@Named("child2") String child) {
+//    throw new AssertionError("Method body unimportant");
+//  }
+}

blackbox-test-inject/src/main/java/org/example/myapp/other/QualifierConsumerComponent.java

@@ -0,0 +1,26 @@
+package org.example.myapp.other;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
+import jakarta.inject.Singleton;
+
+@Singleton
+final class QualifierConsumerComponent {
+
+  private final String parent;
+  private final String child;
+
+  @Inject
+  QualifierConsumerComponent(@Named("parent") String parent, @Named("child") String child) {
+    this.parent = parent;
+    this.child = child;
+  }
+
+  String parent() {
+    return parent;
+  }
+
+  String child() {
+    return child;
+  }
+}

blackbox-test-inject/src/test/java/org/example/myapp/other/QualifierConsumerComponentTest.java

@@ -0,0 +1,19 @@
+package org.example.myapp.other;
+
+import io.avaje.inject.BeanScope;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class QualifierConsumerComponentTest {
+
+  @Test
+  void doStuff() {
+    try (BeanScope beanScope = BeanScope.builder().build()) {
+      var component = beanScope.get(QualifierConsumerComponent.class);
+
+      assertThat(component.parent()).isEqualTo("parent-child");
+      assertThat(component.child()).isEqualTo("child");
+    }
+  }
+}

inject-generator/src/main/java/io/avaje/inject/generator/BeanReader.java

@@ -1,6 +1,15 @@
 package io.avaje.inject.generator;
 
 import static io.avaje.inject.generator.APContext.logError;
+import static java.util.stream.Collectors.toList;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Stream;
 
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
@@ -9,9 +18,6 @@
 
 import io.avaje.inject.generator.MethodReader.MethodParam;
 
-import java.util.*;
-import java.util.stream.Stream;
-
 
 final class BeanReader {
 
@@ -188,11 +194,11 @@ List<MethodReader> factoryMethods() {
   }
 
   List<String> provides() {
-    return typeReader.provides();
+    return Util.addQualifierSuffix(typeReader.provides(), name);
   }
 
   List<String> autoProvides() {
-    return typeReader.autoProvides();
+    return Util.addQualifierSuffix(typeReader.autoProvides(), name);
   }
 
   String providesAspect() {

inject-generator/src/main/java/io/avaje/inject/generator/Dependency.java

@@ -9,26 +9,30 @@ final class Dependency {
   private boolean softDependency;
   private final boolean conditionalDependency;
 
-  Dependency(String name) {
+  Dependency(String type) {
+    this(type, "");
+  }
+
+  Dependency(String type, String qualifier) {
     String nameStr;
-    if (name.startsWith(SOFT_DEPENDENCY)) {
+    if (type.startsWith(SOFT_DEPENDENCY)) {
       this.softDependency = true;
       this.conditionalDependency = false;
-      nameStr = ProcessorUtils.trimAnnotations(name.substring(5));
-    } else if (name.startsWith(CONDITIONAL_DEPENDENCY)) {
+      nameStr = ProcessorUtils.trimAnnotations(type.substring(5));
+    } else if (type.startsWith(CONDITIONAL_DEPENDENCY)) {
       this.softDependency = true;
       this.conditionalDependency = true;
-      nameStr = ProcessorUtils.trimAnnotations(name.substring(4));
+      nameStr = ProcessorUtils.trimAnnotations(type.substring(4));
     } else {
       this.softDependency = false;
       this.conditionalDependency = false;
-      nameStr = ProcessorUtils.trimAnnotations(name);
+      nameStr = ProcessorUtils.trimAnnotations(type);
     }
-    this.name = nameStr.replace(", ", ",");
+    this.name = Util.addQualifierSuffixTrim(qualifier, nameStr);
   }
 
-  Dependency(String name, boolean softDependency) {
-    this.name = ProcessorUtils.trimAnnotations(name).replace(", ", ",");
+  Dependency(String name, String qualifier, boolean softDependency) {
+    this.name = Util.addQualifierSuffixTrim(qualifier, ProcessorUtils.trimAnnotations(name));
     this.softDependency = softDependency;
     this.conditionalDependency = false;
   }
@@ -64,7 +68,9 @@ String dependsOn() {
     return toString();
   }
 
-  /** External dependency */
+  /**
+   * External dependency
+   */
   void markExternal() {
     softDependency = true;
   }

inject-generator/src/main/java/io/avaje/inject/generator/EventPublisherWriter.java

@@ -46,25 +46,26 @@ static void write(Element element) {
   private EventPublisherWriter(Element element) {
     final var asType = element.asType();
     this.utype = UType.parse(asType).param0();
-
-    this.packageName =
-        APContext.elements()
-                .getPackageOf(APContext.typeElement(utype.mainType()))
-                .getQualifiedName()
-                .toString()
-                .replaceFirst("java.", "")
-            + ".events";
+    this.packageName = APContext.elements()
+      .getPackageOf(APContext.typeElement(utype.mainType()))
+      .getQualifiedName()
+      .toString()
+      .replaceFirst("java.", "")
+      + ".events";
     qualifier = Optional.ofNullable(Util.getNamed(element)).orElse("");
     var className =
-        packageName
-            + "."
-            + (qualifier.isEmpty() ? "" : "Qualified")
-            + Util.shortName(utype).replace(".", "_")
-            + "$Publisher";
+      packageName
+        + "."
+        + (qualifier.isEmpty() ? "" : "Qualified")
+        + Util.shortName(utype).replace(".", "_")
+        + "$Publisher";
 
     originName = getUniqueClassName(className, 0);
 
     if (GENERATED_PUBLISHERS.containsKey(originName)) {
+      //in super niche situations when compiling the same module, we need to tell avaje that these types already exist
+      //got this when running both my eclipse compiler and then the terminal build
+      ProcessingContext.addOptionalType(UType.parse(asType).fullWithoutAnnotations(), Util.getNamed(element));
       return;
     }
     importTypes.addAll(utype.importTypes());
@@ -73,13 +74,10 @@ private EventPublisherWriter(Element element) {
   }
 
   private String getUniqueClassName(String className, Integer recursiveIndex) {
-
-    Optional.ofNullable(APContext.typeElement(className))
-        .ifPresent(
-            e ->
-                GENERATED_PUBLISHERS.put(
-                    e.getQualifiedName().toString(),
-                    Optional.ofNullable(Util.getNamed(e)).orElse("")));
+    Optional.ofNullable(APContext.typeElement(className)).ifPresent(e ->
+      GENERATED_PUBLISHERS.put(
+        e.getQualifiedName().toString(),
+        Optional.ofNullable(Util.getNamed(e)).orElse("")));
 
     if (Optional.ofNullable(GENERATED_PUBLISHERS.get(className))
         .filter(not(qualifier::equals))
@@ -99,18 +97,14 @@ private Writer createFileWriter() throws IOException {
   void write() {
     try {
       var writer = new Append(createFileWriter());
-
       final var shortType = utype.shortWithoutAnnotations();
       var typeString = utype.isGeneric() ? getGenericType() : shortType + ".class";
 
       var name = qualifier.isBlank() ? "" : "@Named(\"" + qualifier + "\")\n";
       var className = originName.replace(packageName + ".", "");
-      writer.append(
-          MessageFormat.format(
-              TEMPLATE, packageName, imports(), name, className, shortType, typeString, qualifier));
+      writer.append(MessageFormat.format(TEMPLATE, packageName, imports(), name, className, shortType, typeString, qualifier));
       writer.close();
     } catch (Exception e) {
-      e.printStackTrace();
       logError("Failed to write EventPublisher class " + e);
     }
   }
@@ -125,7 +119,6 @@ String imports() {
       importTypes.add(NamedPrism.PRISM_TYPE);
     }
     if (utype.isGeneric()) {
-
       importTypes.add("io.avaje.inject.spi.GenericType");
     }
 
@@ -146,13 +139,11 @@ String getGenericType() {
     return sb.toString();
   }
 
-  private void writeGenericType(
-      UType type, Map<String, String> seenShortNames, StringBuilder writer) {
+  private void writeGenericType(UType type, Map<String, String> seenShortNames, StringBuilder writer) {
     final var typeShortName = Util.shortName(type.mainType());
     final var mainType = seenShortNames.computeIfAbsent(typeShortName, k -> type.mainType());
     if (type.isGeneric()) {
-      final var shortName =
-          Objects.equals(type.mainType(), mainType) ? typeShortName : type.mainType();
+      final var shortName = Objects.equals(type.mainType(), mainType) ? typeShortName : type.mainType();
       writer.append(shortName);
       writer.append("<");
       boolean first = true;
@@ -167,8 +158,7 @@ private void writeGenericType(
       }
       writer.append(">");
     } else {
-      final var shortName =
-          Objects.equals(type.mainType(), mainType) ? typeShortName : type.mainType();
+      final var shortName = Objects.equals(type.mainType(), mainType) ? typeShortName : type.mainType();
       writer.append(shortName);
     }
   }

inject-generator/src/main/java/io/avaje/inject/generator/FieldReader.java

@@ -27,7 +27,7 @@ final class FieldReader {
     this.type = utype.toUType();
     this.assisted = AssistedPrism.isPresent(element);
     if (nullable || element.asType().toString().startsWith("java.util.Optional<")) {
-      ProcessingContext.addOptionalType(fieldType);
+      ProcessingContext.addOptionalType(fieldType, name);
     }
     if (type.fullWithoutAnnotations().startsWith("io.avaje.inject.events.Event")) {
       EventPublisherWriter.write(element);

inject-generator/src/main/java/io/avaje/inject/generator/InjectProcessor.java

@@ -15,7 +15,6 @@
 import javax.lang.model.util.Elements;
 
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toSet;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -156,11 +155,12 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
 
     maybeElements(roundEnv, ExternalPrism.PRISM_TYPE).stream()
       .flatMap(Set::stream)
-      .map(Element::asType)
-      .map(UType::parse)
-      .map(u -> "java.util.List".equals(u.mainType()) ? u.param0() : u)
-      .map(UType::fullWithoutAnnotations)
-      .forEach(ProcessingContext::addOptionalType);
+      .forEach(e -> {
+        var type = UType.parse(e.asType());
+        type = "java.util.List".equals(type.mainType()) ? type.param0() : type;
+        ProcessingContext.addOptionalType(type.fullWithoutAnnotations(), Util.getNamed(e));
+        ProcessingContext.addOptionalType(type.fullWithoutAnnotations(), null);
+      });
 
     maybeElements(roundEnv, "io.avaje.spi.ServiceProvider").ifPresent(this::registerSPI);
     allScopes.readBeans(roundEnv);

inject-generator/src/main/java/io/avaje/inject/generator/MetaData.java

@@ -1,9 +1,10 @@
 package io.avaje.inject.generator;
 
 import static io.avaje.inject.generator.APContext.typeElement;
-
+import static java.util.stream.Collectors.toList;
 import java.util.*;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 
 /**
@@ -46,9 +47,9 @@ final class MetaData {
     this.shortType = Util.shortName(type);
     this.method = meta.method();
     this.providesAspect = meta.providesAspect();
-    this.provides = meta.provides();
-    this.dependsOn = meta.dependsOn().stream().map(Dependency::new).collect(Collectors.toList());
-    this.autoProvides = meta.autoProvides();
+    this.dependsOn = meta.dependsOn().stream().map(d -> new Dependency(d, name)).collect(Collectors.toList());
+    this.provides = Util.addQualifierSuffix(meta.provides(), name);
+    this.autoProvides = Util.addQualifierSuffix(meta.autoProvides(), name);
     this.importedComponent = meta.importedComponent();
   }
 
@@ -249,21 +250,26 @@ private boolean hasMethod() {
   }
 
   private void appendProvides(Append sb, String attribute, List<String> types) {
+    if (!"dependsOn".equals(attribute)) {
+      types.removeIf(s -> s.contains(":"));
+    }
+
     sb.append(",").eol().append("      ").append(attribute).append(" = {");
     final var size = types.size();
     if (size > 1) {
       sb.eol().append("        ");
     }
     var seen = new HashSet<String>();
     for (int i = 0; i < types.size(); i++) {
-      if (!seen.add(types.get(i))) {
+      final var depType = types.get(i);
+      if (!seen.add(depType)) {
         continue;
       }
       if (i > 0) {
         sb.append(",").eol().append("        ");
       }
       sb.append("\"");
-      sb.append(types.get(i));
+      sb.append(depType);
       sb.append("\"");
     }
     if (size > 1) {
@@ -276,7 +282,7 @@ void setProvides(List<String> provides) {
     this.provides = provides;
   }
 
-  void setDependsOn(List<String> dependsOn) {
+  void setDependsOn(List<String> dependsOn, String name) {
     this.dependsOn = dependsOn.stream().map(Dependency::new).collect(Collectors.toList());
   }