diff --git a/rewrite-core/src/main/java/org/openrewrite/ExecutionContext.java b/rewrite-core/src/main/java/org/openrewrite/ExecutionContext.java index 37ebb28b76f..274c3942dee 100644 --- a/rewrite-core/src/main/java/org/openrewrite/ExecutionContext.java +++ b/rewrite-core/src/main/java/org/openrewrite/ExecutionContext.java @@ -30,6 +30,7 @@ */ public interface ExecutionContext { String CURRENT_RECIPE = "org.openrewrite.currentRecipe"; + String PARENT_RECIPE = "org.openrewrite.parentRecipe"; String UNCAUGHT_EXCEPTION_COUNT = "org.openrewrite.uncaughtExceptionCount"; String DATA_TABLES = "org.openrewrite.dataTables"; @@ -88,9 +89,24 @@ default void putCurrentRecipe(Recipe recipe) { putMessage(CURRENT_RECIPE, recipe); } - default Recipe getCurrentRecipe() { - //noinspection ConstantConditions - return getMessage(CURRENT_RECIPE); + /** + * @return The previous parent recipe. + * @implNote For use in the {@link RecipeScheduler} only. + */ + @Nullable + @Incubating(since = "7.37.0") + default Recipe putParentRecipe(@Nullable Recipe recipe) { + Recipe previousParent = getMessage(PARENT_RECIPE); + putMessage(PARENT_RECIPE, recipe); + return previousParent; + } + + /** + * The recipe that scheduled this recipe's execution via {@link Recipe#getApplicableTests()} or {@link Recipe#getSingleSourceApplicableTests()}. + */ + @Incubating(since = "7.37.0") + default Optional getParentRecipe() { + return Optional.ofNullable(getMessage(PARENT_RECIPE)); } default int incrementAndGetUncaughtExceptionCount() { diff --git a/rewrite-core/src/main/java/org/openrewrite/Recipe.java b/rewrite-core/src/main/java/org/openrewrite/Recipe.java index a8e9713eee7..1a6c893aeba 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Recipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/Recipe.java @@ -18,6 +18,8 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import lombok.EqualsAndHashCode; +import lombok.Value; import org.intellij.lang.annotations.Language; import org.openrewrite.config.DataTableDescriptor; import org.openrewrite.config.RecipeDescriptor; @@ -72,8 +74,8 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { } }; - private transient List> singleSourceApplicableTests; - private transient List> applicableTests; + private transient List singleSourceApplicableTests; + private transient List applicableTests; @Nullable private transient List dataTables; @@ -99,6 +101,40 @@ public TreeVisitor getVisitor() { } } + @Value + @EqualsAndHashCode(callSuper = true) + private static class AdHocRecipe extends Recipe { + @Language("markdown") + String displayName; + @Language("markdown") + String description; + TreeVisitor visitor; + + @Override + public String getDisplayName() { + return displayName; + } + + @Override + public String getDescription() { + return description; + } + + @Override + public TreeVisitor getVisitor() { + return visitor; + } + + @Nullable + static AdHocRecipe fromNullableVisitor( + @Language("markdown") String displayName, + @Language("markdown") String description, + @Nullable TreeVisitor visitor + ) { + return visitor == null ? null : new AdHocRecipe(displayName, description, visitor); + } + } + /** * A human-readable display name for the recipe, initial capped with no period. * For example, "Find text". The display name can be assumed to be rendered in @@ -230,7 +266,7 @@ protected TreeVisitor getApplicableTest() { * particular source file. If multiple applicable tests configured, the final result of the applicable test depends * on all conditions being met, that is, a logical 'AND' relationship. *

- * To identify a {@link SourceFile} as applicable, the visitor should mark or change it at any level. Any mutation + * To identify a {@link SourceFile} as applicable, the {@link TreeVisitor} should mark or change it at any level. Any mutation * that the applicability test visitor makes on the tree will not be included in the results. *

* @@ -238,6 +274,25 @@ protected TreeVisitor getApplicableTest() { */ @SuppressWarnings("unused") public Recipe addApplicableTest(TreeVisitor test) { + return addApplicableTest(AdHocRecipe.fromNullableVisitor( + "Add applicable test for: " + getDisplayName(), + "Add applicable test for: " + getDescription(), + test + )); + } + + /** + * A recipe can be configured with any number of applicable tests that can be used to determine whether it should run on a + * particular source file. If multiple applicable tests configured, the final result of the applicable test depends + * on all conditions being met, that is, a logical 'AND' relationship. + *

+ * To identify a {@link SourceFile} as applicable, the {@link Recipe} should mark or change it at any level. Any mutation + * that the applicability test recipe makes on the tree will not be included in the results. + *

+ * + * @return This recipe. Not the argument passed. + */ + public Recipe addApplicableTest(Recipe test) { if (applicableTests == null) { applicableTests = new ArrayList<>(1); } @@ -252,8 +307,15 @@ public void addDataTable(DataTable dataTable) { dataTables.add(dataTableDescriptorFromDataTable(dataTable)); } - public List> getApplicableTests() { - List> tests = ListUtils.concat(getApplicableTest(), applicableTests); + public List getApplicableTests() { + List tests = ListUtils.concat( + AdHocRecipe.fromNullableVisitor( + "Applicable test for: " + getDisplayName(), + "Applicable test for: " + getDescription(), + getApplicableTest() + ), + applicableTests + ); return tests == null ? emptyList() : tests; } @@ -276,12 +338,31 @@ protected TreeVisitor getSingleSourceApplicableTest() { * particular source file. If multiple applicable tests configured, the final result of the applicable test depends * on all conditions being met, that is, a logical 'AND' relationship. *

- * To identify a {@link SourceFile} as applicable, the visitor should mark or change it at any level. Any mutation + * To identify a {@link SourceFile} as applicable, the {@link TreeVisitor} should mark or change it at any level. Any mutation * that the applicability test visitor makes on the tree will not be included in the results. * * @return This recipe. */ + @SuppressWarnings("unused") public Recipe addSingleSourceApplicableTest(TreeVisitor test) { + return addSingleSourceApplicableTest(AdHocRecipe.fromNullableVisitor( + "Add single source applicable test for: " + getDisplayName(), + "Add single source applicable test for: " + getDescription(), + test + )); + } + + /** + * A recipe can be configured with any number of applicable tests that can be used to determine whether it should run on a + * particular source file. If multiple applicable tests configured, the final result of the applicable test depends + * on all conditions being met, that is, a logical 'AND' relationship. + *

+ * To identify a {@link SourceFile} as applicable, the {@link Recipe} should mark or change it at any level. Any mutation + * that the applicability test recipe makes on the tree will not be included in the results. + * + * @return This recipe. Not the argument passed. + */ + public Recipe addSingleSourceApplicableTest(Recipe test) { if (singleSourceApplicableTests == null) { singleSourceApplicableTests = new ArrayList<>(1); } @@ -289,8 +370,15 @@ public Recipe addSingleSourceApplicableTest(TreeVisitor tes return this; } - public List> getSingleSourceApplicableTests() { - List> tests = ListUtils.concat(getSingleSourceApplicableTest(), singleSourceApplicableTests); + public List getSingleSourceApplicableTests() { + List tests = ListUtils.concat( + AdHocRecipe.fromNullableVisitor( + "Single Source Applicable test for: " + getDisplayName(), + "Single Source Applicable test for: " + getDescription(), + getSingleSourceApplicableTest() + ), + singleSourceApplicableTests + ); return tests == null ? emptyList() : tests; } diff --git a/rewrite-core/src/main/java/org/openrewrite/RecipeScheduler.java b/rewrite-core/src/main/java/org/openrewrite/RecipeScheduler.java index bd1d7c814c2..89c9261a332 100644 --- a/rewrite-core/src/main/java/org/openrewrite/RecipeScheduler.java +++ b/rewrite-core/src/main/java/org/openrewrite/RecipeScheduler.java @@ -123,7 +123,7 @@ default RecipeRun scheduleRun( return recipeRun.withDataTables(ctx.getMessage(ExecutionContext.DATA_TABLES, emptyMap())); } - List results = createAndProcessResults( + List results = RecipeSchedulerUtils.createAndProcessResults( before, after, ctx, @@ -136,76 +136,6 @@ default RecipeRun scheduleRun( .withDataTables(ctx.getMessage(ExecutionContext.DATA_TABLES, emptyMap())); } - static List createAndProcessResults( - List before, - List after, - ExecutionContext ctx, - Map> recipeThatAddedOrDeletedSourceFile - ) { - Map sourceFileIdentities = new HashMap<>(); - for (SourceFile sourceFile : before) { - sourceFileIdentities.put(sourceFile.getId(), sourceFile); - } - List results = new ArrayList<>(); - - // added or changed files - for (SourceFile s : after) { - SourceFile original = sourceFileIdentities.get(s.getId()); - if (original != s) { - if (original == null) { - results.add(new Result(null, s, singletonList(recipeThatAddedOrDeletedSourceFile.get(s.getId())))); - } else { - if (original.getMarkers().findFirst(Generated.class).isPresent()) { - continue; - } - - results.add(new Result( - original, - s, - s.getMarkers() - .findFirst(RecipesThatMadeChanges.class) - .orElseThrow(() -> new IllegalStateException("SourceFile changed but no recipe " + - "reported making a change")) - .getRecipes() - )); - } - } - } - - Set afterIds = after.stream().map(SourceFile::getId).collect(Collectors.toSet()); - - // removed files - for (SourceFile s : before) { - if (!afterIds.contains(s.getId()) && !s.getMarkers().findFirst(Generated.class).isPresent()) { - results.add(new Result(s, null, singleton(recipeThatAddedOrDeletedSourceFile.get(s.getId())))); - } - } - - // Process the Result and add to the results table - for (Result result : results) { - SourcesFileResults resultsTable = new SourcesFileResults(Recipe.noop()); - Stack recipeStack = new Stack<>(); - - for (RecipeDescriptor rd : result.getRecipeDescriptorsThatMadeChanges()) { - recipeStack.push(new RecipeDescriptor[]{null, rd}); - } - - while (!recipeStack.isEmpty()) { - RecipeDescriptor[] recipeThatMadeChange = recipeStack.pop(); - resultsTable.insertRow(ctx, new SourcesFileResults.Row( - result.getBefore() == null ? "" : result.getBefore().getSourcePath().toString(), - result.getAfter() == null ? "" : result.getAfter().getSourcePath().toString(), - recipeThatMadeChange[0] == null ? "" : recipeThatMadeChange[0].getName(), - recipeThatMadeChange[1].getName() - )); - for (RecipeDescriptor rd : recipeThatMadeChange[1].getRecipeList()) { - recipeStack.push(new RecipeDescriptor[]{recipeThatMadeChange[1], rd}); - } - } - } - return results; - } - default List scheduleVisit( RecipeRunStats runStats, Stack recipeStack, @@ -225,10 +155,11 @@ default List scheduleVisit( } try { - if (!recipe.getApplicableTests().isEmpty()) { + List applicableTests = recipe.getApplicableTests(); + if (!applicableTests.isEmpty()) { boolean anySourceMatch = false; for (S s : before) { - if (RecipeSchedulerUtils.applicableListTests(s, recipe.getApplicableTests(), ctx)) { + if (RecipeSchedulerUtils.testAllApplicableTestsMatchSourceFile(s, applicableTests, runStats, this, recipeStack, ctx)) { anySourceMatch = true; break; } @@ -239,18 +170,25 @@ default List scheduleVisit( } } - if (!recipe.getSingleSourceApplicableTests().isEmpty()) { + List singleSourceApplicableTests = recipe.getSingleSourceApplicableTests(); + if (!singleSourceApplicableTests.isEmpty()) { if (singleSourceApplicableTestResult == null || singleSourceApplicableTestResult.isEmpty()) { if (singleSourceApplicableTestResult == null) { singleSourceApplicableTestResult = new HashMap<>(before.size()); } - - for (S s : before) { - singleSourceApplicableTestResult.put( - s.getId(), - RecipeSchedulerUtils.applicableListTests(s, recipe.getSingleSourceApplicableTests(), ctx) - ); - } + } + for (S s : before) { + singleSourceApplicableTestResult.put( + s.getId(), + RecipeSchedulerUtils.testAllApplicableTestsMatchSourceFile( + s, + singleSourceApplicableTests, + runStats, + this, + recipeStack, + ctx + ) + ); } } } catch (Throwable t) { @@ -466,6 +404,76 @@ default List scheduleVisit( } class RecipeSchedulerUtils { + static List createAndProcessResults( + List before, + List after, + ExecutionContext ctx, + Map> recipeThatAddedOrDeletedSourceFile + ) { + Map sourceFileIdentities = new HashMap<>(); + for (SourceFile sourceFile : before) { + sourceFileIdentities.put(sourceFile.getId(), sourceFile); + } + List results = new ArrayList<>(); + + // added or changed files + for (SourceFile s : after) { + SourceFile original = sourceFileIdentities.get(s.getId()); + if (original != s) { + if (original == null) { + results.add(new Result(null, s, singletonList(recipeThatAddedOrDeletedSourceFile.get(s.getId())))); + } else { + if (original.getMarkers().findFirst(Generated.class).isPresent()) { + continue; + } + + results.add(new Result( + original, + s, + s.getMarkers() + .findFirst(RecipesThatMadeChanges.class) + .orElseThrow(() -> new IllegalStateException("SourceFile changed but no recipe " + + "reported making a change")) + .getRecipes() + )); + } + } + } + + Set afterIds = after.stream().map(SourceFile::getId).collect(Collectors.toSet()); + + // removed files + for (SourceFile s : before) { + if (!afterIds.contains(s.getId()) && !s.getMarkers().findFirst(Generated.class).isPresent()) { + results.add(new Result(s, null, singleton(recipeThatAddedOrDeletedSourceFile.get(s.getId())))); + } + } + + // Process the Result and add to the results table + for (Result result : results) { + SourcesFileResults resultsTable = new SourcesFileResults(Recipe.noop()); + Stack recipeStack = new Stack<>(); + + for (RecipeDescriptor rd : result.getRecipeDescriptorsThatMadeChanges()) { + recipeStack.push(new RecipeDescriptor[]{null, rd}); + } + + while (!recipeStack.isEmpty()) { + RecipeDescriptor[] recipeThatMadeChange = recipeStack.pop(); + resultsTable.insertRow(ctx, new SourcesFileResults.Row( + result.getBefore() == null ? "" : result.getBefore().getSourcePath().toString(), + result.getAfter() == null ? "" : result.getAfter().getSourcePath().toString(), + recipeThatMadeChange[0] == null ? "" : recipeThatMadeChange[0].getName(), + recipeThatMadeChange[1].getName() + )); + for (RecipeDescriptor rd : recipeThatMadeChange[1].getRecipeList()) { + recipeStack.push(new RecipeDescriptor[]{recipeThatMadeChange[1], rd}); + } + } + } + return results; + } + public static S addRecipesThatMadeChanges( Stack recipeStack, S afterFile @@ -519,20 +527,47 @@ public static List handleUncaughtException( /** * @return true if the file qualified (file changed) for all applicable tests. */ - public static boolean applicableListTests( + static boolean testAllApplicableTestsMatchSourceFile( S s, - List> applicableTests, + List applicableTests, + RecipeRunStats runStats, + RecipeScheduler recipeScheduler, + Stack recipeStack, ExecutionContext ctx ) { + List sList = singletonList(s); boolean allMatch = true; - for (TreeVisitor applicableTest : applicableTests) { - boolean noChange = - applicableTest.visitSourceFile(s, ctx) == s - && applicableTest.visit(s, ctx) == s; - if (noChange) { + for (Recipe applicableTest : applicableTests) { + RecipeRunStats nextStats = runStats.addCalledRecipe(applicableTest); + // We still need the stack to be accurate for the applicable test, so that ExecutionContext.getRecipeStack() is correct. + Stack stack = new Stack<>(); + stack.addAll(recipeStack); + stack.push(applicableTest); + Recipe previousParent = ctx.putParentRecipe(recipeStack.peek()); + // Recursively schedule the recipe to visit the applicable tests + List next = recipeScheduler.scheduleVisit( + nextStats, + stack, + sList, + ctx, + null, + new HashMap<>() + ); + ctx.putParentRecipe(previousParent); + if (sList == next) { allMatch = false; break; } + // Re-surface any errors generated applying the applicability tests up to the top level + for (S newS : next) { + newS.getMarkers().findFirst(Markup.Error.class).ifPresent(m -> { + if (m.getException() instanceof RecipeRunException) { + throw (RecipeRunException) m.getException(); + } else { + throw new RuntimeException("Applicable Test Failed", m.getException()); + } + }); + } } return allMatch; } diff --git a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java index e5727f9fb5c..b35f532fd7e 100644 --- a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java @@ -118,10 +118,10 @@ public void initialize(Collection availableRecipes) { private void configureByUse(RecipeUse use, Recipe recipe) { switch(use) { case SingleSourceApplicability: - addSingleSourceApplicableTest(getVisitor(recipe)); + addSingleSourceApplicableTest(recipe); break; case AnySourceApplicability: - addApplicableTest(getVisitor(recipe)); + addApplicableTest(recipe); break; case Recipe: doNext(recipe); @@ -129,17 +129,6 @@ private void configureByUse(RecipeUse use, Recipe recipe) { } } - private TreeVisitor getVisitor(Recipe recipe) { - try { - Method getVisitor = recipe.getClass().getDeclaredMethod("getVisitor"); - getVisitor.setAccessible(true); - //noinspection unchecked - return (TreeVisitor) getVisitor.invoke(recipe); - } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException(e); - } - } - public void addUninitialized(RecipeUse use, Recipe recipe) { uninitializedRecipes.computeIfAbsent(use, u -> new ArrayList<>()).add(recipe); } diff --git a/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java b/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java index 5befaf7bf93..221948b36b0 100644 --- a/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java +++ b/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java @@ -79,7 +79,7 @@ static T markup(T t, Markup markup) { @Value @With - class Error implements Markup { + public class Error implements Markup { UUID id; Throwable exception; diff --git a/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java b/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java index 309c90df361..dcae023c51f 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java @@ -44,7 +44,7 @@ public class FindAndReplace extends Recipe { Boolean regex; /** - * @deprecated Use {@link Recipe#addSingleSourceApplicableTest(TreeVisitor)} instead. + * @deprecated Use {@link Recipe#addSingleSourceApplicableTest(Recipe)} instead. */ @SuppressWarnings("DeprecatedIsStillUsed") @Option(displayName = "Optional file Matcher", diff --git a/rewrite-core/src/test/java/org/openrewrite/RecipeLifecycleTest.java b/rewrite-core/src/test/java/org/openrewrite/RecipeLifecycleTest.java index 1ead3b2f686..816866abfe7 100644 --- a/rewrite-core/src/test/java/org/openrewrite/RecipeLifecycleTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/RecipeLifecycleTest.java @@ -21,6 +21,8 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.marker.Markers; +import org.openrewrite.marker.SearchResult; +import org.openrewrite.test.AdHocRecipe; import org.openrewrite.test.RewriteTest; import org.openrewrite.text.ChangeText; import org.openrewrite.text.PlainText; @@ -61,7 +63,7 @@ public Tree visit(@Nullable Tree tree, ExecutionContext executionContext) { } @Test - void notApplicableRecipe() { + void notApplicableVisitor() { rewriteRun( spec -> spec.recipe(toRecipe(() -> new PlainTextVisitor<>() { @Override @@ -73,6 +75,51 @@ public PlainText visitText(PlainText text, ExecutionContext executionContext) { ); } + @Test + void notApplicableRecipe() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new PlainTextVisitor<>() { + @Override + public PlainText visitText(PlainText text, ExecutionContext executionContext) { + return text.withText("goodbye"); + } + }).addApplicableTest(toRecipe())), + text("hello") + ); + } + + static class ReplaceWithGoodbyeVisitor

extends PlainTextVisitor

{ + @Override + public PlainText visitText(PlainText text, P p) { + return text.withText("goodbye"); + } + } + + static class FindEverythingVisitor

extends PlainTextVisitor

{ + @Override + public PlainText visitText(PlainText tree, P p) { + return SearchResult.found(tree); + } + } + + @Test + void recipeApplicabilityWithFindNothingApplicability() { + // Given: + // A recipe `ReplaceWithGoodbyeVisitor` + // And: + // That has another recipe as an applicability test `ReplaceWithGoodbyeVisitor` + // And that second recipe has a `FindEverythingVisitor` as `getSingleSourceApplicableTest` + // Then: + // The recipe should make a change + AdHocRecipe applicableTest = toRecipe() + .withGetSingleSourceApplicableTest(FindEverythingVisitor::new) + .withGetVisitor(ReplaceWithGoodbyeVisitor::new); + rewriteRun( + spec -> spec.recipe(toRecipe(ReplaceWithGoodbyeVisitor::new).addApplicableTest(applicableTest)), + text("hello", "goodbye") + ); + } + @Test void generateFile() { rewriteRun( diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/DoesNotUseRewriteSkipTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/DoesNotUseRewriteSkipTest.java index 52d8cd812e2..5380afa598a 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/DoesNotUseRewriteSkipTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/DoesNotUseRewriteSkipTest.java @@ -26,7 +26,7 @@ class DoesNotUseRewriteSkipTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { spec.recipe(new ChangeType("java.util.List", "java.util.Collection", false) - .addSingleSourceApplicableTest(new DoesNotUseRewriteSkip().getVisitor())) + .addSingleSourceApplicableTest(new DoesNotUseRewriteSkip())) .parser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())); } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaRecipeLifecycleTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaRecipeLifecycleTest.java index 4e22348ec84..805fc988142 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaRecipeLifecycleTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaRecipeLifecycleTest.java @@ -127,8 +127,8 @@ private static FindMethods createFindMethods() { public void defaults(RecipeSpec spec) { spec.recipe( Recipe.noop() - .addApplicableTest(new HasSourceSet("main").getVisitor()) - .addApplicableTest(createFindMethods().getVisitor()) + .addApplicableTest(new HasSourceSet("main")) + .addApplicableTest(createFindMethods()) .doNext(createFindMethods()) ); } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/RecipeExceptionDemonstrationTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/RecipeExceptionDemonstrationTest.java index b24838c174f..f9f9bdb4263 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/RecipeExceptionDemonstrationTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/RecipeExceptionDemonstrationTest.java @@ -83,7 +83,6 @@ void test(List list) { ); } - @Disabled(value = "The exception thrown in getSingleSourceApplicableTest() is caught by RecipeScheduler, so disable this.") @Test void singleSourceApplicableTest() { rewriteRun( @@ -101,15 +100,13 @@ void test(List list) { list.add(42); } } - """, - """ - /*~~(Demonstrating an exception thrown on the single-source applicable test.)~~>*/import java.util.*; - class Test { - void test(List list) { - list.add(42); - } - } """ + ), + text( + null, + "~~(Demonstrating an exception thrown on the single-source applicable test.)~~>" + + "Rewrite encountered an uncaught recipe error in org.openrewrite.java.RecipeExceptionDemonstration.", + spec -> spec.path("recipe-exception-1.txt") ) ); } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/DoesNotUseRewriteSkip.java b/rewrite-java/src/main/java/org/openrewrite/java/DoesNotUseRewriteSkip.java index 6af1c5a6853..9fa3a68430a 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/DoesNotUseRewriteSkip.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/DoesNotUseRewriteSkip.java @@ -15,6 +15,7 @@ */ package org.openrewrite.java; +import org.openrewrite.Applicability; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; @@ -25,6 +26,7 @@ import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.SearchResult; +import java.util.Optional; import java.util.Set; public class DoesNotUseRewriteSkip extends Recipe { @@ -40,19 +42,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return new JavaIsoVisitor() { - final UsesRewriteSkipVisitor usesRewriteSkip = new UsesRewriteSkipVisitor(); - - @Override - public JavaSourceFile visitJavaSourceFile(JavaSourceFile cu, ExecutionContext ctx) { - JavaSourceFile c = cu; - if (c == usesRewriteSkip.visit(c, ctx)) { - // if this source file is NOT skipped, then the recipe is applicable - c = SearchResult.found(c); - } - return c; - } - }; + return Applicability.not(new UsesRewriteSkipVisitor()); } /** @@ -82,9 +72,9 @@ public J.Literal visitLiteral(J.Literal literal, ExecutionContext ctx) { J.Literal l = literal; if (literal.getType() == JavaType.Primitive.String) { assert literal.getValue() != null; - Recipe currentRecipe = ctx.getCurrentRecipe(); - if (literal.getValue().toString().equals(currentRecipe.getClass().getName())) { - l = SearchResult.found(l); + Optional parentRecipe = ctx.getParentRecipe(); + if (parentRecipe.isPresent() && literal.getValue().toString().equals(parentRecipe.get().getClass().getName())) { + return SearchResult.found(l); } } return l; @@ -94,9 +84,9 @@ public J.Literal visitLiteral(J.Literal literal, ExecutionContext ctx) { public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) { J.FieldAccess f = fieldAccess; if (f.getSimpleName().equals("class")) { - Recipe currentRecipe = ctx.getCurrentRecipe(); - if (TypeUtils.isOfClassType(f.getTarget().getType(), currentRecipe.getClass().getName())) { - f = SearchResult.found(f); + Optional parentRecipe = ctx.getParentRecipe(); + if (parentRecipe.isPresent() && TypeUtils.isOfClassType(f.getTarget().getType(), parentRecipe.get().getClass().getName())) { + return SearchResult.found(f); } } return f; diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java index 09e1acc23fc..0c089e12563 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java @@ -22,12 +22,12 @@ import org.openrewrite.Issue; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaParser; -import org.openrewrite.test.AdHocRecipe; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.*; import static org.openrewrite.maven.Assertions.pomXml; +import static org.openrewrite.test.RewriteTest.toRecipe; class AddDependencyTest implements RewriteTest { @@ -803,12 +803,22 @@ void addDependencyToProjectsThatNeedIt() { void rawVisitorDoesNotDuplicate() { rewriteRun( spec -> spec.recipe( - new AdHocRecipe("Add dependency", - "Uses AddDependencyVisitor directly to validate that it will not add a dependency multiple times", - false, - () -> new AddDependencyVisitor("com.google.guava", "guava", "29.0-jre", - null, "test", null, null, null, null, null), - null)), + toRecipe() + .withDisplayName("Add dependency") + .withName("Uses AddDependencyVisitor directly to validate that it will not add a dependency multiple times") + .withGetVisitor(() -> new AddDependencyVisitor( + "com.google.guava", + "guava", + "29.0-jre", + null, + "test", + null, + null, + null, + null, + null + )) + ), mavenProject("project", srcTestJava( java(usingGuavaIntMath) diff --git a/rewrite-test/src/main/java/org/openrewrite/test/AdHocRecipe.java b/rewrite-test/src/main/java/org/openrewrite/test/AdHocRecipe.java index beda21508ac..976e3899b00 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/AdHocRecipe.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/AdHocRecipe.java @@ -53,6 +53,14 @@ public class AdHocRecipe extends Recipe { @With BiFunction, ExecutionContext, List> visit; + @Nullable + @With + Supplier> getSingleSourceApplicableTest; + + @Nullable + @With + Supplier> getApplicableTest; + public String getDisplayName() { return StringUtils.isBlank(displayName) ? "Ad hoc recipe" : displayName; } @@ -75,4 +83,14 @@ protected List visit(List before, ExecutionContext ctx) public TreeVisitor getVisitor() { return getVisitor.get(); } + + @Override + protected @Nullable TreeVisitor getSingleSourceApplicableTest() { + return getSingleSourceApplicableTest == null ? super.getSingleSourceApplicableTest() : getSingleSourceApplicableTest.get(); + } + + @Override + protected @Nullable TreeVisitor getApplicableTest() { + return getApplicableTest == null ? super.getApplicableTest() : getApplicableTest.get(); + } } diff --git a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java index 08fe93519d6..540df403f23 100644 --- a/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java +++ b/rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java @@ -47,11 +47,11 @@ @SuppressWarnings("unused") public interface RewriteTest extends SourceSpecs { static AdHocRecipe toRecipe(Supplier> visitor) { - return new AdHocRecipe(null, null, null, visitor, null); + return new AdHocRecipe(null, null, null, visitor, null, null, null); } static AdHocRecipe toRecipe() { - return new AdHocRecipe(null, null, null, () -> Recipe.NOOP, null); + return new AdHocRecipe(null, null, null, () -> Recipe.NOOP, null, null, null); } static AdHocRecipe toRecipe(Function> visitor) {