Add isAcceptable() call to Preconditions methods

When constructing a precondition using `Preconditions#and()`, `or()`, or `not()` then the returned visitor must also call `isAcceptable()` on the input visitor(s) in order to avoid any `ClassCastException` in case the input visitor overrides `visit(Tree, P)` and as a result probably doesn't call `isAcceptable()` on itself.
openrewrite · Feb 19, 2024 · a438606 · a438606
1 parent dd77edd
commit a438606
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 0 deletions.
diff --git a/rewrite-core/src/main/java/org/openrewrite/Preconditions.java b/rewrite-core/src/main/java/org/openrewrite/Preconditions.java
@@ -80,6 +80,10 @@ public static TreeVisitor<?, ExecutionContext> not(TreeVisitor<?, ExecutionConte
         return new TreeVisitor<Tree, ExecutionContext>() {
             @Override
             public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                SourceFile sourceFile = tree instanceof SourceFile ? (SourceFile) tree : null;
+                if (sourceFile != null && !v.isAcceptable(sourceFile, ctx)) {
+                    return SearchResult.found(tree);
+                }
                 Tree t2 = v.visit(tree, ctx);
                 return tree == t2 && tree != null ?
                         SearchResult.found(tree) :
@@ -93,7 +97,11 @@ public static TreeVisitor<?, ExecutionContext> or(TreeVisitor<?, ExecutionContex
         return new TreeVisitor<Tree, ExecutionContext>() {
             @Override
             public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                SourceFile sourceFile = tree instanceof SourceFile ? (SourceFile) tree : null;
                 for (TreeVisitor<?, ExecutionContext> v : vs) {
+                    if (sourceFile != null && !v.isAcceptable(sourceFile, ctx)) {
+                        continue;
+                    }
                     Tree t2 = v.visit(tree, ctx);
                     if (tree != t2) {
                         return t2;
@@ -109,8 +117,12 @@ public static TreeVisitor<?, ExecutionContext> and(TreeVisitor<?, ExecutionConte
         return new TreeVisitor<Tree, ExecutionContext>() {
             @Override
             public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                SourceFile sourceFile = tree instanceof SourceFile ? (SourceFile) tree : null;
                 Tree t2 = tree;
                 for (TreeVisitor<?, ExecutionContext> v : vs) {
+                    if (sourceFile != null && !v.isAcceptable(sourceFile, ctx)) {
+                        continue;
+                    }
                     t2 = v.visit(tree, ctx);
                     if (tree == t2) {
                         return tree;

diff --git a/rewrite-core/src/test/java/org/openrewrite/PreconditionsTest.java b/rewrite-core/src/test/java/org/openrewrite/PreconditionsTest.java
@@ -16,6 +16,7 @@
 package org.openrewrite;
 
 import org.junit.jupiter.api.Test;
+import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.marker.SearchResult;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
@@ -94,6 +95,72 @@ void checkApplicabilityAgainstOtherSourceTypes() {
         );
     }
 
+    @Test
+    void orOtherSourceType() {
+        rewriteRun(
+          spec -> spec.recipe(toRecipe(() -> Preconditions.check(Preconditions.or(
+              new PlainTextVisitor<>() {
+                  @Nullable
+                  @Override
+                  public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                      return tree != null && ((PlainText) tree).getText().contains("foo") ? SearchResult.found(tree) : tree;
+                  }
+              }),
+            new TreeVisitor<>() {
+                @Override
+                public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                    return SearchResult.found(tree, "recipe");
+                }
+            })
+          )),
+          other("hello")
+        );
+    }
+
+    @Test
+    void andOtherSourceType() {
+        rewriteRun(
+          spec -> spec.recipe(toRecipe(() -> Preconditions.check(Preconditions.and(
+              new PlainTextVisitor<>() {
+                  @Nullable
+                  @Override
+                  public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                      return tree != null && ((PlainText) tree).getText().contains("foo") ? SearchResult.found(tree) : tree;
+                  }
+              }),
+            new TreeVisitor<>() {
+                @Override
+                public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                    return SearchResult.found(tree, "recipe");
+                }
+            })
+          )),
+          other("hello")
+        );
+    }
+
+    @Test
+    void notOtherSourceType() {
+        rewriteRun(
+          spec -> spec.recipe(toRecipe(() -> Preconditions.check(Preconditions.not(
+              new PlainTextVisitor<>() {
+                  @Nullable
+                  @Override
+                  public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                      return tree != null && ((PlainText) tree).getText().contains("foo") ? SearchResult.found(tree) : tree;
+                  }
+              }),
+            new TreeVisitor<>() {
+                @Override
+                public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
+                    return SearchResult.found(tree, "recipe");
+                }
+            })
+          )),
+          other("hello", "~~(recipe)~~>⚛⚛⚛ The contents of this file are not visible. ⚛⚛⚛")
+        );
+    }
+
     Recipe recipe(TreeVisitor<?, ExecutionContext> applicability) {
         return toRecipe(() -> Preconditions.check(applicability, new PlainTextVisitor<>() {
             @Override