Skip to content

Commit

Permalink
Add isAcceptable() call to Preconditions methods (#4021)
Browse files Browse the repository at this point in the history 
* 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.

* Add explanatory comment

* Add test case to `IsOrIsNotLikelyTestTest`
  • Loading branch information
@knutwannheden
knutwannheden authored Feb 19, 2024
1 parent 79327eb commit fc4714a
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 4 deletions.
15 changes: 15 additions & 0 deletions rewrite-core/src/main/java/org/openrewrite/Preconditions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ 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;
// calling `isAcceptable()` in case `v` overrides `visit(Tree, P)`
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) :
Expand All @@ -93,7 +98,12 @@ 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) {
// calling `isAcceptable()` in case `v` overrides `visit(Tree, P)`
if (sourceFile != null && !v.isAcceptable(sourceFile, ctx)) {
continue;
}
Tree t2 = v.visit(tree, ctx);
if (tree != t2) {
return t2;
Expand All @@ -109,8 +119,13 @@ 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) {
// calling `isAcceptable()` in case `v` overrides `visit(Tree, P)`
if (sourceFile != null && !v.isAcceptable(sourceFile, ctx)) {
continue;
}
t2 = v.visit(tree, ctx);
if (tree == t2) {
return tree;
Expand Down
67 changes: 67 additions & 0 deletions rewrite-core/src/test/java/org/openrewrite/PreconditionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
29 changes: 25 additions & 4 deletions rewrite-java-test/src/test/java/org/openrewrite/java/search/IsOrIsNotLikelyTestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@
import org.intellij.lang.annotations.Language;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Tree;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaParser;
import org.openrewrite.marker.SearchResult;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.SourceSpecs;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.java.Assertions.sourceSet;
import static org.openrewrite.java.Assertions.srcMainJava;
import static org.openrewrite.java.Assertions.srcTestJava;
import static org.openrewrite.java.Assertions.*;
import static org.openrewrite.test.RewriteTest.toRecipe;
import static org.openrewrite.test.SourceSpecs.dir;
import static org.openrewrite.test.SourceSpecs.text;

public class IsOrIsNotLikelyTestTest {

Expand Down Expand Up @@ -163,6 +168,22 @@ void standardMainAndTestSourceSet() {
);
}

@Test
void preconditionForOtherSourceType() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> Preconditions.check(
Preconditions.or(new IsLikelyTest().getVisitor()),
new TreeVisitor<>() {
@Nullable
@Override
public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
return tree != null ? SearchResult.found(tree, "recipe") : null;
}
}))),
text("foo")
);
}

@Test
@SuppressWarnings("SpellCheckingInspection")
void standardMainAndIntegTestSourceSet() {
Expand Down

0 comments on commit fc4714a

Please sign in to comment.