diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index c79dc6b8ac7..9751024bf3d 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,8 +1,8 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.11-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.11.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionSha256Sum=57dafb5c2622c6cc08b993c85b7c06956a2f53536432a30ead46166dbca0f1e9 +distributionSha256Sum=f397b287023acdba1e9f6fc5ea72d22dd63669d59ed4a289a29b1a76eee151c6 diff --git a/rewrite-core/src/main/java/org/openrewrite/ExcludeFileFromGitignore.java b/rewrite-core/src/main/java/org/openrewrite/ExcludeFileFromGitignore.java index 9a65ecc0480..2db8c94f8e0 100644 --- a/rewrite-core/src/main/java/org/openrewrite/ExcludeFileFromGitignore.java +++ b/rewrite-core/src/main/java/org/openrewrite/ExcludeFileFromGitignore.java @@ -103,7 +103,7 @@ public PlainText visitText(PlainText text, ExecutionContext ctx) { private List sortRules(String[] originalRules, List newRules) { LinkedList results = new LinkedList<>(); Arrays.stream(originalRules).filter(line -> { - if (line.startsWith("#") || StringUtils.isBlank(line)) { + if (StringUtils.isBlank(line) || line.startsWith("#")) { return true; } return newRules.stream().anyMatch(line::equalsIgnoreCase); @@ -127,17 +127,18 @@ private List sortRules(String[] originalRules, List newRules) { return distinctValuesStartingReversed(results); } - private List distinctValuesStartingReversed(List list) { - Set set = new LinkedHashSet<>(); - ListIterator iterator = list.listIterator(list.size()); + private List distinctValuesStartingReversed(List list) { + LinkedList filteredList = new LinkedList<>(); + ListIterator iterator = list.listIterator(list.size()); while (iterator.hasPrevious()) { - set.add(iterator.previous()); + String previous = iterator.previous(); + if (StringUtils.isBlank(previous) || previous.startsWith("#") || !filteredList.contains(previous)) { + filteredList.addFirst(previous); + } } - List result = new ArrayList<>(set); - Collections.reverse(result); - return result; + return filteredList; } }); } diff --git a/rewrite-core/src/main/java/org/openrewrite/FindParseFailures.java b/rewrite-core/src/main/java/org/openrewrite/FindParseFailures.java index 6451ab0aaba..4e9168b6439 100644 --- a/rewrite-core/src/main/java/org/openrewrite/FindParseFailures.java +++ b/rewrite-core/src/main/java/org/openrewrite/FindParseFailures.java @@ -18,6 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.jspecify.annotations.Nullable; +import org.openrewrite.marker.DeserializationError; import org.openrewrite.marker.Markup; import org.openrewrite.table.ParseFailures; @@ -35,9 +36,9 @@ public class FindParseFailures extends Recipe { Integer maxSnippetLength; @Option(displayName = "Parser type", - description = "Only display failures from parsers with this fully qualified name.", + description = "Only display failures from parsers with this simple name.", required = false, - example = "org.openrewrite.yaml.YamlParser") + example = "YamlParser") @Nullable String parserType; @@ -67,33 +68,54 @@ public TreeVisitor getVisitor() { @Override public Tree postVisit(Tree tree, ExecutionContext ctx) { return tree.getMarkers().findFirst(ParseExceptionResult.class) - .map(exceptionResult -> { - if (parserType != null && !Objects.equals(exceptionResult.getParserType(), parserType)) { - return tree; - } - - if (stackTrace != null && !exceptionResult.getMessage().contains(stackTrace)) { - return tree; - } - - String snippet = tree instanceof SourceFile ? null : tree.printTrimmed(getCursor().getParentTreeCursor()); - if (snippet != null && maxSnippetLength != null && snippet.length() > maxSnippetLength) { - snippet = snippet.substring(0, maxSnippetLength); - } - - failures.insertRow(ctx, new ParseFailures.Row( - exceptionResult.getParserType(), - (tree instanceof SourceFile ? (SourceFile) tree : getCursor().firstEnclosingOrThrow(SourceFile.class)) - .getSourcePath().toString(), - exceptionResult.getExceptionType(), - exceptionResult.getTreeType(), - snippet, - exceptionResult.getMessage() - )); - - return Markup.info(tree, exceptionResult.getMessage()); - }) - .orElse(tree); + .map(exceptionResult -> report(tree, exceptionResult, ctx)) + .orElse(tree.getMarkers().findFirst(DeserializationError.class) + .map(error -> report(tree, error, ctx)) + .orElse(tree) + ); + } + + private Tree report(Tree tree, DeserializationError error, ExecutionContext ctx) { + if (stackTrace != null && !error.getDetail().contains(stackTrace)) { + return tree; + } + + failures.insertRow(ctx, new ParseFailures.Row( + "Unknown", + (tree instanceof SourceFile ? (SourceFile) tree : getCursor().firstEnclosingOrThrow(SourceFile.class)) + .getSourcePath().toString(), + "DeserializationError", + null, + null, + error.getDetail() + )); + + return Markup.info(tree, error.getMessage()); + } + + private Tree report(Tree tree, ParseExceptionResult exceptionResult, ExecutionContext ctx) { + if (parserType != null && !Objects.equals(exceptionResult.getParserType(), parserType)) { + return tree; + } else if (stackTrace != null && !exceptionResult.getMessage().contains(stackTrace)) { + return tree; + } + + String snippet = tree instanceof SourceFile ? null : tree.printTrimmed(getCursor().getParentTreeCursor()); + if (snippet != null && maxSnippetLength != null && snippet.length() > maxSnippetLength) { + snippet = snippet.substring(0, maxSnippetLength); + } + + failures.insertRow(ctx, new ParseFailures.Row( + exceptionResult.getParserType(), + (tree instanceof SourceFile ? (SourceFile) tree : getCursor().firstEnclosingOrThrow(SourceFile.class)) + .getSourcePath().toString(), + exceptionResult.getExceptionType(), + exceptionResult.getTreeType(), + snippet, + exceptionResult.getMessage() + )); + + return Markup.info(tree, exceptionResult.getMessage()); } }; } diff --git a/rewrite-core/src/main/java/org/openrewrite/Recipe.java b/rewrite-core/src/main/java/org/openrewrite/Recipe.java index f9cd6e6649e..9ae4586230c 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Recipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/Recipe.java @@ -211,10 +211,11 @@ public final RecipeDescriptor getDescriptor() { protected RecipeDescriptor createRecipeDescriptor() { List options = getOptionDescriptors(); - List recipeList1 = new ArrayList<>(); + ArrayList recipeList1 = new ArrayList<>(); for (Recipe next : getRecipeList()) { recipeList1.add(next.getDescriptor()); } + recipeList1.trimToSize(); URI recipeSource; try { recipeSource = getClass().getProtectionDomain().getCodeSource().getLocation().toURI(); @@ -233,7 +234,7 @@ private List getOptionDescriptors() { recipe = ((DelegatingRecipe) this).getDelegate(); } - List options = new ArrayList<>(); + ArrayList options = new ArrayList<>(); for (Field field : recipe.getClass().getDeclaredFields()) { Object value; @@ -255,6 +256,7 @@ private List getOptionDescriptors() { value)); } } + options.trimToSize(); return options; } diff --git a/rewrite-core/src/main/java/org/openrewrite/SourceFileWithReferences.java b/rewrite-core/src/main/java/org/openrewrite/SourceFileWithReferences.java new file mode 100644 index 00000000000..d53fa550e7a --- /dev/null +++ b/rewrite-core/src/main/java/org/openrewrite/SourceFileWithReferences.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024 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; + +import lombok.AccessLevel; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import org.jspecify.annotations.Nullable; +import org.openrewrite.trait.Reference; + +import java.util.*; + +@Incubating(since = "8.39.0") +public interface SourceFileWithReferences extends SourceFile { + + References getReferences(); + + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + @Getter + class References { + private final SourceFile sourceFile; + private final Set references; + + public Collection findMatches(Reference.Matcher matcher) { + return findMatchesInternal(matcher, null); + } + + public Collection findMatches(Reference.Matcher matcher, Reference.Kind kind) { + return findMatchesInternal(matcher, kind); + } + + private List findMatchesInternal(Reference.Matcher matcher, Reference.@Nullable Kind kind) { + List list = new ArrayList<>(); + for (Reference ref : references) { + if ((kind == null || ref.getKind().equals(kind)) && ref.matches(matcher) ) { + list.add(ref); + } + } + return list; + } + + public static References build(SourceFile sourceFile) { + Set references = new HashSet<>(); + ServiceLoader loader = ServiceLoader.load(Reference.Provider.class); + loader.forEach(provider -> { + if (provider.isAcceptable(sourceFile)) { + references.addAll(provider.getReferences(sourceFile)); + } + }); + return new References(sourceFile, references); + } + } +} diff --git a/rewrite-core/src/main/java/org/openrewrite/SourceFileWithTypeReferences.java b/rewrite-core/src/main/java/org/openrewrite/SourceFileWithTypeReferences.java deleted file mode 100644 index e4f8974a02b..00000000000 --- a/rewrite-core/src/main/java/org/openrewrite/SourceFileWithTypeReferences.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2024 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; - -import lombok.AccessLevel; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import org.openrewrite.trait.TypeReference; - -import java.util.*; - -@Incubating(since = "8.39.0") -public interface SourceFileWithTypeReferences extends SourceFile { - - TypeReferences getTypeReferences(); - - @RequiredArgsConstructor(access = AccessLevel.PRIVATE) - @Getter - class TypeReferences { - private final SourceFile sourceFile; - private final Set typeReferences; - - public Collection findMatches(TypeReference.Matcher matcher) { - List list = new ArrayList<>(); - for (TypeReference ref : typeReferences) { - if (ref.matches(matcher)) { - list.add(ref); - } - } - return list; - } - - public static TypeReferences build(SourceFile sourceFile) { - Set typeReferences = new HashSet<>(); - ServiceLoader loader = ServiceLoader.load(TypeReference.Provider.class); - loader.forEach(provider -> { - if (provider.isAcceptable(sourceFile)) { - typeReferences.addAll(provider.getTypeReferences(sourceFile)); - } - }); - return new TypeReferences(sourceFile, typeReferences); - } - } -} diff --git a/rewrite-core/src/main/java/org/openrewrite/TypeReferenceProvider.java b/rewrite-core/src/main/java/org/openrewrite/TypeReferenceProvider.java deleted file mode 100644 index b89ea2c767e..00000000000 --- a/rewrite-core/src/main/java/org/openrewrite/TypeReferenceProvider.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2024 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; - -import org.openrewrite.trait.TypeReference; - -import java.util.Set; - -public interface TypeReferenceProvider { - - Set getTypeReferences(SourceFile sourceFile); - - boolean isAcceptable(SourceFile sourceFile); -} diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java b/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java index bb3ca917e87..bd58337f85b 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java @@ -27,6 +27,17 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +/** + * Utility class for list transformations and manipulations, providing a variety of methods to modify or + * extend lists in a functional style while preserving immutability principles. + *

+ * If the transformation does not involve modifying LST elements or broader set of stream operations + * is needed, the Java Streams API may be more suitable. + * @implNote Most transformation methods in this class accept endomorphic mapping functions, i.e., functions + * of the form {@code f: T -> T}. The primary goal of these methods is to produce minimal change in a list’s structure, + * ensuring unnecessary memory allocations are avoided, + * See also Recipes must not mutate LSTs. + */ public final class ListUtils { private ListUtils() { } @@ -77,6 +88,7 @@ public static List insertInOrder(@Nullable List ls, T insert, Comparat /** * Insert element to a list at the specified position in the list. * Throws the same exceptions as List.add() + * * @param ls The original list. * @param t The element to add. * @param index index at which the specified element is to be inserted @@ -96,6 +108,15 @@ public static List insert(@Nullable List ls, @Nullable T t, int index) return newLs; } + /** + * Applies the specified mapping function to the last element of the list. + * If the resulting element is null, the last element is removed from the list. + * + * @param ls The list to modify. + * @param mapLast The mapping function to apply to the last element. + * @param The type of elements in the list. + * @return A new list with the modified last element, or the original list if unchanged. + */ public static List mapLast(@Nullable List ls, UnaryOperator<@Nullable T> mapLast) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions @@ -115,6 +136,15 @@ public static List mapLast(@Nullable List ls, UnaryOperator<@Nullable return ls; } + /** + * Applies the specified mapping function to the first element of the list. + * If the resulting element is null, the first element is removed from the list. + * + * @param ls The list to modify. + * @param mapFirst The mapping function to apply to the first element. + * @param The type of elements in the list. + * @return A new list with the modified first element, or the original list if unchanged. + */ public static List mapFirst(@Nullable List ls, UnaryOperator<@Nullable T> mapFirst) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions @@ -134,6 +164,15 @@ public static List mapFirst(@Nullable List ls, UnaryOperator<@Nullable return ls; } + /** + * Applies a mapping function to each element in the list along with its index. + * If any mapped element is null, it will be removed from the resulting list. + * + * @param ls The list to modify. + * @param map The mapping function that takes an index and an element. + * @param The type of elements in the list. + * @return A new list with modified elements, or the original list if unchanged. + */ public static List map(@Nullable List ls, BiFunction map) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions @@ -163,6 +202,14 @@ public static List map(@Nullable List ls, BiFunction The type of elements in the list. + * @return A new list with modified elements, or the original list if unchanged. + */ // inlined version of `map(List, BiFunction)` for memory efficiency (no overhead for lambda) public static List map(@Nullable List ls, UnaryOperator<@Nullable T> map) { if (ls == null || ls.isEmpty()) { @@ -193,6 +240,16 @@ public static List map(@Nullable List ls, UnaryOperator<@Nullable T> m return newLs; } + /** + * Applies a flat-mapping function to each element in the list with its index. + * Each element may map to a single object or an iterable of objects. + * If any mapped element is null, it will be removed from the list. + * + * @param ls The list to modify. + * @param flatMap The flat-mapping function that takes an index and an element. + * @param The type of elements in the list. + * @return A new list with expanded or modified elements, or the original list if unchanged. + */ public static List flatMap(@Nullable List ls, BiFunction flatMap) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions @@ -257,10 +314,26 @@ public static List flatMap(@Nullable List ls, BiFunction The type of elements in the list. + * @return A new list with expanded or modified elements, or the original list if unchanged. + */ public static List flatMap(@Nullable List ls, Function flatMap) { return flatMap(ls, (i, t) -> flatMap.apply(t)); } + /** + * Concatenates a single element to the end of the list. If the element is null, returns the original list. + * + * @param ls The original list. + * @param t The element to concatenate. + * @param The type of elements in the list. + * @return A new list with the added element, or an empty list if the element or list is null. + */ public static List concat(@Nullable List ls, @Nullable T t) { //noinspection DuplicatedCode if (t == null && ls == null) { @@ -273,6 +346,15 @@ public static List concat(@Nullable List ls, @Nullable T t) { return newLs; } + /** + * Concatenates a single element to the beginning of the list. If the element is null, returns the original list. + * + * @param t The element to add. + * @param ls The original list. + * @param The type of elements in the list. + * @return A new list with the added element at the start, the original list if the element is null or a null + * object if both element and list are null. + */ public static @Nullable List concat(@Nullable T t, @Nullable List ls) { if (t == null && ls == null) { //noinspection ConstantConditions @@ -288,6 +370,14 @@ public static List concat(@Nullable List ls, @Nullable T t) { return newLs; } + /** + * Concatenates two lists. If both are null, returns null. If only one is null, returns the other. + * + * @param ls The original list. + * @param t The list to concatenate. + * @param The type of elements in the lists. + * @return A new list containing both lists, or one of the lists if the other is null. + */ public static @Nullable List concatAll(@Nullable List ls, @Nullable List t) { if (ls == null && t == null) { //noinspection ConstantConditions @@ -306,6 +396,15 @@ public static List concat(@Nullable List ls, @Nullable T t) { return newLs; } + /** + * Inserts all elements of a list at the specified position in another list. + * + * @param ls The original list. + * @param index The position to insert the elements. + * @param t The list of elements to insert. + * @param The type of elements in the list. + * @return A new list with the elements inserted, or the original list if `t` is null or empty. + */ public static List insertAll(@Nullable List ls, int index, @Nullable List t) { if (ls == null && t == null) { return emptyList(); @@ -322,10 +421,25 @@ public static List insertAll(@Nullable List ls, int index, @Nullable L return newLs; } + /** + * Returns null if the list is empty; otherwise, returns the list. + * + * @param ls The list to check. + * @param The type of elements in the list. + * @return Null if the list is empty; otherwise, the list. + */ public static @Nullable List nullIfEmpty(@Nullable List ls) { return ls == null || ls.isEmpty() ? null : ls; } + /** + * Converts a list to an array if the list is non-empty; otherwise, returns null. + * + * @param list The list to convert. + * @param array The array type to populate. + * @param The type of elements in the list. + * @return The array representation of the list, or null if the list is empty. + */ public static T @Nullable [] arrayOrNullIfEmpty(@Nullable List list, T[] array) { if (list == null || list.isEmpty()) { return null; @@ -333,8 +447,15 @@ public static List insertAll(@Nullable List ls, int index, @Nullable L return list.toArray(array); } - public static T @Nullable [] nullIfEmpty(T @Nullable [] list) { - return list == null || list.length == 0 ? null : list; + /** + * Returns null if the array is empty; otherwise, returns the array. + * + * @param array The array to check. + * @param The type of elements in the array. + * @return Null if the array is empty; otherwise, the array. + */ + public static T @Nullable [] nullIfEmpty(T @Nullable [] array) { + return array == null || array.length == 0 ? null : array; } } diff --git a/rewrite-core/src/main/java/org/openrewrite/table/RewriteRecipeSource.java b/rewrite-core/src/main/java/org/openrewrite/table/RewriteRecipeSource.java index 201881218b9..3e6594d35e7 100644 --- a/rewrite-core/src/main/java/org/openrewrite/table/RewriteRecipeSource.java +++ b/rewrite-core/src/main/java/org/openrewrite/table/RewriteRecipeSource.java @@ -53,6 +53,7 @@ public static class Row { public enum RecipeType { Java, + Refaster, Yaml } } diff --git a/rewrite-core/src/main/java/org/openrewrite/trait/TypeReference.java b/rewrite-core/src/main/java/org/openrewrite/trait/Reference.java similarity index 55% rename from rewrite-core/src/main/java/org/openrewrite/trait/TypeReference.java rename to rewrite-core/src/main/java/org/openrewrite/trait/Reference.java index 8d2e89140e6..dbaea758e6f 100644 --- a/rewrite-core/src/main/java/org/openrewrite/trait/TypeReference.java +++ b/rewrite-core/src/main/java/org/openrewrite/trait/Reference.java @@ -15,29 +15,48 @@ */ package org.openrewrite.trait; -import org.openrewrite.Incubating; -import org.openrewrite.SourceFile; -import org.openrewrite.Tree; +import org.openrewrite.*; import java.util.Set; @Incubating(since = "8.39.0") -public interface TypeReference extends Trait { +public interface Reference extends Trait { - String getName(); + enum Kind { + TYPE, + PACKAGE + } + + Kind getKind(); + + String getValue(); + + default boolean supportsRename() { + return false; + } default boolean matches(Matcher matcher) { - return matcher.matchesName(getName()); + return matcher.matchesReference(this); + } + + default TreeVisitor rename(Renamer renamer, String replacement) { + return renamer.rename(replacement); } interface Provider { - Set getTypeReferences(SourceFile sourceFile); + Set getReferences(SourceFile sourceFile); boolean isAcceptable(SourceFile sourceFile); } interface Matcher { - boolean matchesName(String name); + boolean matchesReference(Reference value); + } + + interface Renamer { + default TreeVisitor rename(String replacement) { + throw new UnsupportedOperationException(); + } } } diff --git a/rewrite-core/src/test/java/org/openrewrite/ExcludeFileFromGitignoreTest.java b/rewrite-core/src/test/java/org/openrewrite/ExcludeFileFromGitignoreTest.java index af3233c90bf..fe4dc7468a8 100644 --- a/rewrite-core/src/test/java/org/openrewrite/ExcludeFileFromGitignoreTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/ExcludeFileFromGitignoreTest.java @@ -117,12 +117,18 @@ void commentsAndEmptyLinesUntouched() { # comment test.yml + # comment + + file.yaml """, """ # comment test.yml !/directory/test.yml + # comment + + file.yaml """, spec -> spec.path(".gitignore") ) @@ -411,7 +417,6 @@ void ignoreWildcardedDirectoryAtEnd() { ); } - @Test void ignoreWildcardedFile() { rewriteRun( diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependency.java index fa7565211e5..990a0606d65 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependency.java @@ -115,8 +115,6 @@ public class AddDependency extends ScanningRecipe { @Nullable Boolean acceptTransitive; - static final String DEPENDENCY_PRESENT = "org.openrewrite.gradle.AddDependency.DEPENDENCY_PRESENT"; - @Override public String getDisplayName() { return "Add Gradle dependency"; @@ -155,7 +153,8 @@ public Scanned getInitialValue(ExecutionContext ctx) { public TreeVisitor getScanner(Scanned acc) { return new TreeVisitor() { - UsesType usesType; + @Nullable + UsesType usesType = null; private boolean usesType(SourceFile sourceFile, ExecutionContext ctx) { if(onlyIfUsing == null) { return true; @@ -168,14 +167,17 @@ private boolean usesType(SourceFile sourceFile, ExecutionContext ctx) { @Override public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { - SourceFile sourceFile = (SourceFile) requireNonNull(tree); + if (!(tree instanceof SourceFile)) { + return tree; + } + SourceFile sourceFile = (SourceFile) tree; sourceFile.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> sourceFile.getMarkers().findFirst(JavaSourceSet.class).ifPresent(sourceSet -> { if (usesType(sourceFile, ctx)) { acc.usingType = true; - Set configurations = acc.configurationsByProject.computeIfAbsent(javaProject, ignored -> new HashSet<>()); - configurations.add("main".equals(sourceSet.getName()) ? "implementation" : sourceSet.getName() + "Implementation"); } + Set configurations = acc.configurationsByProject.computeIfAbsent(javaProject, ignored -> new HashSet<>()); + configurations.add("main".equals(sourceSet.getName()) ? "implementation" : sourceSet.getName() + "Implementation"); })); return tree; } @@ -184,7 +186,7 @@ private boolean usesType(SourceFile sourceFile, ExecutionContext ctx) { @Override public TreeVisitor getVisitor(Scanned acc) { - return Preconditions.check(acc.usingType && !acc.configurationsByProject.isEmpty(), + return Preconditions.check((onlyIfUsing == null || acc.usingType) && !acc.configurationsByProject.isEmpty(), Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { @Override @@ -225,7 +227,7 @@ public TreeVisitor getVisitor(Scanned acc) { tmpConfigurations = new HashSet<>(resolvedConfigurations); for (String tmpConfiguration : tmpConfigurations) { - GradleDependencyConfiguration gdc = gp.getConfiguration(tmpConfiguration); + GradleDependencyConfiguration gdc = requireNonNull((gp.getConfiguration(tmpConfiguration))); for (GradleDependencyConfiguration transitive : gp.configurationsExtendingFrom(gdc, true)) { if (resolvedConfigurations.contains(transitive.getName()) || (Boolean.TRUE.equals(acceptTransitive) && transitive.findResolvedDependency(groupId, artifactId) != null)) { @@ -248,7 +250,7 @@ public TreeVisitor getVisitor(Scanned acc) { } private boolean isTopLevel(Cursor cursor) { - return cursor.getParent().firstEnclosing(J.MethodInvocation.class) == null; + return cursor.getParentOrThrow().firstEnclosing(J.MethodInvocation.class) == null; } }) ); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java index 618c8a7504e..80fa69ba0c0 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java @@ -15,10 +15,7 @@ */ package org.openrewrite.gradle; -import lombok.AccessLevel; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.RequiredArgsConstructor; +import lombok.*; import lombok.experimental.FieldDefaults; import lombok.experimental.NonFinal; import org.jspecify.annotations.Nullable; @@ -158,6 +155,7 @@ private GradleWrapper getGradleWrapper(ExecutionContext ctx) { return gradleWrapper; } + @Data public static class GradleWrapperState { boolean gradleProject = false; boolean needsWrapperUpdate = false; @@ -251,10 +249,6 @@ public Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) { new TreeVisitor() { @Override public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { - if (!super.isAcceptable(sourceFile, ctx)) { - return false; - } - if (new FindGradleProject(FindGradleProject.SearchCriteria.Marker).getVisitor().visitNonNull(sourceFile, ctx) != sourceFile) { acc.gradleProject = true; } @@ -277,6 +271,12 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { return false; } + + @Override + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + // "work" already performed by `isAcceptable()`; no need to visit anymore + return tree; + } } ); } @@ -380,6 +380,7 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { } } + GradleWrapper gradleWrapper = getGradleWrapper(ctx); if (sourceFile instanceof PlainText && PathUtils.matchesGlob(sourceFile.getSourcePath(), "**/" + WRAPPER_SCRIPT_LOCATION_RELATIVE_PATH)) { String gradlewText = unixScript(gradleWrapper, ctx); PlainText gradlew = (PlainText) setExecutable(sourceFile); @@ -476,7 +477,7 @@ private String renderTemplate(String source, Map parameters, Str return script; } - private class WrapperPropertiesVisitor extends PropertiesVisitor { + private static class WrapperPropertiesVisitor extends PropertiesVisitor { private static final String DISTRIBUTION_SHA_256_SUM_KEY = "distributionSha256Sum"; private final GradleWrapper gradleWrapper; diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/search/FindPlugins.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/search/FindPlugins.java index 14a52457859..f1a068caeac 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/search/FindPlugins.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/search/FindPlugins.java @@ -91,7 +91,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) // Even if we couldn't find a declaration the metadata might show the plugin is in use GradleProject gp = s.getMarkers().findFirst(GradleProject.class).orElse(null); if (!found.get() && gp != null && gp.getPlugins().stream() - .anyMatch(it -> pluginId.equals(it.getId()))) { + .anyMatch(it -> pluginId.equals(it.getId()))) { s = SearchResult.found(s); } @@ -117,17 +117,18 @@ public static List find(J j, String pluginIdPattern) { MethodMatcher idMatcher = new MethodMatcher("PluginSpec id(..)", false); MethodMatcher versionMatcher = new MethodMatcher("Plugin version(..)", false); - List pluginsWithVersion = plugins.stream().flatMap(plugin -> { - if (versionMatcher.matches(plugin) && idMatcher.matches(plugin.getSelect())) { - return Stream.of(new GradlePlugin( - plugin, - requireNonNull(((J.Literal) requireNonNull(((J.MethodInvocation) plugin.getSelect())) - .getArguments().get(0)).getValue()).toString(), - requireNonNull(((J.Literal) plugin.getArguments().get(0)).getValue()).toString() - )); - } - return Stream.empty(); - }).collect(toList()); + List pluginsWithVersion = plugins.stream() + .flatMap(plugin -> { + if (versionMatcher.matches(plugin) && idMatcher.matches(plugin.getSelect()) && plugin.getArguments().get(0) instanceof J.Literal) { + return Stream.of(new GradlePlugin( + plugin, + requireNonNull(((J.Literal) requireNonNull(((J.MethodInvocation) plugin.getSelect())) + .getArguments().get(0)).getValue()).toString(), + requireNonNull(((J.Literal) plugin.getArguments().get(0)).getValue()).toString() + )); + } + return Stream.empty(); + }).collect(toList()); List pluginsWithoutVersion = plugins.stream().flatMap(plugin -> { if (idMatcher.matches(plugin) && pluginsWithVersion.stream() .noneMatch(it -> it.getPluginId().equals(plugin.getSimpleName()))) { diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/AddDependencyTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/AddDependencyTest.java index 42596fb4002..6330f931018 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/AddDependencyTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/AddDependencyTest.java @@ -76,7 +76,7 @@ void onlyIfUsingTestScope(String onlyIfUsing) { plugins { id "java-library" } - + repositories { mavenCentral() } @@ -85,11 +85,11 @@ void onlyIfUsingTestScope(String onlyIfUsing) { plugins { id "java-library" } - + repositories { mavenCentral() } - + dependencies { testImplementation "com.google.guava:guava:29.0-jre" } @@ -115,11 +115,11 @@ void onlyIfUsingSmokeTestScope(String onlyIfUsing) { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" @@ -131,17 +131,17 @@ void onlyIfUsingSmokeTestScope(String onlyIfUsing) { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" } } - + dependencies { smokeTestImplementation "com.google.guava:guava:29.0-jre" } @@ -165,7 +165,7 @@ void onlyIfUsingCompileScope(String onlyIfUsing) { plugins { id 'java-library' } - + repositories { mavenCentral() } @@ -174,11 +174,11 @@ void onlyIfUsingCompileScope(String onlyIfUsing) { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:29.0-jre" } @@ -211,7 +211,7 @@ void onlyIfUsingMultipleScopes(String onlyIfUsing) { plugins { id "java-library" } - + repositories { mavenCentral() } @@ -220,11 +220,11 @@ void onlyIfUsingMultipleScopes(String onlyIfUsing) { plugins { id "java-library" } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:29.0-jre" } @@ -257,11 +257,11 @@ void usedInMultipleSourceSetsUsingExplicitSourceSet(String onlyIfUsing) { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" @@ -273,17 +273,17 @@ void usedInMultipleSourceSetsUsingExplicitSourceSet(String onlyIfUsing) { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" } } - + dependencies { implementation "com.google.guava:guava:29.0-jre" } @@ -312,11 +312,11 @@ void usedInTransitiveSourceSet() { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" @@ -328,17 +328,17 @@ void usedInTransitiveSourceSet() { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "test" } } - + dependencies { testImplementation "com.google.guava:guava:29.0-jre" } @@ -367,11 +367,11 @@ void addDependencyIfNotUsedInATransitive() { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "main" @@ -383,20 +383,20 @@ void addDependencyIfNotUsedInATransitive() { id "java-library" id "com.netflix.nebula.facet" version "10.1.3" } - + repositories { mavenCentral() } - + facets { smokeTest { parentSourceSet = "main" } } - + dependencies { smokeTestImplementation "com.google.guava:guava:29.0-jre" - + testImplementation "com.google.guava:guava:29.0-jre" } """ @@ -419,7 +419,7 @@ void addDependencyWithClassifier() { plugins { id "java-library" } - + repositories { mavenCentral() } @@ -428,11 +428,11 @@ void addDependencyWithClassifier() { plugins { id "java-library" } - + repositories { mavenCentral() } - + dependencies { implementation "io.netty:netty-tcnative-boringssl-static:2.0.54.Final:linux-x86_64" } @@ -456,7 +456,7 @@ void addDependencyWithoutVersion() { plugins { id "java-library" } - + repositories { mavenCentral() } @@ -465,11 +465,11 @@ void addDependencyWithoutVersion() { plugins { id "java-library" } - + repositories { mavenCentral() } - + dependencies { implementation "io.netty:netty-tcnative-boringssl-static" } @@ -494,7 +494,7 @@ void addDependencyWithoutVersionWithClassifier() { plugins { id "java-library" } - + repositories { mavenCentral() } @@ -503,11 +503,11 @@ void addDependencyWithoutVersionWithClassifier() { plugins { id "java-library" } - + repositories { mavenCentral() } - + dependencies { implementation "io.netty:netty-tcnative-boringssl-static" } @@ -546,11 +546,11 @@ void addInOrder() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "commons-lang:commons-lang:1.0" } @@ -559,11 +559,11 @@ void addInOrder() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:29.0-jre" implementation "commons-lang:commons-lang:1.0" @@ -587,11 +587,11 @@ void addTestDependenciesAfterCompile() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "commons-lang:commons-lang:1.0" } @@ -600,14 +600,14 @@ void addTestDependenciesAfterCompile() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "commons-lang:commons-lang:1.0" - + testImplementation "com.google.guava:guava:29.0-jre" } """ @@ -629,11 +629,11 @@ void addDependenciesKeepFormatting() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "org.openrewrite:rewrite-core:7.40.8" testImplementation "junit:junit:4.12" @@ -643,11 +643,11 @@ void addDependenciesKeepFormatting() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "org.openrewrite:rewrite-core:7.40.8" implementation "org.slf4j:slf4j-api:2.0.7" @@ -667,7 +667,7 @@ void addDependencyToNewGrouping() { srcMainJava( java(""" import lombok.Value; - + @Value class A { String b; @@ -680,11 +680,11 @@ class A { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "commons-lang:commons-lang:2.6" @@ -695,16 +695,16 @@ class A { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { annotationProcessor "org.projectlombok:lombok:1.18.26" - + implementation "commons-lang:commons-lang:2.6" - + testImplementation "junit:junit:4.13" } """ @@ -726,11 +726,11 @@ void addDependenciesToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" @@ -742,14 +742,14 @@ void addDependenciesToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + testImplementation group: "com.google.guava", name: "guava", version: "29.0-jre" def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion @@ -773,14 +773,14 @@ void addDependenciesWithoutVersionToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion } @@ -789,14 +789,14 @@ void addDependenciesWithoutVersionToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + testImplementation group: "com.google.guava", name: "guava" def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion @@ -820,14 +820,14 @@ void addDependenciesWithClassifierToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion } @@ -836,14 +836,14 @@ void addDependenciesWithClassifierToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + testImplementation group: "com.google.guava", name: "guava", version: "29.0-jre", classifier: "test" def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion @@ -868,14 +868,14 @@ void addDependenciesWithoutVersionWithClassifierToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion } @@ -884,14 +884,14 @@ void addDependenciesWithoutVersionWithClassifierToExistingGrouping() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" - + testImplementation group: "io.netty", name: "netty-tcnative-boringssl-static", classifier: "linux-x86_64" def junitVersion = "4.12" testImplementation group: "junit", name: "junit", version: junitVersion @@ -915,11 +915,11 @@ void matchesDependencyDeclarationStyle() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" } @@ -928,11 +928,11 @@ void matchesDependencyDeclarationStyle() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation group: "commons-lang", name: "commons-lang", version: "1.0" @@ -957,11 +957,11 @@ void addDependencyDoesntAddWhenExistingDependency() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:28.0-jre" } @@ -991,7 +991,7 @@ public class A { plugins { id 'java-library' } - + repositories { mavenCentral() } @@ -1000,11 +1000,11 @@ public class A { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { %s "com.fasterxml.jackson.core:jackson-core:2.12.0" } @@ -1038,7 +1038,7 @@ void addDependencyToProjectWithOtherSourceTypes() { plugins { id 'java-library' } - + repositories { mavenCentral() } @@ -1047,11 +1047,11 @@ void addDependencyToProjectWithOtherSourceTypes() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:29.0-jre" } @@ -1091,7 +1091,7 @@ void addDependencyToProjectsThatNeedIt() { plugins { id 'java-library' } - + repositories { mavenCentral() } @@ -1100,11 +1100,11 @@ void addDependencyToProjectsThatNeedIt() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation "com.google.guava:guava:29.0-jre" } @@ -1131,7 +1131,7 @@ void addDynamicVersionDependency() { groovy( """ import java.util.* - + class MyClass { static void main(String[] args) { Date date = new Date() @@ -1146,7 +1146,7 @@ static void main(String[] args) { plugins { id 'java' } - + repositories { mavenCentral() } @@ -1155,11 +1155,11 @@ static void main(String[] args) { plugins { id 'java' } - + repositories { mavenCentral() } - + dependencies { implementation "org.openrewrite:rewrite-core:7.39.1" } @@ -1196,24 +1196,24 @@ void addDependencyWithVariable() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + def guavaVersion = "29.0-jre" """, """ plugins { id 'java-library' } - + repositories { mavenCentral() } - + def guavaVersion = "29.0-jre" - + dependencies { implementation "com.google.guava:guava:${guavaVersion}" } @@ -1238,7 +1238,7 @@ void defaultConfigurationEscaped() { plugins { id 'java' } - + repositories { mavenCentral() } @@ -1247,11 +1247,11 @@ void defaultConfigurationEscaped() { plugins { id 'java' } - + repositories { mavenCentral() } - + dependencies { 'default' "com.google.guava:guava:29.0-jre" } @@ -1312,11 +1312,51 @@ void doNotAddToIncorrectBlocks() { ); } - private AddDependency addDependency(String gav, String onlyIfUsing) { + @Test + void addUnconditionally() { + rewriteRun( + spec -> spec.recipe(addDependency("org.apache.logging.log4j:log4j-core:2.22.1")), + mavenProject("project", + srcMainJava( + java(usingGuavaIntMath) + ), + buildGradle(""" + plugins { + id "java-library" + } + repositories { + mavenCentral() + } + dependencies { + implementation 'org.openrewrite:rewrite-core:8.35.0' + } + """, + """ + plugins { + id "java-library" + } + repositories { + mavenCentral() + } + dependencies { + implementation "org.apache.logging.log4j:log4j-core:2.22.1" + implementation 'org.openrewrite:rewrite-core:8.35.0' + } + """, + spec -> spec.path("build.gradle") + )) + ); + } + + private AddDependency addDependency(@SuppressWarnings("SameParameterValue") String gav) { + return addDependency(gav, null, null); + } + + private AddDependency addDependency(String gav, @Nullable String onlyIfUsing) { return addDependency(gav, onlyIfUsing, null); } - private AddDependency addDependency(String gav, String onlyIfUsing, @Nullable String configuration) { + private AddDependency addDependency(String gav, @Nullable String onlyIfUsing, @Nullable String configuration) { String[] gavParts = gav.split(":"); return new AddDependency( gavParts[0], gavParts[1], (gavParts.length < 3) ? null : gavParts[2], null, configuration, onlyIfUsing, diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java index c4f9f476ec5..78cd8c053fc 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java @@ -26,7 +26,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.gradle.Assertions.buildGradle; +import static org.openrewrite.gradle.Assertions.settingsGradle; import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; +import static org.openrewrite.properties.Assertions.properties; import static org.openrewrite.test.SourceSpecs.text; class FindPluginsTest implements RewriteTest { @@ -43,21 +45,84 @@ void findPlugin() { buildGradle( """ plugins { + id 'java' id 'org.openrewrite.rewrite' version '6.18.0' } """, """ plugins { + id 'java' /*~~>*/id 'org.openrewrite.rewrite' version '6.18.0' } """, - spec -> spec - .beforeRecipe(cu -> assertThat(FindPlugins.find(cu, "org.openrewrite.rewrite")) - .isNotEmpty() - .anySatisfy(p -> { - assertThat(p.getPluginId()).isEqualTo("org.openrewrite.rewrite"); - assertThat(p.getVersion()).isEqualTo("6.18.0"); - })) + spec -> spec.beforeRecipe(cu -> assertThat(FindPlugins.find(cu, "org.openrewrite.rewrite")) + .isNotEmpty() + .anySatisfy(p -> { + assertThat(p.getPluginId()).isEqualTo("org.openrewrite.rewrite"); + assertThat(p.getVersion()).isEqualTo("6.18.0"); + })) + ) + ); + } + + @Test + void settingsResolutionStrategy() { + rewriteRun( + spec -> spec.beforeRecipe(withToolingApi()), + settingsGradle( + """ + pluginManagement { + repositories { + mavenLocal() + gradlePluginPortal() + } + resolutionStrategy { + eachPlugin { + if (requested.id.id == 'org.openrewrite.rewrite') { + useVersion('6.22.0') + } + } + } + } + """ + ), + buildGradle( + """ + plugins { + id "org.openrewrite.rewrite" + } + """, + """ + plugins { + /*~~>*/id "org.openrewrite.rewrite" + } + """, + spec -> spec.beforeRecipe(cu -> assertThat(FindPlugins.find(cu, "org.openrewrite.rewrite")) + .isNotEmpty() + .anySatisfy(p -> assertThat(p.getPluginId()).isEqualTo("org.openrewrite.rewrite"))) + ) + ); + } + + @Test + void property() { + rewriteRun( + spec -> spec.beforeRecipe(withToolingApi()), + properties("rewritePluginVersion=6.22.0", spec -> spec.path("gradle.properties")), + buildGradle( + """ + plugins { + id "org.openrewrite.rewrite" version "${rewritePluginVersion}" + } + """, + """ + plugins { + /*~~>*/id "org.openrewrite.rewrite" version "${rewritePluginVersion}" + } + """, + spec -> spec.beforeRecipe(cu -> assertThat(FindPlugins.find(cu, "org.openrewrite.rewrite")) + .isNotEmpty() + .anySatisfy(p -> assertThat(p.getPluginId()).isEqualTo("org.openrewrite.rewrite"))) ) ); } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/cleanup/SimplifyBooleanExpressionVisitorTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/cleanup/SimplifyBooleanExpressionVisitorTest.java index 8a30277affa..fd8aba9c7c3 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/cleanup/SimplifyBooleanExpressionVisitorTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/cleanup/SimplifyBooleanExpressionVisitorTest.java @@ -414,6 +414,23 @@ def m() { ); } + @Test + void mapFields() { + rewriteRun( + groovy( + """ + class A { + def foo(boolean a, Map m) { + if (a || m.foo || m.bar) { + System.out.println("") + } + } + } + """ + ) + ); + } + @ParameterizedTest @Issue("https://github.com/openrewrite/rewrite-templating/issues/28") // Mimic what would be inserted by a Refaster template using two nullable parameters, with the second one a literal @@ -468,7 +485,6 @@ def m() { a == null || !a.isEmpty() && "b" != null && !"b".isEmpty() // a == null || !a.isEmpty() """) void simplifyLiteralNull(String before, String after) { - //language=java String template = """ class A { void foo(String a) { diff --git a/rewrite-hcl/src/main/java/org/openrewrite/hcl/JsonPathMatcher.java b/rewrite-hcl/src/main/java/org/openrewrite/hcl/JsonPathMatcher.java index 977965b883e..452023b810b 100644 --- a/rewrite-hcl/src/main/java/org/openrewrite/hcl/JsonPathMatcher.java +++ b/rewrite-hcl/src/main/java/org/openrewrite/hcl/JsonPathMatcher.java @@ -20,6 +20,7 @@ import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.tree.ParseTree; +import org.antlr.v4.runtime.tree.RuleNode; import org.antlr.v4.runtime.tree.TerminalNode; import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; @@ -48,28 +49,28 @@ public class JsonPathMatcher { private final String jsonPath; + private JsonPathParser.@Nullable JsonPathContext parsed; public JsonPathMatcher(String jsonPath) { this.jsonPath = jsonPath; } public Optional find(Cursor cursor) { - LinkedList cursorPath = cursor.getPathAsStream() - .filter(o -> o instanceof Tree) - .map(Tree.class::cast) - .collect(Collectors.toCollection(LinkedList::new)); + return find0(cursor, resolvedAncestors(cursor)); + } + + private Optional find0(Cursor cursor, List cursorPath) { if (cursorPath.isEmpty()) { return Optional.empty(); } - Collections.reverse(cursorPath); Tree start; if (jsonPath.startsWith(".") && !jsonPath.startsWith("..")) { start = cursor.getValue(); } else { - start = cursorPath.peekFirst(); + start = cursorPath.get(0); } - JsonPathParser.JsonPathContext ctx = jsonPath().jsonPath(); + JsonPathParser.JsonPathContext ctx = parse(); // The stop may be optimized by interpreting the ExpressionContext and pre-determining the last visit. JsonPathParser.ExpressionContext stop = (JsonPathParser.ExpressionContext) ctx.children.get(ctx.children.size() - 1); @SuppressWarnings("ConstantConditions") JsonPathParserVisitor v = new JsonPathParserHclVisitor(cursorPath, start, stop, false); @@ -80,8 +81,8 @@ public Optional find(Cursor cursor) { } public boolean matches(Cursor cursor) { - List cursorPath = cursor.getPathAsStream().collect(Collectors.toList()); - return find(cursor).map(o -> { + List cursorPath = resolvedAncestors(cursor); + return find0(cursor, cursorPath).map(o -> { if (o instanceof List) { //noinspection unchecked List l = (List) o; @@ -92,6 +93,22 @@ public boolean matches(Cursor cursor) { }).orElse(false); } + private static List resolvedAncestors(Cursor cursor) { + ArrayDeque deque = new ArrayDeque<>(); + for (Iterator it = cursor.getPath(Tree.class::isInstance); it.hasNext(); ) { + Tree tree = (Tree) it.next(); + deque.addFirst(tree); + } + return new ArrayList<>(deque); + } + + private JsonPathParser.JsonPathContext parse() { + if (parsed == null) { + parsed = jsonPath().jsonPath(); + } + return parsed; + } + private JsonPathParser jsonPath() { return new JsonPathParser(new CommonTokenStream(new JsonPathLexer(CharStreams.fromString(this.jsonPath)))); } @@ -121,6 +138,11 @@ protected Object aggregateResult(Object aggregate, Object nextResult) { return (scope = nextResult); } + @Override + protected boolean shouldVisitNextChild(RuleNode node, Object currentResult) { + return scope != null; + } + @Override public Object visitJsonPath(JsonPathParser.JsonPathContext ctx) { if (ctx.ROOT() != null || "[".equals(ctx.start.getText())) { diff --git a/rewrite-hcl/src/test/java/org/openrewrite/hcl/tree/HclStringTest.java b/rewrite-hcl/src/test/java/org/openrewrite/hcl/tree/HclStringTest.java index 19bbf88d8f9..0c9262c4ab6 100644 --- a/rewrite-hcl/src/test/java/org/openrewrite/hcl/tree/HclStringTest.java +++ b/rewrite-hcl/src/test/java/org/openrewrite/hcl/tree/HclStringTest.java @@ -16,7 +16,6 @@ package org.openrewrite.hcl.tree; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; @@ -24,7 +23,6 @@ class HclStringTest implements RewriteTest { - @ExpectedToFail @Issue("https://github.com/openrewrite/rewrite/issues/4612") @Test void quoteEscaping() { @@ -32,7 +30,7 @@ void quoteEscaping() { hcl( """ locals { - quotedText = "this is a double quote: \". Look at me" + quotedText = "this is a double quote: \\". Look at me" } """ ) diff --git a/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11JavadocVisitor.java b/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11JavadocVisitor.java index cec6ca39d64..0ff9afbed53 100644 --- a/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11JavadocVisitor.java +++ b/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11JavadocVisitor.java @@ -125,7 +125,7 @@ private void init() { } else { // Handle consecutive new lines. if ((prev == '\n' || - prev == '\r' && source.charAt(i - 2) == '\n')) { + prev == '\r' && source.charAt(i - 2) == '\n')) { String prevLineLine = prev == '\n' ? "\n" : "\r\n"; lineBreaks.put(javadocContent.length(), new Javadoc.LineBreak(randomId(), prevLineLine, Markers.EMPTY)); } else if (marginBuilder != null) { // A new line with no '*' that only contains whitespace. @@ -581,14 +581,14 @@ public Tree visitProvides(ProvidesTree node, List body) { if (ref.qualifierExpression != null) { try { attr.attribType(ref.qualifierExpression, symbol); - } catch(NullPointerException ignored) { + } catch (NullPointerException ignored) { // best effort, can result in: // java.lang.NullPointerException: Cannot read field "info" because "env" is null // at com.sun.tools.javac.comp.Attr.attribType(Attr.java:404) } } - if(ref.qualifierExpression != null) { + if (ref.qualifierExpression != null) { qualifier = (TypedTree) javaVisitor.scan(ref.qualifierExpression, Space.EMPTY); qualifierType = qualifier.getType(); if (ref.memberName != null) { @@ -678,32 +678,23 @@ public Tree visitProvides(ProvidesTree node, List body) { } private JavaType.@Nullable Method methodReferenceType(DCTree.DCReference ref, @Nullable JavaType type) { - if (type instanceof JavaType.Class) { + if (type instanceof JavaType.Class) { JavaType.Class classType = (JavaType.Class) type; nextMethod: for (JavaType.Method method : classType.getMethods()) { if (method.getName().equals(ref.memberName.toString())) { if (ref.paramTypes != null) { - for (JCTree param : ref.paramTypes) { - for (JavaType testParamType : method.getParameterTypes()) { - Type paramType = attr.attribType(param, symbol); - if (testParamType instanceof JavaType.GenericTypeVariable) { - List bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds(); - if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) { - return method; - } - for (JavaType bound : bounds) { - if (paramTypeMatches(bound, paramType)) { - return method; - } - } - continue nextMethod; - } - - if (paramTypeMatches(testParamType, paramType)) { - continue nextMethod; - } + List parameterTypes = method.getParameterTypes(); + if (ref.paramTypes.size() != parameterTypes.size()) { + continue; + } + for (int i = 0; i < ref.paramTypes.size(); i++) { + JCTree param = ref.paramTypes.get(i); + JavaType testParamType = parameterTypes.get(i); + Type paramType = attr.attribType(param, symbol); + if (!paramTypeMatches(testParamType, paramType)) { + continue nextMethod; } } } @@ -724,13 +715,25 @@ public Tree visitProvides(ProvidesTree node, List body) { return null; } - private boolean paramTypeMatches(JavaType testParamType, Type paramType) { - if (paramType instanceof Type.ClassType) { - JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType); - return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym) - .fullname.toString()); + private boolean paramTypeMatches(JavaType parameterType, Type javadocType) { + return paramTypeMatches(parameterType, typeMapping.type(javadocType)); + } + + // Javadoc type references typically don't have generic type parameters + private static boolean paramTypeMatches(JavaType parameterType, JavaType mappedJavadocType) { + if (parameterType instanceof JavaType.Array && mappedJavadocType instanceof JavaType.Array) { + if (((JavaType.Array) parameterType).getElemType() instanceof JavaType.Primitive) { + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); + } + return paramTypeMatches(((JavaType.Array) parameterType).getElemType(), ((JavaType.Array) mappedJavadocType).getElemType()); + } else if (parameterType instanceof JavaType.GenericTypeVariable && !((JavaType.GenericTypeVariable) parameterType).getBounds().isEmpty()) { + return paramTypeMatches(((JavaType.GenericTypeVariable) parameterType).getBounds().get(0), mappedJavadocType); + } else if (parameterType instanceof JavaType.GenericTypeVariable) { + return TypeUtils.isObject(mappedJavadocType); + } else if (parameterType instanceof JavaType.Parameterized && !(mappedJavadocType instanceof JavaType.Parameterized)) { + return paramTypeMatches(((JavaType.Parameterized) parameterType).getType(), mappedJavadocType); } - return false; + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); } private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) { @@ -865,14 +868,14 @@ public Tree visitVersion(VersionTree node, List body) { @Override public Tree visitText(TextTree node, List body) { throw new UnsupportedOperationException("Anywhere text can occur, we need to call the visitText override that " + - "returns a list of Javadoc elements."); + "returns a list of Javadoc elements."); } public List visitText(String node) { List texts = new ArrayList<>(); if (!node.isEmpty() && Character.isWhitespace(node.charAt(0)) && - !Character.isWhitespace(source.charAt(cursor))) { + !Character.isWhitespace(source.charAt(cursor))) { node = node.stripLeading(); } diff --git a/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java b/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java index 5f3295c40a7..d0b90bea744 100644 --- a/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java +++ b/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java @@ -579,7 +579,11 @@ public JavaType.Primitive primitive(TypeTag tag) { .map(attr -> attr.getValue().toString()) .collect(Collectors.toList()); } else { - defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + try { + defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + } catch(UnsupportedOperationException e) { + // not all Attribute implementations define `getValue()` + } } } JavaType.Method method = new JavaType.Method( diff --git a/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17JavadocVisitor.java b/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17JavadocVisitor.java index 9ec2daba8d6..c88faeda547 100644 --- a/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17JavadocVisitor.java +++ b/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17JavadocVisitor.java @@ -688,25 +688,16 @@ public Tree visitProvides(ProvidesTree node, List body) { for (JavaType.Method method : classType.getMethods()) { if (method.getName().equals(ref.memberName.toString())) { if (ref.paramTypes != null) { - for (JCTree param : ref.paramTypes) { - for (JavaType testParamType : method.getParameterTypes()) { - Type paramType = attr.attribType(param, symbol); - if (testParamType instanceof JavaType.GenericTypeVariable) { - List bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds(); - if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) { - return method; - } - for (JavaType bound : bounds) { - if (paramTypeMatches(bound, paramType)) { - return method; - } - } - continue nextMethod; - } - - if (paramTypeMatches(testParamType, paramType)) { - continue nextMethod; - } + List parameterTypes = method.getParameterTypes(); + if (ref.paramTypes.size() != parameterTypes.size()) { + continue; + } + for (int i = 0; i < ref.paramTypes.size(); i++) { + JCTree param = ref.paramTypes.get(i); + JavaType testParamType = parameterTypes.get(i); + Type paramType = attr.attribType(param, symbol); + if (!paramTypeMatches(testParamType, paramType)) { + continue nextMethod; } } } @@ -727,13 +718,25 @@ public Tree visitProvides(ProvidesTree node, List body) { return null; } - private boolean paramTypeMatches(JavaType testParamType, Type paramType) { - if (paramType instanceof Type.ClassType) { - JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType); - return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym) - .fullname.toString()); + private boolean paramTypeMatches(JavaType parameterType, Type javadocType) { + return paramTypeMatches(parameterType, typeMapping.type(javadocType)); + } + + // Javadoc type references typically don't have generic type parameters + private static boolean paramTypeMatches(JavaType parameterType, JavaType mappedJavadocType) { + if (parameterType instanceof JavaType.Array && mappedJavadocType instanceof JavaType.Array) { + if (((JavaType.Array) parameterType).getElemType() instanceof JavaType.Primitive) { + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); + } + return paramTypeMatches(((JavaType.Array) parameterType).getElemType(), ((JavaType.Array) mappedJavadocType).getElemType()); + } else if (parameterType instanceof JavaType.GenericTypeVariable && !((JavaType.GenericTypeVariable) parameterType).getBounds().isEmpty()) { + return paramTypeMatches(((JavaType.GenericTypeVariable) parameterType).getBounds().get(0), mappedJavadocType); + } else if (parameterType instanceof JavaType.GenericTypeVariable) { + return TypeUtils.isObject(mappedJavadocType); + } else if (parameterType instanceof JavaType.Parameterized && !(mappedJavadocType instanceof JavaType.Parameterized)) { + return paramTypeMatches(((JavaType.Parameterized) parameterType).getType(), mappedJavadocType); } - return false; + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); } private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) { diff --git a/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java b/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java index b7c7ff483ff..7d5ad9da3f6 100644 --- a/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java +++ b/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java @@ -588,7 +588,11 @@ public JavaType.Primitive primitive(TypeTag tag) { .map(attr -> attr.getValue().toString()) .collect(Collectors.toList()); } else { - defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + try { + defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + } catch(UnsupportedOperationException e) { + // not all Attribute implementations define `getValue()` + } } } JavaType.Method method = new JavaType.Method( diff --git a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21JavadocVisitor.java b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21JavadocVisitor.java index ee0a986d231..05fbb3bbe95 100644 --- a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21JavadocVisitor.java +++ b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21JavadocVisitor.java @@ -688,25 +688,16 @@ public Tree visitProvides(ProvidesTree node, List body) { for (JavaType.Method method : classType.getMethods()) { if (method.getName().equals(ref.memberName.toString())) { if (ref.paramTypes != null) { - for (JCTree param : ref.paramTypes) { - for (JavaType testParamType : method.getParameterTypes()) { - Type paramType = attr.attribType(param, symbol); - if (testParamType instanceof JavaType.GenericTypeVariable) { - List bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds(); - if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) { - return method; - } - for (JavaType bound : bounds) { - if (paramTypeMatches(bound, paramType)) { - return method; - } - } - continue nextMethod; - } - - if (paramTypeMatches(testParamType, paramType)) { - continue nextMethod; - } + List parameterTypes = method.getParameterTypes(); + if (ref.paramTypes.size() != parameterTypes.size()) { + continue; + } + for (int i = 0; i < ref.paramTypes.size(); i++) { + JCTree param = ref.paramTypes.get(i); + JavaType testParamType = parameterTypes.get(i); + Type paramType = attr.attribType(param, symbol); + if (!paramTypeMatches(testParamType, paramType)) { + continue nextMethod; } } } @@ -727,13 +718,25 @@ public Tree visitProvides(ProvidesTree node, List body) { return null; } - private boolean paramTypeMatches(JavaType testParamType, Type paramType) { - if (paramType instanceof Type.ClassType) { - JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType); - return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym) - .fullname.toString()); + private boolean paramTypeMatches(JavaType parameterType, Type javadocType) { + return paramTypeMatches(parameterType, typeMapping.type(javadocType)); + } + + // Javadoc type references typically don't have generic type parameters + private static boolean paramTypeMatches(JavaType parameterType, JavaType mappedJavadocType) { + if (parameterType instanceof JavaType.Array && mappedJavadocType instanceof JavaType.Array) { + if (((JavaType.Array) parameterType).getElemType() instanceof JavaType.Primitive) { + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); + } + return paramTypeMatches(((JavaType.Array) parameterType).getElemType(), ((JavaType.Array) mappedJavadocType).getElemType()); + } else if (parameterType instanceof JavaType.GenericTypeVariable && !((JavaType.GenericTypeVariable) parameterType).getBounds().isEmpty()) { + return paramTypeMatches(((JavaType.GenericTypeVariable) parameterType).getBounds().get(0), mappedJavadocType); + } else if (parameterType instanceof JavaType.GenericTypeVariable) { + return TypeUtils.isObject(mappedJavadocType); + } else if (parameterType instanceof JavaType.Parameterized && !(mappedJavadocType instanceof JavaType.Parameterized)) { + return paramTypeMatches(((JavaType.Parameterized) parameterType).getType(), mappedJavadocType); } - return false; + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); } private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) { diff --git a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java index f55a32132b8..10d9b202c6f 100644 --- a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java +++ b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java @@ -599,7 +599,11 @@ public JavaType.Primitive primitive(TypeTag tag) { .map(attr -> attr.getValue().toString()) .collect(Collectors.toList()); } else { - defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + try { + defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + } catch(UnsupportedOperationException e) { + // not all Attribute implementations define `getValue()` + } } } JavaType.Method method = new JavaType.Method( diff --git a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8JavadocVisitor.java b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8JavadocVisitor.java index 96afd0332ba..d3e9cada02a 100644 --- a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8JavadocVisitor.java +++ b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8JavadocVisitor.java @@ -647,25 +647,16 @@ public Tree visitParam(ParamTree node, List body) { for (JavaType.Method method : classType.getMethods()) { if (method.getName().equals(ref.memberName.toString())) { if (ref.paramTypes != null) { - for (JCTree param : ref.paramTypes) { - for (JavaType testParamType : method.getParameterTypes()) { - Type paramType = attr.attribType(param, symbol); - if (testParamType instanceof JavaType.GenericTypeVariable) { - List bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds(); - if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) { - return method; - } - for (JavaType bound : bounds) { - if (paramTypeMatches(bound, paramType)) { - return method; - } - } - continue nextMethod; - } - - if (paramTypeMatches(testParamType, paramType)) { - continue nextMethod; - } + List parameterTypes = method.getParameterTypes(); + if (ref.paramTypes.size() != parameterTypes.size()) { + continue; + } + for (int i = 0; i < ref.paramTypes.size(); i++) { + JCTree param = ref.paramTypes.get(i); + JavaType testParamType = parameterTypes.get(i); + Type paramType = attr.attribType(param, symbol); + if (!paramTypeMatches(testParamType, paramType)) { + continue nextMethod; } } } @@ -678,13 +669,25 @@ public Tree visitParam(ParamTree node, List body) { return null; } - private boolean paramTypeMatches(JavaType testParamType, Type paramType) { - if (paramType instanceof Type.ClassType) { - JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType); - return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym) - .fullname.toString()); + private boolean paramTypeMatches(JavaType parameterType, Type javadocType) { + return paramTypeMatches(parameterType, typeMapping.type(javadocType)); + } + + // Javadoc type references typically don't have generic type parameters + private static boolean paramTypeMatches(JavaType parameterType, JavaType mappedJavadocType) { + if (parameterType instanceof JavaType.Array && mappedJavadocType instanceof JavaType.Array) { + if (((JavaType.Array) parameterType).getElemType() instanceof JavaType.Primitive) { + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); + } + return paramTypeMatches(((JavaType.Array) parameterType).getElemType(), ((JavaType.Array) mappedJavadocType).getElemType()); + } else if (parameterType instanceof JavaType.GenericTypeVariable && !((JavaType.GenericTypeVariable) parameterType).getBounds().isEmpty()) { + return paramTypeMatches(((JavaType.GenericTypeVariable) parameterType).getBounds().get(0), mappedJavadocType); + } else if (parameterType instanceof JavaType.GenericTypeVariable) { + return TypeUtils.isObject(mappedJavadocType); + } else if (parameterType instanceof JavaType.Parameterized && !(mappedJavadocType instanceof JavaType.Parameterized)) { + return paramTypeMatches(((JavaType.Parameterized) parameterType).getType(), mappedJavadocType); } - return false; + return TypeUtils.isAssignableTo(parameterType, mappedJavadocType, TypeUtils.TypePosition.In); } private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) { diff --git a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java index 0d8408dd026..988b0920206 100644 --- a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java +++ b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java @@ -581,7 +581,11 @@ public JavaType.Primitive primitive(TypeTag tag) { .map(attr -> attr.getValue().toString()) .collect(Collectors.toList()); } else { - defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + try { + defaultValues = Collections.singletonList(methodSymbol.getDefaultValue().getValue().toString()); + } catch(UnsupportedOperationException e) { + // not all Attribute implementations define `getValue()` + } } } JavaType.Method method = new JavaType.Method( diff --git a/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java b/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java index 5ba6384ee71..1946e924c83 100644 --- a/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java +++ b/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java @@ -1662,7 +1662,7 @@ void arrayTypeLiterals2() { rewriteRun( java("" + "/**\n" + - " *

Values are converted to strings using {@link java.util.Arrays#compare(Comparable[], Comparable[])}}.\n" + + " *

Values are converted to strings using {@link java.util.Arrays#compare(Comparable[], Comparable[])}.\n" + " */\n" + "class A {}" ) diff --git a/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/TypeUtilsTest.java b/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/TypeUtilsTest.java index b484c7a3cc8..a400ea4b0b3 100644 --- a/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/TypeUtilsTest.java +++ b/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/TypeUtilsTest.java @@ -20,14 +20,19 @@ import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.Issue; import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MinimumJava11; import org.openrewrite.test.RewriteTest; import org.openrewrite.test.SourceSpec; +import java.util.EnumSet; import java.util.function.Consumer; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.tree.TypeUtils.TypePosition.*; import static org.openrewrite.test.RewriteTest.toRecipe; @SuppressWarnings("ConstantConditions") @@ -233,18 +238,22 @@ void isParameterizedTypeOfType() { java( """ class Test { - java.util.List integer; + java.util.List li; + java.util.List lo; } """, spec -> spec.afterRecipe(cu -> new JavaIsoVisitor<>() { @Override - public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, Object o) { - JavaType type = variable.getVariableType().getType(); - assertThat(type).isInstanceOf(JavaType.Parameterized.class); - assertThat(TypeUtils.isAssignableTo("java.util.List", type)).isTrue(); - assertThat(TypeUtils.isAssignableTo("java.util.List", type)).isTrue(); - assertThat(TypeUtils.isAssignableTo("java.util.List", type)).isFalse(); - return super.visitVariable(variable, o); + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Object o) { + J.VariableDeclarations.NamedVariable li = ((J.VariableDeclarations) classDecl.getBody().getStatements().get(0)).getVariables().get(0); + J.VariableDeclarations.NamedVariable lo = ((J.VariableDeclarations) classDecl.getBody().getStatements().get(1)).getVariables().get(0); + JavaType.Parameterized listIntegerType = ((JavaType.Parameterized) li.getVariableType().getType()); + JavaType.Parameterized listObjectType = ((JavaType.Parameterized) lo.getVariableType().getType()); + + assertThat(TypeUtils.isAssignableTo(listIntegerType.getType(), listIntegerType)).isTrue(); + assertThat(TypeUtils.isAssignableTo(listObjectType, listIntegerType)).isFalse(); + assertThat(TypeUtils.isAssignableTo(listIntegerType, listObjectType)).isFalse(); + return classDecl; } }.visit(cu, new InMemoryExecutionContext())) ) @@ -263,12 +272,13 @@ class Test { spec -> spec.afterRecipe(cu -> new JavaIsoVisitor<>() { @Override public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, Object o) { - JavaType type = variable.getVariableType().getType(); - JavaType exprType = variable.getInitializer().getType(); - assertThat(type).isInstanceOf(JavaType.Parameterized.class); - assertThat(exprType).isInstanceOf(JavaType.Parameterized.class); - assertThat(TypeUtils.isAssignableTo(type, exprType)).isTrue(); - return super.visitVariable(variable, o); + JavaType.Parameterized wildcardList = (JavaType.Parameterized) variable.getVariableType().getType(); + JavaType.FullyQualified rawList = wildcardList.getType(); + JavaType.Parameterized stringArrayList = (JavaType.Parameterized) variable.getInitializer().getType(); + assertThat(TypeUtils.isAssignableTo(wildcardList, stringArrayList)).isTrue(); + assertThat(TypeUtils.isAssignableTo(wildcardList, stringArrayList)).isTrue(); + assertThat(TypeUtils.isAssignableTo(wildcardList, rawList)).isTrue(); + return variable; } }.visit(cu, new InMemoryExecutionContext())) ) @@ -332,6 +342,270 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Objec ); } + @Test + @MinimumJava11 + void isAssignableToGenericTypeVariable2() { + rewriteRun( + java( + """ + import java.util.Collection; + import java.util.List; + + class Test { + public > T test() { + return (T) get(); + } + public List get() { + return List.of("a", "b", "c"); + } + } + """, + spec -> spec.afterRecipe(cu -> new JavaIsoVisitor<>() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Object o) { + if (method.getSimpleName().equals("test")) { + J.Return return_ = (J.Return) method.getBody().getStatements().get(0); + J.TypeCast cast = (J.TypeCast) return_.getExpression(); + assertThat(TypeUtils.isAssignableTo(cast.getType(), cast.getExpression().getType(), Invariant)).isFalse(); + assertThat(TypeUtils.isAssignableTo(cast.getType(), cast.getExpression().getType(), Out)).isTrue(); + } + return method; + } + }.visit(cu, new InMemoryExecutionContext())) + ) + ); + } + + @Test + void isAssignableToGenericTypeVariable3() { + rewriteRun( + java( + """ + import java.util.Collection; + import java.util.List; + + import static java.util.Collections.singletonList; + + class Test> { + + void consumeClass(T collection) { + } + + > void consumeMethod(T collection) { + } + + void test() { + List list = singletonList("hello"); + consumeMethod(null); + consumeClass(null); + } + } + """, + spec -> spec.afterRecipe(cu -> new JavaIsoVisitor<>() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Object o) { + if (method.getSimpleName().equals("test")) { + J.Block block = getCursor().getParentTreeCursor().getValue(); + J.MethodDeclaration consumeClass = (J.MethodDeclaration) block.getStatements().get(0); + J.MethodDeclaration consumeMethod = (J.MethodDeclaration) block.getStatements().get(1); + J.VariableDeclarations.NamedVariable list = ((J.VariableDeclarations) method.getBody().getStatements().get(0)).getVariables().get(0); + JavaType consumeClassParamType = ((J.VariableDeclarations) consumeClass.getParameters().get(0)).getVariables().get(0).getType(); + JavaType consumeMethodParamType = ((J.VariableDeclarations) consumeMethod.getParameters().get(0)).getVariables().get(0).getType(); + + assertThat(TypeUtils.isAssignableTo(consumeClassParamType, list.getType(), Out)).isTrue(); + assertThat(TypeUtils.isAssignableTo(consumeMethodParamType, list.getType(), Out)).isTrue(); + } + return method; + } + }.visit(cu, new InMemoryExecutionContext())) + ) + ); + } + + @Test + void isAssignableToLong() { + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Long)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Int)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Double)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Long, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToInt() { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Long)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Int)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Double)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Int, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToShort() { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Long)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Int)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Double)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Short, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToChar() { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Long)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Int)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Char)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Double)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Char, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToByte() { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Long)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Int)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Short)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Double)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Byte, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToDouble() { + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Long)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Int)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Byte)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Double)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Double, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToFloat() { + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Long)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Int)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Short)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Char)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Byte)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Double)); + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Float)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Boolean)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.None)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Void)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.String)); + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Float, JavaType.Primitive.Null)); + } + + @Test + void isAssignableToBoolean() { + EnumSet others = EnumSet.complementOf(EnumSet.of(JavaType.Primitive.Boolean)); + for (JavaType.Primitive other : others) { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Boolean, other)); + } + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Boolean, JavaType.Primitive.Boolean)); + } + + @Test + void isAssignableToNone() { + EnumSet others = EnumSet.complementOf(EnumSet.of(JavaType.Primitive.None)); + for (JavaType.Primitive other : others) { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.None, other)); + } + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.None, JavaType.Primitive.None)); + } + + @Test + void isAssignableToVoid() { + EnumSet others = EnumSet.complementOf(EnumSet.of(JavaType.Primitive.Void)); + for (JavaType.Primitive other : others) { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.Void, other)); + } + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.Void, JavaType.Primitive.Void)); + } + + @Test + void isAssignableToString() { + EnumSet others = EnumSet.complementOf(EnumSet.of(JavaType.Primitive.String)); + for (JavaType.Primitive other : others) { + assertFalse(TypeUtils.isAssignableTo(JavaType.Primitive.String, other)); + } + assertTrue(TypeUtils.isAssignableTo(JavaType.Primitive.String, JavaType.Primitive.String)); + } + + @Test + void isAssignableToPrimitiveArrays() { + JavaType.Array intArray = new JavaType.Array(null, JavaType.Primitive.Int, null); + JavaType.Array longArray = new JavaType.Array(null, JavaType.Primitive.Long, null); + assertTrue(TypeUtils.isAssignableTo(intArray, intArray)); + assertFalse(TypeUtils.isAssignableTo(longArray, intArray)); + assertFalse(TypeUtils.isAssignableTo(intArray, longArray)); + } + + @Test + void isAssignableToNonPrimitiveArrays() { + rewriteRun( + java( + """ + class Test { + Object[] oa; + String[] sa; + } + """, + spec -> spec.afterRecipe(cu -> new JavaIsoVisitor<>() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Object o) { + J.VariableDeclarations oa = (J.VariableDeclarations) classDecl.getBody().getStatements().get(0); + J.VariableDeclarations sa = (J.VariableDeclarations) classDecl.getBody().getStatements().get(1); + JavaType objectArray = oa.getType(); + JavaType stringArray = sa.getType(); + assertTrue(TypeUtils.isAssignableTo(objectArray, objectArray)); + assertFalse(TypeUtils.isAssignableTo(stringArray, objectArray)); + assertTrue(TypeUtils.isAssignableTo(objectArray, stringArray)); + return classDecl; + } + }.visit(cu, new InMemoryExecutionContext())) + ) + ); + } + @SuppressWarnings("RedundantCast") @Test void isAssignableFromIntersection() { diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java index e74411bd724..ac1b1bc8117 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java @@ -25,7 +25,7 @@ class AddOrUpdateAnnotationAttributeTest implements RewriteTest { @Test void addValueAttribute() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "hello", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "hello", null, null)), java( """ package org.example; @@ -56,7 +56,7 @@ public class A { @Test void addValueAttributeClass() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null, null)), java( """ package org.example; @@ -87,7 +87,7 @@ public class A { @Test void addValueAttributeFullyQualifiedClass() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "java.math.BigDecimal.class", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "java.math.BigDecimal.class", null, null)), java( """ package org.example; @@ -118,7 +118,7 @@ public class A { @Test void updateValueAttribute() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "hello", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "hello", null, null)), java( """ package org.example; @@ -150,7 +150,7 @@ public class A { @Test void updateValueAttributeClass() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null, null)), java( """ package org.example; @@ -182,7 +182,7 @@ public class A { @Test void removeValueAttribute() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, null, null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, null, null, null)), java( """ package org.example; @@ -214,7 +214,7 @@ public class A { @Test void removeValueAttributeClass() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, null, null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, null, null, null)), java( """ package org.example; @@ -245,7 +245,7 @@ public class A { @Test void addNamedAttribute() { - rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null)), + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null, null)), java( """ package org.junit; @@ -282,7 +282,7 @@ void foo() { @Test void replaceAttribute() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null, null)), java( """ package org.junit; @@ -319,7 +319,7 @@ void foo() { @Test void removeAttribute() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", null, null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", null, null, null)), java( """ package org.junit; @@ -356,7 +356,7 @@ void foo() { @Test void preserveExistingAttributes() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null, null)), java( """ package org.junit; @@ -394,7 +394,7 @@ void foo() { @Test void implicitValueToExplicitValue() { - rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)), + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null, null)), java( """ package org.junit; @@ -431,7 +431,7 @@ void foo() { @Test void implicitValueToExplicitValueClass() { - rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)), + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null, null)), java( """ package org.junit; @@ -469,7 +469,7 @@ void foo() { @Test void dontChangeWhenSetToAddOnly() { rewriteRun( - spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", true)), + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", true, false)), java( """ package org.junit; @@ -500,6 +500,7 @@ void arrayInAnnotationAttribute() { "org.example.Foo", "array", "newTest", + false, false)), java( """ @@ -535,6 +536,7 @@ void arrayInputMoreThanOneInAnnotationAttribute() { "org.example.Foo", "array", "newTest1,newTest2", + false, false)), java( """ @@ -570,6 +572,7 @@ void addArrayInputInAnnotationAttribute() { "org.example.Foo", "array", "newTest1,newTest2", + false, false)), java( """ @@ -598,6 +601,78 @@ public class A { ); } + @Test + void addArrayInputInAnnotationAttributeWithAppendTrue() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "newTest1,newTest2", + false, + true)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"newTest1", "newTest2"}) + public class A { + } + """ + ) + ); + } + + @Test + void addArrayInputInAnnotationAttributeEmptyBraces() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "newTest1,newTest2", + false, + null)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo() + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"newTest1", "newTest2"}) + public class A { + } + """ + ) + ); + } + @Test void removeArrayInputInAnnotationAttribute() { rewriteRun( @@ -605,7 +680,8 @@ void removeArrayInputInAnnotationAttribute() { "org.example.Foo", "array", null, - null)), + null, + false)), java( """ package org.example; @@ -640,7 +716,8 @@ void addOtherAttributeInArrayAnnotation() { "org.example.Foo", "string", "test", - null)), + null, + false)), java( """ package org.example; @@ -668,4 +745,151 @@ public class A { ) ); } + + @Test + void appendSingleValueToExistingArrayAttribute() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "b", + false, + true)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo(array = {"a"}) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"a", "b"}) + public class A { + } + """ + ) + ); + } + + @Test + void appendMultipleValuesToExistingArrayAttribute() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "b,c", + false, + true)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo(array = {"a"}) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"a", "b", "c"}) + public class A { + } + """ + ) + ); + } + + @Test + void appendMultipleValuesToExistingArrayAttributeWithOverlap() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "b,c", + false, + true)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo(array = {"a", "b"}) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"a", "b", "c"}) + public class A { + } + """ + ) + ); + } + + @Test + void appendMultipleValuesToExistingArrayAttributeNonSet() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute( + "org.example.Foo", + "array", + "b,c", + true, + true)), + java( + """ + package org.example; + public @interface Foo { + String[] array() default {}; + } + + public class A { + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo(array = {"a", "b"}) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(array = {"a", "b", "b", "c"}) + public class A { + } + """ + ) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java index f21841e0a08..c307f330f27 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java @@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.xml.Assertions.xml; @SuppressWarnings("ConstantConditions") class ChangePackageTest implements RewriteTest { @@ -1702,4 +1703,26 @@ public enum MyEnum { ) ); } + + @Test + void changePackageInSpringXml() { + rewriteRun( + spec -> spec.recipe(new ChangePackage("test.type", "test.test.type", true)), + xml( + """ + + + + + """, + """ + + + + + """ + ) + ); + + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java index b2234b43a47..d957ef4cf5e 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java @@ -18,9 +18,8 @@ import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.openrewrite.DocumentExample; -import org.openrewrite.Issue; -import org.openrewrite.PathUtils; +import org.openrewrite.*; +import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.test.RecipeSpec; @@ -28,7 +27,9 @@ import org.openrewrite.test.SourceSpec; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.xml.Assertions.xml; @SuppressWarnings("ConstantConditions") class ChangeTypeTest implements RewriteTest { @@ -1981,27 +1982,27 @@ void shouldNotFullyQualifyWhenNewTypeIsAlreadyUsed() { true)), java( """ - package org.a; - - public class A { - public static String A = "A"; - } - """), + package org.a; + + public class A { + public static String A = "A"; + } + """), java( """ - package org.ab; - - public class AB { - public static String A = "A"; - public static String B = "B"; - } - """), + package org.ab; + + public class AB { + public static String A = "A"; + public static String B = "B"; + } + """), // language=java java( """ import org.a.A; import org.ab.AB; - + class Letters { String a = A.A; String b = AB.B; @@ -2009,7 +2010,7 @@ class Letters { """, """ import org.ab.AB; - + class Letters { String a = AB.A; String b = AB.B; @@ -2019,4 +2020,46 @@ class Letters { ); } + @Test + void changeTypeInSpringXml() { + rewriteRun( + spec -> spec.recipe(new ChangeType("test.type.A", "test.type.B", true)), + xml( + """ + + + + + """, + """ + + + + + """ + ) + ); + + } + + @Test + void changeTypeWorksOnDirectInvocations() { + rewriteRun( + spec -> spec.recipe(Recipe.noop()), // do not run the default recipe + java( + """ + package hello; + public class HelloClass {} + """, + spec -> spec.beforeRecipe((source) -> { + TreeVisitor visitor = new ChangeType("hello.HelloClass", "hello.GoodbyeClass", false).getVisitor(); + + J.CompilationUnit cu = (J.CompilationUnit) visitor.visit(source, new InMemoryExecutionContext()); + assertEquals("GoodbyeClass", cu.getClasses().get(0).getSimpleName()); + + J.ClassDeclaration cd = (J.ClassDeclaration) visitor.visit(source.getClasses().get(0), new InMemoryExecutionContext()); + assertEquals("GoodbyeClass", cd.getSimpleName()); + })) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteMethodArgumentTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteMethodArgumentTest.java index 6949a868520..5ad21d41e15 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteMethodArgumentTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/DeleteMethodArgumentTest.java @@ -17,11 +17,14 @@ import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; +import org.openrewrite.Issue; +import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; class DeleteMethodArgumentTest implements RewriteTest { + @Language("java") String b = """ class B { @@ -34,11 +37,15 @@ public B(int n) {} } """; + @Override + public void defaults(RecipeSpec spec) { + spec.parser(JavaParser.fromJavaVersion().dependsOn(b)); + } + @Test void deleteMiddleArgumentDeclarative() { rewriteRun( spec -> spec.recipes(new DeleteMethodArgument("B foo(int, int, int)", 1)), - java(b), java( "public class A {{ B.foo(0, 1, 2); }}", "public class A {{ B.foo(0, 2); }}" @@ -50,7 +57,6 @@ void deleteMiddleArgumentDeclarative() { void deleteMiddleArgument() { rewriteRun( spec -> spec.recipe(new DeleteMethodArgument("B foo(int, int, int)", 1)), - java(b), java( "public class A {{ B.foo(0, 1, 2); }}", "public class A {{ B.foo(0, 2); }}" @@ -65,8 +71,8 @@ void deleteArgumentsConsecutively() { new DeleteMethodArgument("B foo(int, int, int)", 1), new DeleteMethodArgument("B foo(int, int)", 1) ), - java(b), - java("public class A {{ B.foo(0, 1, 2); }}", + java( + "public class A {{ B.foo(0, 1, 2); }}", "public class A {{ B.foo(0); }}" ) ); @@ -76,7 +82,6 @@ void deleteArgumentsConsecutively() { void doNotDeleteEmptyContainingFormatting() { rewriteRun( spec -> spec.recipe(new DeleteMethodArgument("B foo(..)", 0)), - java(b), java("public class A {{ B.foo( ); }}") ); } @@ -85,7 +90,6 @@ void doNotDeleteEmptyContainingFormatting() { void insertEmptyWhenLastArgumentIsDeleted() { rewriteRun( spec -> spec.recipe(new DeleteMethodArgument("B foo(..)", 0)), - java(b), java( "public class A {{ B.foo(1); }}", "public class A {{ B.foo(); }}" @@ -97,11 +101,26 @@ void insertEmptyWhenLastArgumentIsDeleted() { void deleteConstructorArgument() { rewriteRun( spec -> spec.recipe(new DeleteMethodArgument("B (int)", 0)), - java(b), java( "public class A { B b = new B(0); }", "public class A { B b = new B(); }" ) ); } + + @Issue("https://github.com/openrewrite/rewrite/issues/4676") + @Test + void deleteFirstArgument() { + rewriteRun( + spec -> spec.recipe(new DeleteMethodArgument("B foo(int, int, int)", 0)), + java( + "public class A {{ B.foo(0, 1, 2); }}", + "public class A {{ B.foo(1, 2); }}" + ), + java( + "public class C {{ B.foo(\n\t\t0, 1, 2); }}", + "public class C {{ B.foo(\n\t\t1, 2); }}" + ) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java index 090bad88ba9..9e21f9be66d 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java @@ -36,32 +36,32 @@ class JavaTemplateTest implements RewriteTest { @Test void addAnnotation() { - rewriteRun( - spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() { - private final JavaTemplate template = JavaTemplate.builder("@SuppressWarnings({\"ALL\"})") - .build(); - @Override - public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext executionContext) { - J.ClassDeclaration cd = classDecl; - if(!cd.getLeadingAnnotations().isEmpty()) { - return cd; - } - cd = template.apply(updateCursor(cd), cd.getCoordinates().addAnnotation(comparing(J.Annotation::getSimpleName))); - return cd; - } - })), - java( - """ - public class A { - } - """, - """ - @SuppressWarnings({"ALL"}) - public class A { - } - """ - ) - ); + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() { + private final JavaTemplate template = JavaTemplate.builder("@SuppressWarnings({\"ALL\"})").build(); + + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = classDecl; + if (!cd.getLeadingAnnotations().isEmpty()) { + return cd; + } + cd = template.apply(updateCursor(cd), cd.getCoordinates().addAnnotation(comparing(J.Annotation::getSimpleName))); + return cd; + } + })), + java( + """ + public class A { + } + """, + """ + @SuppressWarnings({"ALL"}) + public class A { + } + """ + ) + ); } @Issue("https://github.com/openrewrite/rewrite/issues/2090") @@ -370,7 +370,7 @@ class T { void m() { hashCode(); } - + void m2() { hashCode(); } @@ -594,7 +594,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) """ import java.math.BigDecimal; import java.math.RoundingMode; - + class A { void m() { StringBuilder sb = new StringBuilder(); @@ -605,7 +605,7 @@ void m() { """ import java.math.BigDecimal; import java.math.RoundingMode; - + class A { void m() { StringBuilder sb = new StringBuilder(); @@ -793,20 +793,20 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { class A { public enum Type { One; - + public Type(String t) { } - + String t; - + public static Type fromType(String type) { return null; } } - + public A(Type type) {} public A() {} - + public void method(Type type) { new A(type); } @@ -816,20 +816,20 @@ public void method(Type type) { class A { public enum Type { One; - + public Type(String t) { } - + String t; - + public static Type fromType(String type) { return null; } } - + public A(Type type) {} public A() {} - + public void method(Type type) { new A(); } @@ -861,7 +861,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu java( """ package abc; - + public class ArrayHelper { public static Object[] of(Object... objects){ return null;} } @@ -871,7 +871,7 @@ public class ArrayHelper { """ import abc.ArrayHelper; import java.util.Arrays; - + class A { Object[] o = new Object[] { ArrayHelper.of(1, 2, 3) @@ -881,7 +881,7 @@ class A { """ import abc.ArrayHelper; import java.util.Arrays; - + class A { Object[] o = new Object[] { Arrays.asList(1, 2, 3) @@ -937,7 +937,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) { java( """ import java.util.Collection; - + class Test { void doSomething(Collection c) { assert c.size() > 0; @@ -946,7 +946,7 @@ void doSomething(Collection c) { """, """ import java.util.Collection; - + class Test { void doSomething(Collection c) { assert !c.isEmpty(); @@ -971,13 +971,13 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) { """ enum Outer { A, B; - + enum Inner { C, D } - + private final String s; - + Outer() { s = "a" + "b"; } @@ -986,13 +986,13 @@ enum Inner { """ enum Outer { A, B; - + enum Inner { C, D } - + private final String s; - + Outer() { s = "ab"; } @@ -1156,7 +1156,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu """ import java.util.Map; import org.junit.jupiter.api.Assertions; - + class T { void m(String one, Map map) { Assertions.assertEquals(one, map.get("one")); @@ -1165,9 +1165,9 @@ void m(String one, Map map) { """, """ import java.util.Map; - + import static org.assertj.core.api.Assertions.assertThat; - + class T { void m(String one, Map map) { assertThat(map.get("one")).isEqualTo(one); @@ -1212,7 +1212,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu import java.util.Objects; import java.util.Map; import java.util.HashMap; - + class T { void m() { Map map = new HashMap<>(); @@ -1223,10 +1223,10 @@ void m() { """ import java.util.Objects; import java.util.Map; - + import static java.util.Objects.requireNonNull; import java.util.HashMap; - + class T { void m() { Map map = new HashMap<>(); @@ -1254,13 +1254,13 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, Executi java( """ interface Test { - + String a; } """, """ interface Test { - + String a(); } """ @@ -1275,13 +1275,13 @@ void finalMethodParameter() { java( """ import org.jetbrains.annotations.NotNull; - + class A { String testMethod(@NotNull final String test) {} } """, """ import lombok.NonNull; - + class A { String testMethod(@NonNull final String test) {} } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/FindRecipesTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/FindRecipesTest.java index 4426ea61e75..3624f6bfe8e 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/FindRecipesTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/recipes/FindRecipesTest.java @@ -19,21 +19,25 @@ import org.openrewrite.DocumentExample; import org.openrewrite.java.JavaParser; import org.openrewrite.table.RewriteRecipeSource; +import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.yaml.Assertions.yaml; class FindRecipesTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FindRecipes()).parser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath())); + } + @DocumentExample @Test void findRecipes() { rewriteRun( spec -> spec - .recipe(new FindRecipes()) - .parser(JavaParser.fromJavaVersion() - .classpath(JavaParser.runtimeClasspath())) .dataTable(RewriteRecipeSource.Row.class, rows -> { assertThat(rows).hasSize(1); RewriteRecipeSource.Row row = rows.get(0); @@ -66,7 +70,7 @@ class MyRecipe extends Recipe { public String getDisplayName() { return "My recipe"; } - + @Override public String getDescription() { return "This is my recipe."; @@ -85,19 +89,19 @@ class /*~~>*/MyRecipe extends Recipe { description = "A method pattern that is used to find matching method declarations/invocations.", example = "org.mockito.Matchers anyVararg()") String methodPattern; - + @Option(displayName = "New access level", description = "New method access level to apply to the method, like \\"public\\".", example = "public", valid = {"private", "protected", "package", "public"}, required = false) String newAccessLevel; - + @Override public String getDisplayName() { return "My recipe"; } - + @Override public String getDescription() { return "This is my recipe."; @@ -111,7 +115,6 @@ public String getDescription() { @Test void returnInLambda() { rewriteRun( - spec -> spec.recipe(new FindRecipes()), java( """ import java.util.function.UnaryOperator; @@ -126,4 +129,122 @@ class SomeTest { ) ); } + + @Test + void findRefasterRecipe() { + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion().dependsOn( + """ + package org.openrewrite.java.template; + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + @Target(ElementType.TYPE) + public @interface RecipeDescriptor { + String name(); + String description(); + } + """ + )) + .dataTable(RewriteRecipeSource.Row.class, rows -> { + assertThat(rows).hasSize(1); + RewriteRecipeSource.Row row = rows.get(0); + assertThat(row.getDisplayName()).isEqualTo("Some refaster rule"); + assertThat(row.getDescription()).isEqualTo("This is a refaster rule."); + }), + java( + """ + import org.openrewrite.java.template.RecipeDescriptor; + + @RecipeDescriptor( + name = "Some refaster rule", + description = "This is a refaster rule." + ) + class SomeRefasterRule { + } + """, + """ + import org.openrewrite.java.template.RecipeDescriptor; + + /*~~>*/@RecipeDescriptor( + name = "Some refaster rule", + description = "This is a refaster rule." + ) + class SomeRefasterRule { + } + """ + ) + ); + } + + @Test + void findYamlRecipe() { + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion().dependsOn( + """ + package org.openrewrite.java.template; + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + @Target(ElementType.TYPE) + public @interface RecipeDescriptor { + String name(); + String description(); + } + """ + )) + .dataTable(RewriteRecipeSource.Row.class, rows -> { + assertThat(rows).hasSize(2); + assertThat(rows.get(0).getDisplayName()).isEqualTo("Migrates to Apache Commons Lang 3.x"); + assertThat(rows.get(0).getSourceCode()).startsWith( + "---\ntype: specs.openrewrite.org/v1beta/recipe\nname: org.openrewrite.apache.commons.lang.UpgradeApacheCommonsLang_2_3"); + assertThat(rows.get(1).getDisplayName()).isEqualTo("Migrates to Apache POI 3.17"); + assertThat(rows.get(1).getSourceCode()).startsWith( + "---\ntype: specs.openrewrite.org/v1beta/recipe\nname: org.openrewrite.apache.poi.UpgradeApachePoi_3_17"); + }), + yaml( + """ + # Apache Commons Lang + --- + type: specs.openrewrite.org/v1beta/recipe + name: org.openrewrite.apache.commons.lang.UpgradeApacheCommonsLang_2_3 + displayName: Migrates to Apache Commons Lang 3.x + description: >- + Migrate applications to the latest Apache Commons Lang 3.x release. This recipe modifies\s + application's build files, and changes the package as per [the migration release notes](https://commons.apache.org/proper/commons-lang/article3_0.html). + tags: + - apache + - commons + - lang + recipeList: + - org.openrewrite.java.dependencies.ChangeDependency: + oldGroupId: commons-lang + oldArtifactId: commons-lang + newGroupId: org.apache.commons + newArtifactId: commons-lang3 + newVersion: 3.x + - org.openrewrite.java.ChangePackage: + oldPackageName: org.apache.commons.lang + newPackageName: org.apache.commons.lang3 + --- + type: specs.openrewrite.org/v1beta/recipe + name: org.openrewrite.apache.poi.UpgradeApachePoi_3_17 + displayName: Migrates to Apache POI 3.17 + description: Migrates to the last Apache POI 3.x release. This recipe modifies build files and makes changes to deprecated/preferred APIs that have changed between versions. + tags: + - apache + - poi + recipeList: + - org.openrewrite.java.dependencies.ChangeDependency: + oldGroupId: poi + oldArtifactId: poi + newGroupId: org.apache.poi + newArtifactId: poi + newVersion: 3.x + """, + spec -> spec.path("rewrite.yml").after(after -> { + assertThat(after).contains("~~>"); + return after; + }) + ) + ); + } } diff --git a/rewrite-java/build.gradle.kts b/rewrite-java/build.gradle.kts index f84627b4c2b..92121e1ec1b 100644 --- a/rewrite-java/build.gradle.kts +++ b/rewrite-java/build.gradle.kts @@ -64,6 +64,7 @@ dependencies { testRuntimeOnly(project(":rewrite-java-17")) testImplementation("com.tngtech.archunit:archunit:1.0.1") testImplementation("com.tngtech.archunit:archunit-junit5:1.0.1") + testImplementation("org.junit-pioneer:junit-pioneer:2.0.0") // For use in ClassGraphTypeMappingTest testRuntimeOnly("org.eclipse.persistence:org.eclipse.persistence.core:3.0.2") diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java index 8b4af613bd7..be098009e16 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -17,22 +17,24 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import lombok.With; import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Marker; import org.openrewrite.marker.Markers; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import static org.openrewrite.Tree.randomId; + @Value @EqualsAndHashCode(callSuper = false) public class AddOrUpdateAnnotationAttribute extends Recipe { @@ -70,6 +72,14 @@ public String getDescription() { @Nullable Boolean addOnly; + @Option(displayName = "Append array", + description = "If the attribute is an array, setting this option to `true` will append the value(s). " + + "In conjunction with `addOnly`, it is possible to control duplicates: " + + "`addOnly=true`, always append. " + + "`addOnly=false`, only append if the value is not already present.") + @Nullable + Boolean appendArray; + @Override public TreeVisitor getVisitor() { return Preconditions.check(new UsesType<>(annotationType, false), new JavaIsoVisitor() { @@ -82,7 +92,7 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { String newAttributeValue = maybeQuoteStringArgument(attributeName, attributeValue, a); List currentArgs = a.getArguments(); - if (currentArgs == null || currentArgs.isEmpty()) { + if (currentArgs == null || currentArgs.isEmpty() || currentArgs.get(0) instanceof J.Empty) { if (newAttributeValue == null) { return a; } @@ -123,10 +133,28 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { return null; } - if (as.getAssignment() instanceof J.NewArray){ + if (as.getAssignment() instanceof J.NewArray) { List jLiteralList = ((J.NewArray) as.getAssignment()).getInitializer(); String attributeValueCleanedUp = attributeValue.replaceAll("\\s+","").replaceAll("[\\s+{}\"]",""); List attributeList = Arrays.asList(attributeValueCleanedUp.contains(",") ? attributeValueCleanedUp.split(",") : new String[]{attributeValueCleanedUp}); + + if (as.getMarkers().findFirst(AlreadyAppended.class).filter(ap -> ap.getValues().equals(newAttributeValue)).isPresent()) { + return as; + } + + if (Boolean.TRUE.equals(appendArray)) { + boolean changed = false; + for (String attrListValues : attributeList) { + String newAttributeListValue = maybeQuoteStringArgument(attributeName, attrListValues, finalA); + if (Boolean.FALSE.equals(addOnly) && attributeValIsAlreadyPresent(jLiteralList, newAttributeListValue)) { + continue; + } + changed = true; + jLiteralList.add(new J.Literal(randomId(), Space.EMPTY, Markers.EMPTY, newAttributeListValue, newAttributeListValue, null, JavaType.Primitive.String)); + } + return changed ? as.withAssignment(((J.NewArray) as.getAssignment()).withInitializer(jLiteralList)) + .withMarkers(as.getMarkers().add(new AlreadyAppended(randomId(), newAttributeValue))) : as; + } int m = 0; for (int i = 0; i< Objects.requireNonNull(jLiteralList).size(); i++){ if (i >= attributeList.size()){ @@ -152,7 +180,7 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { } for (int j = m; j < attributeList.size(); j++){ String newAttributeListValue = maybeQuoteStringArgument(attributeName, attributeList.get(j), finalA); - jLiteralList.add(j, new J.Literal(Tree.randomId(), jLiteralList.get(j-1).getPrefix(), Markers.EMPTY, newAttributeListValue, newAttributeListValue, null, JavaType.Primitive.String)); + jLiteralList.add(j, new J.Literal(randomId(), jLiteralList.get(j - 1).getPrefix(), Markers.EMPTY, newAttributeListValue, newAttributeListValue, null, JavaType.Primitive.String)); } } @@ -255,4 +283,23 @@ private static boolean attributeIsString(@Nullable String attributeName, J.Annot } return false; } + + private static boolean attributeValIsAlreadyPresent(List expression, @Nullable String attributeValue) { + for (Expression e : expression) { + if (e instanceof J.Literal) { + J.Literal literal = (J.Literal) e; + if (literal.getValueSource() != null && literal.getValueSource().equals(attributeValue)) { + return true; + } + } + } + return false; + } + + @Value + @With + private static class AlreadyAppended implements Marker { + UUID id; + String values; + } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java index 7d28b412ca0..876410cc5cf 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java @@ -23,8 +23,10 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.java.tree.*; import org.openrewrite.marker.SearchResult; +import org.openrewrite.trait.Reference; import java.nio.file.Paths; +import java.util.HashMap; import java.util.IdentityHashMap; import java.util.Map; @@ -83,11 +85,12 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - JavaIsoVisitor condition = new JavaIsoVisitor() { + TreeVisitor condition = new TreeVisitor() { @Override - public @Nullable J preVisit(J tree, ExecutionContext ctx) { + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + stopAfterPreVisit(); if (tree instanceof JavaSourceFile) { - JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree); + JavaSourceFile cu = (JavaSourceFile) tree; if (cu.getPackageDeclaration() != null) { String original = cu.getPackageDeclaration().getExpression() .printTrimmed(getCursor()).replaceAll("\\s", ""); @@ -111,16 +114,48 @@ public TreeVisitor getVisitor() { } } } - stopAfterPreVisit(); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences cu = (SourceFileWithReferences) tree; + boolean recursive = Boolean.TRUE.equals(ChangePackage.this.recursive); + String recursivePackageNamePrefix = oldPackageName + "."; + for (Reference ref : cu.getReferences().getReferences()) { + if (ref.getValue().equals(oldPackageName) || recursive && ref.getValue().startsWith(recursivePackageNamePrefix)) { + return SearchResult.found(cu); + } + } } - return super.preVisit(tree, ctx); + return tree; } }; - return Preconditions.check(condition, new ChangePackageVisitor()); + return Preconditions.check(condition, new TreeVisitor() { + @Override + public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { + return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithReferences; + } + + @Override + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + stopAfterPreVisit(); + if (tree instanceof JavaSourceFile) { + return new JavaChangePackageVisitor().visit(tree, ctx, requireNonNull(getCursor().getParent())); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences sourceFile = (SourceFileWithReferences) tree; + SourceFileWithReferences.References references = sourceFile.getReferences(); + boolean recursive = Boolean.TRUE.equals(ChangePackage.this.recursive); + PackageMatcher matcher = new PackageMatcher(oldPackageName, recursive); + Map matches = new HashMap<>(); + for (Reference ref : references.findMatches(matcher)) { + matches.put(ref.getTree(), ref); + } + return new ReferenceChangePackageVisitor(matches, matcher, newPackageName).visit(tree, ctx, requireNonNull(getCursor().getParent())); + } + return tree; + } + }); } - private class ChangePackageVisitor extends JavaVisitor { + private class JavaChangePackageVisitor extends JavaVisitor { private static final String RENAME_TO_KEY = "renameTo"; private static final String RENAME_FROM_KEY = "renameFrom"; @@ -349,4 +384,22 @@ private boolean isTargetRecursivePackageName(String packageName) { } } + + @Value + @EqualsAndHashCode(callSuper = false) + private static class ReferenceChangePackageVisitor extends TreeVisitor { + Map matches; + Reference.Renamer renamer; + String newPackageName; + + @Override + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + Reference reference = matches.get(tree); + if (reference != null && reference.supportsRename()) { + return reference.rename(renamer, newPackageName).visit(tree, ctx, getCursor().getParent()); + } + return tree; + } + } + } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java index b3ce211b049..f8f1a24941a 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java @@ -24,6 +24,7 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.marker.SearchResult; +import org.openrewrite.trait.Reference; import java.nio.file.Path; import java.nio.file.Paths; @@ -81,22 +82,49 @@ public String getDescription() { public TreeVisitor getVisitor() { TreeVisitor condition = new TreeVisitor() { @Override - public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + stopAfterPreVisit(); if (tree instanceof JavaSourceFile) { - JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree); + JavaSourceFile cu = (JavaSourceFile) tree; if (!Boolean.TRUE.equals(ignoreDefinition) && containsClassDefinition(cu, oldFullyQualifiedTypeName)) { return SearchResult.found(cu); } return new UsesType<>(oldFullyQualifiedTypeName, true).visitNonNull(cu, ctx); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences cu = (SourceFileWithReferences) tree; + return new UsesType<>(oldFullyQualifiedTypeName, true).visitNonNull(cu, ctx); } return tree; } }; - return Preconditions.check(condition, new ChangeTypeVisitor(oldFullyQualifiedTypeName, newFullyQualifiedTypeName, ignoreDefinition)); + return Preconditions.check(condition, new TreeVisitor() { + @Override + public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { + return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithReferences; + } + + @Override + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + stopAfterPreVisit(); + if (tree instanceof J) { + return new JavaChangeTypeVisitor(oldFullyQualifiedTypeName, newFullyQualifiedTypeName, ignoreDefinition).visit(tree, ctx, requireNonNull(getCursor().getParent())); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences sourceFile = (SourceFileWithReferences) tree; + SourceFileWithReferences.References references = sourceFile.getReferences(); + TypeMatcher matcher = new TypeMatcher(oldFullyQualifiedTypeName); + Map matches = new HashMap<>(); + for (Reference ref : references.findMatches(matcher, Reference.Kind.TYPE)) { + matches.put(ref.getTree(), ref); + } + return new ReferenceChangeTypeVisitor(matches, matcher, newFullyQualifiedTypeName).visit(tree, ctx, requireNonNull(getCursor().getParent())); + } + return tree; + } + }); } - private static class ChangeTypeVisitor extends JavaVisitor { + private static class JavaChangeTypeVisitor extends JavaVisitor { private final JavaType.Class originalType; private final JavaType targetType; @@ -109,7 +137,7 @@ private static class ChangeTypeVisitor extends JavaVisitor { private final Map oldNameToChangedType = new IdentityHashMap<>(); private final Set topLevelClassnames = new HashSet<>(); - private ChangeTypeVisitor(String oldFullyQualifiedTypeName, String newFullyQualifiedTypeName, @Nullable Boolean ignoreDefinition) { + private JavaChangeTypeVisitor(String oldFullyQualifiedTypeName, String newFullyQualifiedTypeName, @Nullable Boolean ignoreDefinition) { this.originalType = JavaType.ShallowClass.build(oldFullyQualifiedTypeName); this.targetType = JavaType.buildType(newFullyQualifiedTypeName); this.ignoreDefinition = ignoreDefinition; @@ -527,6 +555,23 @@ private boolean hasNoConflictingImport(@Nullable JavaSourceFile sf) { } } + @Value + @EqualsAndHashCode(callSuper = false) + private static class ReferenceChangeTypeVisitor extends TreeVisitor { + Map matches; + Reference.Renamer renamer; + String newFullyQualifiedName; + + @Override + public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { + Reference reference = matches.get(tree); + if (reference != null && reference.supportsRename()) { + return reference.rename(renamer, newFullyQualifiedName).visit(tree, ctx, getCursor().getParent()); + } + return tree; + } + } + private static class ChangeClassDefinition extends JavaIsoVisitor { private final JavaType.Class originalType; private final JavaType.Class targetType; @@ -579,7 +624,7 @@ private boolean updatePath(JavaSourceFile sf, String oldPath, String newPath) { } @Override - public J.@Nullable Package visitPackage(J.Package pkg, ExecutionContext ctx) { + public J.@Nullable Package visitPackage(J.Package pkg, ExecutionContext ctx) { Boolean updatePackage = getCursor().pollNearestMessage("UPDATE_PACKAGE"); if (updatePackage != null && updatePackage) { String original = pkg.getExpression().printTrimmed(getCursor()).replaceAll("\\s", ""); diff --git a/rewrite-java/src/main/java/org/openrewrite/java/DeleteMethodArgument.java b/rewrite-java/src/main/java/org/openrewrite/java/DeleteMethodArgument.java index 2fd26e66556..c6c4676656d 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/DeleteMethodArgument.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/DeleteMethodArgument.java @@ -100,9 +100,11 @@ private MethodCall visitMethodCall(MethodCall methodCall) { .count() >= argumentIndex + 1) { List args = new ArrayList<>(originalArgs); - args.remove(argumentIndex); + Expression removed = args.remove(argumentIndex); if (args.isEmpty()) { args = singletonList(new J.Empty(randomId(), Space.EMPTY, Markers.EMPTY)); + } else if (argumentIndex == 0) { + args.set(0, args.get(0).withPrefix(removed.getPrefix())); } m = m.withArguments(args); diff --git a/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java b/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java new file mode 100644 index 00000000000..c6a88019001 --- /dev/null +++ b/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java @@ -0,0 +1,86 @@ +/* + * Copyright 2021 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.java; + +import lombok.Getter; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.StringUtils; +import org.openrewrite.trait.Reference; +import org.openrewrite.xml.tree.Xml; + +@Getter +public class PackageMatcher implements Reference.Renamer, Reference.Matcher { + + private final @Nullable String targetPackage; + + @Getter + private final Boolean recursive; + + public PackageMatcher(@Nullable String targetPackage) { + this(targetPackage, false); + } + + public PackageMatcher(@Nullable String targetPackage, boolean recursive) { + this.targetPackage = targetPackage; + this.recursive = recursive; + } + + @Override + public boolean matchesReference(Reference reference) { + if (reference.getKind().equals(Reference.Kind.TYPE) || reference.getKind().equals(Reference.Kind.PACKAGE)) { + String recursivePackageNamePrefix = targetPackage + "."; + if (reference.getValue().equals(targetPackage) || recursive && reference.getValue().startsWith(recursivePackageNamePrefix)) { + return true; + } + } + return false; + } + + @Override + public TreeVisitor rename(String newValue) { + return new TreeVisitor() { + @Override + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + if (StringUtils.isNotEmpty(newValue)) { + if (tree instanceof Xml.Attribute) { + return ((Xml.Attribute) tree).withValue(((Xml.Attribute) tree).getValue().withValue(getReplacement(((Xml.Attribute) tree).getValueAsString(), targetPackage, newValue))); + } + if (tree instanceof Xml.Tag) { + if (((Xml.Tag) tree).getValue().isPresent()) { + return ((Xml.Tag) tree).withValue(getReplacement(((Xml.Tag) tree).getValue().get(), targetPackage, newValue)); + } + } + } + return super.visit(tree, ctx); + } + }; + } + + String getReplacement(String value, @Nullable String oldValue, String newValue) { + if (oldValue != null) { + if (recursive) { + return value.replace(oldValue, newValue); + } else if (value.startsWith(oldValue) && Character.isUpperCase(value.charAt(oldValue.length() + 1))) { + return value.replace(oldValue, newValue); + } + } + return value; + } + +} diff --git a/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocations.java b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocations.java new file mode 100644 index 00000000000..82babb6c930 --- /dev/null +++ b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocations.java @@ -0,0 +1,52 @@ +/* + * Copyright 2024 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.java; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.search.UsesMethod; + +import static java.util.Collections.singletonList; + +@Value +@EqualsAndHashCode(callSuper = false) +public class RemoveMethodInvocations extends Recipe { + @Option(displayName = "Method pattern", + description = "A pattern to match method invocations for removal.", + example = "java.lang.StringBuilder append(java.lang.String)") + String methodPattern; + + @Override + public String getDisplayName() { + return "Remove method invocations"; + } + + @Override + public String getDescription() { + return "Remove method invocations if syntactically safe."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check(new UsesMethod<>(methodPattern), + new RemoveMethodInvocationsVisitor(singletonList(methodPattern))); + } +} diff --git a/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java new file mode 100644 index 00000000000..7cf86789aeb --- /dev/null +++ b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java @@ -0,0 +1,256 @@ +/* + * Copyright 2023 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.java; + +import lombok.Value; +import lombok.With; +import org.jspecify.annotations.Nullable; +import org.openrewrite.*; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Marker; + +import java.util.*; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import static org.openrewrite.Tree.randomId; + +/** + * This visitor removes method calls matching some criteria. + * Tries to intelligently remove within chains without breaking other methods in the chain. + */ +public class RemoveMethodInvocationsVisitor extends JavaVisitor { + private final Map>> matchers; + + public RemoveMethodInvocationsVisitor(Map>> matchers) { + this.matchers = matchers; + } + + public RemoveMethodInvocationsVisitor(List methodSignatures) { + this(methodSignatures.stream().collect(Collectors.toMap( + MethodMatcher::new, + signature -> args -> true + ))); + } + + @Override + public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + + if (inMethodCallChain()) { + List newArgs = ListUtils.map(m.getArguments(), arg -> (Expression) this.visit(arg, ctx)); + return m.withArguments(newArgs); + } + + J j = removeMethods(m, 0, isLambdaBody(), new Stack<>()); + if (j != null) { + j = j.withPrefix(m.getPrefix()); + // There should always be + if (!m.getArguments().isEmpty() && m.getArguments().stream().allMatch(ToBeRemoved::hasMarker)) { + return ToBeRemoved.withMarker(j); + } + } + + //noinspection DataFlowIssue allow returning null to remove the element + return j; + } + + private @Nullable J removeMethods(Expression expression, int depth, boolean isLambdaBody, Stack selectAfter) { + if (!(expression instanceof J.MethodInvocation)) { + return expression; + } + + boolean isStatement = isStatement(); + J.MethodInvocation m = (J.MethodInvocation) expression; + + if (m.getMethodType() == null || m.getSelect() == null) { + return expression; + } + + if (matchers.entrySet().stream().anyMatch(entry -> matches(m, entry.getKey(), entry.getValue()))) { + boolean hasSameReturnType = TypeUtils.isAssignableTo(m.getMethodType().getReturnType(), m.getSelect().getType()); + boolean removable = (isStatement && depth == 0) || hasSameReturnType; + if (!removable) { + return expression; + } + + if (m.getSelect() instanceof J.Identifier || m.getSelect() instanceof J.NewClass) { + boolean keepSelect = depth != 0; + if (keepSelect) { + selectAfter.add(getSelectAfter(m)); + return m.getSelect(); + } else { + if (isStatement) { + return null; + } else if (isLambdaBody) { + return ToBeRemoved.withMarker(J.Block.createEmptyBlock()); + } else { + return m.getSelect(); + } + } + } else if (m.getSelect() instanceof J.MethodInvocation) { + return removeMethods(m.getSelect(), depth, isLambdaBody, selectAfter); + } + } + + J.MethodInvocation method = m.withSelect((Expression) removeMethods(m.getSelect(), depth + 1, isLambdaBody, selectAfter)); + + // inherit prefix + if (!selectAfter.isEmpty()) { + method = inheritSelectAfter(method, selectAfter); + } + + return method; + } + + private boolean matches(J.MethodInvocation m, MethodMatcher matcher, Predicate> argsMatches) { + return matcher.matches(m) && argsMatches.test(m.getArguments()); + } + + private boolean isStatement() { + return getCursor().dropParentUntil(p -> p instanceof J.Block || + p instanceof J.Assignment || + p instanceof J.VariableDeclarations.NamedVariable || + p instanceof J.Return || + p instanceof JContainer || + p == Cursor.ROOT_VALUE + ).getValue() instanceof J.Block; + } + + private boolean isLambdaBody() { + if (getCursor().getParent() == null) { + return false; + } + Object parent = getCursor().getParent().getValue(); + return parent instanceof J.Lambda && ((J.Lambda) parent).getBody() == getCursor().getValue(); + } + + private boolean inMethodCallChain() { + return getCursor().dropParentUntil(p -> !(p instanceof JRightPadded)).getValue() instanceof J.MethodInvocation; + } + + private J.MethodInvocation inheritSelectAfter(J.MethodInvocation method, Stack prefix) { + return (J.MethodInvocation) new JavaIsoVisitor() { + @Override + public @Nullable JRightPadded visitRightPadded(@Nullable JRightPadded right, + JRightPadded.Location loc, + ExecutionContext executionContext) { + if (right == null) return null; + return prefix.isEmpty() ? right : right.withAfter(prefix.pop()); + } + }.visitNonNull(method, new InMemoryExecutionContext()); + } + + private Space getSelectAfter(J.MethodInvocation method) { + return new JavaIsoVisitor>() { + @Override + public @Nullable JRightPadded visitRightPadded(@Nullable JRightPadded right, + JRightPadded.Location loc, + List selectAfter) { + if (selectAfter.isEmpty()) { + selectAfter.add(right == null ? Space.EMPTY : right.getAfter()); + } + return right; + } + }.reduce(method, new ArrayList<>()).get(0); + } + + @SuppressWarnings("unused") // used in rewrite-spring / convenient for consumers + public static Predicate> isTrueArgument() { + return args -> args.size() == 1 && isTrue(args.get(0)); + } + + @SuppressWarnings("unused") // used in rewrite-spring / convenient for consumers + public static Predicate> isFalseArgument() { + return args -> args.size() == 1 && isFalse(args.get(0)); + } + + public static boolean isTrue(Expression expression) { + return isBoolean(expression, Boolean.TRUE); + } + + public static boolean isFalse(Expression expression) { + return isBoolean(expression, Boolean.FALSE); + } + + private static boolean isBoolean(Expression expression, Boolean b) { + if (expression instanceof J.Literal) { + return expression.getType() == JavaType.Primitive.Boolean && b.equals(((J.Literal) expression).getValue()); + } + return false; + } + + @Override + public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext ctx) { + lambda = (J.Lambda) super.visitLambda(lambda, ctx); + J body = lambda.getBody(); + if (body instanceof J.MethodInvocation && ToBeRemoved.hasMarker(body)) { + Expression select = ((J.MethodInvocation) body).getSelect(); + List parameters = lambda.getParameters().getParameters(); + if (select instanceof J.Identifier && !parameters.isEmpty() && parameters.get(0) instanceof J.VariableDeclarations) { + J.VariableDeclarations declarations = (J.VariableDeclarations) parameters.get(0); + if (((J.Identifier) select).getSimpleName().equals(declarations.getVariables().get(0).getSimpleName())) { + return ToBeRemoved.withMarker(lambda); + } + } else if (select instanceof J.MethodInvocation) { + return lambda.withBody(select.withPrefix(body.getPrefix())); + } + } else if (body instanceof J.Block && ToBeRemoved.hasMarker(body)) { + return ToBeRemoved.withMarker(lambda.withBody(ToBeRemoved.removeMarker(body))); + } + return lambda; + } + + @Override + public J.Block visitBlock(J.Block block, ExecutionContext ctx) { + int statementsCount = block.getStatements().size(); + + block = (J.Block) super.visitBlock(block, ctx); + List statements = block.getStatements(); + if (!statements.isEmpty() && statements.stream().allMatch(ToBeRemoved::hasMarker)) { + return ToBeRemoved.withMarker(block.withStatements(Collections.emptyList())); + } + + if (statementsCount > 0 && statements.isEmpty()) { + return ToBeRemoved.withMarker(block.withStatements(Collections.emptyList())); + } + + if (statements.stream().anyMatch(ToBeRemoved::hasMarker)) { + //noinspection DataFlowIssue + return block.withStatements(statements.stream() + .filter(s -> !ToBeRemoved.hasMarker(s) || s instanceof J.MethodInvocation && ((J.MethodInvocation) s).getSelect() instanceof J.MethodInvocation) + .map(s -> s instanceof J.MethodInvocation && ToBeRemoved.hasMarker(s) ? ((J.MethodInvocation) s).getSelect().withPrefix(s.getPrefix()) : s) + .collect(Collectors.toList())); + } + return block; + } + + @Value + @With + static class ToBeRemoved implements Marker { + UUID id; + static J2 withMarker(J2 j) { + return j.withMarkers(j.getMarkers().addIfAbsent(new ToBeRemoved(randomId()))); + } + static J2 removeMarker(J2 j) { + return j.withMarkers(j.getMarkers().removeByType(ToBeRemoved.class)); + } + static boolean hasMarker(J j) { + return j.getMarkers().findFirst(ToBeRemoved.class).isPresent(); + } + } +} diff --git a/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java b/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java index 66149adb535..fe5a8cdea9f 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java @@ -19,6 +19,9 @@ import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; import org.openrewrite.internal.StringUtils; import org.openrewrite.java.internal.grammar.MethodSignatureLexer; import org.openrewrite.java.internal.grammar.MethodSignatureParser; @@ -26,14 +29,15 @@ import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeTree; import org.openrewrite.java.tree.TypeUtils; -import org.openrewrite.trait.TypeReference; +import org.openrewrite.trait.Reference; +import org.openrewrite.xml.tree.Xml; import java.util.regex.Pattern; import static org.openrewrite.java.tree.TypeUtils.fullyQualifiedNamesAreEqual; @Getter -public class TypeMatcher implements TypeReference.Matcher { +public class TypeMatcher implements Reference.Renamer, Reference.Matcher { private static final String ASPECTJ_DOT_PATTERN = StringUtils.aspectjNameToPattern("."); @SuppressWarnings("NotNullFieldNotInitialized") @@ -112,7 +116,26 @@ private static boolean isPlainIdentifier(MethodSignatureParser.TargetTypePattern } @Override - public boolean matchesName(String name) { - return matchesTargetTypeName(name); + public boolean matchesReference(Reference reference) { + return reference.getKind().equals(Reference.Kind.TYPE) && matchesTargetTypeName(reference.getValue()); } + + @Override + public TreeVisitor rename(String newValue) { + return new TreeVisitor() { + @Override + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + if (StringUtils.isNotEmpty(newValue)) { + if (tree instanceof Xml.Attribute) { + return ((Xml.Attribute) tree).withValue(((Xml.Attribute) tree).getValue().withValue(newValue)); + } + if (tree instanceof Xml.Tag) { + return ((Xml.Tag) tree).withValue(newValue); + } + } + return super.visit(tree, ctx); + } + }; + } + } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/recipes/FindRecipes.java b/rewrite-java/src/main/java/org/openrewrite/java/recipes/FindRecipes.java index 9be46a8dd1e..d460a4ffa7a 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/recipes/FindRecipes.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/recipes/FindRecipes.java @@ -19,11 +19,9 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.ValueNode; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Preconditions; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; import org.openrewrite.java.AnnotationMatcher; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; @@ -33,9 +31,13 @@ import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.SearchResult; import org.openrewrite.table.RewriteRecipeSource; +import org.openrewrite.yaml.YamlIsoVisitor; +import org.openrewrite.yaml.search.FindProperty; +import org.openrewrite.yaml.tree.Yaml; import java.util.ArrayList; import java.util.List; +import java.util.Set; import static java.util.Objects.requireNonNull; @@ -55,6 +57,67 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { + TreeVisitor findImperativeRecipes = findImperativeRecipes(); + TreeVisitor findRefasterRecipes = findRefasterRecipes(); + TreeVisitor findYamlRecipes = findYamlRecipes(); + return new TreeVisitor() { + @Override + public @Nullable Tree preVisit(@NonNull Tree tree, ExecutionContext ctx) { + stopAfterPreVisit(); + tree = findImperativeRecipes.visit(tree, ctx); + tree = findRefasterRecipes.visit(tree, ctx); + tree = findYamlRecipes.visit(tree, ctx); + return tree; + } + }; + } + + private TreeVisitor findRefasterRecipes() { + String recipeDescriptor = "org.openrewrite.java.template.RecipeDescriptor"; + AnnotationMatcher annotationMatcher = new AnnotationMatcher("@" + recipeDescriptor); + return Preconditions.check(new UsesType<>(recipeDescriptor, false), new JavaIsoVisitor() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); + for (J.Annotation annotation : cd.getLeadingAnnotations()) { + if (annotationMatcher.matches(annotation)) { + String name = null; + String description = null; + for (Expression argument : annotation.getArguments()) { + if (argument instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) argument; + if (assignment.getVariable() instanceof J.Identifier) { + String simpleName = ((J.Identifier) assignment.getVariable()).getSimpleName(); + if ("name".equals(simpleName)) { + if (assignment.getAssignment() instanceof J.Literal) { + name = (String) ((J.Literal) assignment.getAssignment()).getValue(); + } + } else if ("description".equals(simpleName)) { + if (assignment.getAssignment() instanceof J.Literal) { + description = (String) ((J.Literal) assignment.getAssignment()).getValue(); + } + } + } + } + } + if (name != null && description != null) { + recipeSource.insertRow(ctx, new RewriteRecipeSource.Row( + name, + description, + RewriteRecipeSource.RecipeType.Refaster, + cd.printTrimmed(getCursor()), + "[]" + )); + return SearchResult.found(cd); + } + } + } + return cd; + } + }); + } + + private TreeVisitor findImperativeRecipes() { MethodMatcher getDisplayName = new MethodMatcher("org.openrewrite.Recipe getDisplayName()", true); MethodMatcher getDescription = new MethodMatcher("org.openrewrite.Recipe getDescription()", true); AnnotationMatcher optionAnnotation = new AnnotationMatcher("@org.openrewrite.Option"); @@ -156,4 +219,43 @@ private ValueNode mapValue(@Nullable Object value) { } }); } + + private TreeVisitor findYamlRecipes() { + return Preconditions.check( + new FindProperty("type", false, "specs.openrewrite.org/v1beta/recipe"), + new YamlIsoVisitor() { + @Override + public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { + Yaml.Document doc = super.visitDocument(document, ctx); + + if ("specs.openrewrite.org/v1beta/recipe".equals(extractValue(doc, "type"))) { + String displayName = extractValue(doc, "displayName"); + String description = extractValue(doc, "description"); + if (displayName != null && description != null) { + recipeSource.insertRow(ctx, new RewriteRecipeSource.Row( + displayName, + description, + RewriteRecipeSource.RecipeType.Yaml, + doc.withPrefix("").printTrimmed(getCursor()), + "[]" + )); + return SearchResult.found(doc); + } + } + return doc; + } + + private @Nullable String extractValue(Yaml yaml, String key) { + Set blocks = FindProperty.find(yaml, key, false); + if (!blocks.isEmpty()) { + Yaml.Block first = blocks.iterator().next(); + if (first instanceof Yaml.Scalar) { + return ((Yaml.Scalar) first).getValue(); + } + } + return null; + } + } + ); + } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/recipes/MissingOptionExample.java b/rewrite-java/src/main/java/org/openrewrite/java/recipes/MissingOptionExample.java index 51d668ac0d2..847f84fdc87 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/recipes/MissingOptionExample.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/recipes/MissingOptionExample.java @@ -80,7 +80,7 @@ public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ct } } - AddOrUpdateAnnotationAttribute addOrUpdateAnnotationAttribute = new AddOrUpdateAnnotationAttribute(ORG_OPENREWRITE_OPTION, "example", "TODO Provide a usage example for the docs", true); + AddOrUpdateAnnotationAttribute addOrUpdateAnnotationAttribute = new AddOrUpdateAnnotationAttribute(ORG_OPENREWRITE_OPTION, "example", "TODO Provide a usage example for the docs", true, false); return (J.Annotation) addOrUpdateAnnotationAttribute.getVisitor().visitNonNull(an, ctx, getCursor().getParent()); } }); diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/FindEscapingVariables.java b/rewrite-java/src/main/java/org/openrewrite/java/search/FindEscapingVariables.java new file mode 100644 index 00000000000..7ad603d3e7d --- /dev/null +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/FindEscapingVariables.java @@ -0,0 +1,72 @@ +/* + * Copyright 2024 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.java.search; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.marker.SearchResult; + +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Identify local variables that escape there defining scope. + * + * @see FindVariableEscapeLocations + */ +public class FindEscapingVariables extends JavaIsoVisitor { + /** + * Finds named local variables from the given subtree that will escape their defining scope + */ + public static Set find(J subtree) { + return TreeVisitor.collect(new FindEscapingVariables(), subtree, new HashSet<>()) + .stream() + .filter(J.VariableDeclarations.NamedVariable.class::isInstance) + .map(J.VariableDeclarations.NamedVariable.class::cast) + .collect(Collectors.toSet()); + } + + /** + * Determine if a local named variable from the given subtree will escape its defining scope + */ + public static boolean willLeakFrom(J.VariableDeclarations.NamedVariable variable, J subtree) { + return find(subtree).contains(variable); + } + + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext executionContext) { + variable = super.visitVariable(variable, executionContext); + + if (variable.isField(getCursor())) { + return variable; + } + + J.Block definingScope = getCursor().firstEnclosing(J.Block.class); + if (definingScope != null && variable.getName().getFieldType() != null) { + boolean willLeak = FindVariableEscapeLocations.findLeakingVars(definingScope).stream() + .map(J.Identifier::getFieldType) + .anyMatch(variable.getName().getFieldType()::equals); + if (willLeak) { + variable = SearchResult.found(variable); + } + } + + return variable; + } +} diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/FindMissingTypes.java b/rewrite-java/src/main/java/org/openrewrite/java/search/FindMissingTypes.java index 036e98ef66a..384add4c471 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/search/FindMissingTypes.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/FindMissingTypes.java @@ -143,6 +143,18 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu // A different object in one implies a type has changed, either in the method signature or deeper in the type tree. mi = SearchResult.found(mi, "MethodInvocation#name#type is not the same instance as the MethodType of MethodInvocation."); } + if (type != null) { + int argCount = 0; + for (Expression argument : mi.getArguments()) { + if (!(argument instanceof J.Empty)) { + argCount++; + } + } + int minCount = type.hasFlags(Flag.Varargs) ? type.getParameterTypes().size() - 1 : type.getParameterTypes().size(); + if (argCount < minCount) { + mi = SearchResult.found(mi, "argument count mismatch: " + argCount + " != " + type.getParameterTypes().size()); + } + } } return mi; } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/FindTypes.java b/rewrite-java/src/main/java/org/openrewrite/java/search/FindTypes.java index 97a468dbffa..9c936327614 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/search/FindTypes.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/FindTypes.java @@ -28,6 +28,7 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.SearchResult; import org.openrewrite.trait.Trait; +import org.openrewrite.trait.Reference; import java.util.HashSet; import java.util.Set; @@ -68,19 +69,19 @@ public TreeVisitor getVisitor() { return Preconditions.check(new UsesType<>(fullyQualifiedTypeName, false), new TreeVisitor() { @Override public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { - return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithTypeReferences; + return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithReferences; } @Override public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if (tree instanceof JavaSourceFile) { return new JavaSourceFileVisitor(fullyQualifiedType).visit(tree, ctx); - } else if (tree instanceof SourceFileWithTypeReferences) { - SourceFileWithTypeReferences sourceFile = (SourceFileWithTypeReferences) tree; - SourceFileWithTypeReferences.TypeReferences typeReferences = sourceFile.getTypeReferences(); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences sourceFile = (SourceFileWithReferences) tree; + SourceFileWithReferences.References references = sourceFile.getReferences(); TypeMatcher matcher = new TypeMatcher(fullyQualifiedTypeName); - Set matches = typeReferences.findMatches(matcher).stream().map(Trait::getTree).collect(Collectors.toSet()); - return new TypeReferenceVisitor(matches).visit(tree, ctx); + Set matches = references.findMatches(matcher, Reference.Kind.TYPE).stream().map(Trait::getTree).collect(Collectors.toSet()); + return new ReferenceVisitor(matches).visit(tree, ctx); } return tree; } @@ -149,7 +150,7 @@ private static boolean typeMatches(boolean checkAssignability, Pattern pattern, @Value @EqualsAndHashCode(callSuper = false) - private static class TypeReferenceVisitor extends TreeVisitor { + private static class ReferenceVisitor extends TreeVisitor { Set matches; @Override diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/FindVariableEscapeLocations.java b/rewrite-java/src/main/java/org/openrewrite/java/search/FindVariableEscapeLocations.java new file mode 100644 index 00000000000..74344a7e773 --- /dev/null +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/FindVariableEscapeLocations.java @@ -0,0 +1,196 @@ +/* + * Copyright 2024 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.java.search; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.marker.SearchResult; + +import javax.annotation.Nullable; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * Find escaping points of local variables. + * An escaping point is the statement where a reference to a local variable leafs it scope an is now accessible from outside the method. + * In some situation such escaping is wanted, think of getters, but everytime its importent to rethink. + * Is mutability, synchronization or information hiding a problem here? + */ +public class FindVariableEscapeLocations extends JavaIsoVisitor { + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) { + method = super.visitMethodInvocation(method, executionContext); + + boolean noLeaks = findLocalArguments(method.getArguments()).isEmpty(); + + return noLeaks ? method : SearchResult.found(method); + } + + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext executionContext) { + assignment = super.visitAssignment(assignment, executionContext); + + J.Identifier assigmentTarget = (J.Identifier) assignment.getVariable(); + if (isPrimitive(assigmentTarget)) { + return assignment; + } + + boolean targetIsOutsider = isNotLocalVar(assigmentTarget); + boolean assignedByLocalVar = extractIdentifiers(assignment.getAssignment()).stream() + .map(FindVariableEscapeLocations::isLocalVar) + .reduce(false, Boolean::logicalOr); + boolean leaksVariable = targetIsOutsider && assignedByLocalVar; + + return leaksVariable ? SearchResult.found(assignment) : assignment; + } + + @Override + public J.Return visitReturn(J.Return _return, ExecutionContext executionContext) { + _return = super.visitReturn(_return, executionContext); + + if (_return.getExpression() == null) { + return _return; + } + + boolean returnsLocalVar = extractIdentifiers(_return.getExpression()).stream() + .map(FindVariableEscapeLocations::extractIdentifiers) + .flatMap(Collection::stream) + .filter(i -> !isPrimitive(i)) + .map(FindVariableEscapeLocations::isLocalVar) + .reduce(false, Boolean::logicalOr); + + return returnsLocalVar ? SearchResult.found(_return) : _return; + } + + @Override + public J.NewClass visitNewClass(J.NewClass newClass, ExecutionContext executionContext) { + newClass = super.visitNewClass(newClass, executionContext); + + boolean noLeaks = findLocalArguments(newClass.getArguments()).isEmpty(); + + return noLeaks ? newClass : SearchResult.found(newClass); + } + + /** + * Finds statements that enable escaping of local variables from the given subtree + */ + public static Set find(J subtree) { + return TreeVisitor.collect(new FindVariableEscapeLocations(), subtree, new HashSet<>()) + .stream() + .filter(Statement.class::isInstance) + .map(Statement.class::cast) + .collect(Collectors.toSet()); + } + + /** + * Finds identifier of local variables that escape from the given subtree + */ + public static Set findLeakingVars(J subtree) { + Function> extractIdentifiers = t -> { + Set identifiers = new HashSet<>(); + if (t instanceof J.Return) { + identifiers.addAll(extractIdentifiers(((J.Return) t).getExpression())); + } else if (t instanceof J.Assignment) { + identifiers.addAll(extractIdentifiers(((J.Assignment) t).getAssignment())); + } else if (t instanceof J.NewClass) { + identifiers.addAll(findLocalArguments(((J.NewClass) t).getArguments())); + } else if(t instanceof J.MethodInvocation) { + identifiers.addAll(findLocalArguments(((J.MethodInvocation) t).getArguments())); + } + + return identifiers; + }; + + return TreeVisitor.collect(new FindVariableEscapeLocations(), subtree, new HashSet<>()) + .stream() + .map(extractIdentifiers) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + } + + /** + * Extracts Set of Identifiers from J.Identifiers and Ternary operators. + * These two potentially participate in very case but will not lead to escaping themselves. + */ + private static Set extractIdentifiers(@Nullable Expression assignment) { + Set identifiers = new HashSet<>(); + // fast return if already an J.Identifier + if (assignment instanceof J.Identifier) { + identifiers.add((J.Identifier) assignment); + } + if (assignment instanceof J.Ternary) { + // if ternary we only need to care about direct assigment. + // the other possibilities (J.MethodInvocation, J.NewClass) are handled by the visitor itself. + if (((J.Ternary) assignment).getFalsePart() instanceof J.Identifier) { + identifiers.add((J.Identifier) ((J.Ternary) assignment).getFalsePart()); + } + if (((J.Ternary) assignment).getTruePart() instanceof J.Identifier) { + identifiers.add((J.Identifier) ((J.Ternary) assignment).getTruePart()); + } + } + + return identifiers; + } + + private static boolean isLocalVar(J.Identifier identifier) { + JavaType.Variable fieldType = identifier.getFieldType(); + + if (fieldType != null) { + JavaType owner = fieldType.getOwner(); + + boolean isOwnedByMethod = owner instanceof JavaType.Method; + if (isOwnedByMethod) { + boolean isMethodParameter = ((JavaType.Method) owner).getParameterNames().contains(identifier.getSimpleName()); + return !isMethodParameter; + } + } + + return false; + } + + private static boolean isNotLocalVar(J.Identifier identifier) { + return !isLocalVar(identifier); + } + + private static boolean isPrimitive(J.Identifier identifier) { + JavaType.Variable fieldType = identifier.getFieldType(); + return fieldType != null && fieldType.getType() instanceof JavaType.Primitive; + } + + private static boolean isNotPrimitive(J.Identifier identifier) { + return !isPrimitive(identifier); + } + + private static List findLocalArguments(List arguments) { + return arguments.stream() + .map(FindVariableEscapeLocations::extractIdentifiers) + .flatMap(Collection::stream) + .filter(FindVariableEscapeLocations::isNotPrimitive) + .filter(FindVariableEscapeLocations::isLocalVar) + .collect(Collectors.toList()); + } +} diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java b/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java index b6bbcdce598..5d0697b676c 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java @@ -41,24 +41,12 @@ public static boolean areEqual(J firstElem, J secondElem) { return semanticallyEqualVisitor.isEqual(); } - /** - * Compares method invocations and new class constructors based on the `JavaType.Method` instead of checking - * each types of each parameter. - * I.E. void foo(Object obj) {} invoked with `java.lang.String` or `java.lang.Integer` will return true. - */ - public static boolean areSemanticallyEqual(J firstElem, J secondElem) { - SemanticallyEqualVisitor semanticallyEqualVisitor = new SemanticallyEqualVisitor(false); - semanticallyEqualVisitor.visit(firstElem, secondElem); - return semanticallyEqualVisitor.isEqual.get(); - } - @SuppressWarnings("ConstantConditions") protected static class SemanticallyEqualVisitor extends JavaIsoVisitor { private final boolean compareMethodArguments; protected final AtomicBoolean isEqual = new AtomicBoolean(true); private final Deque> variableScope = new ArrayDeque<>(); - private final Set seen = new HashSet<>(); public SemanticallyEqualVisitor(boolean compareMethodArguments) { this.compareMethodArguments = compareMethodArguments; @@ -137,10 +125,7 @@ protected void visitList(@Nullable List list1, @Nullable List scope = variableScope.peek(); + if (j instanceof J.FieldAccess) { + J.FieldAccess field = (J.FieldAccess) j; + Expression fieldTarget = field.getTarget(); + + // Consider implicit-this "a" equivalent to "this.a" + // Definitely does not account for every possible edge case around "this", but handles the common case + if(fieldTarget instanceof J.Identifier && "this".equals(((J.Identifier) fieldTarget).getSimpleName())) { + visit(identifier, field.getName()); + return identifier; + } + + // Identifier name and type aren't the same as the field access they are obviously different + if(!identifier.getSimpleName().equals(field.getSimpleName()) || !TypeUtils.isOfType(identifier.getFieldType(), field.getName().getFieldType())) { isEqual.set(false); + return identifier; } + + String identifierTypeFqn = identifier.getType() instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified)identifier.getType()).getFullyQualifiedName() : ""; + // If the field access is a static field access, the identifier must be a class reference + // e.g.: Pattern is semantically equal to java.util.regex.Pattern + if (field.isFullyQualifiedClassReference(identifierTypeFqn)) { + return identifier; + } + + // Similarly, might be comparing a statically imported field to a fully qualified + // e.g. java.util.regex.Pattern.CASE_INSENSITIVE is semantically equal to CASE_INSENSITIVE when the latter is a static import of the former + JavaType identifierOwner = identifier.getFieldType().getOwner(); + String identifierOwnerFqn = identifierOwner instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified) identifierOwner).getFullyQualifiedName() : ""; + if(field.getTarget() instanceof J.FieldAccess && ((J.FieldAccess) field.getTarget()).isFullyQualifiedClassReference(identifierOwnerFqn)) { + return identifier; + } + + // Identifier is statically imported field name "CASE_INSENSITIVE", field is field access "Pattern.CASE_INSENSITIVE" + if (identifierOwner instanceof JavaType.FullyQualified && ((JavaType.FullyQualified) identifierOwner).getClassName().equals(fieldTarget.toString()) && TypeUtils.isOfType(identifier.getFieldType().getOwner(), fieldTarget.getType())) { + return identifier; + } + + isEqual.set(false); + return identifier; + } + if (!(j instanceof J.Identifier)) { + isEqual.set(false); return identifier; } J.Identifier compareTo = (J.Identifier) j; if (identifier.getFieldType() != null) { - Map scope = variableScope.peek(); if (scope != null && scope.containsKey(identifier.getSimpleName()) && scope.get(identifier.getSimpleName()).equals(compareTo.getSimpleName())) { return identifier; } @@ -856,8 +877,7 @@ public J.MemberReference visitMemberReference(J.MemberReference memberRef, J j) return memberRef; } - visit(memberRef.getContaining(), compareTo.getContaining()); - this.visitList(memberRef.getTypeParameters(), compareTo.getTypeParameters()); + visitList(memberRef.getTypeParameters(), compareTo.getTypeParameters()); } return memberRef; } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/UsesType.java b/rewrite-java/src/main/java/org/openrewrite/java/search/UsesType.java index 3d348fc8cbe..2e701f37491 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/search/UsesType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/UsesType.java @@ -18,7 +18,7 @@ import lombok.Getter; import org.jspecify.annotations.Nullable; import org.openrewrite.SourceFile; -import org.openrewrite.SourceFileWithTypeReferences; +import org.openrewrite.SourceFileWithReferences; import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.StringUtils; @@ -28,7 +28,7 @@ import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.SearchResult; -import org.openrewrite.trait.TypeReference; +import org.openrewrite.trait.Reference; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -77,7 +77,7 @@ public UsesType(String fullyQualifiedType, @Nullable Boolean includeImplicit) { @Override public boolean isAcceptable(SourceFile sourceFile, P p) { - return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithTypeReferences; + return sourceFile instanceof JavaSourceFile || sourceFile instanceof SourceFileWithReferences; } @Override @@ -119,11 +119,11 @@ public boolean isAcceptable(SourceFile sourceFile, P p) { } } } - } else if (tree instanceof SourceFileWithTypeReferences) { - SourceFileWithTypeReferences sourceFile = (SourceFileWithTypeReferences) tree; - SourceFileWithTypeReferences.TypeReferences typeReferences = sourceFile.getTypeReferences(); + } else if (tree instanceof SourceFileWithReferences) { + SourceFileWithReferences sourceFile = (SourceFileWithReferences) tree; + SourceFileWithReferences.References references = sourceFile.getReferences(); TypeMatcher matcher = typeMatcher != null ? typeMatcher : new TypeMatcher(fullyQualifiedType); - for (TypeReference ignored : typeReferences.findMatches(matcher)) { + for (Reference ignored : references.findMatches(matcher, Reference.Kind.TYPE)) { return SearchResult.found(sourceFile); } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java index 1f4ab2a97a6..070d38ea124 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java @@ -134,7 +134,7 @@ public AnnotatedType withType(@Nullable JavaType type) { } /** - * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(J)} instead. + * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(Cursor)} instead. */ @Deprecated public List getAllAnnotations() { @@ -1222,6 +1222,12 @@ public ClassDeclaration withPrimaryConstructor(@Nullable List primary @Nullable JLeftPadded extendings; + /** + * This is used to access the parent class. + * + * @return The parent class of the ClassDeclaration. If the ClassDeclaration is a class, this will return the + * class specified by the 'extends' keyword. If the ClassDeclaration is an interface, this will return null. + */ public @Nullable TypeTree getExtends() { return extendings == null ? null : extendings.getElement(); } @@ -1233,6 +1239,13 @@ public ClassDeclaration withExtends(@Nullable TypeTree extendings) { @Nullable JContainer implementings; + /** + * This is used to access the parent interfaces. + * + * @return A list of the parent interfaces of the ClassDeclaration. If the ClassDeclaration is a class, this + * will return the interfaces specified by the 'implements' keyword. If the ClassDeclaration is an interface, + * this will return the interfaces specified by the 'extends' keyword. + */ public @Nullable List getImplements() { return implementings == null ? null : implementings.getElements(); } @@ -1279,7 +1292,7 @@ public

J acceptJava(JavaVisitor

v, P p) { } /** - * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(J)} instead. + * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(Cursor)} instead. */ @Deprecated // gather annotations from everywhere they may occur @@ -3674,7 +3687,7 @@ public CoordinateBuilder.MethodDeclaration getCoordinates() { } /** - * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(J)} instead. + * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(Cursor)} instead. */ @Deprecated // gather annotations from everywhere they may occur @@ -5797,7 +5810,7 @@ public CoordinateBuilder.VariableDeclarations getCoordinates() { } /** - * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(J)} instead. + * @deprecated Use {@link org.openrewrite.java.service.AnnotationService#getAllAnnotations(Cursor)} instead. */ @Deprecated // gather annotations from everywhere they may occur diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java index 789c1bea8fe..fb550d501cb 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java @@ -23,7 +23,6 @@ import java.util.*; import java.util.function.Predicate; import java.util.regex.Pattern; -import java.util.stream.IntStream; public class TypeUtils { private static final JavaType.Class TYPE_OBJECT = JavaType.ShallowClass.build("java.lang.Object"); @@ -78,8 +77,8 @@ public static boolean isObject(@Nullable JavaType type) { public static boolean isString(@Nullable JavaType type) { return type == JavaType.Primitive.String || - (type instanceof JavaType.FullyQualified && - "java.lang.String".equals(((JavaType.FullyQualified) type).getFullyQualifiedName()) + (type instanceof JavaType.Class && + "java.lang.String".equals(((JavaType.Class) type).getFullyQualifiedName()) ); } @@ -224,132 +223,281 @@ public static boolean isOfTypeWithName( return false; } + public enum TypePosition { + In, Out, Invariant + } + public static boolean isAssignableTo(@Nullable JavaType to, @Nullable JavaType from) { + return isAssignableTo(to, from, TypePosition.Invariant); + } + + public static boolean isAssignableTo(@Nullable JavaType to, @Nullable JavaType from, TypePosition position) { try { if (to instanceof JavaType.Unknown || from instanceof JavaType.Unknown) { return false; } if (to == from) { return true; - } else if (to instanceof JavaType.Parameterized) { + } + + // Handle parameterized types (e.g., List) + if (to instanceof JavaType.Parameterized) { JavaType.Parameterized toParameterized = (JavaType.Parameterized) to; + + // If 'from' is not parameterized but the 'to' type is, + // this would be an unsafe raw type conversion - disallow it unless for wildcards if (!(from instanceof JavaType.Parameterized)) { - return from instanceof JavaType.FullyQualified && - isAssignableTo(toParameterized.getType(), from) && - toParameterized.getTypeParameters().stream() - .allMatch(p -> (p instanceof JavaType.GenericTypeVariable && - ((JavaType.GenericTypeVariable) p).getName().equals("?")) || - isAssignableTo(p, TYPE_OBJECT)); + for (JavaType typeParameter : toParameterized.getTypeParameters()) { + if (typeParameter instanceof JavaType.GenericTypeVariable && ((JavaType.GenericTypeVariable) typeParameter).getName().equals("?")) { + continue; + } + return false; + } + // all wildcards case + return isAssignableTo(toParameterized.getType(), from); } + JavaType.Parameterized fromParameterized = (JavaType.Parameterized) from; List toParameters = toParameterized.getTypeParameters(); List fromParameters = fromParameterized.getTypeParameters(); - int parameterCount = toParameters.size(); - return parameterCount == fromParameters.size() && - isAssignableTo(toParameterized.getType(), fromParameterized.getType()) && - IntStream.range(0, parameterCount).allMatch(i -> isAssignableTo(toParameters.get(i), fromParameters.get(i))); - } else if (to instanceof JavaType.FullyQualified) { + + // First check if the raw types are assignable + if (toParameters.size() != fromParameters.size() || + !isAssignableTo(toParameterized.getType(), fromParameterized.getType(), position)) { + return false; + } + + // Check type parameters with appropriate variance + for (int i = 0; i < toParameters.size(); i++) { + JavaType toParam = toParameters.get(i); + JavaType fromParam = fromParameters.get(i); + + if (toParam instanceof JavaType.GenericTypeVariable) { + JavaType.GenericTypeVariable toGeneric = (JavaType.GenericTypeVariable) toParam; + + // Special handling for wildcards + if (toGeneric.getName().equals("?")) { + // If both are wildcards, check their compatibility + if (fromParam instanceof JavaType.GenericTypeVariable && + ((JavaType.GenericTypeVariable) fromParam).getName().equals("?")) { + + // If both are unbounded wildcards, they're compatible + if (toGeneric.getBounds().isEmpty() && + ((JavaType.GenericTypeVariable) fromParam).getBounds().isEmpty()) { + continue; // Skip to next parameter, these wildcards match + } + + // If they have bounds, check bound compatibility + return areWildcardBoundsCompatible(toGeneric, (JavaType.GenericTypeVariable) fromParam, position); + } + + // Wildcard to non-wildcard case + if (toGeneric.getBounds().isEmpty()) { + // Unbounded wildcard accepts anything + return true; + } else { + // Bounded wildcard - use the variance from the wildcard + TypePosition wildcardPosition = convertVarianceToPosition(toGeneric.getVariance()); + for (JavaType bound : toGeneric.getBounds()) { + if (!isAssignableTo(bound, fromParam, wildcardPosition)) { + return false; + } + } + return true; + } + } + } + + // For all non-wildcard cases, use invariant position + if (!isOfType(toParam, fromParam)) { + return false; + } + } + return true; + } + + // Handle generic type variables (e.g., T extends Collection) + else if (to instanceof JavaType.GenericTypeVariable) { + JavaType.GenericTypeVariable toGeneric = (JavaType.GenericTypeVariable) to; + + switch (position) { + case In: + // In parameter position (contravariant), the provided type must be a supertype + // of at least one possible type that could satisfy the bounds + for (JavaType bound : toGeneric.getBounds()) { + if (isAssignableTo(from, bound, TypePosition.Invariant)) { + return true; + } + } + return false; + + case Out: + // In return position (covariant), we can assign any subtype that satisfies the bounds + for (JavaType bound : toGeneric.getBounds()) { + if (!isAssignableTo(bound, from, TypePosition.Invariant)) { + return false; + } + } + return true; + + case Invariant: + // In invariant position, types must match exactly + if (from instanceof JavaType.GenericTypeVariable) { + return toGeneric.getName().equals(((JavaType.GenericTypeVariable) from).getName()); + } + return false; + } + } + + // Handle fully qualified types (e.g., java.util.List) + else if (to instanceof JavaType.FullyQualified) { JavaType.FullyQualified toFq = (JavaType.FullyQualified) to; if (from instanceof JavaType.Primitive) { JavaType.Primitive toPrimitive = JavaType.Primitive.fromClassName(toFq.getFullyQualifiedName()); if (toPrimitive != null) { - return isAssignableTo(toPrimitive, from); - } else if (toFq.getFullyQualifiedName().equals("java.lang.Object")) { + return isAssignableTo(toPrimitive, from, position); + } else if (isObject(toFq)) { return true; } } else if (from instanceof JavaType.Intersection) { for (JavaType intersectionType : ((JavaType.Intersection) from).getBounds()) { - if (isAssignableTo(to, intersectionType)) { + if (isAssignableTo(to, intersectionType, position)) { return true; } } return false; } return !(from instanceof JavaType.GenericTypeVariable) && isAssignableTo(toFq.getFullyQualifiedName(), from); - } else if (to instanceof JavaType.GenericTypeVariable) { - if (from instanceof JavaType.GenericTypeVariable && isOfType(to, from)) { - return true; + } + + // Rest of the existing cases, passing through the position parameter + else if (to instanceof JavaType.Variable) { + return isAssignableTo(((JavaType.Variable) to).getType(), from, position); + } else if (to instanceof JavaType.Method) { + return isAssignableTo(((JavaType.Method) to).getReturnType(), from, position); + } else if (to instanceof JavaType.Array && from instanceof JavaType.Array) { + JavaType.Array toArray = (JavaType.Array) to; + JavaType.Array fromArray = (JavaType.Array) from; + if (toArray.getElemType() instanceof JavaType.Primitive) { + return isOfType(toArray.getElemType(), fromArray.getElemType()); } - JavaType.GenericTypeVariable toGeneric = (JavaType.GenericTypeVariable) to; - List toBounds = toGeneric.getBounds(); - if (!toGeneric.getName().equals("?")) { - return false; - } else if (toGeneric.getVariance() == JavaType.GenericTypeVariable.Variance.COVARIANT) { - for (JavaType toBound : toBounds) { - if (!isAssignableTo(toBound, from)) { - return false; - } - } - return true; - } else if (toGeneric.getVariance() == JavaType.GenericTypeVariable.Variance.CONTRAVARIANT) { - for (JavaType toBound : toBounds) { - if (!isAssignableTo(from, toBound)) { - return false; - } + // Arrays are invariant in Java + return isAssignableTo(toArray.getElemType(), fromArray.getElemType(), TypePosition.Invariant); + } + + // Handle primitives with their existing logic + else if (to instanceof JavaType.Primitive) { + // Primitive handling remains unchanged as they don't involve variance + return handlePrimitiveAssignability((JavaType.Primitive) to, from); + } + + } catch (Exception ignored) { + } + return false; + } + + private static boolean areWildcardBoundsCompatible(JavaType.GenericTypeVariable to, JavaType.GenericTypeVariable from, TypePosition position) { + // If both wildcards are unbounded, they're compatible + if (to.getBounds().isEmpty() && from.getBounds().isEmpty()) { + return true; + } + + // If we have a bounded and unbounded wildcard: + if (to.getBounds().isEmpty()) { + // Unbounded target only accepts unbounded source + return false; + } + if (from.getBounds().isEmpty()) { + // Source being unbounded is never safe when target is bounded + return false; + } + + // Both wildcards are bounded + switch (position) { + case Out: + // In covariant position, source bounds must be more specific than target bounds + for (JavaType bound : to.getBounds()) { + if (!isAssignableTo(bound, from.getBounds().get(0), TypePosition.Invariant)) { + return false; } - return true; - } else if (toBounds.isEmpty()) { - return from instanceof JavaType.FullyQualified; } - return false; - } else if (to instanceof JavaType.Variable) { - return isAssignableTo(((JavaType.Variable) to).getType(), from); - } else if (to instanceof JavaType.Method) { - return isAssignableTo(((JavaType.Method) to).getReturnType(), from); - } else if (to instanceof JavaType.Primitive) { - JavaType.Primitive toPrimitive = (JavaType.Primitive) to; - if (from instanceof JavaType.FullyQualified) { - // Account for auto-unboxing - JavaType.FullyQualified boxed = JavaType.ShallowClass.build(toPrimitive.getClassName()); - return isAssignableTo(boxed, from); - } else if (from instanceof JavaType.Primitive) { - JavaType.Primitive fromPrimitive = (JavaType.Primitive) from; - if (fromPrimitive == JavaType.Primitive.Boolean || fromPrimitive == JavaType.Primitive.Void) { + return true; + case In: + // In contravariant position, target bounds must be more specific than source bounds + for (JavaType bound : from.getBounds()) { + if (!isAssignableTo(bound, to.getBounds().get(0), TypePosition.Invariant)) { return false; - } else { - switch (toPrimitive) { - case Char: - return fromPrimitive == JavaType.Primitive.Byte; - case Short: - switch (fromPrimitive) { - case Byte: - case Char: - return true; - } - return false; - case Int: - switch (fromPrimitive) { - case Byte: - case Char: - case Short: - return true; - } - return false; - case Long: - switch (fromPrimitive) { - case Byte: - case Char: - case Short: - case Int: - return true; - } - return false; - case Float: - return fromPrimitive != JavaType.Primitive.Double; - case Double: - return true; - case String: - return fromPrimitive == JavaType.Primitive.Null; - default: - return false; - } } } - } else if (to instanceof JavaType.Array && from instanceof JavaType.Array) { - return isAssignableTo(((JavaType.Array) to).getElemType(), ((JavaType.Array) from).getElemType()); + return true; + case Invariant: + // In invariant position, bounds must match exactly + return to.getBounds().equals(from.getBounds()); + } + return false; + } + + private static TypePosition convertVarianceToPosition(JavaType.GenericTypeVariable.Variance variance) { + switch (variance) { + case COVARIANT: + return TypePosition.Out; + case CONTRAVARIANT: + return TypePosition.In; + default: + return TypePosition.Invariant; + } + } + + private static boolean handlePrimitiveAssignability(JavaType.Primitive to, @Nullable JavaType from) { + if (from instanceof JavaType.FullyQualified) { + // Account for auto-unboxing + JavaType.FullyQualified boxed = JavaType.ShallowClass.build(to.getClassName()); + return isAssignableTo(boxed, from); + } else if (from instanceof JavaType.Primitive) { + JavaType.Primitive fromPrimitive = (JavaType.Primitive) from; + switch (fromPrimitive) { + case Boolean: + case Void: + case None: + case Null: + case String: + return false; + default: + switch (to) { + case Char: + return false; + case Short: + switch (fromPrimitive) { + case Byte: + case Char: + return true; + } + return false; + case Int: + switch (fromPrimitive) { + case Byte: + case Char: + case Short: + return true; + } + return false; + case Long: + switch (fromPrimitive) { + case Byte: + case Char: + case Short: + case Int: + return true; + } + return false; + case Float: + return fromPrimitive != JavaType.Primitive.Double; + case Double: + return true; + default: + return false; + } } - } catch (Exception e) { - return false; } return false; } diff --git a/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java b/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java new file mode 100644 index 00000000000..c94f455eb16 --- /dev/null +++ b/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java @@ -0,0 +1,495 @@ +/* + * Copyright 2023 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.java; + +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; +import org.openrewrite.DocumentExample; +import org.openrewrite.Recipe; +import org.openrewrite.test.RewriteTest; + +import java.util.List; + +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.test.RewriteTest.toRecipe; + +@SuppressWarnings({"ResultOfMethodCallIgnored", "CodeBlock2Expr", "RedundantThrows", "Convert2MethodRef", "EmptyTryBlock", "CatchMayIgnoreException", "EmptyFinallyBlock", "StringBufferReplaceableByString", "UnnecessaryLocalVariable"}) +class RemoveMethodInvocationsVisitorTest implements RewriteTest { + + private Recipe createRemoveMethodsRecipe(String... methods) { + return toRecipe(() -> new RemoveMethodInvocationsVisitor(List.of(methods))); + } + + @DocumentExample + @Test + void removeFromEnd() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder toString()")) + , + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello") + .append(" ") + .append("World") + .reverse() + .append(" ") + .reverse() + .append("Yeah") + .toString(); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello") + .append(" ") + .append("World") + .reverse() + .append(" ") + .reverse() + .append("Yeah"); + } + } + """ + ) + ); + } + + @Test + void removeMultipleMethodsFromEnd() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder toString()", "java.lang.StringBuilder append(java.lang.String)")), + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello") + .append(" ") + .append("World") + .reverse() + .append(" ") + .reverse() + .append("Yeah") + .toString(); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.reverse() + .reverse(); + } + } + """ + ) + ); + } + + @Test + void removeFromMiddle() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder reverse()")), + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello") + .append(" ") + .append("World") + .reverse() + .append(" ") + .reverse() + .append("Yeah") + .toString(); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello") + .append(" ") + .append("World") + .append(" ") + .append("Yeah") + .toString(); + } + } + """ + ) + ); + } + + @Test + void removeEntireStatement() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + sb.append("Hello"); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + } + } + """ + ) + ); + } + + @Test + @ExpectedToFail + void removeWithoutSelect() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("Test foo()")), + //language=java + java( + """ + public class Test { + void foo() {} + void method() { + StringBuilder sb = new StringBuilder(); + foo(); + } + } + """, + """ + public class Test { + void foo() {} + void method() { + StringBuilder sb = new StringBuilder(); + } + } + """ + ) + ); + } + + @Test + void removeFromWithinArguments() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + StringBuilder sb2 = new StringBuilder(); + sb.append(1) + .append(((java.util.function.Supplier) () -> sb2 + .append("foo") + .append('b') + .toString() + .charAt(0))) + .append(2) + .toString(); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + StringBuilder sb2 = new StringBuilder(); + sb.append(1) + .append(((java.util.function.Supplier) () -> sb2 + .append('b') + .toString() + .charAt(0))) + .append(2) + .toString(); + } + } + """ + ) + ); + } + + @Test + void keepSelectForAssignment() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + //language=java + java( + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + StringBuilder sb2 = sb.append("foo"); + sb2.append("bar"); + sb2.reverse(); + } + } + """, + """ + public class Test { + void method() { + StringBuilder sb = new StringBuilder(); + StringBuilder sb2 = sb; + sb2.reverse(); + } + } + """ + ) + ); + } + + @Test + void chainedCallsAsParameter() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + class Test { + void method() { + print(new StringBuilder() + .append("Hello") + .append(" ") + .append("World") + .reverse() + .append(" ") + .reverse() + .append("Yeah") + .toString()); + } + void print(String str) {} + } + """, + """ + class Test { + void method() { + print(new StringBuilder() + .reverse() + .reverse() + .toString()); + } + void print(String str) {} + } + """ + ) + ); + } + + @Test + void removeFromLambda() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + import java.util.List; + + public class Test { + void method(List names) { + names.forEach(name -> new StringBuilder() + .append("hello") + .append(" ") + .append(name) + .reverse() + .toString()); + } + } + """, + """ + import java.util.List; + + public class Test { + void method(List names) { + names.forEach(name -> new StringBuilder() + .reverse() + .toString()); + } + } + """ + ) + ); + } + + @Test + void complexCase() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + import java.util.List; + import java.util.function.Consumer; + + public class Test { + void method(List names) { + this.consume(s -> names.forEach(name -> { + new StringBuilder() + .append("hello") + .append(" ") + .append(name) + .reverse() + .toString(); + } + ) + ).toString(); + } + StringBuilder consume(Consumer consumer) {return new StringBuilder();} + } + """, + """ + import java.util.List; + import java.util.function.Consumer; + + public class Test { + void method(List names) { + this.consume(s -> names.forEach(name -> { + new StringBuilder() + .reverse() + .toString(); + } + ) + ).toString(); + } + StringBuilder consume(Consumer consumer) {return new StringBuilder();} + } + """ + ) + ); + } + + @Test + void returnEmptyLambdaBody() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + import java.util.function.Consumer; + + public class Test { + public void method() throws Exception { + this.customize(sb -> sb + .append("Hello") + ); + } + + public void customize(Consumer securityContextCustomizer) { + } + } + """, + """ + import java.util.function.Consumer; + + public class Test { + public void method() throws Exception { + } + + public void customize(Consumer securityContextCustomizer) { + } + } + """ + ) + ); + } + + @Test + void lambdaAssignment() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + import java.util.function.Consumer; + + public class Test { + public void method() { + StringBuilder sb = new StringBuilder(); + Consumer consumer = name -> { + sb.append(name); + }; + consumer.accept("hello"); + } + } + """, + """ + import java.util.function.Consumer; + + public class Test { + public void method() { + StringBuilder sb = new StringBuilder(); + Consumer consumer = name -> { + }; + consumer.accept("hello"); + } + } + """ + ) + ); + } + + @Test + void tryCatchBlocks() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.lang.StringBuilder append(java.lang.String)")), + // language=java + java( + """ + public class Test { + public void method() { + StringBuilder sb = new StringBuilder(); + try { + sb.append("Hello"); + } catch (Exception e) { + sb.append("Hello"); + } finally { + sb.append("Hello"); + } + } + } + """, + """ + public class Test { + public void method() { + StringBuilder sb = new StringBuilder(); + try { + } catch (Exception e) { + } finally { + } + } + } + """ + ) + ); + } +} diff --git a/rewrite-java/src/test/java/org/openrewrite/java/search/FindEscapingVariablesTest.java b/rewrite-java/src/test/java/org/openrewrite/java/search/FindEscapingVariablesTest.java new file mode 100644 index 00000000000..e43bc1520a1 --- /dev/null +++ b/rewrite-java/src/test/java/org/openrewrite/java/search/FindEscapingVariablesTest.java @@ -0,0 +1,453 @@ +/* + * Copyright 2024 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.java.search; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class FindEscapingVariablesTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(RewriteTest.toRecipe(FindEscapingVariables::new)); + } + + @Nested + class Escaping { + @Nested + class Directly { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return o; + } + } + """, """ + package com.sample; + public class Foo{ + Object test() { + Object /*~~>*/o = new Object(); + return o; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = o; + } + } + """, """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object /*~~>*/o = new Object(); + someField = o; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(o); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + Object /*~~>*/o = new Object(); + System.out.print(o); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(o); + } + } + """, """ + package com.sample; + public class Foo{ + Runnable test() { + Object /*~~>*/o = new Object(); + Runnable r = () -> System.out.print(o); + } + } + """)); + } + + @Test + void viaNew() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + StringBuilder /*~~>*/sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb); + } + } + """)); + } + } + + @Nested + class WrappedInTernary { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return o != null ? o : "null"; + } + } + """, """ + package com.sample; + public class Foo{ + Object test() { + Object /*~~>*/o = new Object(); + return o != null ? o : "null"; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = o != null ? o : "null"; + } + } + """, """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object /*~~>*/o = new Object(); + someField = o != null ? o : "null"; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(o != null ? o : "null"); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + Object /*~~>*/o = new Object(); + System.out.print(o != null ? o : "null"); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(o != null ? o : "null"); + } + } + """, """ + package com.sample; + public class Foo{ + Runnable test() { + Object /*~~>*/o = new Object(); + Runnable r = () -> System.out.print(o != null ? o : "null"); + } + } + """)); + } + + @Test + void viaNew() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb != null ? sb : new StringBuilder()); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + StringBuilder /*~~>*/sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb != null ? sb : new StringBuilder()); + } + } + """)); + } + } + } + + @Nested + class Secure { + @Nested + class OtherObject { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return new Object(); + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = new Object(); + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(new Object()); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(new Object()); + } + } + """)); + } + } + + @Nested + class Primitives { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + int test() { + int o = 1; + return o; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + int someField; + void test() { + int o = 1; + someField = o; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + int o = 1; + System.out.print(o); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + int o = 1; + Runnable r = () -> System.out.print(o); + } + } + """)); + } + } + + @Nested + class Parameter { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test(Object other) { + Object o = new Object(); + return other; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test(Object other) { + Object o = new Object(); + someField = other; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test(Object other) { + Object o = new Object(); + System.out.print(other); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test(Object other) { + Object o = new Object(); + Runnable r = () -> System.out.print(other); + } + } + """)); + } + } + } +} diff --git a/rewrite-java/src/test/java/org/openrewrite/java/search/FindVariableEscapeLocationsTest.java b/rewrite-java/src/test/java/org/openrewrite/java/search/FindVariableEscapeLocationsTest.java new file mode 100644 index 00000000000..1bd466a55dc --- /dev/null +++ b/rewrite-java/src/test/java/org/openrewrite/java/search/FindVariableEscapeLocationsTest.java @@ -0,0 +1,453 @@ +/* + * Copyright 2024 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.java.search; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class FindVariableEscapeLocationsTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(RewriteTest.toRecipe(FindVariableEscapeLocations::new)); + } + + @Nested + class Escaping { + @Nested + class Directly { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return o; + } + } + """, """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + /*~~>*/return o; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = o; + } + } + """, """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + /*~~>*/someField = o; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(o); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + /*~~>*/System.out.print(o); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(o); + } + } + """, """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> /*~~>*/System.out.print(o); + } + } + """)); + } + + @Test + void viaNew() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = /*~~>*/new StringBuilder(sb); + } + } + """)); + } + } + + @Nested + class WrappedInTernary { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return o != null ? o : "null"; + } + } + """, """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + /*~~>*/return o != null ? o : "null"; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = o != null ? o : "null"; + } + } + """, """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + /*~~>*/someField = o != null ? o : "null"; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(o != null ? o : "null"); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + /*~~>*/System.out.print(o != null ? o : "null"); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(o != null ? o : "null"); + } + } + """, """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> /*~~>*/System.out.print(o != null ? o : "null"); + } + } + """)); + } + + @Test + void viaNew() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = new StringBuilder(sb != null ? sb : new StringBuilder()); + } + } + """, """ + package com.sample; + public class Foo{ + void test() { + StringBuilder sb = new StringBuilder(); + StringBuilder other = /*~~>*/new StringBuilder(sb != null ? sb : new StringBuilder()); + } + } + """)); + } + } + } + + @Nested + class Secure { + @Nested + class OtherObject { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test() { + Object o = new Object(); + return new Object(); + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test() { + Object o = new Object(); + someField = new Object(); + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + Object o = new Object(); + System.out.print(new Object()); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + Object o = new Object(); + Runnable r = () -> System.out.print(new Object()); + } + } + """)); + } + } + + @Nested + class Primitives { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + int test() { + int o = 1; + return o; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + int someField; + void test() { + int o = 1; + someField = o; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test() { + int o = 1; + System.out.print(o); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test() { + int o = 1; + Runnable r = () -> System.out.print(o); + } + } + """)); + } + } + + @Nested + class Parameter { + @Test + @DocumentExample + void viaReturnValue() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object test(Object other) { + Object o = new Object(); + return other; + } + } + """)); + } + + @Test + void viaField() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Object someField; + void test(Object other) { + Object o = new Object(); + someField = other; + } + } + """)); + } + + @Test + void viaMethodCall() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + void test(Object other) { + Object o = new Object(); + System.out.print(other); + } + } + """)); + } + + @Test + void viaLambdaBody() { + rewriteRun(java( + """ + package com.sample; + public class Foo{ + Runnable test(Object other) { + Object o = new Object(); + Runnable r = () -> System.out.print(other); + } + } + """)); + } + } + } +} diff --git a/rewrite-json/src/main/java/org/openrewrite/json/JsonPathMatcher.java b/rewrite-json/src/main/java/org/openrewrite/json/JsonPathMatcher.java index e098741979d..492a5dc9bfe 100644 --- a/rewrite-json/src/main/java/org/openrewrite/json/JsonPathMatcher.java +++ b/rewrite-json/src/main/java/org/openrewrite/json/JsonPathMatcher.java @@ -20,6 +20,7 @@ import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.tree.ParseTree; +import org.antlr.v4.runtime.tree.RuleNode; import org.antlr.v4.runtime.tree.TerminalNode; import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; @@ -48,28 +49,28 @@ public class JsonPathMatcher { private final String jsonPath; + private JsonPathParser.@Nullable JsonPathContext parsed; public JsonPathMatcher(String jsonPath) { this.jsonPath = jsonPath; } public Optional find(Cursor cursor) { - LinkedList cursorPath = cursor.getPathAsStream() - .filter(o -> o instanceof Tree) - .map(Tree.class::cast) - .collect(Collectors.toCollection(LinkedList::new)); + return find0(cursor, resolvedAncestors(cursor)); + } + + private Optional find0(Cursor cursor, List cursorPath) { if (cursorPath.isEmpty()) { return Optional.empty(); } - Collections.reverse(cursorPath); Tree start; if (jsonPath.startsWith(".") && !jsonPath.startsWith("..")) { start = cursor.getValue(); } else { - start = cursorPath.peekFirst(); + start = cursorPath.get(0); } - JsonPathParser.JsonPathContext ctx = jsonPath().jsonPath(); + JsonPathParser.JsonPathContext ctx = parse(); // The stop may be optimized by interpreting the ExpressionContext and pre-determining the last visit. JsonPathParser.ExpressionContext stop = (JsonPathParser.ExpressionContext) ctx.children.get(ctx.children.size() - 1); @SuppressWarnings("ConstantConditions") JsonPathParserVisitor v = new JsonPathMatcher.JsonPathParserJsonVisitor(cursorPath, start, stop, false); @@ -80,8 +81,8 @@ public Optional find(Cursor cursor) { } public boolean matches(Cursor cursor) { - List cursorPath = cursor.getPathAsStream().collect(Collectors.toList()); - return find(cursor).map(o -> { + List cursorPath = resolvedAncestors(cursor); + return find0(cursor, cursorPath).map(o -> { if (o instanceof List) { //noinspection unchecked List l = (List) o; @@ -92,6 +93,22 @@ public boolean matches(Cursor cursor) { }).orElse(false); } + private static List resolvedAncestors(Cursor cursor) { + ArrayDeque deque = new ArrayDeque<>(); + for (Iterator it = cursor.getPath(Tree.class::isInstance); it.hasNext(); ) { + Tree tree = (Tree) it.next(); + deque.addFirst(tree); + } + return new ArrayList<>(deque); + } + + private JsonPathParser.JsonPathContext parse() { + if (parsed == null) { + parsed = jsonPath().jsonPath(); + } + return parsed; + } + private JsonPathParser jsonPath() { return new JsonPathParser(new CommonTokenStream(new JsonPathLexer(CharStreams.fromString(this.jsonPath)))); } @@ -121,6 +138,11 @@ protected Object aggregateResult(Object aggregate, Object nextResult) { return (scope = nextResult); } + @Override + protected boolean shouldVisitNextChild(RuleNode node, Object currentResult) { + return scope != null; + } + @Override public Object visitJsonPath(JsonPathParser.JsonPathContext ctx) { if (ctx.ROOT() != null || "[".equals(ctx.start.getText())) { diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java index 340f1865aec..885a3a2eeca 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java @@ -24,7 +24,6 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.xml.AddToTagVisitor; import org.openrewrite.xml.RemoveContentVisitor; -import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.util.Optional; @@ -32,7 +31,6 @@ @Value @EqualsAndHashCode(callSuper = false) public class AddProfile extends Recipe { - private static final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); @Option(displayName = "id", description = "The profile id.", @@ -81,7 +79,7 @@ private class AddProfileVisitor extends MavenIsoVisitor { public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (PROJECT_MATCHER.matches(getCursor())) { + if (isProjectTag()) { Optional maybeProfiles = t.getChild("profiles"); Xml.Tag profiles; if (maybeProfiles.isPresent()) { diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangePackaging.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangePackaging.java index 2b9c21f7b60..1d54870edc6 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangePackaging.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangePackaging.java @@ -26,7 +26,6 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.maven.tree.MavenResolutionResult; import org.openrewrite.maven.tree.ResolvedPom; -import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.util.Optional; @@ -38,7 +37,6 @@ @Value @EqualsAndHashCode(callSuper = false) public class ChangePackaging extends Recipe { - private static final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); @Option(displayName = "Group", description = "The groupId of the project whose packaging should be changed. Accepts glob patterns.", @@ -102,7 +100,7 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (PROJECT_MATCHER.matches(getCursor())) { + if (isProjectTag()) { Optional maybePackaging = t.getChild("packaging"); if (!maybePackaging.isPresent() || oldPackaging == null || oldPackaging.equals(maybePackaging.get().getValue().orElse(null))) { if (packaging == null || "jar".equals(packaging)) { diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeProjectVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeProjectVersion.java index 8f3c5891079..957c90bc989 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeProjectVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeProjectVersion.java @@ -25,7 +25,6 @@ import org.openrewrite.maven.tree.ResolvedPom; import org.openrewrite.xml.AddToTagVisitor; import org.openrewrite.xml.ChangeTagValueVisitor; -import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.util.Arrays; @@ -83,13 +82,11 @@ public String getDescription() { public TreeVisitor getVisitor() { return new MavenIsoVisitor() { - private final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); - @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (PROJECT_MATCHER.matches(getCursor())) { + if (isProjectTag()) { ResolvedPom resolvedPom = getResolutionResult().getPom(); if (matchesGlob(resolvedPom.getValue(t.getChildValue("groupId").orElse(null)), groupId) && diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java index df6b9fa3c8d..780127619be 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java @@ -22,7 +22,6 @@ import org.openrewrite.maven.tree.GroupArtifact; import org.openrewrite.maven.tree.ResolvedPom; import org.openrewrite.xml.ChangeTagValue; -import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.util.*; @@ -81,7 +80,6 @@ public Map getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(Map acc) { - final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); final Pattern SEMVER_PATTERN = Pattern.compile("(\\d+)\\.(\\d+)\\.(\\d+)\\.?(\\d+)?(-.+)?$"); return new MavenIsoVisitor() { @@ -89,7 +87,7 @@ public TreeVisitor getScanner(Map ac public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (!PROJECT_MATCHER.matches(getCursor())) { + if (!isProjectTag()) { return t; } ResolvedPom resolvedPom = getResolutionResult().getPom(); @@ -158,14 +156,11 @@ private String incrementSemverDigit(String oldVersion) { @Override public TreeVisitor getVisitor(Map acc) { return new MavenIsoVisitor() { - final XPathMatcher PARENT_MATCHER = new XPathMatcher("/project/parent"); - final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); - @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if ((!(PROJECT_MATCHER.matches(getCursor()) || PARENT_MATCHER.matches(getCursor()))) || + if (!(isProjectTag() || isParentTag()) || t.getMarkers().findFirst(AlreadyIncremented.class).isPresent()) { return t; } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java index 342e08ddb04..5a5b77a16f9 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java @@ -45,6 +45,7 @@ public class MavenVisitor

extends XmlVisitor

{ static final XPathMatcher PLUGIN_MATCHER = new XPathMatcher("//plugins/plugin"); static final XPathMatcher MANAGED_PLUGIN_MATCHER = new XPathMatcher("//pluginManagement/plugins/plugin"); static final XPathMatcher PARENT_MATCHER = new XPathMatcher("/project/parent"); + static final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); private transient Xml.@Nullable Document document; @@ -259,6 +260,10 @@ public boolean isParentTag() { return isTag("parent") && PARENT_MATCHER.matches(getCursor()); } + public boolean isProjectTag() { + return isTag("project") && PROJECT_MATCHER.matches(getCursor()); + } + private boolean isTag(String name) { // `XPathMatcher` is still a bit expensive return getCursor().getValue() instanceof Xml.Tag && name.equals(getCursor().getValue().getName()); diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveUnusedProperties.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveUnusedProperties.java new file mode 100644 index 00000000000..3251d64f736 --- /dev/null +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveUnusedProperties.java @@ -0,0 +1,275 @@ +/* + * Copyright 2024 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.maven; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.*; +import org.openrewrite.java.tree.JavaSourceFile; +import org.openrewrite.maven.internal.MavenPomDownloader; +import org.openrewrite.maven.tree.MavenResolutionResult; +import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion; +import org.openrewrite.maven.tree.ResolvedPom; +import org.openrewrite.text.PlainText; +import org.openrewrite.text.PlainTextParser; +import org.openrewrite.text.PlainTextVisitor; +import org.openrewrite.xml.RemoveContentVisitor; +import org.openrewrite.xml.XPathMatcher; +import org.openrewrite.xml.tree.Xml; + +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +@Value +@EqualsAndHashCode(callSuper = false) +public class RemoveUnusedProperties extends ScanningRecipe { + @Option(displayName = "Property pattern", + description = "A pattern to filter properties to remove. Defaults to `.+?` to match anything", + required = false, + example = ".+\\.version") + @Nullable + String propertyPattern; + + @Override + public String getDisplayName() { + return "Remove unused properties"; + } + + @Override + public String getDescription() { + return "Detect and remove Maven property declarations which do not have any usage within the project."; + } + + public static class Accumulator { + public Map> propertiesToUsingPoms = new HashMap<>(); + public Map filteredResourcePathsToDeclaringPoms = new HashMap<>(); + public Map> nonPomPathsToUsages = new HashMap<>(); + + public Map> getFilteredResourceUsages() { + Map> result = new HashMap<>(); + filteredResourcePathsToDeclaringPoms.forEach((filteredResourcePath, mrr) -> + nonPomPathsToUsages.forEach((usagePath, properties) -> { + if (usagePath.startsWith(filteredResourcePath)) { + properties.forEach(property -> { + result.putIfAbsent(property, new HashSet<>()); + result.get(property).add(mrr); + }); + } + } + )); + return result; + } + } + + @Override + public RemoveUnusedProperties.Accumulator getInitialValue(ExecutionContext ctx) { + return new RemoveUnusedProperties.Accumulator(); + } + + private String getPropertyPattern() { + return propertyPattern != null ? propertyPattern : ".+?"; + } + + @Override + public TreeVisitor getScanner(RemoveUnusedProperties.Accumulator acc) { + String patternOrDefault = getPropertyPattern(); + MavenIsoVisitor findPomUsagesVisitor = new FindPomUsagesVisitor(dollarPropertyMatcher(patternOrDefault), acc); + MavenIsoVisitor findFilteredResourcePathsVisitor = new FindFilteredResourcePathsVisitor(acc); + PlainTextVisitor findResourceUsagesVisitor = new FindResourceUsagesVisitor(patternOrDefault, acc); + + return new TreeVisitor() { + @Override + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + if (tree instanceof SourceFile) { + SourceFile sf = (SourceFile) tree; + if (findPomUsagesVisitor.isAcceptable(sf, ctx)) { // ie: is a pom + findPomUsagesVisitor.visit(sf, ctx); + findFilteredResourcePathsVisitor.visit(sf, ctx); + } else if (!(tree instanceof JavaSourceFile)) { // optimization: avoid visiting code files which are almost always not filtered resources + findResourceUsagesVisitor.visit(PlainTextParser.convert(sf), ctx); + } + } + return tree; + } + }; + } + + private static Pattern dollarPropertyMatcher(String patternOrDefault) { + return Pattern.compile("[^$]*\\$\\{(" + patternOrDefault + ")}[^$]*"); + } + + private static Pattern atPropertyMatcher(String patternOrDefault) { + return Pattern.compile("@(" + patternOrDefault + ")@"); + } + + @Override + public TreeVisitor getVisitor(RemoveUnusedProperties.Accumulator acc) { + Pattern propertyMatcher = Pattern.compile(getPropertyPattern()); + Map> filteredResourceUsages = acc.getFilteredResourceUsages(); + return new MavenIsoVisitor() { + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + Xml.Tag t = super.visitTag(tag, ctx); + String propertyName = t.getName(); + if (isPropertyTag() && propertyMatcher.matcher(propertyName).matches()) { + if (isMavenBuiltinProperty(propertyName)) { + return t; + } + + if (parentHasProperty(getResolutionResult(), propertyName, ctx)) { + return t; + } + + if (acc.propertiesToUsingPoms.containsKey(propertyName)) { + for (MavenResolutionResult pomWhereUsed : acc.propertiesToUsingPoms.get(propertyName)) { + if (isAncestor(pomWhereUsed, getResolutionResult().getPom().getGav())) { + return t; + } + } + } + + if (filteredResourceUsages.containsKey(propertyName)) { + for (MavenResolutionResult pomWhereUsed : filteredResourceUsages.get(propertyName)) { + if (isAncestor(pomWhereUsed, getResolutionResult().getPom().getGav())) { + return t; + } + } + } + + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); + maybeUpdateModel(); + } + return t; + } + + private boolean isMavenBuiltinProperty(String propertyName) { + return propertyName.startsWith("project.") || propertyName.startsWith("maven."); + } + + private boolean isAncestor(MavenResolutionResult project, ResolvedGroupArtifactVersion possibleAncestorGav) { + MavenResolutionResult projectAncestor = project; + while (projectAncestor != null) { + if (projectAncestor.getPom().getGav().equals(possibleAncestorGav)) { + return true; + } + projectAncestor = projectAncestor.getParent(); + } + return false; + } + + private boolean parentHasProperty(MavenResolutionResult resolutionResult, String propertyName, + ExecutionContext ctx) { + MavenPomDownloader downloader = new MavenPomDownloader(resolutionResult.getProjectPoms(), ctx, + resolutionResult.getMavenSettings(), resolutionResult.getActiveProfiles()); + try { + ResolvedPom resolvedBarePom = resolutionResult.getPom().getRequested() + .withProperties(Collections.emptyMap()) + .withDependencies(Collections.emptyList()) + .withDependencyManagement(Collections.emptyList()) + .withPlugins(Collections.emptyList()) + .withPluginManagement(Collections.emptyList()) + .resolve(resolutionResult.getActiveProfiles(), downloader, ctx); + return resolvedBarePom.getProperties().containsKey(propertyName); + } catch (MavenDownloadingException e) { + // assume parent *does* have property if error to do no harm + return true; + } + } + }; + } + + private static class FindPomUsagesVisitor extends MavenIsoVisitor { + private final Pattern propertyUsageMatcher; + private final Accumulator acc; + + public FindPomUsagesVisitor(Pattern propertyUsageMatcher, Accumulator acc) { + this.propertyUsageMatcher = propertyUsageMatcher; + this.acc = acc; + } + + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + Xml.Tag t = super.visitTag(tag, ctx); + Optional value = t.getValue(); + if (value.isPresent()) { + Matcher matcher = propertyUsageMatcher.matcher(value.get()); + while (matcher.find()) { + acc.propertiesToUsingPoms.putIfAbsent(matcher.group(1), new HashSet<>()); + acc.propertiesToUsingPoms.get(matcher.group(1)).add(getResolutionResult()); + } + } + return t; + } + } + + private static class FindFilteredResourcePathsVisitor extends MavenIsoVisitor { + private final XPathMatcher resourceMatcher = new XPathMatcher("/project/build/resources/resource"); + private final Accumulator acc; + + public FindFilteredResourcePathsVisitor(Accumulator acc) { + this.acc = acc; + } + + @Override + public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { + if (resourceMatcher.matches(getCursor())) { + String directory = tag.getChildValue("directory").orElse(null); + if (tag.getChildValue("filtering").map(Boolean::valueOf).orElse(false) + && directory != null) { + Path path = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + try { + acc.filteredResourcePathsToDeclaringPoms.put(path.getParent().resolve(directory), getResolutionResult()); + } catch (InvalidPathException ignored) { + } // fail quietly + } + return tag; + } else { + return super.visitTag(tag, ctx); + } + } + } + + private static class FindResourceUsagesVisitor extends PlainTextVisitor { + private final Pattern dollarMatcher; + private final Pattern atMatcher; + private final Accumulator acc; + + public FindResourceUsagesVisitor(String pattern, Accumulator acc) { + this.dollarMatcher = dollarPropertyMatcher(pattern); + this.atMatcher = atPropertyMatcher(pattern); + this.acc = acc; + } + + @Override + public PlainText visitText(PlainText text, ExecutionContext ctx) { + Matcher matcher = dollarMatcher.matcher(text.getText()); + while (matcher.find()) { + acc.nonPomPathsToUsages.putIfAbsent(text.getSourcePath(), new HashSet<>()); + acc.nonPomPathsToUsages.get(text.getSourcePath()).add(matcher.group(1)); + } + matcher = atMatcher.matcher(text.getText()); + while (matcher.find()) { + acc.nonPomPathsToUsages.putIfAbsent(text.getSourcePath(), new HashSet<>()); + acc.nonPomPathsToUsages.get(text.getSourcePath()).add(matcher.group(1)); + } + return text; + } + } +} diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 2fa42d3482a..94e9523f0d5 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -27,7 +27,6 @@ import org.openrewrite.semver.VersionComparator; import org.openrewrite.xml.AddToTagVisitor; import org.openrewrite.xml.ChangeTagValueVisitor; -import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.nio.file.Path; @@ -202,7 +201,6 @@ private void storeParentPomProperty( @Override public TreeVisitor getVisitor(Accumulator accumulator) { return new MavenIsoVisitor() { - private final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project"); private final VersionComparator versionComparator = requireNonNull(Semver.validate(newVersion, versionPattern).getValue()); @@ -242,7 +240,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { return e.warn(t); } - if (t != tag && PROJECT_MATCHER.matches(getCursor())) { + if (t != tag && isProjectTag()) { maybeUpdateModel(); doAfterVisit(new RemoveRedundantDependencyVersions(groupId, artifactId, (RemoveRedundantDependencyVersions.Comparator) null, null).getVisitor()); } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveUnusedPropertiesTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveUnusedPropertiesTest.java new file mode 100644 index 00000000000..f65bbf7418d --- /dev/null +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveUnusedPropertiesTest.java @@ -0,0 +1,877 @@ +/* + * Copyright 2024 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.maven; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.mavenProject; +import static org.openrewrite.java.Assertions.srcMainResources; +import static org.openrewrite.maven.Assertions.pomXml; +import static org.openrewrite.test.SourceSpecs.text; +import static org.openrewrite.xml.Assertions.xml; + +class RemoveUnusedPropertiesTest implements RewriteTest { + @Override + public void defaults(final RecipeSpec spec) { + spec.recipe(new RemoveUnusedProperties(null)); + } + + @Test + void removesWholePropertiesSection() { + rewriteRun( + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + xyz + xyz + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + """ + ) + ); + } + + @Test + void ignoresMavenDefaultProperties() { + rewriteRun( + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + yyyy-MM-dd'T'HH:mm:ss'Z' + + + + """ + ) + ); + } + + @Test + void regexPropertyPattern() { + rewriteRun( + spec -> spec.recipe(new RemoveUnusedProperties(".+\\.version")), + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 1.2.3 + 1.2.3 + xyz + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + xyz + + + + """ + ) + ); + } + + @Test + void regexPropertyPatternNegativeLookahead() { + rewriteRun( + spec -> spec.recipe(new RemoveUnusedProperties("(?!java.|spring.).+")), + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 21 + 1.2.3 + 1.2.3 + xyz + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 21 + 1.2.3 + + + + """ + ) + ); + } + + @Test + void requiresCyclesForCascadingRemovals() { + rewriteRun( + spec -> spec.expectedCyclesThatMakeChanges(2), + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + xyz + ${foo} + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + """ + ) + ); + } + + @Test + void keepsUsedProperty() { + rewriteRun( + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 6.0.0 + xyz + + + + + org.springframework + spring-beans + ${spring.version} + + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 6.0.0 + + + + + org.springframework + spring-beans + ${spring.version} + + + + + """ + ) + ); + } + + @Test + void catchesMultiplePropertyUsagesInSameElement() { + rewriteRun( + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 6 + 0 + 10 + + + + + org.springframework + spring-beans + ${six}.${zero}.${zero} + + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 6 + 0 + + + + + org.springframework + spring-beans + ${six}.${zero}.${zero} + + + + + """ + ) + ); + } + + @Test + void keepsOverrideOfParentProperty() { + rewriteRun( + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + org.springframework.boot + spring-boot-starter-parent + 3.0.0 + + + + + 2.11.0 + + + + """ + ) + ); + } + + @Test + void keepsPropertyUsedByChild() { + rewriteRun( + mavenProject("parent", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + module1 + + + + 6.0.0 + + + + """ + ), + mavenProject("module1", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module1 + + + + org.springframework + spring-beans + ${spring.version} + + + + """ + )) + ) + ); + } + + @ParameterizedTest + @ValueSource(strings = { + "Hello ${a}", + "Hello @a@" + }) + void keepsPropertyUsedByFilteredResource(String text) { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + b + + + + + + src/main/resources + true + + + + + + """, + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + + + + + + src/main/resources + true + + + + + + """ + ), + srcMainResources( + text(text) + ) + ) + ); + } + + @Test + void removesPropertyUsedByNonFilteredResource_filteringFalse() { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + b + + + + + + src/main/resources + false + + + + + + """, """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + src/main/resources + false + + + + + + """ + ), + srcMainResources( + text("Hello ${a}") + ) + ) + ); + } + + @Test + void removesPropertyUsedByNonFilteredResource_noFiltering() { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + b + + + + """, """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + """ + ), + srcMainResources( + text("Hello ${a}") + ) + ) + ); + } + + @Test + void removesPropertyUsedByNonFilteredResource_wrongDir() { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + b + + + + + + src/main/resources-filtered + false + + + + + + """, """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + + + src/main/resources-filtered + false + + + + + + """ + ), + srcMainResources( + text("Hello ${a}") + ) + ) + ); + } + + @Test + void keepsMultiplePropertiesUsedBySameFilteredResource() { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + b + c + + + + + + src/main/resources + true + + + + + + """, """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + a + c + + + + + + src/main/resources + true + + + + + + """ + ), + srcMainResources( + text("Hello ${a} ${c}") + ) + ) + ); + } + + @Test + void keepsPropertyUsedByResourceFilter_Xml() { + rewriteRun( + mavenProject("my-project", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + 6.0.0 + + + + + + src/main/resources + true + + + + + + """ + ), + srcMainResources( + xml(""" + + + ${spring.version} + + """) + ) + ) + ); + } + + @Test + void removesIrrelevantPropertyDeclaration() { + rewriteRun( + mavenProject("parent", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + module1 + module2 + + + + """ + ), + mavenProject("module1", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module1 + + + 6.0.0 + + + + + org.springframework + spring-beans + ${spring.version} + + + + """ + )), + mavenProject("module2", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + + 6.0.0 + + + """, """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + """ + ) + ) + ) + ); + } + + @Test + void removesIrrelevantPropertyDeclarationForFilteredResourceUsage() { + rewriteRun( + mavenProject("parent", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + module1 + module2 + + + + """ + ), + mavenProject("module1", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module1 + + + a + + + + + + src/main/resources + true + + + + + + """ + ), + srcMainResources( + text("Hello ${a}") + ) + ), + mavenProject("module2", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + + a + + + """, """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + """ + ) + ) + ) + ); + } +} diff --git a/rewrite-test/src/test/java/org/openrewrite/FindSourceFilesTest.java b/rewrite-test/src/test/java/org/openrewrite/FindSourceFilesTest.java index d321d6e36fe..ecb8a32977b 100644 --- a/rewrite-test/src/test/java/org/openrewrite/FindSourceFilesTest.java +++ b/rewrite-test/src/test/java/org/openrewrite/FindSourceFilesTest.java @@ -170,4 +170,13 @@ void eitherOr() { ) ); } + + @Test + void negation() { + rewriteRun( + spec -> spec.recipe(new FindSourceFiles("!(**/not-this.txt)")), + text("not-this", spec -> spec.path("not-this.txt")), + text("this", "~~>this", spec -> spec.path("this.txt")) + ); + } } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringTypeReference.java b/rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringReference.java similarity index 54% rename from rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringTypeReference.java rename to rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringReference.java index edc9bdc6e83..6ef6cf9e93d 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringTypeReference.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/trait/SpringReference.java @@ -17,29 +17,34 @@ import lombok.Value; import org.jspecify.annotations.Nullable; -import org.openrewrite.Cursor; -import org.openrewrite.SourceFile; -import org.openrewrite.Tree; +import org.openrewrite.*; import org.openrewrite.trait.SimpleTraitMatcher; -import org.openrewrite.trait.TypeReference; +import org.openrewrite.trait.Reference; import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.tree.Xml; import java.util.HashSet; +import java.util.Optional; import java.util.Set; import java.util.regex.Pattern; @Value -class SpringTypeReference implements TypeReference { +class SpringReference implements Reference { Cursor cursor; + Kind kind; @Override public Tree getTree() { - return TypeReference.super.getTree(); + return Reference.super.getTree(); } @Override - public String getName() { + public Kind getKind() { + return kind; + } + + @Override + public String getValue() { if (getTree() instanceof Xml.Attribute) { Xml.Attribute attribute = (Xml.Attribute) getTree(); return attribute.getValueAsString(); @@ -52,54 +57,70 @@ public String getName() { throw new IllegalArgumentException("getTree() must be an Xml.Attribute or Xml.Tag: " + getTree().getClass()); } - static class Matcher extends SimpleTraitMatcher { - private final Pattern typeReference = Pattern.compile("(?:[a-zA-Z_][a-zA-Z0-9_]*\\.)+[A-Z*][a-zA-Z0-9_]*(?:<[a-zA-Z0-9_,?<> ]*>)?"); + @Override + public boolean supportsRename() { + return true; + } + + static class Matcher extends SimpleTraitMatcher { + private final Pattern referencePattern = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(?:\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*"); private final XPathMatcher classXPath = new XPathMatcher("//@class"); private final XPathMatcher typeXPath = new XPathMatcher("//@type"); + private final XPathMatcher keyTypeXPath = new XPathMatcher("//@key-type"); + private final XPathMatcher valueTypeXPath = new XPathMatcher("//@value-type"); private final XPathMatcher tags = new XPathMatcher("//value"); @Override - protected @Nullable SpringTypeReference test(Cursor cursor) { + protected @Nullable SpringReference test(Cursor cursor) { Object value = cursor.getValue(); if (value instanceof Xml.Attribute) { Xml.Attribute attrib = (Xml.Attribute) value; - if (classXPath.matches(cursor) || typeXPath.matches(cursor)) { - if (typeReference.matcher(attrib.getValueAsString()).matches()) { - return new SpringTypeReference(cursor); + if (classXPath.matches(cursor) || typeXPath.matches(cursor) || keyTypeXPath.matches(cursor) || valueTypeXPath.matches(cursor)) { + String stringVal = attrib.getValueAsString(); + if (referencePattern.matcher(stringVal).matches()) { + return new SpringReference(cursor, determineKind(stringVal)); } } } else if (value instanceof Xml.Tag) { Xml.Tag tag = (Xml.Tag) value; if (tags.matches(cursor)) { - if (tag.getValue().isPresent() && typeReference.matcher(tag.getValue().get()).matches()) { - return new SpringTypeReference(cursor); + Optional stringVal = tag.getValue(); + if (stringVal.isPresent() && referencePattern.matcher(stringVal.get()).matches()) { + return new SpringReference(cursor, determineKind(stringVal.get())); } } } return null; } + + Kind determineKind(String value) { + return Character.isUpperCase(value.charAt(value.lastIndexOf('.') + 1)) ? Kind.TYPE : Kind.PACKAGE; + } } @SuppressWarnings("unused") - public static class Provider implements TypeReference.Provider { + public static class Provider implements Reference.Provider { @Override - public Set getTypeReferences(SourceFile sourceFile) { - Set typeReferences = new HashSet<>(); + public Set getReferences(SourceFile sourceFile) { + Set references = new HashSet<>(); new Matcher().asVisitor(reference -> { - typeReferences.add(reference); + references.add(reference); return reference.getTree(); }).visit(sourceFile, 0); - return typeReferences; + return references; } @Override public boolean isAcceptable(SourceFile sourceFile) { if (sourceFile instanceof Xml.Document) { Xml.Document doc = (Xml.Document) sourceFile; - for (Xml.Attribute attrib : doc.getRoot().getAttributes()) { - if (attrib.getKeyAsString().equals("xsi:schemaLocation") && attrib.getValueAsString().contains("www.springframework.org/schema/beans")) { - return true; + //noinspection ConstantValue + if (doc.getRoot() != null) { + for (Xml.Attribute attrib : doc.getRoot().getAttributes()) { + if (attrib.getKeyAsString().equals("xsi:schemaLocation") && attrib.getValueAsString().contains("www.springframework.org/schema/beans")) { + return true; + } } } } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/tree/Xml.java b/rewrite-xml/src/main/java/org/openrewrite/xml/tree/Xml.java index 4e0eb50db4d..1fabf24388c 100755 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/tree/Xml.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/tree/Xml.java @@ -83,7 +83,7 @@ default

boolean isAcceptable(TreeVisitor v, P p) { @EqualsAndHashCode(callSuper = false, onlyExplicitlyIncluded = true) @RequiredArgsConstructor @AllArgsConstructor(access = AccessLevel.PRIVATE) - class Document implements Xml, SourceFileWithTypeReferences { + class Document implements Xml, SourceFileWithReferences { @With @EqualsAndHashCode.Include UUID id; @@ -164,25 +164,25 @@ public T service(Class service) { if (WhitespaceValidationService.class.getName().equals(service.getName())) { return (T) new XmlWhitespaceValidationService(); } - return SourceFileWithTypeReferences.super.service(service); + return SourceFileWithReferences.super.service(service); } @Nullable @NonFinal - transient SoftReference typeReferences; + transient SoftReference references; @Transient @Override - public TypeReferences getTypeReferences() { - TypeReferences cache; - if (this.typeReferences == null) { - cache = TypeReferences.build(this); - this.typeReferences = new SoftReference<>(cache); + public References getReferences() { + References cache; + if (this.references == null) { + cache = References.build(this); + this.references = new SoftReference<>(cache); } else { - cache = this.typeReferences.get(); + cache = this.references.get(); if (cache == null || cache.getSourceFile() != this) { - cache = TypeReferences.build(this); - this.typeReferences = new SoftReference<>(cache); + cache = References.build(this); + this.references = new SoftReference<>(cache); } } return cache; diff --git a/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider b/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider new file mode 100644 index 00000000000..72d76d75ee4 --- /dev/null +++ b/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider @@ -0,0 +1 @@ +org.openrewrite.xml.trait.SpringReference$Provider \ No newline at end of file diff --git a/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.TypeReference$Provider b/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.TypeReference$Provider deleted file mode 100644 index 6f58258f8bc..00000000000 --- a/rewrite-xml/src/main/resources/META-INF/services/org.openrewrite.trait.TypeReference$Provider +++ /dev/null @@ -1 +0,0 @@ -org.openrewrite.xml.trait.SpringTypeReference$Provider \ No newline at end of file diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/XmlParserTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/XmlParserTest.java index 7eda3abcda9..cff46087276 100755 --- a/rewrite-xml/src/test/java/org/openrewrite/xml/XmlParserTest.java +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/XmlParserTest.java @@ -24,6 +24,7 @@ import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.trait.Reference; import org.openrewrite.xml.tree.Xml; import java.nio.file.Paths; @@ -120,7 +121,7 @@ void parseXmlDocument() { } @Test - void javaTypeReferenceDocument() { + void javaReferenceDocument() { rewriteRun( xml( """ @@ -135,13 +136,18 @@ void javaTypeReferenceDocument() { java.lang.String + + java.lang + """, spec -> spec.afterRecipe(doc -> { - assertThat(doc.getTypeReferences().getTypeReferences().stream().anyMatch(typeRef -> typeRef.getName().equals("java.lang.String"))); + assertThat(doc.getReferences().getReferences().stream().anyMatch(typeRef -> typeRef.getValue().equals("java.lang.String"))).isTrue(); + assertThat(doc.getReferences().getReferences().stream().anyMatch(typeRef -> typeRef.getKind().equals(Reference.Kind.TYPE))).isTrue(); + assertThat(doc.getReferences().getReferences().stream().anyMatch(typeRef -> typeRef.getKind().equals(Reference.Kind.PACKAGE))).isTrue(); }) ) ); diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringTypeReferenceTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringReferenceTest.java similarity index 78% rename from rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringTypeReferenceTest.java rename to rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringReferenceTest.java index 5259faf3cf5..d60f5a2e8a9 100644 --- a/rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringTypeReferenceTest.java +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/trait/SpringReferenceTest.java @@ -23,12 +23,12 @@ import static org.openrewrite.xml.Assertions.xml; -class SpringTypeReferenceTest implements RewriteTest { +class SpringReferenceTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(RewriteTest.toRecipe(() -> new SpringTypeReference.Matcher() - .asVisitor(springJavaTypeReference -> SearchResult.found(springJavaTypeReference.getTree(), springJavaTypeReference.getName())))); + spec.recipe(RewriteTest.toRecipe(() -> new SpringReference.Matcher() + .asVisitor(springJavaTypeReference -> SearchResult.found(springJavaTypeReference.getTree(), springJavaTypeReference.getValue())))); } @SuppressWarnings("SpringXmlModelInspection") @@ -50,6 +50,12 @@ void xmlConfiguration() { java.lang.String + + java.lang + + + + @@ -68,6 +74,12 @@ void xmlConfiguration() { java.lang.String + + java.lang + + + -->key-type="java.lang.String" value-type="java.lang.String"/> + diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/JsonPathMatcher.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/JsonPathMatcher.java index 128e2cce40d..260c9f24974 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/JsonPathMatcher.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/JsonPathMatcher.java @@ -20,6 +20,7 @@ import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.tree.ParseTree; +import org.antlr.v4.runtime.tree.RuleNode; import org.antlr.v4.runtime.tree.TerminalNode; import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; @@ -33,7 +34,6 @@ import java.util.*; import java.util.function.BiPredicate; import java.util.regex.Pattern; -import java.util.stream.Collectors; import static java.util.Collections.disjoint; @@ -48,33 +48,31 @@ public class JsonPathMatcher { private final String jsonPath; + private JsonPathParser.@Nullable JsonPathContext parsed; public JsonPathMatcher(String jsonPath) { this.jsonPath = jsonPath; } public Optional find(Cursor cursor) { - LinkedList cursorPath = cursor.getPathAsStream() - .filter(o -> o instanceof Tree) - .map(Tree.class::cast) - .map(t -> new ReplaceAliasWithAnchorValueVisitor() - .visitNonNull(t, 0)) - .collect(Collectors.toCollection(LinkedList::new)); + return find0(cursor, resolvedAncestors(cursor)); + } + + private Optional find0(Cursor cursor, List cursorPath) { if (cursorPath.isEmpty()) { return Optional.empty(); } - Collections.reverse(cursorPath); Tree start; if (jsonPath.startsWith(".") && !jsonPath.startsWith("..")) { start = cursor.getValue(); } else { - start = cursorPath.peekFirst(); + start = cursorPath.get(0); } - JsonPathParser.JsonPathContext ctx = jsonPath().jsonPath(); + JsonPathParser.JsonPathContext ctx = parse(); // The stop may be optimized by interpreting the ExpressionContext and pre-determining the last visit. JsonPathParser.ExpressionContext stop = (JsonPathParser.ExpressionContext) ctx.children.get(ctx.children.size() - 1); - @SuppressWarnings("ConstantConditions") JsonPathParserVisitor v = new JsonPathMatcher.JsonPathYamlVisitor(cursorPath, start, stop, false); + @SuppressWarnings("ConstantConditions") JsonPathParserVisitor v = new JsonPathYamlVisitor(cursorPath, start, stop, false); Object result = v.visit(ctx); //noinspection unchecked @@ -82,16 +80,9 @@ public Optional find(Cursor cursor) { } public boolean matches(Cursor cursor) { - Object cursorValue = new ReplaceAliasWithAnchorValueVisitor().visit((Tree)cursor.getValue(), 0); - List cursorPath = cursor.getPathAsStream() - .map(cp -> { - if (cp instanceof Yaml) { - cp = new ReplaceAliasWithAnchorValueVisitor().visit((Yaml) cp, 0); - } - return cp; - }) - .collect(Collectors.toList()); - return find(cursor).map(o -> { + List cursorPath = resolvedAncestors(cursor); + Object cursorValue = cursorPath.get(cursorPath.size() - 1); + return find0(cursor, cursorPath).map(o -> { if (o instanceof List) { //noinspection unchecked List l = (List) o; @@ -102,6 +93,50 @@ public boolean matches(Cursor cursor) { }).orElse(false); } + private static List resolvedAncestors(Cursor cursor) { + ArrayDeque deque = new ArrayDeque<>(); + Map resolved = new IdentityHashMap<>(); + for (Iterator it = cursor.getPath(Tree.class::isInstance); it.hasNext(); ) { + Tree tree = (Tree) it.next(); + if (tree instanceof Yaml.Document) { + tree = new ReplaceAliasWithAnchorValueVisitor() { + @Override + public @Nullable Yaml visit(@Nullable Tree tree, Integer p) { + // NOTE: not calling `super.visit()` for performance reasons + if (tree instanceof Yaml) { + Yaml updated = ((Yaml) tree).acceptYaml(this, p); + if (updated != tree) { + resolved.put(tree, updated); + } + return updated; + } + return (Yaml) tree; + } + }.visitNonNull(tree, 0); + deque.addFirst(tree); + break; + } + deque.addFirst(tree); + } + ArrayList list = new ArrayList<>(deque); + if (!resolved.isEmpty()) { + for (int i = 0; i < list.size(); i++) { + Tree tree = list.get(i); + if (resolved.containsKey(tree)) { + list.set(i, resolved.get(tree)); + } + } + } + return list; + } + + private JsonPathParser.JsonPathContext parse() { + if (parsed == null) { + parsed = jsonPath().jsonPath(); + } + return parsed; + } + private JsonPathParser jsonPath() { return new JsonPathParser(new CommonTokenStream(new JsonPathLexer(CharStreams.fromString(this.jsonPath)))); } @@ -131,17 +166,27 @@ protected Object aggregateResult(Object aggregate, Object nextResult) { return (scope = nextResult); } + @Override + protected boolean shouldVisitNextChild(RuleNode node, Object currentResult) { + return scope != null; + } + @Override public Object visitJsonPath(JsonPathParser.JsonPathContext ctx) { - if (ctx.ROOT() != null || "[".equals(ctx.start.getText())) { - scope = cursorPath.stream() - .filter(t -> t instanceof Yaml.Mapping) - .findFirst() - .orElseGet(() -> cursorPath.stream() - .filter(t -> t instanceof Yaml.Document && ((Yaml.Document) t).getBlock() instanceof Yaml.Mapping) - .map(t -> ((Yaml.Document) t).getBlock()) - .findFirst() - .orElse(null)); + MATCH: + if (ctx.ROOT() != null || ctx.start.getType() == JsonPathLexer.LBRACK) { + Tree first = cursorPath.get(0); + if (first instanceof Yaml.Document) { + scope = ((Yaml.Document) first).getBlock(); + break MATCH; + } + for (Tree tree : cursorPath) { + if (tree instanceof Yaml.Mapping) { + scope = tree; + break MATCH; + } + } + scope = null; } return super.visitJsonPath(ctx); } @@ -167,7 +212,7 @@ private JsonPathParser.ExpressionContext getExpressionContext(ParserRuleContext if (previous.indexOf(current) - 1 < 0 || "$".equals(previous.get(previous.indexOf(current) - 1).getText())) { List results = new ArrayList<>(); for (Tree path : cursorPath) { - JsonPathMatcher.JsonPathYamlVisitor v = new JsonPathMatcher.JsonPathYamlVisitor(cursorPath, path, null, false); + JsonPathYamlVisitor v = new JsonPathYamlVisitor(cursorPath, path, null, false); for (int i = 1; i < ctx.getChildCount(); i++) { result = v.visit(ctx.getChild(i)); if (result != null) { @@ -178,7 +223,7 @@ private JsonPathParser.ExpressionContext getExpressionContext(ParserRuleContext return results; // Otherwise, the recursive descent is scoped to the previous match. `$.foo..['find-in-foo']`. } else { - JsonPathMatcher.JsonPathYamlVisitor v = new JsonPathMatcher.JsonPathYamlVisitor(cursorPath, scope, null, true); + JsonPathYamlVisitor v = new JsonPathYamlVisitor(cursorPath, scope, null, true); for (int i = 1; i < ctx.getChildCount(); i++) { result = v.visit(ctx.getChild(i)); if (result != null) { @@ -197,9 +242,11 @@ private JsonPathParser.ExpressionContext getExpressionContext(ParserRuleContext } // Return a list if more than 1 property is specified. - return ctx.property().stream() - .map(this::visitProperty) - .collect(Collectors.toList()); + List list = new ArrayList<>(); + for (JsonPathParser.PropertyContext propertyContext : ctx.property()) { + list.add(visitProperty(propertyContext)); + } + return list; } else if (ctx.slice() != null) { return visitSlice(ctx.slice()); } else if (ctx.indexes() != null) { @@ -229,7 +276,7 @@ public Object visitSlice(JsonPathParser.SliceContext ctx) { // A wildcard will use these initial values, so it is not checked in the conditions. int start = 0; - int limit = Integer.MAX_VALUE; + int limit = results.size(); if (ctx.PositiveNumber() != null) { // [:n], Selects the first n elements of the array. @@ -245,10 +292,7 @@ public Object visitSlice(JsonPathParser.SliceContext ctx) { limit = ctx.end() != null ? Integer.parseInt(ctx.end().getText()) + 1 : limit; } - return results.stream() - .skip(start) - .limit(limit) - .collect(Collectors.toList()); + return results.subList(start, Math.min(start + limit, results.size())); } @Override @@ -325,36 +369,31 @@ public Object visitIndexes(JsonPathParser.IndexesContext ctx) { scope = member.getValue(); return visitProperty(ctx); } else if (scope instanceof Yaml.Sequence) { - Object matches = ((Yaml.Sequence) scope).getEntries().stream() - .map(o -> { - scope = o; - return visitProperty(ctx); - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + List matches = new ArrayList<>(); + for (Yaml.Sequence.Entry entry : ((Yaml.Sequence) scope).getEntries()) { + scope = entry; + Object result = visitProperty(ctx); + if (result != null) { + matches.add(result); + } + } return getResultFromList(matches); } else if (scope instanceof Yaml.Sequence.Entry) { Yaml.Sequence.Entry entry = (Yaml.Sequence.Entry) scope; scope = entry.getBlock(); return visitProperty(ctx); } else if (scope instanceof List) { - List results = ((List) scope).stream() - .map(o -> { - scope = o; - return visitProperty(ctx); - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - // Unwrap lists of results from visitProperty to match the position of the cursor. List matches = new ArrayList<>(); - for (Object result : results) { + for (Object object : ((List) scope)) { + scope = object; + Object result = visitProperty(ctx); if (result instanceof List) { + // Unwrap lists of results from visitProperty to match the position of the cursor. matches.addAll(((List) result)); - } else { + } else if (result != null) { matches.add(result); } } - return getResultFromList(matches); } @@ -370,25 +409,27 @@ public Object visitIndexes(JsonPathParser.IndexesContext ctx) { Yaml.Mapping.Entry member = (Yaml.Mapping.Entry) scope; return member.getValue(); } else if (scope instanceof Yaml.Sequence) { - Object matches = ((Yaml.Sequence) scope).getEntries().stream() - .map(o -> { - scope = o; - return visitWildcard(ctx); - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + List matches = new ArrayList<>(); + for (Yaml.Sequence.Entry entry : ((Yaml.Sequence) scope).getEntries()) { + scope = entry; + Object result = visitWildcard(ctx); + if (result != null) { + matches.add(result); + } + } return getResultFromList(matches); } else if (scope instanceof Yaml.Sequence.Entry) { Yaml.Sequence.Entry entry = (Yaml.Sequence.Entry) scope; return entry.getBlock(); } else if (scope instanceof List) { - List results = ((List) scope).stream() - .map(o -> { - scope = o; - return visitWildcard(ctx); - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + List results = new ArrayList<>(); + for (Object object : ((List) scope)) { + scope = object; + Object result = visitWildcard(ctx); + if (result != null) { + results.add(result); + } + } List matches = new ArrayList<>(); if (stop != null && stop == getExpressionContext(ctx)) { @@ -399,7 +440,7 @@ public Object visitIndexes(JsonPathParser.IndexesContext ctx) { for (Object result : results) { if (result instanceof List) { matches.addAll(((List) result)); - } else { + } else if (result != null) { matches.add(result); } } @@ -459,20 +500,14 @@ public Object visitLiteralExpression(JsonPathParser.LiteralExpressionContext ctx return getResultFromList(entry.getBlock()); } } else if (scope instanceof List) { - List results = ((List) scope).stream() - .map(o -> { - scope = o; - return visitUnaryExpression(ctx); - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - - // Unwrap lists of results from visitUnaryExpression to match the position of the cursor. List matches = new ArrayList<>(); - for (Object result : results) { + for (Object object : ((List) scope)) { + scope = object; + Object result = visitUnaryExpression(ctx); if (result instanceof List) { + // Unwrap lists of results from visitUnaryExpression to match the position of the cursor. matches.addAll(((List) result)); - } else { + } else if (result != null) { matches.add(result); } } @@ -526,11 +561,13 @@ public Object visitLiteralExpression(JsonPathParser.LiteralExpressionContext ctx Yaml.Mapping.Entry entry = (Yaml.Mapping.Entry) lhs; if (entry.getValue() instanceof Yaml.Sequence) { Yaml.Sequence sequence = (Yaml.Sequence) entry.getValue(); - if (sequence.getEntries().stream() - .filter(o -> o.getBlock() instanceof Yaml.Scalar) - .map(o -> (Yaml.Scalar) o.getBlock()) - .anyMatch(o -> o.getValue().contains(String.valueOf(rhs)))) { - return originalScope; + for (Yaml.Sequence.Entry o : sequence.getEntries()) { + if (o.getBlock() instanceof Yaml.Scalar) { + Yaml.Scalar block = (Yaml.Scalar) o.getBlock(); + if (block.getValue().contains(String.valueOf(rhs))) { + return originalScope; + } + } } } else if (entry.getValue() instanceof Yaml.Scalar) { Yaml.Scalar scalar = (Yaml.Scalar) entry.getValue(); @@ -585,8 +622,8 @@ public Object visitLiteralExpression(JsonPathParser.LiteralExpressionContext ctx scope = scopeOfLogicalOp; rhs = getBinaryExpressionResult(rhs); if ("&&".equals(operator) && - ((lhs != null && (!(lhs instanceof List) || !((List) lhs).isEmpty())) && - (rhs != null && (!(rhs instanceof List) || !((List) rhs).isEmpty())))) { + ((lhs != null && (!(lhs instanceof List) || !((List) lhs).isEmpty())) && + (rhs != null && (!(rhs instanceof List) || !((List) rhs).isEmpty())))) { // Return the result of the evaluated expression. if (lhs instanceof Yaml) { return rhs; @@ -600,8 +637,8 @@ public Object visitLiteralExpression(JsonPathParser.LiteralExpressionContext ctx } return scopeOfLogicalOp; } else if ("||".equals(operator) && - ((lhs != null && (!(lhs instanceof List) || !((List) lhs).isEmpty())) || - (rhs != null && (!(rhs instanceof List) || !((List) rhs).isEmpty())))) { + ((lhs != null && (!(lhs instanceof List) || !((List) lhs).isEmpty())) || + (rhs != null && (!(rhs instanceof List) || !((List) rhs).isEmpty())))) { return scopeOfLogicalOp; } } else if (ctx.EQUALITY_OPERATOR() != null) { @@ -672,14 +709,14 @@ public Object visitLiteralExpression(JsonPathParser.LiteralExpressionContext ctx Yaml.Mapping mapping = (Yaml.Mapping) lhs; for (Yaml.Mapping.Entry entry : mapping.getEntries()) { if (entry.getValue() instanceof Yaml.Scalar && - checkObjectEquality(((Yaml.Scalar) entry.getValue()).getValue(), operator, rhs)) { + checkObjectEquality(((Yaml.Scalar) entry.getValue()).getValue(), operator, rhs)) { return mapping; } } } else if (lhs instanceof Yaml.Mapping.Entry) { Yaml.Mapping.Entry entry = (Yaml.Mapping.Entry) lhs; if (entry.getValue() instanceof Yaml.Scalar && - checkObjectEquality(((Yaml.Scalar) entry.getValue()).getValue(), operator, rhs)) { + checkObjectEquality(((Yaml.Scalar) entry.getValue()).getValue(), operator, rhs)) { return entry; } } else if (lhs instanceof Yaml.Scalar) { @@ -749,10 +786,14 @@ private Object getResultFromList(Object results) { } else if (result instanceof Yaml.Mapping) { return ((Yaml.Mapping) result).getEntries(); } else if (result instanceof List) { - return ((List) result).stream() - .map(this::getValue) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + List list = new ArrayList<>(); + for (Object o : ((List) result)) { + Object value = getValue(o); + if (value != null) { + list.add(value); + } + } + return list; } else if (result instanceof Yaml.Sequence) { return ((Yaml.Sequence) result).getEntries(); } else if (result instanceof Yaml.Scalar) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/search/FindProperty.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/search/FindProperty.java index af06020f761..9af08fcaeaf 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/search/FindProperty.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/search/FindProperty.java @@ -28,6 +28,7 @@ import java.util.HashSet; import java.util.Iterator; +import java.util.Objects; import java.util.Set; @Value @@ -46,6 +47,13 @@ public class FindProperty extends Recipe { @Nullable Boolean relaxedBinding; + @Option(displayName = "Property value", + example = "false", + description = "If provided, only properties specified in propertyKey having this value will be found. Works only for scalar values", + required = false) + @Nullable + String propertyValue; + @Override public String getDisplayName() { return "Find YAML properties"; @@ -66,7 +74,16 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC if (!Boolean.FALSE.equals(relaxedBinding) ? NameCaseConvention.matchesGlobRelaxedBinding(prop, propertyKey) : StringUtils.matchesGlob(prop, propertyKey)) { - e = e.withValue(SearchResult.found(e.getValue())); + if (!Objects.isNull(propertyValue)) { + if (entry.getValue() instanceof Yaml.Scalar) { + Yaml.Scalar scalar = (Yaml.Scalar) entry.getValue(); + if (scalar.getValue().equals(propertyValue)) { + e = e.withValue(SearchResult.found(e.getValue())); + } + } + } else { + e = e.withValue(SearchResult.found(e.getValue())); + } } return e; } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/JsonPathMatcherTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/JsonPathMatcherTest.java index 2a90331d9c8..74664a4317f 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/JsonPathMatcherTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/JsonPathMatcherTest.java @@ -112,6 +112,33 @@ void findsAlias() { ); } + @Test + void complex() { + assertMatched( + "..spec.containers[:1].resources.limits.cpu", + List.of( + //language=yaml + """ + apiVersion: apps/v1 + kind: Deployment + metadata: + labels: + app: application + spec: + template: + spec: + containers: + - image: nginx:latest + name: nginx + resources: + limits: + cpu: "64Mi" + """ + ), + List.of("cpu: \"64Mi\"") + ); + } + @Test void doesNotMatchMissingProperty() { assertNotMatched( @@ -456,6 +483,36 @@ void allElements() { ); } + @Test + void documentLevelSequenceWildcard() { + assertMatched( + "$[*].id", + List.of( + """ + - id: 0 + - id: 1 + - id: 2 + """ + ), + List.of("id: 0", "id: 1", "id: 2") + ); + } + + @Test + void documentLevelSequenceSingle() { + assertMatched( + "$[1].id", + List.of( + """ + - id: 0 + - id: 1 + - id: 2 + """ + ), + List.of("id: 1") + ); + } + @Test void bracketOperatorByNames() { assertMatched( @@ -777,8 +834,8 @@ private void assertMatched(String jsonPath, List before, List af var results = visit(before, jsonPath, printMatches); assertThat(results).hasSize(after.size()); for (int i = 0; i < results.size(); i++) { - assertThat(StringUtils.trimIndent(results.get(i)).replaceAll("\s+", " ")) - .isEqualTo(StringUtils.trimIndent(after.get(i)).replaceAll("\s+", " ")); + assertThat(StringUtils.trimIndent(results.get(i)).replaceAll("\\s+", " ")) + .isEqualTo(StringUtils.trimIndent(after.get(i)).replaceAll("\\s+", " ")); } } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/search/FindPropertyTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/search/FindPropertyTest.java index 83bc5c7afa0..8a89adf1ce1 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/search/FindPropertyTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/search/FindPropertyTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.yaml.search; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -30,7 +31,7 @@ class FindPropertyTest implements RewriteTest { @Test void findProperty() { rewriteRun( - spec -> spec.recipe(new FindProperty("management.metrics.binders.files.enabled", null)), + spec -> spec.recipe(new FindProperty("management.metrics.binders.files.enabled", null, null)), yaml( "management.metrics.binders.files.enabled: true", "management.metrics.binders.files.enabled: ~~>true" @@ -41,7 +42,7 @@ void findProperty() { @Test void findGlobProperty() { rewriteRun( - spec -> spec.recipe(new FindProperty("management.metrics.binders.*.enabled", null)), + spec -> spec.recipe(new FindProperty("management.metrics.binders.*.enabled", null, null)), yaml( "management.metrics.binders.files.enabled: true", "management.metrics.binders.files.enabled: ~~>true" @@ -49,6 +50,51 @@ void findGlobProperty() { ); } + @Test + void findPropertyWithSpecificValueMatch() { + rewriteRun( + spec -> spec.recipe(new FindProperty("my.cool.property", null, "my-matching-value")), + yaml( + "my.cool.property: my-matching-value", + "my.cool.property: ~~>my-matching-value" + ) + ); + } + + @Test + void findPropertyWithSpecificValueMatchSingleQuotes() { + rewriteRun( + spec -> spec.recipe(new FindProperty("my.cool.property", null, "my-matching-value")), + yaml( + "my.cool.property: 'my-matching-value'", + "my.cool.property: ~~>'my-matching-value'" + ) + ); + } + + @Test + void findPropertyWithSpecificValueMatchDoubleQuotes() { + rewriteRun( + spec -> spec.recipe(new FindProperty("my.cool.property", null, "my-matching-value")), + yaml( + "my.cool.property: \"my-matching-value\"", + "my.cool.property: ~~>\"my-matching-value\"" + ) + ); + } + + @Test + @Disabled("how do I test that the search has no hits?") + void findPropertyWithSpecificValueNoMatch() { + rewriteRun( + spec -> spec.recipe(new FindProperty("my.cool.property", null, "my-non-matching-value")), + yaml( + "my.cool.property: my-matching-value", + "my.cool.property: my-matching-value" + ) + ); + } + @ParameterizedTest @ValueSource(strings = { "acme.my-project.person.first-name", @@ -58,9 +104,9 @@ void findGlobProperty() { @Issue("https://github.com/openrewrite/rewrite/issues/1168") void relaxedBinding(String propertyKey) { rewriteRun( - spec -> spec.recipe(new FindProperty(propertyKey, true)), + spec -> spec.recipe(new FindProperty(propertyKey, true, null)), yaml( - """ + """ acme.my-project.person.first-name: example acme.myProject.person.firstName: example acme.my_project.person.first_name: example @@ -78,9 +124,9 @@ void relaxedBinding(String propertyKey) { @Issue("https://github.com/openrewrite/rewrite/issues/1168") void exactMatch() { rewriteRun( - spec -> spec.recipe(new FindProperty("acme.my-project.person.first-name", false)), + spec -> spec.recipe(new FindProperty("acme.my-project.person.first-name", false, null)), yaml( - """ + """ acme.my-project.person.first-name: example acme.myProject.person.firstName: example acme.my_project.person.first_name: example