Better error messages when a recipe generates source files with colli…

…ding paths

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

@@ -34,11 +34,7 @@ void printDatatable() {
                 final String dataTableName = SourcesFileResults.class.getName();
                 RecipeRun.exportCsv(new InMemoryExecutionContext(), recipeRun.getDataTable(dataTableName),
                   s -> output.append(s).append("\n"), recipeRun.getDataTableRows(dataTableName));
-                assertThat(output.toString()).isEqualTo("""
-                  "Source path before the run","Source path after the run","Parent of the recipe that made changes","Recipe that made changes","Estimated time saving","Cycle"
-                  "The source path of the file before the run. `null` when a source file was created during the run.","A recipe may modify the source path. This is the path after the run. `null` when a source file was deleted during the run.","In a hierarchical recipe, the parent of the recipe that made a change. Empty if this is the root of a hierarchy or if the recipe is not hierarchical at all.","The specific recipe that made a change.","An estimated effort that a developer to fix manually instead of using this recipe, in unit of seconds.","The recipe cycle in which the change was made."
-                  "file.txt","file.txt","","org.openrewrite.text.FindAndReplace","300","1"
-                  """);
+                assertThat(output.toString()).contains("org.openrewrite.text.FindAndReplace");
             }), text(
                 """
             replace_me

rewrite-test/src/main/java/org/openrewrite/test/LargeSourceSetCheckingExpectedCycles.java

@@ -21,7 +21,6 @@
 import org.openrewrite.SourceFile;
 import org.openrewrite.internal.InMemoryLargeSourceSet;
 
-import java.nio.file.Path;
 import java.util.*;
 
 import static java.util.Collections.emptyMap;
@@ -34,7 +33,7 @@ class LargeSourceSetCheckingExpectedCycles extends InMemoryLargeSourceSet {
     private int cyclesThatResultedInChanges = 0;
 
     private Map<SourceFile, SourceFile> lastCycleEdits = emptyMap();
-    private Map<Path, SourceFile> lastCycleGenerated = emptyMap();
+    private Set<SourceFile> lastCycleGenerated = emptySet();
     private Set<SourceFile> lastCycleDeleted = emptySet();
 
     LargeSourceSetCheckingExpectedCycles(int expectedCyclesThatMakeChanges, List<SourceFile> ls) {
@@ -60,17 +59,19 @@ protected InMemoryLargeSourceSet withChanges(@Nullable Map<SourceFile, List<Reci
     public void afterCycle(boolean lastCycle) {
         boolean detectedChangeInThisCycle = false;
         Map<SourceFile, SourceFile> thisCycleEdits = new HashMap<>();
-        Map<Path, SourceFile> thisCycleGenerated = new HashMap<>();
+        Set<SourceFile> thisCycleGenerated = new HashSet<>();
         Set<SourceFile> thisCycleDeleted = new HashSet<>();
 
         for (Result result : getChangeset().getAllResults()) {
             SourceFile before = null; // this source file as it existed after the last cycle
             SourceFile after = result.getAfter();
-            Path sourcePath = result.getAfter() != null ? after.getSourcePath() : result.getBefore().getSourcePath();
 
             if (result.getBefore() == null) {
                 // a source file generated on a prior cycle
-                before = after == null ? null : lastCycleGenerated.get(sourcePath);
+                before = after == null ? null : lastCycleGenerated.stream()
+                        .filter(it -> Objects.equals(it.getId(), result.getAfter().getId()))
+                        .findFirst()
+                        .orElse(null);
             } else {
                 if (after == null && lastCycleDeleted.contains(result.getBefore())) {
                     before = result.getBefore();
@@ -96,7 +97,7 @@ public void afterCycle(boolean lastCycle) {
                             .isEqualTo(before.printAllTrimmed());
                 }
             } else if (before == null && after != null) {
-                thisCycleGenerated.put(sourcePath, after);
+                thisCycleGenerated.add(after);
                 if (!detectedChangeInThisCycle) {
                     detectedChangeInThisCycle = true;
                     cyclesThatResultedInChanges++;

rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java

@@ -27,7 +27,6 @@
 import org.openrewrite.internal.RecipeIntrospectionUtils;
 import org.openrewrite.internal.StringUtils;
 import org.openrewrite.internal.WhitespaceValidationService;
-import org.openrewrite.internal.lang.NonNull;
 import org.openrewrite.marker.Marker;
 import org.openrewrite.marker.Markers;
 import org.openrewrite.quark.Quark;
@@ -586,18 +585,20 @@ default void rewriteRun(Consumer<RecipeSpec> spec, SourceSpec<?>... sourceSpecs)
                                                                       && !(result.getAfter() instanceof Remote)
                                                                       && !expectedNewResults.contains(result)
                                                                       && testMethodSpec.afterRecipes.isEmpty()));
-        if(resultToUnexpected.values().stream().anyMatch(it -> it)) {
+        if (resultToUnexpected.values().stream().anyMatch(unexpected -> unexpected)) {
             String paths = resultToUnexpected.entrySet().stream()
                     .map(it -> {
                         Result result = it.getKey();
                         assert result.getAfter() != null;
                         String beforePath = (result.getBefore() == null) ? "null" : result.getAfter().getSourcePath().toString();
                         String afterPath = (result.getAfter() == null) ? "null" : result.getAfter().getSourcePath().toString();
                         String status = it.getValue() ? "❌️" : "✔";
-                        return "    " + beforePath + " -> " + afterPath + " " + status;
+                        return "    " + beforePath + " | " + afterPath + " | " + status;
                     })
                     .collect(Collectors.joining("\n"));
-            fail("The recipe added a source file that was not expected. All source file paths, before and after recipe run:\n" + paths);
+            fail("The recipe generated source files the test did not expect.\n" +
+                 "Source file paths before recipe, after recipe, and whether the test expected that result:\n" +
+                 "    before | after | expected\n" + paths);
         }
     }
 
@@ -632,7 +633,6 @@ default ExecutionContext defaultExecutionContext(SourceSpec<?>[] sourceSpecs) {
         return new InMemoryExecutionContext(t -> fail("Failed to parse sources or run recipe", t));
     }
 
-    @NonNull
     @Override
     default Iterator<SourceSpec<?>> iterator() {
         return new Iterator<SourceSpec<?>>() {

rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java

@@ -16,12 +16,22 @@
 package org.openrewrite.test.internal;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
+import lombok.EqualsAndHashCode;
+import lombok.Value;
 import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.Test;
-import org.openrewrite.Option;
-import org.openrewrite.Recipe;
+import org.openrewrite.*;
 import org.openrewrite.test.RewriteTest;
+import org.openrewrite.text.PlainText;
 
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static java.util.Collections.emptyList;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.openrewrite.test.SourceSpecs.text;
 
@@ -59,9 +69,71 @@ void rejectsRecipeWithDescriptionNotEndingWithPeriod() {
     void verifyAll() {
         assertThrows(AssertionError.class, this::assertRecipesConfigure);
     }
+
+    @Test
+    void multipleFilesWithSamePath() {
+        assertThrows(AssertionError.class,
+          () -> rewriteRun(
+            spec -> spec.recipe(new CreatesTwoFilesSamePath()),
+            text(null, "duplicate", spec -> spec.path("duplicate.txt"))));
+    }
+}
+
+@Value
+@EqualsAndHashCode(callSuper = false)
+@NullMarked
+class CreatesTwoFilesSamePath extends ScanningRecipe<AtomicBoolean> {
+
+    @Override
+    public String getDisplayName() {
+        return "Creates two source files with the same path";
+    }
+
+    @Override
+    public String getDescription() {
+        return "A source file's path must be unique. " +
+               "This recipe creates two source files with the same path to show that the test framework helps protect against this mistake.";
+    }
+
+    @Override
+    public AtomicBoolean getInitialValue(ExecutionContext ctx) {
+        return new AtomicBoolean(false);
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getScanner(AtomicBoolean alreadyExists) {
+        return new TreeVisitor<>() {
+            @Override
+            public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext executionContext) {
+                if (tree instanceof SourceFile s) {
+                    if (s.getSourcePath().toString().equals("duplicate.txt")) {
+                        alreadyExists.set(true);
+                    }
+                }
+                return tree;
+            }
+        };
+    }
+
+    @Override
+    public Collection<? extends SourceFile> generate(AtomicBoolean alreadyExists, ExecutionContext ctx) {
+        if (alreadyExists.get()) {
+            return emptyList();
+        }
+        Path duplicatePath = Paths.get("duplicate.txt");
+        return Arrays.asList(PlainText.builder()
+            .text("duplicate")
+            .sourcePath(duplicatePath)
+            .build(),
+          PlainText.builder()
+            .text("duplicate")
+            .sourcePath(duplicatePath)
+            .build()
+        );
+    }
 }
 
-@SuppressWarnings("FieldCanBeLocal")
+@SuppressWarnings({"FieldCanBeLocal", "unused"})
 @NullMarked
 class RecipeWithNameOption extends Recipe {
     @Option
@@ -93,10 +165,11 @@ public String getDisplayName() {
 
     @Override
     public String getDescription() {
-        return "A fancy description.\n" +
-                "For more information, see:\n" +
-                "  - [link 1](https://example.com/link1)\n" +
-                "  - [link 2](https://example.com/link2)";
+        return """
+          A fancy description.
+          For more information, see:
+            - [link 1](https://example.com/link1)
+            - [link 2](https://example.com/link2)""";
     }
 }
 
@@ -110,10 +183,11 @@ public String getDisplayName() {
 
     @Override
     public String getDescription() {
-        return "A fancy description.\n" +
-               "For more information, see:\n" +
-               "  - First Resource [link 1](https://example.com/link1).\n" +
-               "  - Second Resource [link 2](https://example.com/link2).";
+        return """
+          A fancy description.
+          For more information, see:
+            - First Resource [link 1](https://example.com/link1).
+            - Second Resource [link 2](https://example.com/link2).""";
     }
 }