Fix Manual SPI Detection

.editorconfig

@@ -0,0 +1,12 @@
+# editorconfig.org
+
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+indent_size = 2
+indent_style = space
+insert_final_newline = true
+trim_trailing_whitespace = true
+spaces_around_operators = true

.gitignore

@@ -11,3 +11,7 @@ build/
 */bin/
 .DS_Store
 
+/bin
+blackbox-test-spi/META-INF/services/io.avaje.spi.test.SPIInterface
+blackbox-test-spi/META-INF/services/io.avaje.spi.test.SPIInterface$NestedSPIInterface
+blackbox-test-spi/io.avaje.spi.test.SPIInterface$NestedSPIInterface

avaje-spi-core/pom.xml

@@ -13,7 +13,7 @@
 		<tag>HEAD</tag>
 	</scm>
   <properties>
-    <avaje.prisms.version>1.21</avaje.prisms.version>
+    <avaje.prisms.version>1.25</avaje.prisms.version>
   </properties>
 
   <dependencies>
@@ -23,12 +23,18 @@
       <version>${avaje.prisms.version}</version>
       <optional>true</optional>
       <scope>provided</scope>
+      <exclusions>
+      	<exclusion>
+          <groupId>io.avaje</groupId>
+          <artifactId>avaje-spi-service</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
-    
+
 	<dependency>
 	  <groupId>io.avaje</groupId>
 	  <artifactId>junit</artifactId>
-	  <version>1.4</version>
+	  <version>1.5</version>
 	  <scope>test</scope>
 	</dependency>
 

avaje-spi-core/src/main/java/io/avaje/spi/internal/ModuleReader.java

@@ -10,22 +10,22 @@
 
 import javax.lang.model.element.ModuleElement;
 
-import io.avaje.prism.GenerateModuleInfoReader;
-
-@GenerateModuleInfoReader
 final class ModuleReader {
-  private final Map<String, Set<String>> missingServicesMap = new HashMap<>();
 
+  private final Map<String, Set<String>> missingServicesMap = new HashMap<>();
   private boolean staticWarning;
-
   private boolean coreWarning;
 
   ModuleReader(Map<String, Set<String>> services) {
     services.forEach(this::add);
   }
 
   private void add(String k, Set<String> v) {
-    missingServicesMap.put(Utils.fqnFromBinaryType(k), v.stream().map(Utils::fqnFromBinaryType).collect(toSet()));
+    missingServicesMap.put(replace$(k), v.stream().map(ModuleReader::replace$).collect(toSet()));
+  }
+
+  private static String replace$(String k) {
+    return k.replace('$', '.');
   }
 
   void read(BufferedReader reader, ModuleElement element) throws IOException {
@@ -42,18 +42,17 @@ void read(BufferedReader reader, ModuleElement element) throws IOException {
         break;
       }
     }
-
     module.provides().forEach(p -> {
-      if (!missingServicesMap.containsKey(p.service())) {
+      final var contract = replace$(p.service());
+      if (!missingServicesMap.containsKey(contract)) {
         return;
       }
-
       var impls = p.implementations();
-      var missing = missingServicesMap.get(p.service());
+      var missing = missingServicesMap.get(contract);
       if (missing.size() != impls.size()) {
         return;
       }
-      impls.stream().map(Utils::fqnFromBinaryType).forEach(missing::remove);
+      impls.stream().map(ModuleReader::replace$).forEach(missing::remove);
     });
   }
 

avaje-spi-core/src/main/java/io/avaje/spi/internal/ServiceProcessor.java

@@ -18,7 +18,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -111,7 +110,7 @@ public synchronized void init(ProcessingEnvironment env) {
                       .replace(
                           "META-INF/services/spi-service-locator", "META-INF/generated-services")));
     } catch (IOException e) {
-      logError("Failed to write service locator file");
+      // not an issue worth failing over
     }
   }
 
@@ -130,18 +129,7 @@ public boolean process(Set<? extends TypeElement> tes, RoundEnvironment roundEnv
     if (roundEnv.processingOver()) {
       //load generated service files into main services
       var generatedSpis = loadMetaInfServices(generatedSpisDir);
-      generatedSpis.forEach(
-          (key, value) ->
-              services.merge(
-                  key,
-                  value,
-                  (oldValue, newValue) -> {
-                    if (oldValue == null) {
-                      oldValue = new HashSet<>();
-                    }
-                    oldValue.addAll(newValue);
-                    return oldValue;
-                  }));
+      Utils.mergeServices(generatedSpis, services);
       write();
       validateModule();
     }
@@ -186,14 +174,17 @@ private void validate(final TypeElement type) {
   private void write() {
     // Read the existing service files
     var allServices = loadMetaInfServices(servicesDirectory);
-    allServices.putAll(services);
+
+    // add loaded services without messing with other annotation processors' service generation
+    allServices.forEach((key, value) ->
+      services.computeIfPresent(key, (k, v) -> {
+        v.addAll(value);
+        return v;
+      }));
+
     // Write the service files
-    for (final Map.Entry<String, Set<String>> e : allServices.entrySet()) {
+    for (final var e : services.entrySet()) {
       final String contract = e.getKey();
-      // avoid messing with other annotation processors' service generation
-      if (!services.containsKey(contract)) {
-        continue;
-      }
       logNote("Writing META-INF/services/%s", contract);
       try (final var file =
               processingEnv
@@ -210,11 +201,16 @@ private void write() {
         logError("Failed to write service definition files: %s", x);
       }
     }
-    services.putAll(allServices);
+
+    // merge all services for module validation
+    Utils.mergeServices(allServices, services);
   }
 
   private Map<String, Set<String>> loadMetaInfServices(Path servicesDirectory) {
     var allServices = new HashMap<String, Set<String>>();
+    if (servicesDirectory == null) {
+      return allServices;
+    }
 
     // Read the existing service files
     try (var servicePaths = Files.walk(servicesDirectory, 1).skip(1)) {
@@ -236,11 +232,7 @@ private Map<String, Set<String>> loadMetaInfServices(Path servicesDirectory) {
         } catch (final FileNotFoundException | java.nio.file.NoSuchFileException x) {
           // missing and thus not created yet
         } catch (final IOException x) {
-          logError(
-              "Failed to load existing service definition file. SPI: "
-                  + contract
-                  + " exception: "
-                  + x);
+          logError("Failed to load existing service definition file. SPI: " + contract + " exception: " + x);
         }
       }
     } catch (NoSuchFileException e) {
@@ -291,8 +283,7 @@ private List<TypeElement> getServiceInterfaces(TypeElement type) {
     return typeElementList;
   }
 
-  // if a @Service Annotation is present on a superclass/interface, use that as the inferred service
-  // type
+  // if a @Service Annotation is present on a superclass/interface, use that as the inferred service type
   private boolean checkSPI(TypeMirror typeMirror, final List<TypeElement> typeElementList) {
     var type = asTypeElement(typeMirror);
     if (type == null) {
@@ -377,17 +368,24 @@ void validateModule() {
   }
 
   private void logModuleError(ModuleReader moduleReader) {
-    moduleReader.missing().forEach((k, v) -> {
-      if (!v.isEmpty()) {
-        logError(
-                moduleElement,
-                "Missing `provides %s with %s;` Please ensure that all META-INF service classes are registered correctly",
-                k,
-                services.get(k).stream()
+    moduleReader
+        .missing()
+        .forEach(
+            (k, v) -> {
+              if (!v.isEmpty()) {
+                var contract =
+                    services.keySet().stream()
+                        .filter(s -> s.replace('$', '.').equals(k.replace('$', '.')))
+                        .findAny()
+                        .orElseThrow();
+                var missingImpls =
+                    services.get(contract).stream()
                         .map(Utils::fqnFromBinaryType)
-                        .collect(joining(", ")));
-      }
-    });
+                        .collect(joining(", "));
+
+                logError(moduleElement, "Missing `provides %s with %s;`", Utils.fqnFromBinaryType(contract), missingImpls);
+              }
+            });
   }
 
   private static boolean buildPluginAvailable() {

avaje-spi-core/src/main/java/io/avaje/spi/internal/Utils.java

@@ -2,10 +2,14 @@
 
 import static io.avaje.spi.internal.APContext.typeElement;
 
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 final class Utils {
 
   static String fqnFromBinaryType(String binaryType) {
-    var type = typeElement(binaryType.replace("$", "."));
+    var type = typeElement(binaryType.replace('$', '.'));
     if (type != null) {
       return type.getQualifiedName().toString();
     }
@@ -37,4 +41,12 @@ static String replaceDollar(String str) {
     }
     return sb.toString();
   }
+
+  static void mergeServices(Map<String, Set<String>> newMap, Map<String, Set<String>> oldMap) {
+    newMap.forEach((key, value) ->
+      oldMap.merge(key, value, (oldValue, newValue) -> {
+        oldValue.addAll(newValue);
+        return oldValue;
+      }));
+  }
 }