Added SearchResults as a global datatable

rewrite-core/src/main/java/org/openrewrite/Recipe.java

@@ -37,6 +37,7 @@
 import org.openrewrite.internal.StringUtils;
 import org.openrewrite.internal.lang.NullUtils;
 import org.openrewrite.table.RecipeRunStats;
+import org.openrewrite.table.SearchResults;
 import org.openrewrite.table.SourcesFileErrors;
 import org.openrewrite.table.SourcesFileResults;
 
@@ -315,6 +316,7 @@ private List<OptionDescriptor> getOptionDescriptors() {
 
     private static final List<DataTableDescriptor> GLOBAL_DATA_TABLES = Arrays.asList(
             dataTableDescriptorFromDataTable(new SourcesFileResults(Recipe.noop())),
+            dataTableDescriptorFromDataTable(new SearchResults(Recipe.noop())),
             dataTableDescriptorFromDataTable(new SourcesFileErrors(Recipe.noop())),
             dataTableDescriptorFromDataTable(new RecipeRunStats(Recipe.noop()))
     );

rewrite-core/src/main/java/org/openrewrite/RecipeScheduler.java

@@ -18,6 +18,7 @@
 import org.openrewrite.scheduling.RecipeRunCycle;
 import org.openrewrite.scheduling.WatchableExecutionContext;
 import org.openrewrite.table.RecipeRunStats;
+import org.openrewrite.table.SearchResults;
 import org.openrewrite.table.SourcesFileErrors;
 import org.openrewrite.table.SourcesFileResults;
 
@@ -56,6 +57,7 @@ private LargeSourceSet runRecipeCycles(Recipe recipe, LargeSourceSet sourceSet,
 
         RecipeRunStats recipeRunStats = new RecipeRunStats(Recipe.noop());
         SourcesFileErrors errorsTable = new SourcesFileErrors(Recipe.noop());
+        SearchResults searchResults = new SearchResults(Recipe.noop());
         SourcesFileResults sourceFileResults = new SourcesFileResults(Recipe.noop());
 
         LargeSourceSet after = sourceSet;
@@ -72,7 +74,7 @@ private LargeSourceSet runRecipeCycles(Recipe recipe, LargeSourceSet sourceSet,
                 Cursor rootCursor = new Cursor(null, Cursor.ROOT_VALUE);
                 try {
                     RecipeRunCycle<LargeSourceSet> cycle = new RecipeRunCycle<>(recipe, i, rootCursor, ctxWithWatch,
-                            recipeRunStats, sourceFileResults, errorsTable, LargeSourceSet::edit);
+                            recipeRunStats, searchResults, sourceFileResults, errorsTable, LargeSourceSet::edit);
                     ctxWithWatch.putCycle(cycle);
                     after.beforeCycle(i == maxCycles);
 

rewrite-core/src/main/java/org/openrewrite/rpc/request/Generate.java

@@ -24,6 +24,7 @@
 import org.openrewrite.scheduling.RecipeRunCycle;
 import org.openrewrite.scheduling.WatchableExecutionContext;
 import org.openrewrite.table.RecipeRunStats;
+import org.openrewrite.table.SearchResults;
 import org.openrewrite.table.SourcesFileErrors;
 import org.openrewrite.table.SourcesFileResults;
 
@@ -58,8 +59,9 @@ protected Object handle(Generate request) throws Exception {
             if (ctx.getMessage(CURRENT_RECIPE) == null) {
                 WatchableExecutionContext wctx = new WatchableExecutionContext(ctx);
                 wctx.putCycle(new RecipeRunCycle<>(recipe, 0, new Cursor(null, Cursor.ROOT_VALUE), wctx,
-                        new RecipeRunStats(Recipe.noop()), new SourcesFileResults(Recipe.noop()),
-                        new SourcesFileErrors(Recipe.noop()), LargeSourceSet::edit));
+                        new RecipeRunStats(Recipe.noop()), new SearchResults(Recipe.noop()),
+                        new SourcesFileResults(Recipe.noop()), new SourcesFileErrors(Recipe.noop()),
+                        LargeSourceSet::edit));
                 ctx.putCurrentRecipe(recipe);
             }
 

rewrite-core/src/main/java/org/openrewrite/rpc/request/Visit.java

@@ -24,6 +24,7 @@
 import org.openrewrite.scheduling.RecipeRunCycle;
 import org.openrewrite.scheduling.WatchableExecutionContext;
 import org.openrewrite.table.RecipeRunStats;
+import org.openrewrite.table.SearchResults;
 import org.openrewrite.table.SourcesFileErrors;
 import org.openrewrite.table.SourcesFileResults;
 
@@ -91,8 +92,9 @@ private Object getVisitorP(Visit request) {
                     // removed from OpenRewrite in the future.
                     WatchableExecutionContext ctx = new WatchableExecutionContext((ExecutionContext) p);
                     ctx.putCycle(new RecipeRunCycle<>(recipe, 0, new Cursor(null, Cursor.ROOT_VALUE), ctx,
-                            new RecipeRunStats(Recipe.noop()), new SourcesFileResults(Recipe.noop()),
-                            new SourcesFileErrors(Recipe.noop()), LargeSourceSet::edit));
+                            new RecipeRunStats(Recipe.noop()), new SearchResults(Recipe.noop()),
+                            new SourcesFileResults(Recipe.noop()), new SourcesFileErrors(Recipe.noop()),
+                            LargeSourceSet::edit));
                     ctx.putCurrentRecipe(recipe);
                     return ctx;
                 }

rewrite-core/src/main/java/org/openrewrite/scheduling/RecipeRunCycle.java

@@ -26,11 +26,10 @@
 import org.openrewrite.internal.ExceptionUtils;
 import org.openrewrite.internal.FindRecipeRunException;
 import org.openrewrite.internal.RecipeRunException;
-import org.openrewrite.marker.Generated;
-import org.openrewrite.marker.Markers;
-import org.openrewrite.marker.RecipesThatMadeChanges;
+import org.openrewrite.marker.*;
 import org.openrewrite.quark.Quark;
 import org.openrewrite.table.RecipeRunStats;
+import org.openrewrite.table.SearchResults;
 import org.openrewrite.table.SourcesFileErrors;
 import org.openrewrite.table.SourcesFileResults;
 
@@ -41,8 +40,7 @@
 import java.util.function.BiFunction;
 import java.util.function.UnaryOperator;
 
-import static java.util.Collections.newSetFromMap;
-import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.*;
 import static org.openrewrite.ExecutionContext.SCANNING_MUTATION_VALIDATION;
 import static org.openrewrite.Recipe.PANIC;
 
@@ -64,6 +62,7 @@ public class RecipeRunCycle<LSS extends LargeSourceSet> {
     Cursor rootCursor;
     WatchableExecutionContext ctx;
     RecipeRunStats recipeRunStats;
+    SearchResults searchResults;
     SourcesFileResults sourcesFileResults;
     SourcesFileErrors errorsTable;
     BiFunction<LSS, UnaryOperator<@Nullable SourceFile>, LSS> sourceSetEditor;
@@ -101,9 +100,9 @@ public LSS scanSources(LSS sourceSet) {
                                         Tree maybeMutated = scanner.visit(source, ctx, rootCursor);
                                         assert maybeMutated == source || !ctx.getMessage(SCANNING_MUTATION_VALIDATION, false) :
                                                 "Edits made from within ScanningRecipe.getScanner() are discarded. " +
-                                                "The purpose of a scanner is to aggregate information for use in subsequent phases. " +
-                                                "Use ScanningRecipe.getVisitor() for making edits. " +
-                                                "To disable this warning set TypeValidation.immutableScanning to false in your tests.";
+                                                        "The purpose of a scanner is to aggregate information for use in subsequent phases. " +
+                                                        "Use ScanningRecipe.getVisitor() for making edits. " +
+                                                        "To disable this warning set TypeValidation.immutableScanning to false in your tests.";
                                     }
                                     return source;
                                 });
@@ -152,7 +151,7 @@ public LSS generateSources(LSS sourceSet) {
                         generated.replaceAll(source -> addRecipesThatMadeChanges(recipeStack, source));
                         if (!generated.isEmpty()) {
                             acc.addAll(generated);
-                            generated.forEach(source -> recordSourceFileResult(null, source, recipeStack, ctx));
+                            generated.forEach(source -> recordSourceFileResultAndSearchResults(null, source, recipeStack, ctx));
                             madeChangesInThisCycle.add(recipe);
                         }
                     } catch (Throwable t) {
@@ -214,7 +213,7 @@ public LSS editSources(LSS sourceSet) {
 
                             if (after != source) {
                                 madeChangesInThisCycle.add(recipe);
-                                recordSourceFileResult(source, after, recipeStack, ctx);
+                                recordSourceFileResultAndSearchResults(source, after, recipeStack, ctx);
                                 if (source.getMarkers().findFirst(Generated.class).isPresent()) {
                                     // skip edits made to generated source files so that they don't show up in a diff
                                     // that later fails to apply on a freshly cloned repository
@@ -230,50 +229,62 @@ public LSS editSources(LSS sourceSet) {
                             after = handleError(recipe, source, after, t);
                         }
                         if (after != null && after != source) {
+                            //TODO: question for Jonathan: should we not do the handling of searchResults here instead or is it impossible that a throwable was thrown and that a search result is present or are we not interested in these results?
                             after = addRecipesThatMadeChanges(recipeStack, after);
                         }
                         return after;
                     }, sourceFile);
-        }
+                }
         );
     }
 
-    private void recordSourceFileResult(@Nullable SourceFile before, @Nullable SourceFile after, Stack<Recipe> recipeStack, ExecutionContext ctx) {
+    private void recordSourceFileResultAndSearchResults(@Nullable SourceFile before, @Nullable SourceFile after, Stack<Recipe> recipeStack, ExecutionContext ctx) {
         String beforePath = (before == null) ? "" : before.getSourcePath().toString();
         String afterPath = (after == null) ? "" : after.getSourcePath().toString();
         Recipe recipe = recipeStack.peek();
         Long effortSeconds = (recipe.getEstimatedEffortPerOccurrence() == null || Result.isLocalAndHasNoChanges(before, after)) ?
                 0L : recipe.getEstimatedEffortPerOccurrence().getSeconds();
 
         String parentName = "";
+        String parentInstanceName = "";
         boolean hierarchical = recipeStack.size() > 1;
         if (hierarchical) {
             parentName = recipeStack.get(recipeStack.size() - 2).getName();
+            parentInstanceName = recipeStack.get(recipeStack.size() - 2).getInstanceName();
         }
-        String recipeName = recipe.getName();
         sourcesFileResults.insertRow(ctx, new SourcesFileResults.Row(
                 beforePath,
                 afterPath,
                 parentName,
-                recipeName,
+                recipe.getName(),
                 effortSeconds,
                 cycle));
+
+        List<String> searchMarkers = collectSearchResults(before, after);
+        for (String searchResult : searchMarkers) {
+            searchResults.insertRow(ctx, new SearchResults.Row(afterPath, searchResult, parentInstanceName, recipe.getInstanceName()));
+        }
+
         if (hierarchical) {
-            recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), effortSeconds, ctx);
+            //TODO: Question for Jonathan: Do we need this "overhead" of having an entry per recipe in the stack like we have for SourceFileResults?
+            recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), effortSeconds, searchMarkers, ctx);
         }
     }
 
-    private void recordSourceFileResult(@Nullable String beforePath, @Nullable String afterPath, List<Recipe> recipeStack, Long effortSeconds, ExecutionContext ctx) {
+    private void recordSourceFileResult(@Nullable String beforePath, @Nullable String afterPath, List<Recipe> recipeStack, Long effortSeconds, List<String> searchMarkers, ExecutionContext ctx) {
         if (recipeStack.size() <= 1) {
             // No reason to record the synthetic root recipe which contains the recipe run
             return;
         }
         String parentName;
+        String parentInstanceName;
         if (recipeStack.size() == 2) {
             // Record the parent name as blank rather than CompositeRecipe when the parent is the synthetic root recipe
             parentName = "";
+            parentInstanceName = "";
         } else {
             parentName = recipeStack.get(recipeStack.size() - 2).getName();
+            parentInstanceName = recipeStack.get(recipeStack.size() - 2).getInstanceName();
         }
         Recipe recipe = recipeStack.get(recipeStack.size() - 1);
         sourcesFileResults.insertRow(ctx, new SourcesFileResults.Row(
@@ -283,7 +294,10 @@ private void recordSourceFileResult(@Nullable String beforePath, @Nullable Strin
                 recipe.getName(),
                 effortSeconds,
                 cycle));
-        recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), effortSeconds, ctx);
+        for (String searchResult : searchMarkers) {
+            searchResults.insertRow(ctx, new SearchResults.Row(afterPath, searchResult, parentInstanceName, recipe.getInstanceName()));
+        }
+        recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), effortSeconds, searchMarkers, ctx);
     }
 
     private @Nullable SourceFile handleError(Recipe recipe, SourceFile sourceFile, @Nullable SourceFile after,
@@ -319,6 +333,7 @@ private static <S extends SourceFile> S addRecipesThatMadeChanges(List<Recipe> r
     @NonFinal
     @Nullable
     transient Boolean isScanningRecipe;
+
     private boolean isScanningRequired() {
         if (isScanningRecipe == null) {
             isScanningRecipe = isScanningRequired(recipe);
@@ -330,7 +345,7 @@ private static boolean isScanningRequired(Recipe recipe) {
         if (recipe instanceof ScanningRecipe) {
             // DeclarativeRecipe is technically a ScanningRecipe, but it only needs the
             // scanning phase if it or one of its sub-recipes or preconditions is a ScanningRecipe
-            if(recipe instanceof DeclarativeRecipe) {
+            if (recipe instanceof DeclarativeRecipe) {
                 for (Recipe precondition : ((DeclarativeRecipe) recipe).getPreconditions()) {
                     if (isScanningRequired(precondition)) {
                         return true;
@@ -347,4 +362,36 @@ private static boolean isScanningRequired(Recipe recipe) {
         }
         return false;
     }
+
+    private List<String> collectSearchResults(@Nullable SourceFile before, @Nullable SourceFile after) {
+        if (after == null) {
+            return emptyList();
+        }
+        Set<SearchResult> alreadyPresentMarkers = new TreeVisitor<Tree, Set<SearchResult>>() {
+            @Override
+            public <M extends Marker> M visitMarker(Marker marker, Set<SearchResult> ctx) {
+                if (marker instanceof SearchResult) {
+                    ctx.add((SearchResult) marker);
+                }
+                return super.visitMarker(marker, ctx);
+            }
+        }.reduce(before, newSetFromMap(new IdentityHashMap<>()));
+
+        return new TreeVisitor<Tree, List<String>>() {
+            @Override
+            public <M extends Marker> M visitMarker(Marker marker, List<String> ctx) {
+                if (marker instanceof SearchResult && !alreadyPresentMarkers.contains(marker)) {
+                    Cursor cursor = getCursor();
+                    if (!(cursor.getValue() instanceof Tree)) {
+                        cursor = cursor.getParentTreeCursor();
+                    }
+                    if (cursor.getValue() instanceof Tree) {
+                        //TODO: Question for Jonathan: what should the truncation length be?
+                        ctx.add(((Tree) cursor.getValue()).print(getCursor(), new PrintOutputCapture<>(0, PrintOutputCapture.MarkerPrinter.SANITIZED)));
+                    }
+                }
+                return super.visitMarker(marker, ctx);
+            }
+        }.reduce(after, new ArrayList<>());
+    }
 }

rewrite-core/src/main/java/org/openrewrite/table/SearchResults.java

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2022 the original author or authors.
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.table;
+
+import lombok.Value;
+import org.jspecify.annotations.Nullable;
+import org.openrewrite.Column;
+import org.openrewrite.DataTable;
+import org.openrewrite.Recipe;
+
+public class SearchResults extends DataTable<SearchResults.Row> {
+
+    public SearchResults(Recipe recipe) {
+        super(recipe, "Source files that had search results",
+                "Search results that were found during the recipe run.");
+    }
+
+    @Value
+    public static class Row {
+        //TODO: Question for Jonathan: should we not (similar to SourcesFileErrors) have a afterSourcePath and represent both?
+        // What if a recipe changes the path and adds markers? Do we need both?
+        // If a later recipe removed the sourcefile, we will not have the markers anymore. Not foreseeing that as an issue as typically search and edit/delete will not go together.
+        @Column(displayName = "Source path of search result",
+                description = "The source path of the file with the search result markers present.")
+        @Nullable
+        String sourcePath;
+
+        //TODO: Question for Jonathan: what should the truncation length be?
+        @Column(displayName = "Result",
+                description = "The trimmed printed tree of the LST element that the marker is attached to. Truncated after 1000 chars if longer.")
+        String result;
+
+        //TODO: Question for Jonathan: Should we not report both instance name and recipe name? Searching for the recipe id from the String is not always "easy" (see also the unit tests)
+        @Column(displayName = "Parent of the recipe that had the search marker added",
+                description = "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.")
+        String parentRecipe;
+
+        @Column(displayName = "Recipe that added the search marker",
+                description = "The specific recipe that added the Search marker.")
+        String recipe;
+
+    }
+    //TODO: Question for Jonathan: in `SourcesFileResults`, I see a `build` method which is not used in the OR codebase. Perhaps by our other system? Do we need to have this build method or not? `SourcesFileErrors` does not have it.
+}