fix: Dont generate the same file multiple times & fix CreateTextFile (#…

…4356) * add failing test * fix recipe and RecipeRunCycle to not allow generate to overwrite * use TreeSet instead of manual checking each time * put generated back into a list to avoid any additional unchecked cast issues
openrewrite · Jul 25, 2024 · 84f5858 · 84f5858
1 parent 70b904b
commit 84f5858
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 9 deletions.
diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/InMemoryLargeSourceSet.java b/rewrite-core/src/main/java/org/openrewrite/internal/InMemoryLargeSourceSet.java
@@ -44,8 +44,8 @@ public InMemoryLargeSourceSet(List<SourceFile> ls) {
     }
 
     protected InMemoryLargeSourceSet(@Nullable InMemoryLargeSourceSet initialState,
-                                   @Nullable Map<SourceFile, List<Recipe>> deletions,
-                                   List<SourceFile> ls) {
+                                     @Nullable Map<SourceFile, List<Recipe>> deletions,
+                                     List<SourceFile> ls) {
         this.initialState = initialState;
         this.ls = ls;
         this.deletions = deletions;

diff --git a/rewrite-core/src/main/java/org/openrewrite/scheduling/RecipeRunCycle.java b/rewrite-core/src/main/java/org/openrewrite/scheduling/RecipeRunCycle.java
@@ -36,7 +36,7 @@
 import java.util.function.BiFunction;
 import java.util.function.UnaryOperator;
 
-import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableSet;
 import static java.util.Objects.requireNonNull;
 import static org.openrewrite.Recipe.PANIC;
 
@@ -105,23 +105,23 @@ public LSS scanSources(LSS sourceSet) {
     }
 
     public LSS generateSources(LSS sourceSet) {
-        List<SourceFile> generatedInThisCycle = allRecipeStack.reduce(sourceSet, recipe, ctx, (acc, recipeStack) -> {
+        Set<SourceFile> generatedInThisCycle = allRecipeStack.reduce(sourceSet, recipe, ctx, (acc, recipeStack) -> {
             Recipe recipe = recipeStack.peek();
             if (recipe instanceof ScanningRecipe) {
                 //noinspection unchecked
                 ScanningRecipe<Object> scanningRecipe = (ScanningRecipe<Object>) recipe;
-                List<SourceFile> generated = new ArrayList<>(scanningRecipe.generate(scanningRecipe.getAccumulator(rootCursor, ctx), unmodifiableList(acc), ctx));
+                List<SourceFile> generated = new ArrayList<>(scanningRecipe.generate(scanningRecipe.getAccumulator(rootCursor, ctx), unmodifiableSet(acc), ctx));
                 generated.replaceAll(source -> addRecipesThatMadeChanges(recipeStack, source));
                 acc.addAll(generated);
                 if (!generated.isEmpty()) {
                     madeChangesInThisCycle.add(recipe);
                 }
             }
             return acc;
-        }, new ArrayList<>());
+        }, new TreeSet<>(Comparator.comparing(SourceFile::getSourcePath)));
 
         // noinspection unchecked
-        return (LSS) sourceSet.generate(generatedInThisCycle);
+        return (LSS) sourceSet.generate(new ArrayList<>(generatedInThisCycle));
     }
 
     public LSS editSources(LSS sourceSet) {
@@ -238,7 +238,7 @@ private void recordSourceFileResult(@Nullable String beforePath, @Nullable Strin
     }
 
     private @Nullable SourceFile handleError(Recipe recipe, SourceFile sourceFile, @Nullable SourceFile after,
-                                   Throwable t) {
+                                             Throwable t) {
         ctx.getOnError().accept(t);
 
         if (t instanceof RecipeRunException) {

diff --git a/rewrite-core/src/main/java/org/openrewrite/text/CreateTextFile.java b/rewrite-core/src/main/java/org/openrewrite/text/CreateTextFile.java
@@ -86,7 +86,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(AtomicBoolean created) {
             @Override
             public SourceFile visit(@Nullable Tree tree, ExecutionContext ctx) {
                 SourceFile sourceFile = (SourceFile) requireNonNull(tree);
-                if ((created.get() || Boolean.TRUE.equals(overwriteExisting)) && path.equals(sourceFile.getSourcePath())) {
+                if (Boolean.TRUE.equals(overwriteExisting) && path.equals(sourceFile.getSourcePath())) {
                     if (sourceFile instanceof PlainText) {
                         return ((PlainText) sourceFile).withText(fileContents);
                     }

diff --git a/rewrite-test/src/test/java/org/openrewrite/text/CreateTextFileTest.java b/rewrite-test/src/test/java/org/openrewrite/text/CreateTextFileTest.java
@@ -21,6 +21,8 @@
 import org.openrewrite.Issue;
 import org.openrewrite.test.RewriteTest;
 
+import java.nio.file.Path;
+
 import static org.openrewrite.groovy.Assertions.groovy;
 import static org.openrewrite.test.SourceSpecs.text;
 
@@ -128,4 +130,29 @@ void shouldOverrideDifferentSourceFileType() {
           )
         );
     }
+
+    @Test
+    void shouldNotOverrideIfCreatedInSameRun() {
+        rewriteRun(
+          spec -> spec.recipeFromYaml("""
+            ---
+            type: specs.openrewrite.org/v1beta/recipe
+            name: org.openrewrite.test.CreateTextFileDuplication
+            displayName: Create Text File test recipe
+            description: Create Text File test recipe.
+            recipeList:
+              - org.openrewrite.text.CreateTextFile:
+                  relativeFileName: duplicate.txt
+                  overwriteExisting: false
+                  fileContents: |
+                    hi
+              - org.openrewrite.text.CreateTextFile:
+                  relativeFileName: duplicate.txt
+                  overwriteExisting: false
+                  fileContents: |
+                    hello
+            """, "org.openrewrite.test.CreateTextFileDuplication"),
+          text(null, "hi", spec -> spec.path(Path.of("duplicate.txt")))
+        );
+    }
 }