During declarative recipe loading filter out duplicate copies of singleton recipes. (#7468)

I had proposed something very similar to this before, here: #6560

That didn't get merged because it felt too magical / special case for DeclarativeRecipe to deduplicate as a silent implementation detail. Instead of doing this the first time we decided on creating Singleton.

But with Singleton alone I cannot find a way to guard the scanning phase without breaking ScanningRecipe.generate(). I tried several things. But a common usage for scanning recipes is to verify whether a file already exists, if I skip those then generate() recipes will create files they shouldn't because their scanner was skipped by Singleton.

So I'm resurrecting the declarative recipe duplicate loading skip. But this time I'm keying off the presence of Singleton, which explicitly opts-in a recipe to desiring this behavior.

Author: sambsnyd

rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java

@@ -21,6 +21,7 @@
 import org.intellij.lang.annotations.Language;
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.*;
+import org.openrewrite.internal.ListUtils;
 
 import java.net.URI;
 import java.time.Duration;
@@ -503,17 +504,87 @@ public List<Recipe> getPreconditions() {
 
     @Override
     public final List<Recipe> getRecipeList() {
+        List<Recipe> filtered = deduplicateSingletonsRecursively(recipeList, new HashSet<>());
+
         if (preconditions.isEmpty()) {
-            return recipeList;
+            return filtered;
         }
 
         PreconditionBellwether bellwether = new PreconditionBellwether(this);
-        List<Recipe> recipeListWithBellwether = new ArrayList<>(recipeList.size() + 1);
+        List<Recipe> recipeListWithBellwether = new ArrayList<>(filtered.size() + 1);
         recipeListWithBellwether.add(bellwether);
-        recipeListWithBellwether.addAll(decorateWithPreconditionBellwether(bellwether, recipeList));
+        recipeListWithBellwether.addAll(decorateWithPreconditionBellwether(bellwether, filtered));
         return recipeListWithBellwether;
     }
 
+    /**
+     * Walk {@code recipes} and every {@link DeclarativeRecipe} reachable through it, dropping
+     * later occurrences of any DeclarativeRecipe (by name) whose preconditions include
+     * {@link Singleton}. The {@code seen} set is threaded through the entire traversal so a
+     * Singleton-gated recipe that appears under one branch is also filtered from sibling or
+     * nested branches. The {@code Singleton} precondition would make the later occurrences
+     * no-op at runtime; removing them from the list avoids scheduling them at all.
+     */
+    private static List<Recipe> deduplicateSingletonsRecursively(List<Recipe> recipes, Set<String> seen) {
+        return ListUtils.map(recipes, recipe -> {
+            if (recipe instanceof DeclarativeRecipe) {
+                DeclarativeRecipe dr = (DeclarativeRecipe) recipe;
+                if (hasSingletonPrecondition(dr) && !seen.add(dr.getName())) {
+                    //noinspection DataFlowIssue
+                    return null;
+                }
+                return dr.withDeduplicatedChildren(seen);
+            }
+            return recipe;
+        });
+    }
+
+    /**
+     * Return a view of this recipe whose {@code recipeList} has been recursively filtered to
+     * remove Singleton-gated duplicates using the shared {@code seen} set. If nothing changes
+     * the original instance is returned; otherwise a copy is produced so the original tree is
+     * left untouched.
+     */
+    DeclarativeRecipe withDeduplicatedChildren(Set<String> seen) {
+        List<Recipe> deduplicated = deduplicateSingletonsRecursively(recipeList, seen);
+        if (deduplicated == recipeList) {
+            return this;
+        }
+        return copyWithRecipeList(this, deduplicated);
+    }
+
+    /**
+     * Factory used by {@link #withDeduplicatedChildren(Set)} to produce an otherwise identical
+     * recipe whose {@code recipeList} has been filtered. Shares immutable (or effectively
+     * immutable) state — {@code preconditions}, {@code validation}, {@code initValidation} —
+     * with {@code source} and leaves {@code uninitializedRecipes} empty so {@link #validate()}
+     * doesn't reject the filtered list as "uninitialized". The new instance's
+     * {@link ScanningRecipe} field initializer gives it a fresh {@code recipeAccMessage}.
+     * <p>
+     * Implemented as a static factory rather than an additional constructor so that Jackson
+     * can still unambiguously pick the Lombok-generated required-args constructor during
+     * recipe deserialization.
+     */
+    private static DeclarativeRecipe copyWithRecipeList(DeclarativeRecipe source, List<Recipe> recipeList) {
+        DeclarativeRecipe copy = new DeclarativeRecipe(source.name, source.displayName,
+                source.description, source.tags, source.estimatedEffortPerOccurrence,
+                source.source, source.causesAnotherCycle, source.maintainers);
+        copy.recipeList = recipeList;
+        copy.preconditions = source.preconditions;
+        copy.validation = source.validation;
+        copy.initValidation = source.initValidation;
+        return copy;
+    }
+
+    private static boolean hasSingletonPrecondition(DeclarativeRecipe recipe) {
+        for (Recipe precondition : recipe.getPreconditions()) {
+            if (precondition instanceof Singleton) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     @SuppressWarnings({"rawtypes", "unchecked"})
     private TreeVisitor<?, ExecutionContext> orVisitors(Recipe recipe, Cursor rootCursor, ExecutionContext ctx) {
         List<TreeVisitor<?, ExecutionContext>> conditions = new ArrayList<>();

rewrite-core/src/test/java/org/openrewrite/SingletonTest.java

@@ -29,7 +29,9 @@
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.openrewrite.Singleton.singleton;
 import static org.openrewrite.test.SourceSpecs.text;
 
@@ -240,4 +242,112 @@ void yamlRecipeWithSingletonPreconditionAcrossDifferentYamlFiles() {
           )
         );
     }
+
+    /**
+     * A scanning recipe whose scanner increments a static counter for each visited file.
+     * Used to prove that when it is used behind a Singleton precondition, the scanner
+     * runs once per file for the first recipe instance and is skipped entirely for later
+     * equivalent instances.
+     */
+    @EqualsAndHashCode(callSuper = false)
+    @Value
+    static class CountingScanRecipe extends ScanningRecipe<AtomicInteger> {
+        static final AtomicInteger SCAN_COUNT = new AtomicInteger();
+
+        @Override
+        public String getDisplayName() {
+            return "Counting scan recipe";
+        }
+
+        @Override
+        public String getDescription() {
+            return "Tracks how many files its scanner visits via a shared static counter.";
+        }
+
+        @Override
+        public AtomicInteger getInitialValue(ExecutionContext ctx) {
+            return SCAN_COUNT;
+        }
+
+        @Override
+        public TreeVisitor<?, ExecutionContext> getScanner(AtomicInteger acc) {
+            return new TreeVisitor<>() {
+                @Override
+                public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                    SCAN_COUNT.incrementAndGet();
+                    return tree;
+                }
+            };
+        }
+    }
+
+    @Test
+    void singletonPreconditionDeduplicatesAcrossNestingLevels() {
+        CountingScanRecipe.SCAN_COUNT.set(0);
+
+        // ChildWithCountingScanner appears twice: once at the top level of ParentRecipe, and once
+        // nested inside NestedWrapper. Recursive deduplication should keep the first occurrence
+        // and drop the nested one.
+        String childYaml = """
+          ---
+          type: specs.openrewrite.org/v1beta/recipe
+          name: org.openrewrite.ChildWithCountingScanner
+          displayName: Child recipe with counting scanner behind Singleton
+          description: Uses Singleton precondition so scanning is skipped for duplicate instances.
+          preconditions:
+            - org.openrewrite.Singleton
+          recipeList:
+            - org.openrewrite.SingletonTest$CountingScanRecipe
+          """;
+
+        String nestedWrapperYaml = """
+          ---
+          type: specs.openrewrite.org/v1beta/recipe
+          name: org.openrewrite.NestedWrapper
+          displayName: Wrapper that also pulls in the counting child
+          description: Embeds the counting child under a layer of nesting.
+          recipeList:
+            - org.openrewrite.ChildWithCountingScanner
+          """;
+
+        String parentYaml = """
+          ---
+          type: specs.openrewrite.org/v1beta/recipe
+          name: org.openrewrite.ParentRecipe
+          displayName: Parent recipe that references the counting child at two different depths
+          description: Forces the Singleton-gated child to appear at different levels of the tree.
+          recipeList:
+            - org.openrewrite.ChildWithCountingScanner
+            - org.openrewrite.NestedWrapper
+          """;
+
+        Recipe recipe = Environment.builder()
+          .load(new YamlResourceLoader(
+            new ByteArrayInputStream(childYaml.getBytes(StandardCharsets.UTF_8)),
+            URI.create("child.yml"),
+            new Properties()))
+          .load(new YamlResourceLoader(
+            new ByteArrayInputStream(nestedWrapperYaml.getBytes(StandardCharsets.UTF_8)),
+            URI.create("nested-wrapper.yml"),
+            new Properties()))
+          .load(new YamlResourceLoader(
+            new ByteArrayInputStream(parentYaml.getBytes(StandardCharsets.UTF_8)),
+            URI.create("parent.yml"),
+            new Properties()))
+          .build()
+          .activateRecipes("org.openrewrite.ParentRecipe");
+
+        rewriteRun(
+          spec -> spec
+            .cycles(1)
+            .expectedCyclesThatMakeChanges(0)
+            .recipe(recipe)
+            .validateRecipeSerialization(false)
+            .afterRecipe(run -> assertThat(CountingScanRecipe.SCAN_COUNT.get())
+              .as("scanner should run once per file exactly once across the whole tree; nested occurrence must be filtered by recursive singleton deduplication")
+              .isEqualTo(2)),
+          text("one", spec -> spec.path("a.txt")),
+          text("two", spec -> spec.path("b.txt"))
+        );
+    }
 }