From 777a0f3fd7a9816bd8e09415302646dd9cd0151c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 1 Dec 2023 16:43:38 +0100 Subject: [PATCH] Change HasSourcePath from a Visitor into a Recipe for use in yaml preconditions (#3749) * ChangeText needs a relativePath to be of use Or so I'm told. * Replace ChangeText `relativePath` with hint about preconditions * Turn HasSourcePath into a Recipe * Apply suggestions from code review --- .../java/org/openrewrite/HasSourcePath.java | 73 +++++++++++++------ .../java/org/openrewrite/text/ChangeText.java | 6 +- .../main/java/org/openrewrite/text/Find.java | 11 +-- .../org/openrewrite/text/FindAndReplace.java | 3 +- .../org/openrewrite/text/FindMultiselect.java | 15 ++-- .../org/openrewrite/RecipeBasicsTest.java | 1 - .../openrewrite/config/ChangeTextToSam.java | 54 -------------- .../recipes/MigrateRecipeToRewrite8Test.java | 4 +- .../org/openrewrite/HasSourcePathTest.java | 2 +- .../org/openrewrite/xml/RemoveXmlTag.java | 2 +- .../org/openrewrite/yaml/CopyValueTest.java | 2 +- 11 files changed, 74 insertions(+), 99 deletions(-) delete mode 100644 rewrite-core/src/test/java/org/openrewrite/config/ChangeTextToSam.java diff --git a/rewrite-core/src/main/java/org/openrewrite/HasSourcePath.java b/rewrite-core/src/main/java/org/openrewrite/HasSourcePath.java index 6683c907432..4bbd54f1113 100644 --- a/rewrite-core/src/main/java/org/openrewrite/HasSourcePath.java +++ b/rewrite-core/src/main/java/org/openrewrite/HasSourcePath.java @@ -23,8 +23,17 @@ import java.nio.file.PathMatcher; import java.nio.file.Paths; -public class HasSourcePath

extends TreeVisitor { +public class HasSourcePath extends Recipe { + @Option(displayName = "Syntax", + description = "Either `glob` or `regex`. Defaults to `glob`.", + example = "glob", + required = false) private final String syntax; + @Option(displayName = "File pattern", + description = "A glob or regex pattern to match against the source path of a source file.", + example = "foo/**/bar/*.txt", + required = false) + @Nullable private final String filePattern; public HasSourcePath(@Nullable String filePattern) { @@ -35,34 +44,52 @@ public HasSourcePath(@Nullable String filePattern) { * @param syntax one of "glob" or "regex". * @param filePattern the file pattern. */ - public HasSourcePath(String syntax, @Nullable String filePattern) { - this.syntax = syntax; + public HasSourcePath(@Nullable String syntax, @Nullable String filePattern) { + this.syntax = syntax == null ? "glob" : syntax; this.filePattern = filePattern; } - @Nullable @Override - public Tree preVisit(Tree tree, P p) { - stopAfterPreVisit(); - if (StringUtils.isBlank(filePattern)) { - return SearchResult.found(tree, "has file"); - } + public String getDisplayName() { + return "Has source path"; + } - if (tree instanceof SourceFile) { - SourceFile sourceFile = (SourceFile) tree; - Path sourcePath; - if ("glob".equals(syntax) && filePattern.startsWith("**")) { - sourcePath = Paths.get(".").resolve(sourceFile.getSourcePath().normalize()); - } else { - sourcePath = sourceFile.getSourcePath().normalize(); - } + @Override + public String getDescription() { + return "Matches a source file based on its path, for use as precondition. " + + "The path can be matched using either glob or regex syntax. " + + "If no syntax is specified, glob syntax is used by default. " + + "If no file pattern is specified, all source files are matched. " + + "The path is relative to the root of the project."; + } - PathMatcher pathMatcher = sourcePath.getFileSystem().getPathMatcher(syntax + ":" + filePattern); - if (pathMatcher.matches(sourcePath)) { - return SearchResult.found(sourceFile,"has file"); - } - } + @Override + public TreeVisitor getVisitor() { + return new TreeVisitor() { + @Override + public Tree preVisit(Tree tree, ExecutionContext p) { + stopAfterPreVisit(); + if (StringUtils.isBlank(filePattern)) { + return SearchResult.found(tree, "has file"); + } + + if (tree instanceof SourceFile) { + SourceFile sourceFile = (SourceFile) tree; + Path sourcePath; + if ("glob".equals(syntax) && filePattern.startsWith("**")) { + sourcePath = Paths.get(".").resolve(sourceFile.getSourcePath().normalize()); + } else { + sourcePath = sourceFile.getSourcePath().normalize(); + } - return tree; + PathMatcher pathMatcher = sourcePath.getFileSystem().getPathMatcher(syntax + ":" + filePattern); + if (pathMatcher.matches(sourcePath)) { + return SearchResult.found(sourceFile, "has file"); + } + } + + return tree; + } + }; } } diff --git a/rewrite-core/src/main/java/org/openrewrite/text/ChangeText.java b/rewrite-core/src/main/java/org/openrewrite/text/ChangeText.java index fe05c4fe139..f5b7574e46e 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/ChangeText.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/ChangeText.java @@ -30,7 +30,6 @@ @Value @EqualsAndHashCode(callSuper = false) public class ChangeText extends Recipe { - @Option(displayName = "Text after change", description = "The text file will have only this text after the change.", example = "Some text.") @@ -48,14 +47,15 @@ public String getDisplayName() { @Override public String getDescription() { - return "Completely replaces the contents of the text file with other text."; + return "Completely replaces the contents of the text file with other text. " + + "Use together with a `HasSourcePath` precondition to limit which files are changed."; } @Override public TreeVisitor getVisitor() { return new PlainTextVisitor() { @Override - public PlainText visitText(PlainText text, ExecutionContext executionContext) { + public PlainText visitText(PlainText text, ExecutionContext ctx) { return text .withSnippets(emptyList()) .withText(toText); diff --git a/rewrite-core/src/main/java/org/openrewrite/text/Find.java b/rewrite-core/src/main/java/org/openrewrite/text/Find.java index c2ff43118f2..d963382b305 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/Find.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/Find.java @@ -105,13 +105,13 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { searchStr = Pattern.quote(searchStr); } int patternOptions = 0; - if(!Boolean.TRUE.equals(caseSensitive)) { + if (!Boolean.TRUE.equals(caseSensitive)) { patternOptions |= Pattern.CASE_INSENSITIVE; } - if(Boolean.TRUE.equals(multiline)) { + if (Boolean.TRUE.equals(multiline)) { patternOptions |= Pattern.MULTILINE; } - if(Boolean.TRUE.equals(dotAll)) { + if (Boolean.TRUE.equals(dotAll)) { patternOptions |= Pattern.DOTALL; } Pattern pattern = Pattern.compile(searchStr, patternOptions); @@ -134,10 +134,11 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { } }; //noinspection DuplicatedCode - if(filePattern != null) { + if (filePattern != null) { //noinspection unchecked TreeVisitor check = Preconditions.or(Arrays.stream(filePattern.split(";")) - .map(HasSourcePath::new) + .map(HasSourcePath::new) + .map(Recipe::getVisitor) .toArray(TreeVisitor[]::new)); visitor = Preconditions.check(check, visitor); 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 40aa0f0b258..62897a26079 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/FindAndReplace.java @@ -147,7 +147,8 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if(filePattern != null) { //noinspection unchecked TreeVisitor check = Preconditions.or(Arrays.stream(filePattern.split(";")) - .map(HasSourcePath::new) + .map(HasSourcePath::new) + .map(Recipe::getVisitor) .toArray(TreeVisitor[]::new)); visitor = Preconditions.check(check, visitor); diff --git a/rewrite-core/src/main/java/org/openrewrite/text/FindMultiselect.java b/rewrite-core/src/main/java/org/openrewrite/text/FindMultiselect.java index cf8d7463e4b..bdf9bd2e771 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/FindMultiselect.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/FindMultiselect.java @@ -69,7 +69,7 @@ public String getDescription() { "* Dot all - Allows `.` to match line terminators.", valid = {"Case-sensitive", "Multiline", "Dot all"}, required = false) - @Nullable + @Nullable Set regexOptions; @Option(displayName = "File pattern", @@ -86,7 +86,7 @@ public TreeVisitor getVisitor() { Boolean caseSensitive; Boolean multiline; Boolean dotAll; - if(regexOptions != null) { + if (regexOptions != null) { Set lowerCaseOptions = regexOptions.stream() .map(String::toLowerCase) .collect(toSet()); @@ -112,13 +112,13 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { searchStr = Pattern.quote(searchStr); } int patternOptions = 0; - if(!Boolean.TRUE.equals(caseSensitive)) { + if (!Boolean.TRUE.equals(caseSensitive)) { patternOptions |= Pattern.CASE_INSENSITIVE; } - if(Boolean.TRUE.equals(multiline)) { + if (Boolean.TRUE.equals(multiline)) { patternOptions |= Pattern.MULTILINE; } - if(Boolean.TRUE.equals(dotAll)) { + if (Boolean.TRUE.equals(dotAll)) { patternOptions |= Pattern.DOTALL; } Pattern pattern = Pattern.compile(searchStr, patternOptions); @@ -141,10 +141,11 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { } }; //noinspection DuplicatedCode - if(filePattern != null) { + if (filePattern != null) { //noinspection unchecked TreeVisitor check = Preconditions.or(Arrays.stream(filePattern.split(";")) - .map(HasSourcePath::new) + .map(HasSourcePath::new) + .map(Recipe::getVisitor) .toArray(TreeVisitor[]::new)); visitor = Preconditions.check(check, visitor); diff --git a/rewrite-core/src/test/java/org/openrewrite/RecipeBasicsTest.java b/rewrite-core/src/test/java/org/openrewrite/RecipeBasicsTest.java index 1c8e2ef61f6..a1d07fc4998 100644 --- a/rewrite-core/src/test/java/org/openrewrite/RecipeBasicsTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/RecipeBasicsTest.java @@ -25,7 +25,6 @@ import static org.assertj.core.api.Assertions.assertThat; class RecipeBasicsTest { - @Test void cloneRecipe() throws JsonMappingException { ChangeText ct = new ChangeText("hi"); diff --git a/rewrite-core/src/test/java/org/openrewrite/config/ChangeTextToSam.java b/rewrite-core/src/test/java/org/openrewrite/config/ChangeTextToSam.java deleted file mode 100644 index 1b40fdfc0cf..00000000000 --- a/rewrite-core/src/test/java/org/openrewrite/config/ChangeTextToSam.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - *

- * 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 - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * 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.config; - -import org.openrewrite.ExecutionContext; -import org.openrewrite.Option; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; -import org.openrewrite.internal.lang.Nullable; -import org.openrewrite.text.PlainText; -import org.openrewrite.text.PlainTextVisitor; - -@SuppressWarnings("unused") // Used in a yaml recipe -public class ChangeTextToSam extends Recipe { - - @Override - public String getDisplayName() { - return "Change text to Sam"; - } - - @Override - public String getDescription() { - return "Does the text file say Sam? It will after you run this recipe."; - } - - @Option(displayName = "bool", - description = "Exists to test that optional configuration may be omitted", - required = false) - @Nullable - Boolean bool; - - @Override - public TreeVisitor getVisitor() { - return new PlainTextVisitor<>() { - @Override - public PlainText visitText(PlainText text, ExecutionContext executionContext) { - return text.withText("Sam"); - } - }; - } -} diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/MigrateRecipeToRewrite8Test.java b/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/MigrateRecipeToRewrite8Test.java index 6436a7a5fd1..c20006ee7a9 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/MigrateRecipeToRewrite8Test.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/MigrateRecipeToRewrite8Test.java @@ -589,7 +589,7 @@ public String getDisplayName() { @Override protected TreeVisitor getSingleSourceApplicableTest() { if (fileMatcher != null) { - return new HasSourcePath<>(fileMatcher); + return new HasSourcePath(fileMatcher).getVisitor(); } else { return null; } @@ -633,7 +633,7 @@ public String getDisplayName() { @Override protected TreeVisitor getSingleSourceApplicableTest() { if (fileMatcher != null) { - return new HasSourcePath<>(fileMatcher); + return new HasSourcePath(fileMatcher).getVisitor(); } else { return null; } diff --git a/rewrite-test/src/test/java/org/openrewrite/HasSourcePathTest.java b/rewrite-test/src/test/java/org/openrewrite/HasSourcePathTest.java index c4eee99242e..e2a2790a3cc 100644 --- a/rewrite-test/src/test/java/org/openrewrite/HasSourcePathTest.java +++ b/rewrite-test/src/test/java/org/openrewrite/HasSourcePathTest.java @@ -90,7 +90,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new HasSourcePath<>(syntax, filePattern), + return Preconditions.check(new HasSourcePath(syntax, filePattern), new ChangeText(toText).getVisitor()); } } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java index 2eeeba2df81..95fa685dea5 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java @@ -48,7 +48,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(fileMatcher != null ? new HasSourcePath<>(fileMatcher) : new HasSourcePath<>(""), new XmlIsoVisitor() { + return Preconditions.check(fileMatcher != null ? new HasSourcePath(fileMatcher) : new HasSourcePath(""), new XmlIsoVisitor() { private final XPathMatcher xPathMatcher = new XPathMatcher(xPath); @Override diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/CopyValueTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/CopyValueTest.java index 10faf31bbc8..e6b5570dc72 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/CopyValueTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/CopyValueTest.java @@ -89,7 +89,7 @@ void insertCopyValueAndRemoveSource() { void changeOnlyMatchingFile() { rewriteRun( spec -> spec.recipe( - toRecipe(() -> Preconditions.check(new HasSourcePath<>("**/a.yml"), new YamlIsoVisitor<>() { + toRecipe(() -> Preconditions.check(new HasSourcePath("**/a.yml"), new YamlIsoVisitor<>() { @Override public Yaml.Documents visitDocuments(Yaml.Documents documents, ExecutionContext executionContext) { doAfterVisit(new CopyValue(".source", ".destination").getVisitor());