Skip to content

Commit d634c65

Browse files
Jenson3210Jenson3210
authored
Register accumulators for nested ScanningRecipes in OR preconditions (#6701)
* Add failing tests for nested ScanningRecipes in OR preconditions Reproduces issue #6698 where OR preconditions with nested ScanningRecipes fail with NPE because getInitialValue() only registers direct preconditions, not nested ones. Tests cover: - Basic nested ScanningRecipe in OR precondition - Precondition not met scenario - Same scanning recipe with different parameters - Deeply nested OR preconditions - Multiple branches with same scanning recipe * Register accumulators for nested ScanningRecipes in OR preconditions Fixes #6698. When using OR preconditions with nested ScanningRecipes, getInitialValue() and getScanner() now recursively traverse precondition recipes' recipeLists to register and scan all nested ScanningRecipes. This ensures orVisitors() can find accumulators for deeply nested ScanningRecipes in the accumulator map. * Use @issue annotations instead of comments in tests
1 parent daf9e11 commit d634c65

2 files changed

Lines changed: 205 additions & 9 deletions

File tree

‎rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,33 +169,47 @@ private void initializeDeclarativeRecipe(DeclarativeRecipe declarativeRecipe, St
169169
public Accumulator getInitialValue(ExecutionContext ctx) {
170170
Accumulator acc = new Accumulator();
171171
for (Recipe precondition : preconditions) {
172-
if (precondition instanceof ScanningRecipe && isScanningRequired(precondition)) {
173-
acc.recipeToAccumulator.put(precondition, ((ScanningRecipe<?>) precondition).getInitialValue(ctx));
174-
}
172+
registerNestedScanningRecipes(precondition, acc, ctx);
175173
}
176174
accumulator = acc;
177175
return acc;
178176
}
179177

178+
private void registerNestedScanningRecipes(Recipe recipe, Accumulator acc, ExecutionContext ctx) {
179+
if (recipe instanceof ScanningRecipe && isScanningRequired(recipe)) {
180+
acc.recipeToAccumulator.put(recipe, ((ScanningRecipe<?>) recipe).getInitialValue(ctx));
181+
}
182+
for (Recipe nested : recipe.getRecipeList()) {
183+
registerNestedScanningRecipes(nested, acc, ctx);
184+
}
185+
}
186+
180187
@Override
181188
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
182189
return new TreeVisitor<Tree, ExecutionContext>() {
183190
@SuppressWarnings({"rawtypes", "unchecked"})
184191
@Override
185192
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
186193
for (Recipe precondition : preconditions) {
187-
if (precondition instanceof ScanningRecipe && isScanningRequired(precondition)) {
188-
ScanningRecipe preconditionRecipe = (ScanningRecipe) precondition;
189-
Object preconditionAcc = acc.recipeToAccumulator.get(precondition);
190-
preconditionRecipe.getScanner(preconditionAcc)
191-
.visit(tree, ctx);
192-
}
194+
scanNestedScanningRecipes(precondition, acc, tree, ctx);
193195
}
194196
return tree;
195197
}
196198
};
197199
}
198200

201+
@SuppressWarnings({"rawtypes", "unchecked"})
202+
private void scanNestedScanningRecipes(Recipe recipe, Accumulator acc, @Nullable Tree tree, ExecutionContext ctx) {
203+
if (recipe instanceof ScanningRecipe && isScanningRequired(recipe)) {
204+
ScanningRecipe scanningRecipe = (ScanningRecipe) recipe;
205+
Object recipeAcc = acc.recipeToAccumulator.get(recipe);
206+
scanningRecipe.getScanner(recipeAcc).visit(tree, ctx);
207+
}
208+
for (Recipe nested : recipe.getRecipeList()) {
209+
scanNestedScanningRecipes(nested, acc, tree, ctx);
210+
}
211+
}
212+
199213
public static class Accumulator {
200214
Map<Recipe, Object> recipeToAccumulator = new HashMap<>();
201215
}

‎rewrite-core/src/test/java/org/openrewrite/config/DeclarativeRecipeTest.java‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,188 @@ void mutuallyRecursiveRecipesDetectedAsCycle() {
485485
.hasMessageContaining("RecipeB");
486486
}
487487

488+
@Issue("https://github.com/openrewrite/rewrite/issues/6698")
489+
@Test
490+
void nestedScanningRecipeInOrPrecondition() {
491+
rewriteRun(
492+
spec -> spec.recipeFromYaml(
493+
"""
494+
type: specs.openrewrite.org/v1beta/recipe
495+
name: org.sample.AddJacksonAnnotations
496+
description: Test.
497+
preconditions:
498+
- org.sample.ProjectUsesJackson
499+
recipeList:
500+
- org.openrewrite.text.ChangeText:
501+
toText: changed
502+
---
503+
type: specs.openrewrite.org/v1beta/recipe
504+
name: org.sample.ProjectUsesJackson
505+
description: OR precondition - matches if ANY condition is true
506+
recipeList:
507+
- org.openrewrite.search.RepositoryContainsFile:
508+
filePattern: "**/jackson-config.json"
509+
- org.openrewrite.search.RepositoryContainsFile:
510+
filePattern: "**/jackson.properties"
511+
""",
512+
"org.sample.AddJacksonAnnotations"
513+
),
514+
// jackson-config.json triggers the precondition, so all files get changed
515+
text("config", "changed", spec -> spec.path("jackson-config.json")),
516+
text("original", "changed")
517+
);
518+
}
519+
520+
@Issue("https://github.com/openrewrite/rewrite/issues/6698")
521+
@Test
522+
void nestedScanningRecipeInOrPreconditionNotMet() {
523+
rewriteRun(
524+
spec -> spec.recipeFromYaml(
525+
"""
526+
type: specs.openrewrite.org/v1beta/recipe
527+
name: org.sample.AddJacksonAnnotations
528+
description: Test.
529+
preconditions:
530+
- org.sample.ProjectUsesJackson
531+
recipeList:
532+
- org.openrewrite.text.ChangeText:
533+
toText: changed
534+
---
535+
type: specs.openrewrite.org/v1beta/recipe
536+
name: org.sample.ProjectUsesJackson
537+
description: OR precondition - matches if ANY condition is true
538+
recipeList:
539+
- org.openrewrite.search.RepositoryContainsFile:
540+
filePattern: "**/jackson-config.json"
541+
- org.openrewrite.search.RepositoryContainsFile:
542+
filePattern: "**/jackson.properties"
543+
""",
544+
"org.sample.AddJacksonAnnotations"
545+
),
546+
text("config", spec -> spec.path("some-other-file.txt")),
547+
text("original")
548+
);
549+
}
550+
551+
@Issue("https://github.com/openrewrite/rewrite/issues/6698")
552+
@Test
553+
void sameScanningRecipeWithDifferentParametersInOrPrecondition() {
554+
rewriteRun(
555+
spec -> spec.recipeFromYaml(
556+
"""
557+
type: specs.openrewrite.org/v1beta/recipe
558+
name: org.sample.MultiFileCheck
559+
description: Test.
560+
preconditions:
561+
- org.sample.HasAnyConfigFile
562+
recipeList:
563+
- org.openrewrite.text.ChangeText:
564+
toText: changed
565+
---
566+
type: specs.openrewrite.org/v1beta/recipe
567+
name: org.sample.HasAnyConfigFile
568+
description: OR precondition with same recipe type but different params
569+
recipeList:
570+
- org.openrewrite.search.RepositoryContainsFile:
571+
filePattern: "**/config-a.json"
572+
- org.openrewrite.search.RepositoryContainsFile:
573+
filePattern: "**/config-b.json"
574+
- org.openrewrite.search.RepositoryContainsFile:
575+
filePattern: "**/config-c.json"
576+
""",
577+
"org.sample.MultiFileCheck"
578+
),
579+
// Only config-b.json exists, so should still match due to OR logic
580+
// When matched, all files are changed
581+
text("config b", "changed", spec -> spec.path("config-b.json")),
582+
text("original", "changed")
583+
);
584+
}
585+
586+
@Issue("https://github.com/openrewrite/rewrite/issues/6698")
587+
@Test
588+
void deeplyNestedOrPreconditionsWithScanningRecipe() {
589+
rewriteRun(
590+
spec -> spec.recipeFromYaml(
591+
"""
592+
type: specs.openrewrite.org/v1beta/recipe
593+
name: org.sample.TopLevel
594+
description: Test.
595+
preconditions:
596+
- org.sample.MiddleLevel
597+
recipeList:
598+
- org.openrewrite.text.ChangeText:
599+
toText: changed
600+
---
601+
type: specs.openrewrite.org/v1beta/recipe
602+
name: org.sample.MiddleLevel
603+
description: Middle level precondition
604+
recipeList:
605+
- org.sample.BottomLevel
606+
- org.openrewrite.search.RepositoryContainsFile:
607+
filePattern: "**/middle.json"
608+
---
609+
type: specs.openrewrite.org/v1beta/recipe
610+
name: org.sample.BottomLevel
611+
description: Bottom level with scanning recipes
612+
recipeList:
613+
- org.openrewrite.search.RepositoryContainsFile:
614+
filePattern: "**/bottom-a.json"
615+
- org.openrewrite.search.RepositoryContainsFile:
616+
filePattern: "**/bottom-b.json"
617+
""",
618+
"org.sample.TopLevel"
619+
),
620+
// Only bottom-b.json exists, which should match through the nested OR chain
621+
// When matched, all files are changed
622+
text("bottom b config", "changed", spec -> spec.path("bottom-b.json")),
623+
text("original", "changed")
624+
);
625+
}
626+
627+
@Issue("https://github.com/openrewrite/rewrite/issues/6698")
628+
@Test
629+
void multipleNestedOrPreconditionsWithSameScanningRecipe() {
630+
rewriteRun(
631+
spec -> spec.recipeFromYaml(
632+
"""
633+
type: specs.openrewrite.org/v1beta/recipe
634+
name: org.sample.TopLevel
635+
description: Test.
636+
preconditions:
637+
- org.sample.BranchA
638+
- org.sample.BranchB
639+
recipeList:
640+
- org.openrewrite.text.ChangeText:
641+
toText: changed
642+
---
643+
type: specs.openrewrite.org/v1beta/recipe
644+
name: org.sample.BranchA
645+
description: First branch
646+
recipeList:
647+
- org.openrewrite.search.RepositoryContainsFile:
648+
filePattern: "**/branch-a-1.json"
649+
- org.openrewrite.search.RepositoryContainsFile:
650+
filePattern: "**/shared.json"
651+
---
652+
type: specs.openrewrite.org/v1beta/recipe
653+
name: org.sample.BranchB
654+
description: Second branch
655+
recipeList:
656+
- org.openrewrite.search.RepositoryContainsFile:
657+
filePattern: "**/branch-b-1.json"
658+
- org.openrewrite.search.RepositoryContainsFile:
659+
filePattern: "**/shared.json"
660+
""",
661+
"org.sample.TopLevel"
662+
),
663+
// shared.json exists, which should satisfy both branches (AND of two ORs)
664+
// When matched, all files are changed
665+
text("shared config", "changed", spec -> spec.path("shared.json")),
666+
text("original", "changed")
667+
);
668+
}
669+
488670
@Test
489671
void deeperCyclicReferencesDetectedAsCycle() {
490672
// Test that deeper cyclic references (A -> B -> C -> A) are detected as a cycle

0 commit comments

Comments
 (0)
⚡