From 84f58587f8b91e06765cb9e170b85652d2577edf Mon Sep 17 00:00:00 2001 From: Peter Streef Date: Thu, 25 Jul 2024 21:18:00 +0200 Subject: [PATCH] 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 --- .../internal/InMemoryLargeSourceSet.java | 4 +-- .../scheduling/RecipeRunCycle.java | 12 ++++----- .../org/openrewrite/text/CreateTextFile.java | 2 +- .../openrewrite/text/CreateTextFileTest.java | 27 +++++++++++++++++++ 4 files changed, 36 insertions(+), 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 index 07dea5eee39..1e3d53db3a9 100644 --- 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 ls) { } protected InMemoryLargeSourceSet(@Nullable InMemoryLargeSourceSet initialState, - @Nullable Map> deletions, - List ls) { + @Nullable Map> deletions, + List 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 index d08b85bb17d..6160d74344c 100644 --- 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,12 +105,12 @@ public LSS scanSources(LSS sourceSet) { } public LSS generateSources(LSS sourceSet) { - List generatedInThisCycle = allRecipeStack.reduce(sourceSet, recipe, ctx, (acc, recipeStack) -> { + Set generatedInThisCycle = allRecipeStack.reduce(sourceSet, recipe, ctx, (acc, recipeStack) -> { Recipe recipe = recipeStack.peek(); if (recipe instanceof ScanningRecipe) { //noinspection unchecked ScanningRecipe scanningRecipe = (ScanningRecipe) recipe; - List generated = new ArrayList<>(scanningRecipe.generate(scanningRecipe.getAccumulator(rootCursor, ctx), unmodifiableList(acc), ctx)); + List generated = new ArrayList<>(scanningRecipe.generate(scanningRecipe.getAccumulator(rootCursor, ctx), unmodifiableSet(acc), ctx)); generated.replaceAll(source -> addRecipesThatMadeChanges(recipeStack, source)); acc.addAll(generated); if (!generated.isEmpty()) { @@ -118,10 +118,10 @@ public LSS generateSources(LSS sourceSet) { } } 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 index 46a675afecf..9718096f17c 100644 --- 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 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 index fc1c8fac98b..d707cbb95d2 100644 --- 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"))) + ); + } }