Fixes incremental compilation for Pico services and running unit tests from IDE (helidon-io#6863)

Author: trentjeff

pico/processor/pom.xml

@@ -57,6 +57,11 @@
             <artifactId>hamcrest-all</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>

pico/processor/src/main/java/io/helidon/pico/processor/ActiveProcessorUtils.java

@@ -117,7 +117,10 @@ public void error(String message,
         out(System.Logger.Level.ERROR, Diagnostic.Kind.ERROR, message, null);
     }
 
-    void out(System.Logger.Level level, Diagnostic.Kind kind, String message, Throwable t) {
+    void out(System.Logger.Level level,
+             Diagnostic.Kind kind,
+             String message,
+             Throwable t) {
         if (logger.isLoggable(level)) {
             logger.log(level, getClass().getSimpleName() + ": " + message, t);
         }
@@ -191,10 +194,6 @@ Optional<TypeInfo> toTypeInfo(TypeElement element,
         return typeInfoCreatorProvider.createTypeInfo(element, mirror, processingEnv, isOneWeCareAbout);
     }
 
-    System.Logger.Level loggerLevel() {
-        return (Options.isOptionEnabled(Options.TAG_DEBUG)) ? System.Logger.Level.INFO : System.Logger.Level.DEBUG;
-    }
-
     RoundEnvironment roundEnv() {
         return roundEnv;
     }

pico/processor/src/main/java/io/helidon/pico/processor/CustomAnnotationProcessor.java

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -72,9 +73,7 @@ public CustomAnnotationProcessor() {
     }
 
     static List<CustomAnnotationTemplateCreator> initialize() {
-        // note: it is important to use this class' CL since maven will not give us the "right" one.
-        List<CustomAnnotationTemplateCreator> creators = HelidonServiceLoader.create(ServiceLoader.load(
-                CustomAnnotationTemplateCreator.class, CustomAnnotationTemplateCreator.class.getClassLoader())).asList();
+        List<CustomAnnotationTemplateCreator> creators = HelidonServiceLoader.create(loader()).asList();
         creators.forEach(creator -> {
             try {
                 Set<String> annoTypes = creator.annoTypes();
@@ -237,6 +236,19 @@ CustomAnnotationTemplateRequestDefault.Builder toRequestBuilder(TypeName annoTyp
                 .enclosingTypeInfo(enclosingClassTypeInfo);
     }
 
+    private static ServiceLoader<CustomAnnotationTemplateCreator> loader() {
+        try {
+            // note: it is important to use this class' CL since maven will not give us the "right" one.
+            return ServiceLoader.load(
+                    CustomAnnotationTemplateCreator.class, CustomAnnotationTemplateCreator.class.getClassLoader());
+        } catch (ServiceConfigurationError e) {
+            // see issue #6261 - running inside the IDE?
+            // this version will use the thread ctx classloader
+            System.getLogger(CustomAnnotationProcessor.class.getName()).log(System.Logger.Level.WARNING, e.getMessage(), e);
+            return ServiceLoader.load(CustomAnnotationTemplateCreator.class);
+        }
+    }
+
     private static TypeElement toEnclosingClassTypeElement(Element typeToProcess) {
         while (typeToProcess != null && !(typeToProcess instanceof TypeElement)) {
             typeToProcess = typeToProcess.getEnclosingElement();

pico/processor/src/main/java/io/helidon/pico/processor/PicoAnnotationProcessor.java

@@ -16,6 +16,8 @@
 
 package io.helidon.pico.processor;
 
+import java.io.IOException;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashMap;
@@ -40,6 +42,7 @@
 import io.helidon.common.types.AnnotationAndValueDefault;
 import io.helidon.common.types.TypeInfo;
 import io.helidon.common.types.TypeName;
+import io.helidon.common.types.TypeNameDefault;
 import io.helidon.common.types.TypedElementInfo;
 import io.helidon.pico.api.Activator;
 import io.helidon.pico.api.Contract;
@@ -51,9 +54,11 @@
 import io.helidon.pico.api.ServiceInfoBasics;
 import io.helidon.pico.runtime.Dependencies;
 import io.helidon.pico.tools.ActivatorCreatorCodeGen;
+import io.helidon.pico.tools.ActivatorCreatorCodeGenDefault;
 import io.helidon.pico.tools.ActivatorCreatorConfigOptionsDefault;
 import io.helidon.pico.tools.ActivatorCreatorDefault;
 import io.helidon.pico.tools.ActivatorCreatorRequest;
+import io.helidon.pico.tools.ActivatorCreatorRequestDefault;
 import io.helidon.pico.tools.ActivatorCreatorResponse;
 import io.helidon.pico.tools.InterceptionPlan;
 import io.helidon.pico.tools.InterceptorCreatorProvider;
@@ -68,6 +73,7 @@
 import jakarta.annotation.PreDestroy;
 import jakarta.inject.Inject;
 
+import static io.helidon.builder.processor.tools.BeanUtils.isBuiltInJavaType;
 import static io.helidon.builder.processor.tools.BuilderTypeTools.createTypeNameFromElement;
 import static io.helidon.common.types.TypeNameDefault.createFromTypeName;
 import static io.helidon.pico.processor.ActiveProcessorUtils.MAYBE_ANNOTATIONS_CLAIMED_BY_THIS_PROCESSOR;
@@ -81,6 +87,10 @@
 import static io.helidon.pico.processor.GeneralProcessorUtils.toScopeNames;
 import static io.helidon.pico.processor.GeneralProcessorUtils.toServiceTypeHierarchy;
 import static io.helidon.pico.processor.GeneralProcessorUtils.toWeight;
+import static io.helidon.pico.processor.ProcessingTracker.DEFAULT_SCRATCH_FILE_NAME;
+import static io.helidon.pico.processor.ProcessingTracker.initializeFrom;
+import static io.helidon.pico.tools.CodeGenFiler.scratchClassOutputPath;
+import static io.helidon.pico.tools.CodeGenFiler.targetClassOutputPath;
 import static io.helidon.pico.tools.TypeTools.createTypedElementInfoFromElement;
 import static io.helidon.pico.tools.TypeTools.toAccess;
 import static java.util.Objects.requireNonNull;
@@ -112,6 +122,7 @@ public class PicoAnnotationProcessor extends BaseAnnotationProcessor {
 
     private final Set<TypedElementInfo> allElementsOfInterestInThisModule = new LinkedHashSet<>();
     private final Map<TypeName, TypeInfo> typeInfoToCreateActivatorsForInThisModule = new LinkedHashMap<>();
+    private ProcessingTracker tracker;
     private CreatorHandler creator;
     private boolean autoAddInterfaces;
 
@@ -149,10 +160,7 @@ public void init(ProcessingEnvironment processingEnv) {
         super.init(processingEnv);
         this.autoAddInterfaces = Options.isOptionEnabled(Options.TAG_AUTO_ADD_NON_CONTRACT_INTERFACES);
         this.creator = new CreatorHandler(getClass().getSimpleName(), processingEnv, utils());
-//        if (BaseAnnotationProcessor.ENABLED) {
-//            // we are is simulation mode when the base one is operating...
-//            this.creator.activateSimulationMode();
-//        }
+        this.tracker = initializeFrom(trackerStatePath(), processingEnv);
     }
 
     @Override
@@ -165,7 +173,6 @@ public boolean process(Set<? extends TypeElement> annotations,
         }
 
         ServicesToProcess.onBeginProcessing(utils(), getSupportedAnnotationTypes(), roundEnv);
-//        ServicesToProcess.addOnDoneRunnable(CreatorHandler.reporting());
 
         try {
             // build the model
@@ -243,6 +250,7 @@ protected Set<String> supportedElementTargetAnnotations() {
      * Code generate these {@link io.helidon.pico.api.Activator}'s ad {@link io.helidon.pico.api.ModuleComponent}'s.
      *
      * @param services the services to code generate
+     * @throws ToolsException if there is problem code generating sources or resources
      */
     protected void doFiler(ServicesToProcess services) {
         ActivatorCreatorCodeGen codeGen = ActivatorCreatorDefault.createActivatorCreatorCodeGen(services).orElse(null);
@@ -257,8 +265,28 @@ protected void doFiler(ServicesToProcess services) {
                 .build();
         ActivatorCreatorRequest req = ActivatorCreatorDefault
                 .createActivatorCreatorRequest(services, codeGen, configOptions, creator.filer(), false);
+        Set<TypeName> allActivatorTypeNames = tracker.remainingTypeNames().stream()
+                .map(TypeNameDefault::createFromTypeName)
+                .collect(Collectors.toSet());
+        if (!allActivatorTypeNames.isEmpty()) {
+            req = ActivatorCreatorRequestDefault.toBuilder(req)
+                    .codeGen(ActivatorCreatorCodeGenDefault.toBuilder(req.codeGen())
+                                     .allModuleActivatorTypeNames(allActivatorTypeNames)
+                                     .build())
+                    .build();
+        }
         ActivatorCreatorResponse res = creator.createModuleActivators(req);
-        if (!res.success()) {
+        if (res.success()) {
+            res.activatorTypeNamesPutInComponentModule()
+                    .forEach(it -> tracker.processing(it.name()));
+            if (processingOver) {
+                try {
+                    tracker.close();
+                } catch (IOException e) {
+                    throw new ToolsException(e.getMessage(), e);
+                }
+            }
+        } else {
             ToolsException exc = new ToolsException("Error during codegen", res.error().orElse(null));
             utils().error(exc.getMessage(), exc);
             // should not get here since the error above should halt further processing
@@ -504,14 +532,11 @@ private void gatherContracts(Set<TypeName> contracts,
             if (fqProviderTypeName != null) {
                 if (!genericTypeName.generic()) {
                     providerForSet.add(genericTypeName);
-
-                    Optional<String> moduleName = filterModuleName(typeInfo.moduleNameOf(genericTypeName));
-                    moduleName.ifPresent(externalModuleNamesRequired::add);
-                    if (moduleName.isPresent()) {
-                        externalContracts.add(genericTypeName);
-                    } else {
-                        contracts.add(genericTypeName);
-                    }
+                    extractModuleAndContract(contracts,
+                                             externalContracts,
+                                             externalModuleNamesRequired,
+                                             typeInfo,
+                                             genericTypeName);
                 }
 
                 // if we are dealing with a Provider<> then we should add those too as module dependencies
@@ -529,13 +554,11 @@ private void gatherContracts(Set<TypeName> contracts,
                         || !isTypeAnInterface
                         || AnnotationAndValueDefault.findFirst(Contract.class, typeInfo.annotations()).isPresent();
                 if (isTypeAContract) {
-                    Optional<String> moduleName = filterModuleName(typeInfo.moduleNameOf(genericTypeName));
-                    moduleName.ifPresent(externalModuleNamesRequired::add);
-                    if (moduleName.isPresent()) {
-                        externalContracts.add(genericTypeName);
-                    } else {
-                        contracts.add(genericTypeName);
-                    }
+                    extractModuleAndContract(contracts,
+                                             externalContracts,
+                                             externalModuleNamesRequired,
+                                             typeInfo,
+                                             genericTypeName);
                 }
             }
         }
@@ -574,6 +597,20 @@ private void gatherContracts(Set<TypeName> contracts,
                                                                    true));
     }
 
+    private void extractModuleAndContract(Set<TypeName> contracts,
+                                          Set<TypeName> externalContracts,
+                                          Set<String> externalModuleNamesRequired,
+                                          TypeInfo typeInfo,
+                                          TypeName genericTypeName) {
+        Optional<String> moduleName = filterModuleName(typeInfo.moduleNameOf(genericTypeName));
+        moduleName.ifPresent(externalModuleNamesRequired::add);
+        if (moduleName.isPresent() || isBuiltInJavaType(genericTypeName)) {
+            externalContracts.add(genericTypeName);
+        } else {
+            contracts.add(genericTypeName);
+        }
+    }
+
     private Optional<String> filterModuleName(Optional<String> moduleName) {
         String name = moduleName.orElse(null);
         if (name != null && (name.startsWith("java.") || name.startsWith("jdk"))) {
@@ -727,4 +764,8 @@ private void gatherTypeInfosToProcessInThisModule(Map<TypeName, TypeInfo> result
         });
     }
 
+    private Path trackerStatePath() {
+        return scratchClassOutputPath(targetClassOutputPath(processingEnv.getFiler())).resolve(DEFAULT_SCRATCH_FILE_NAME);
+    }
+
 }

pico/processor/src/main/java/io/helidon/pico/processor/ProcessingTracker.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * 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
+ *
+ *     http://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 io.helidon.pico.processor;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.element.TypeElement;
+
+import io.helidon.pico.tools.ToolsException;
+
+/**
+ * This class adds persistent tracking (typically under ./target/XXX) to allow seamless full and/or incremental processing of
+ * types to be tracked over repeated compilation cycles over time. It is expected to be integrated into a host annotation
+ * processor implementation.
+ * <p>
+ * For example, when incremental processing occurs, the elements passed to process in all rounds will just be a subset of
+ * all of the annotated services since the compiler (from the IDE) only recompiles the files that have been changed. This is
+ * typically different from how maven invokes compilation (doing a full compile where all types will be seen in the round). The
+ * {@link PicoAnnotationProcessor}, for example, would see this reduced subset of types in the round and would otherwise have
+ * created a {@link io.helidon.pico.api.ModuleComponent} only representative of the reduced subset of classes. This would be
+ * incorrect and lead to an invalid module component source file to have been generated.
+ * <p>
+ * We use this tracker to persist the list of generated activators much in the same way that
+ * {@code META-INF/services} are tracked. A target scratch directory (i.e., target/pico in this case) is used instead - in order
+ * to keep it out of the build jar.
+ * <p>
+ * Usage:
+ * <ol>
+ *     <li>{@link #initializeFrom} - during the APT initialization phase</li>
+ *     <li>{@link #processing(String)} - during each processed type that the annotation processor visits in the round</li>
+ *     <li>{@link #removedTypeNames()} or {@link #remainingTypeNames()} as needed - to see the changes over time</li>
+ *     <li>{@link #close()} - during final lifecycle of the APT in order to persist state to be (re)written out to disk</li>
+ * </ol>
+ *
+ * @see PicoAnnotationProcessor
+ */
+class ProcessingTracker implements AutoCloseable {
+    static final String DEFAULT_SCRATCH_FILE_NAME = "activators.lst";
+
+    private final Path path;
+    private final Set<String> allTypeNames;
+    private final TypeElementFinder typeElementFinder;
+    private final Set<String> foundOrProcessed = new LinkedHashSet<>();
+
+    /**
+     * Creates an instance using the given path to keep persistent state.
+     *
+     * @param persistentScratchPath the fully qualified path to carry the state
+     * @param allLines all lines read at initialization
+     * @param typeElementFinder the type element finder (e.g., {@link ProcessingEnvironment#getElementUtils})
+     */
+    ProcessingTracker(Path persistentScratchPath,
+                      List<String> allLines,
+                      TypeElementFinder typeElementFinder) {
+        this.path = persistentScratchPath;
+        this.allTypeNames = new LinkedHashSet<>(allLines);
+        this.typeElementFinder = typeElementFinder;
+    }
+
+    public static ProcessingTracker initializeFrom(Path persistentScratchPath,
+                                                   ProcessingEnvironment processingEnv) {
+        List<String> allLines = List.of();
+        File file = persistentScratchPath.toFile();
+        if (file.exists() && file.canRead()) {
+            try {
+                allLines = Files.readAllLines(persistentScratchPath, StandardCharsets.UTF_8);
+            } catch (IOException e) {
+                throw new ToolsException(e.getMessage(), e);
+            }
+        }
+        return new ProcessingTracker(persistentScratchPath, allLines, toTypeElementFinder(processingEnv));
+    }
+
+    public ProcessingTracker processing(String typeName) {
+        foundOrProcessed.add(Objects.requireNonNull(typeName));
+        return this;
+    }
+
+    public Set<String> allTypeNamesFromInitialization() {
+        return allTypeNames;
+    }
+
+    public Set<String> removedTypeNames() {
+        Set<String> typeNames = new LinkedHashSet<>(allTypeNamesFromInitialization());
+        typeNames.removeAll(remainingTypeNames());
+        return typeNames;
+    }
+
+    public Set<String> remainingTypeNames() {
+        Set<String> typeNames = new LinkedHashSet<>(allTypeNamesFromInitialization());
+        typeNames.addAll(foundOrProcessed);
+        typeNames.removeIf(typeName -> !found(typeName));
+        return typeNames;
+    }
+
+    @Override
+    public void close() throws IOException {
+        Path parent = path.getParent();
+        if (parent == null) {
+            throw new ToolsException("bad path: " + path);
+        }
+        Files.createDirectories(parent);
+        Files.write(path, remainingTypeNames(), StandardCharsets.UTF_8);
+    }
+
+    private boolean found(String typeName) {
+        return (typeElementFinder.apply(typeName) != null);
+    }
+
+    private static TypeElementFinder toTypeElementFinder(ProcessingEnvironment processingEnv) {
+        return typeName -> processingEnv.getElementUtils().getTypeElement(typeName);
+    }
+
+    @FunctionalInterface
+    interface TypeElementFinder extends Function<CharSequence, TypeElement> {
+    }
+
+}