diff --git a/rewrite-core/src/main/java/org/openrewrite/Recipe.java b/rewrite-core/src/main/java/org/openrewrite/Recipe.java index 849c6205308..6ed845bc49f 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Recipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/Recipe.java @@ -20,7 +20,6 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import lombok.Setter; import org.intellij.lang.annotations.Language; -import org.jetbrains.annotations.Nls; import org.openrewrite.config.DataTableDescriptor; import org.openrewrite.config.OptionDescriptor; import org.openrewrite.config.RecipeDescriptor; @@ -131,7 +130,7 @@ public String getInstanceName() { return getDisplayName() + " " + suffix; } - List options = new ArrayList<>(getOptionDescriptors(getClass())); + List options = new ArrayList<>(getOptionDescriptors()); options.removeIf(opt -> !opt.isRequired()); if (options.isEmpty()) { return getDisplayName(); @@ -209,7 +208,7 @@ public final RecipeDescriptor getDescriptor() { } protected RecipeDescriptor createRecipeDescriptor() { - List options = getOptionDescriptors(this.getClass()); + List options = getOptionDescriptors(); List recipeList1 = new ArrayList<>(); for (Recipe next : getRecipeList()) { recipeList1.add(next.getDescriptor()); @@ -226,14 +225,19 @@ protected RecipeDescriptor createRecipeDescriptor() { getMaintainers(), getContributors(), getExamples(), recipeSource); } - private List getOptionDescriptors(Class recipeClass) { + private List getOptionDescriptors() { + Recipe recipe = this; + if (this instanceof DelegatingRecipe) { + recipe = ((DelegatingRecipe) this).getDelegate(); + } + List options = new ArrayList<>(); - for (Field field : recipeClass.getDeclaredFields()) { + for (Field field : recipe.getClass().getDeclaredFields()) { Object value; try { field.setAccessible(true); - value = field.get(this); + value = field.get(recipe); } catch (IllegalAccessException e) { value = null; } @@ -416,4 +420,8 @@ public Object clone() { throw new RuntimeException(e); } } + + public interface DelegatingRecipe { + Recipe getDelegate(); + } } diff --git a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java index 2b5850d5069..e7c1e2b7a3c 100644 --- a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java @@ -168,7 +168,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { @EqualsAndHashCode(callSuper = false) @Value - static class BellwetherDecoratedRecipe extends Recipe { + static class BellwetherDecoratedRecipe extends Recipe implements DelegatingRecipe { DeclarativeRecipe.PreconditionBellwether bellwether; Recipe delegate; @@ -206,7 +206,7 @@ public boolean causesAnotherCycle() { @Value @EqualsAndHashCode(callSuper = false) - static class BellwetherDecoratedScanningRecipe extends ScanningRecipe { + static class BellwetherDecoratedScanningRecipe extends ScanningRecipe implements DelegatingRecipe { DeclarativeRecipe.PreconditionBellwether bellwether; ScanningRecipe delegate; diff --git a/rewrite-core/src/test/java/org/openrewrite/config/DeclarativeRecipeTest.java b/rewrite-core/src/test/java/org/openrewrite/config/DeclarativeRecipeTest.java index 227eb73eef4..016ac24bb02 100644 --- a/rewrite-core/src/test/java/org/openrewrite/config/DeclarativeRecipeTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/config/DeclarativeRecipeTest.java @@ -71,6 +71,36 @@ public PlainText visitText(PlainText text, ExecutionContext ctx) { ); } + @Test + void addingPreconditionsWithOptions() { + DeclarativeRecipe dr = new DeclarativeRecipe("test", "test", "test", null, + null, null, true, null); + dr.addPrecondition( + toRecipe(() -> new PlainTextVisitor<>() { + @Override + public PlainText visitText(PlainText text, ExecutionContext ctx) { + if ("1".equals(text.getText())) { + return SearchResult.found(text); + } + return text; + } + }) + ); + dr.addUninitialized( + new ChangeText("2") + ); + dr.addUninitialized( + new ChangeText("3") + ); + dr.initialize(List.of(), Map.of()); + assertThat(dr.getDescriptor().getRecipeList()) + .hasSize(3) // precondition + 2 recipes with options + .flatExtracting(RecipeDescriptor::getOptions) + .hasSize(2) + .extracting(OptionDescriptor::getName) + .containsOnly("toText"); + } + @Test void uninitializedFailsValidation() { DeclarativeRecipe dr = new DeclarativeRecipe("test", "test", "test", null, diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyConstraintToRule.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyConstraintToRule.java new file mode 100644 index 00000000000..1754fde5f7c --- /dev/null +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyConstraintToRule.java @@ -0,0 +1,398 @@ +/* + * 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.gradle; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.intellij.lang.annotations.Language; +import org.openrewrite.*; +import org.openrewrite.groovy.GroovyIsoVisitor; +import org.openrewrite.groovy.GroovyParser; +import org.openrewrite.groovy.tree.G; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Markers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; + +@Value +@EqualsAndHashCode(callSuper = false) +public class DependencyConstraintToRule extends Recipe { + + private static final MethodMatcher DEPENDENCIES_DSL_MATCHER = new MethodMatcher("RewriteGradleProject dependencies(..)"); + private static final String CONSTRAINT_MATCHER = "org.gradle.api.artifacts.dsl.DependencyHandler *(..)"; + + @Override + public String getDisplayName() { + return "Dependency constraint to resolution rule"; + } + + @Override + public String getDescription() { + return "Gradle [dependency constraints](https://docs.gradle.org/current/userguide/dependency_constraints.html#dependency-constraints) " + + "are useful for managing the versions of transitive dependencies. " + + "Some plugins, such as the Spring Dependency Management plugin, do not respect these constraints. " + + "This recipe converts constraints into [resolution rules](https://docs.gradle.org/current/userguide/resolution_rules.html), " + + "which can achieve similar effects to constraints but are harder for plugins to ignore."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { + @Override + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit compilationUnit, ExecutionContext ctx) { + List gavs = new ArrayList<>(); + Cursor parent = requireNonNull(getCursor().getParent()); + G.CompilationUnit cu = (G.CompilationUnit) new RemoveConstraints().visitNonNull(compilationUnit, gavs, parent); + if (gavs.isEmpty()) { + return compilationUnit; + } + cu = (G.CompilationUnit) new MaybeAddEachDependency().visitNonNull(cu, 0, parent); + cu = (G.CompilationUnit) new UpdateEachDependency().visitNonNull(cu, gavs, parent); + return cu; + } + }); + } + + @Value + static class GroupArtifactVersionBecause { + @Nullable + String groupId; + + String artifactId; + + @Nullable + String version; + + @Nullable + String because; + } + + static class RemoveConstraints extends GroovyIsoVisitor> { + @SuppressWarnings("DataFlowIssue") + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, List groupArtifactVersions) { + J.MethodInvocation m = super.visitMethodInvocation(method, groupArtifactVersions); + if ("constraints".equals(m.getSimpleName()) && isInDependenciesBlock(getCursor())) { + if (!(m.getArguments().get(0) instanceof J.Lambda)) { + return null; + } + J.Lambda closure = (J.Lambda) m.getArguments().get(0); + if (!(closure.getBody() instanceof J.Block)) { + return null; + } + List withoutConvertableConstraints = ListUtils.map(((J.Block) closure.getBody()).getStatements(), statement -> { + J.MethodInvocation constraint = null; + if (statement instanceof J.MethodInvocation) { + constraint = (J.MethodInvocation) statement; + } else if (statement instanceof J.Return) { + constraint = (J.MethodInvocation) ((J.Return) statement).getExpression(); + } + if (constraint == null) { + return statement; + } + if (!(constraint.getArguments().get(0) instanceof J.Literal)) { + return statement; + } + J.Literal rawGav = (J.Literal) constraint.getArguments().get(0); + String[] gav = rawGav.getValue().toString().split(":"); + if (gav.length != 3) { + return statement; + } + AtomicReference because = new AtomicReference<>(null); + new GroovyIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integer integer) { + J.MethodInvocation m1 = super.visitMethodInvocation(method, integer); + if("because".equals(m1.getSimpleName()) && m1.getArguments().get(0) instanceof J.Literal) { + because.set(((J.Literal) m1.getArguments().get(0)).getValue().toString()); + } + return m1; + } + }.visit(constraint.getArguments(), 0); + + groupArtifactVersions.add(new GroupArtifactVersionBecause(gav[0], gav[1], gav[2], because.get())); + return null; + }); + // If nothing remains in the constraints{} it can be removed entirely + if(withoutConvertableConstraints.isEmpty()) { + return null; + } else { + return m.withArguments(singletonList(closure.withBody(((J.Block) closure.getBody()).withStatements(withoutConvertableConstraints)))); + } + } + return m; + } + } + + static class UpdateEachDependency extends GroovyIsoVisitor> { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, List groupArtifactVersions) { + J.MethodInvocation m = super.visitMethodInvocation(method, groupArtifactVersions); + if (isEachDependency(m)) { + Cursor parent = requireNonNull(getCursor().getParent()); + for (GroupArtifactVersionBecause gav : groupArtifactVersions) { + m = (J.MethodInvocation) new MaybeAddIf().visitNonNull(m, gav, parent); + m = (J.MethodInvocation) new UpdateIf().visitNonNull(m, gav, parent); + } + } + return m; + } + } + + static class MaybeAddIf extends GroovyIsoVisitor { + boolean containsAnyIfStatement; + boolean containsMatchingIfStatement; + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, GroupArtifactVersionBecause groupArtifactVersion) { + J.MethodInvocation m = super.visitMethodInvocation(method, groupArtifactVersion); + if (containsMatchingIfStatement) { + return m; + } + if (!isEachDependency(m)) { + return m; + } + // Identify the parameter name used in the eachDependency closure + Expression maybeClosure = m.getArguments().get(0); + if (!(maybeClosure instanceof J.Lambda) || !(((J.Lambda) maybeClosure).getBody() instanceof J.Block)) { + return m; + } + J.Lambda closure = (J.Lambda) maybeClosure; + J.Block closureBody = (J.Block) closure.getBody(); + J rawParam = ((J.Lambda) maybeClosure).getParameters().getParameters().get(0); + if (!(rawParam instanceof J.VariableDeclarations)) { + return m; + } + String p = ((J.VariableDeclarations) rawParam).getVariables().get(0).getSimpleName(); + @SuppressWarnings("GroovyEmptyStatementBody") @Language("groovy") + String snippet = "Object " + p + " = null\n" + + "if (" + p + ".requested.group == '" + groupArtifactVersion.getGroupId() + "' && " + + p + ".requested.name == '" + groupArtifactVersion.getArtifactId() + "') {\n}"; + J.If newIf = GroovyParser.builder().build() + .parse(snippet) + .map(G.CompilationUnit.class::cast) + .map(cu -> cu.getStatements().get(1)) + .map(J.If.class::cast) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to produce a new if statement")); + if (containsAnyIfStatement) { + m = (J.MethodInvocation) new GroovyIsoVisitor() { + boolean inserted; + + @Override + public J.If visitIf(J.If iff, Integer integer) { + J.If anIf = super.visitIf(iff, integer); + J.If.Else currentElse = anIf.getElsePart(); + if (!inserted && (currentElse == null || currentElse.getBody() instanceof J.Block)) { + inserted = true; + J.If.Else newElsePart = new J.If.Else(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, + JRightPadded.build(newIf + .withPrefix(Space.SINGLE_SPACE) + .withElsePart(currentElse))); + anIf = autoFormat(anIf.withElsePart(newElsePart), 0, requireNonNull(getCursor().getParent())); + } + return anIf; + } + }.visitNonNull(m, 0, requireNonNull(getCursor().getParent())); + } else { + J.Block newBody = autoFormat(closureBody.withStatements(ListUtils.concat(newIf, closureBody.getStatements())), groupArtifactVersion, getCursor()); + m = m.withArguments(singletonList(closure.withBody(newBody))); + } + return m; + } + + + @Override + public J.If visitIf(J.If iff, GroupArtifactVersionBecause groupArtifactVersion) { + containsAnyIfStatement = true; + J.If f = super.visitIf(iff, groupArtifactVersion); + if (predicateRelatesToGav(f, groupArtifactVersion)) { + containsMatchingIfStatement = true; + } + return iff; + } + + @Override + public @Nullable J visit(@Nullable Tree tree, GroupArtifactVersionBecause groupArtifactVersion) { + // Avoid wasting time if we've already found it + if (containsMatchingIfStatement) { + return (J) tree; + } + return super.visit(tree, groupArtifactVersion); + } + } + + static class UpdateIf extends GroovyIsoVisitor { + @Override + public J.If visitIf(J.If iff, GroupArtifactVersionBecause groupArtifactVersionBecause) { + J.If anIf = super.visitIf(iff, groupArtifactVersionBecause); + if (predicateRelatesToGav(anIf, groupArtifactVersionBecause)) { + // The predicate of the if condition will already contain the relevant variable name + AtomicReference variableName = new AtomicReference<>(); + new GroovyIsoVisitor() { + @Override + public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer integer) { + // Comparison will involve ".requested.group" + J.FieldAccess field = super.visitFieldAccess(fieldAccess, integer); + if(field.getTarget() instanceof J.Identifier) { + variableName.set(((J.Identifier) field.getTarget()).getSimpleName()); + } + return fieldAccess; + } + }.visit(anIf.getIfCondition(), 0); + @Language("groovy") + String snippet = variableName + ".useVersion('" + groupArtifactVersionBecause.getVersion() + "')\n"; + if(groupArtifactVersionBecause.getBecause() != null) { + snippet += variableName + ".because('" + groupArtifactVersionBecause.getBecause() + "')\n"; + } + List newStatements = GroovyParser.builder() + .build() + .parse(snippet) + .map(G.CompilationUnit.class::cast) + .map(G.CompilationUnit::getStatements) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to produce a new block statement")); + J.Block block = (J.Block) anIf.getThenPart(); + block = block.withStatements(newStatements); + block = autoFormat(block, groupArtifactVersionBecause, getCursor()); + anIf = anIf.withThenPart(block); + } + return anIf; + } + } + + static class MaybeAddEachDependency extends GroovyIsoVisitor { + boolean alreadyExists; + + @Override + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit compilationUnit, Integer integer) { + G.CompilationUnit cu = super.visitCompilationUnit(compilationUnit, integer); + if (alreadyExists) { + return cu; + } + // Prefer to insert before the dependencies block for readability + int insertionIndex = 0; + while (insertionIndex < cu.getStatements().size()) { + Statement s = cu.getStatements().get(insertionIndex); + if (s instanceof J.MethodInvocation && DEPENDENCIES_DSL_MATCHER.matches((J.MethodInvocation) s)) { + break; + } + insertionIndex++; + } + J.MethodInvocation m = GradleParser.builder() + .build() + .parse("\n" + + "configurations.all {\n" + + " resolutionStrategy.eachDependency { details ->\n" + + " }\n" + + "}") + .map(G.CompilationUnit.class::cast) + .map(G.CompilationUnit::getStatements) + .map(it -> it.get(0)) + .map(J.MethodInvocation.class::cast) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to create a new configurations.all block")); + cu = cu.withStatements(ListUtils.insert(cu.getStatements(), m, insertionIndex)); + return cu; + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integer integer) { + J.MethodInvocation m = super.visitMethodInvocation(method, integer); + if (isEachDependency(m)) { + alreadyExists = true; + } + return m; + } + + @Override + public @Nullable J visit(@Nullable Tree tree, Integer integer) { + // Avoid wasting time if we've already found it + if (alreadyExists) { + return (J) tree; + } + return super.visit(tree, integer); + } + } + + private static boolean isInDependenciesBlock(Cursor cursor) { + Cursor c = cursor.dropParentUntil(value -> + value == Cursor.ROOT_VALUE + || (value instanceof J.MethodInvocation && DEPENDENCIES_DSL_MATCHER.matches((J.MethodInvocation) value))); + return c.getValue() instanceof J.MethodInvocation; + } + + private static boolean isEachDependency(J.MethodInvocation m) { + return "eachDependency".equals(m.getSimpleName()) + && (m.getSelect() instanceof J.Identifier + && "resolutionStrategy".equals(((J.Identifier) m.getSelect()).getSimpleName())); + } + + private static boolean predicateRelatesToGav(J.If iff, GroupArtifactVersionBecause groupArtifactVersion) { + Expression predicate = iff.getIfCondition().getTree(); + if (!(predicate instanceof J.Binary)) { + return false; + } + J.Binary and = (J.Binary) predicate; + // Looking for a comparison of group id && artifact id + if (and.getOperator() != J.Binary.Type.And) { + return false; + } + // GroupId and artifactId might be compared in either order or this could be an unrelated comparison + AtomicBoolean groupIdCompared = new AtomicBoolean(); + AtomicBoolean artifactIdCompared = new AtomicBoolean(); + new GroovyIsoVisitor() { + @Override + public J.Binary visitBinary(J.Binary binary, GroupArtifactVersionBecause groupArtifactVersion) { + J.Binary b = super.visitBinary(binary, groupArtifactVersion); + if (b.getOperator() != J.Binary.Type.Equal) { + return b; + } + J.FieldAccess access = null; + J.Literal literal = null; + if (b.getLeft() instanceof J.FieldAccess && b.getRight() instanceof J.Literal) { + access = (J.FieldAccess) b.getLeft(); + literal = (J.Literal) b.getRight(); + } else if (b.getRight() instanceof J.FieldAccess && b.getLeft() instanceof J.Literal) { + access = (J.FieldAccess) b.getRight(); + literal = (J.Literal) b.getLeft(); + } + //noinspection ConstantValue + if (access == null || literal == null) { + return b; + } + if ("group".equals(access.getSimpleName()) && Objects.equals(groupArtifactVersion.getGroupId(), literal.getValue())) { + groupIdCompared.set(true); + } else if ("name".equals(access.getSimpleName()) && Objects.equals(groupArtifactVersion.getArtifactId(), literal.getValue())) { + artifactIdCompared.set(true); + } + return b; + } + }.visit(and, groupArtifactVersion); + + return groupIdCompared.get() && artifactIdCompared.get(); + } +} 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 eaa8cddc26e..10118812886 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpdateGradleWrapper.java @@ -38,6 +38,7 @@ import org.openrewrite.semver.VersionComparator; import org.openrewrite.text.PlainText; +import java.net.URI; import java.time.ZonedDateTime; import java.util.*; @@ -94,13 +95,13 @@ public String getDescription() { final Boolean addIfMissing; @Getter - @Option(example = "https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip", displayName = "Wrapper URI", - description = "The URI of the Gradle wrapper distribution. " + - "Lookup of available versions still requires access to https://services.gradle.org " + - "When this is specified the exact literal values supplied for `version` and `distribution` " + - "will be interpolated into this string wherever `${version}` and `${distribution}` appear respectively. " + - "Defaults to https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip.", - required = false) + @Option(example = "https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip", displayName = "Wrapper URI", + description = "The URI of the Gradle wrapper distribution. " + + "Lookup of available versions still requires access to https://services.gradle.org " + + "When this is specified the exact literal values supplied for `version` and `distribution` " + + "will be interpolated into this string wherever `${version}` and `${distribution}` appear respectively. " + + "Defaults to https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip.", + required = false) @Nullable final String wrapperUri; @@ -118,7 +119,32 @@ public Validated validate() { private GradleWrapper getGradleWrapper(ExecutionContext ctx) { if (gradleWrapper == null) { - gradleWrapper = GradleWrapper.create(distribution, version, null, ctx); + try { + gradleWrapper = GradleWrapper.create(distribution, version, null, ctx); + } catch (Exception e) { + // services.gradle.org is unreachable, possibly because of a firewall + // But if the user specified a wrapperUri to an internal repository things might still be workable + if (wrapperUri == null) { + throw new IllegalArgumentException( + "Could not reach services.gradle.org and no alternative wrapper URI is provided. " + + "To use this recipe in environments where services.gradle.org is unavailable specify a wrapperUri.", e); + } + if (wrapperUri.contains("${version})")) { + if (version == null) { + throw new IllegalArgumentException( + "wrapperUri contains a ${version} interpolation specifier but no version parameter was specified.", e); + } + if(!version.matches("[0-9.]+")) { + throw new IllegalArgumentException( + "Version selectors like \"" + version + "\" are unavailable when services.gradle.org cannot be reached. " + + "Specify an exact, literal version number.", e); + } + } + String effectiveWrapperUri = wrapperUri + .replace("${version}", version == null ? "" : version) + .replace("${distribution}", distribution == null ? "bin" : distribution); + gradleWrapper = GradleWrapper.create(URI.create(effectiveWrapperUri), ctx); + } } return gradleWrapper; } @@ -436,12 +462,12 @@ public Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) { String currentUrl = value.getText(); // Prefer wrapperUri specified directly in the recipe over other options // If that isn't set, prefer the existing artifact repository URL over changing to services.gradle.org - if(wrapperUri != null) { + if (wrapperUri != null) { String effectiveWrapperUri = formatUriForPropertiesFile(wrapperUri .replace("${version}", gradleWrapper.getVersion()) .replace("${distribution}", distribution == null ? "bin" : distribution)); return entry.withValue(value.withText(effectiveWrapperUri)); - } else if(currentUrl.startsWith("https\\://services.gradle.org/distributions/")) { + } else if (currentUrl.startsWith("https\\://services.gradle.org/distributions/")) { return entry.withValue(value.withText(gradleWrapper.getPropertiesFormattedUrl())); } else { String gradleServicesDistributionUrl = gradleWrapper.getDistributionUrl(); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java index 91159ede86f..4403cb9ad0a 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java @@ -655,7 +655,7 @@ public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ct } } - static GradleProject replaceVersion(GradleProject gp, ExecutionContext ctx, GroupArtifactVersion gav, Set configurations) { + public static GradleProject replaceVersion(GradleProject gp, ExecutionContext ctx, GroupArtifactVersion gav, Set configurations) { try { if (gav.getGroupId() == null || gav.getArtifactId() == null) { return gp; @@ -671,26 +671,23 @@ static GradleProject replaceVersion(GradleProject gp, ExecutionContext ctx, Grou MavenPomDownloader mpd = new MavenPomDownloader(ctx); Pom pom = mpd.download(gav, null, null, gp.getMavenRepositories()); ResolvedPom resolvedPom = pom.resolve(emptyList(), mpd, gp.getMavenRepositories(), ctx); - ResolvedGroupArtifactVersion resolvedGav = resolvedPom.getGav(); List transitiveDependencies = resolvedPom.resolveDependencies(Scope.Runtime, mpd, ctx); + org.openrewrite.maven.tree.Dependency newRequested = org.openrewrite.maven.tree.Dependency.builder() + .gav(gav) + .build(); + ResolvedDependency newDep = ResolvedDependency.builder() + .gav(resolvedPom.getGav()) + .requested(newRequested) + .dependencies(transitiveDependencies) + .build(); + Map nameToConfiguration = gp.getNameToConfiguration(); Map newNameToConfiguration = new HashMap<>(nameToConfiguration.size()); boolean anyChanged = false; for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { - GradleDependencyConfiguration newGdc = gdc; - newGdc = newGdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { - if (!Objects.equals(requested.getGroupId(), gav.getGroupId()) || !Objects.equals(requested.getArtifactId(), gav.getArtifactId())) { - return requested; - } - return requested.withGav(gav); - })); - newGdc = newGdc.withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> { - if (!Objects.equals(resolved.getGroupId(), resolvedGav.getGroupId()) || !Objects.equals(resolved.getArtifactId(), resolvedGav.getArtifactId())) { - return resolved; - } - return resolved.withGav(resolvedGav) - .withDependencies(transitiveDependencies); - })); + GradleDependencyConfiguration newGdc = gdc + .withRequested(ListUtils.map(gdc.getRequested(), requested -> maybeUpdateDependency(requested, newRequested))) + .withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> maybeUpdateResolvedDependency(resolved, newDep))); anyChanged |= newGdc != gdc; newNameToConfiguration.put(newGdc.getName(), newGdc); } @@ -702,4 +699,22 @@ static GradleProject replaceVersion(GradleProject gp, ExecutionContext ctx, Grou } return gp; } + + private static org.openrewrite.maven.tree.Dependency maybeUpdateDependency( + org.openrewrite.maven.tree.Dependency dep, + org.openrewrite.maven.tree.Dependency newDep) { + if(Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId())) { + return newDep; + } + return dep; + } + + private static ResolvedDependency maybeUpdateResolvedDependency( + ResolvedDependency dep, + ResolvedDependency newDep) { + if(Objects.equals(dep.getGroupId(), newDep.getGroupId()) && Objects.equals(dep.getArtifactId(), newDep.getArtifactId())) { + return newDep; + } + return dep.withDependencies(ListUtils.map(dep.getDependencies(), d -> maybeUpdateResolvedDependency(d, newDep))); + } } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java index 847fd86f9a5..b28cab06b92 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java @@ -46,6 +46,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import static java.util.Collections.singletonMap; import static java.util.Objects.requireNonNull; @@ -139,7 +140,7 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { gradleProject = cu.getMarkers().findFirst(GradleProject.class) .orElseThrow(() -> new IllegalStateException("Unable to find GradleProject marker.")); - Map> toUpdate = new HashMap<>(); + Map> toUpdate = new LinkedHashMap<>(); DependencyVersionSelector versionSelector = new DependencyVersionSelector(metadataFailures, gradleProject, null); for (GradleDependencyConfiguration configuration : gradleProject.getConfigurations()) { @@ -162,7 +163,7 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { new GroupArtifact(resolved.getGroupId(), resolved.getArtifactId()), singletonMap(constraintConfig, selected), (existing, update) -> { - Map all = new HashMap<>(existing); + Map all = new LinkedHashMap<>(existing); all.putAll(update); all.keySet().removeIf(c -> { if (c == null) { @@ -210,11 +211,43 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { update.getKey().getArtifactId(), config.getValue()), because).visitNonNull(cu, ctx); } } + + // Update dependency model so chained recipes will have correct information on what dependencies are present + cu = cu.withMarkers(cu.getMarkers() + .removeByType(GradleProject.class) + .add(updatedModel(gradleProject, toUpdate, ctx))); + + // Spring dependency management plugin stomps on constraints. Use an alternative mechanism it does not override + if (gradleProject.getPlugins().stream() + .anyMatch(plugin -> "io.spring.dependency-management".equals(plugin.getId()))) { + cu = (G.CompilationUnit) new DependencyConstraintToRule().getVisitor().visitNonNull(cu, ctx); + } } return cu; } + private GradleProject updatedModel(GradleProject gradleProject, Map> toUpdate, ExecutionContext ctx) { + GradleProject gp = gradleProject; + Set configNames = gp.getConfigurations().stream() + .map(GradleDependencyConfiguration::getName) + .collect(Collectors.toSet()); + Set gavs = new LinkedHashSet<>(); + for (Map.Entry> update : toUpdate.entrySet()) { + Map configs = update.getValue(); + String groupId = update.getKey().getGroupId(); + String artifactId = update.getKey().getArtifactId(); + for (Map.Entry configToVersion : configs.entrySet()) { + String newVersion = configToVersion.getValue(); + gavs.add(new GroupArtifactVersion(groupId, artifactId, newVersion)); + } + } + for (GroupArtifactVersion gav : gavs) { + gp = UpgradeDependencyVersion.replaceVersion(gp, ctx, gav, configNames); + } + return gp; + } + /* * It is typical in Gradle for there to be a number of unresolvable configurations that developers put * dependencies into in their build files, like implementation. Other configurations like compileClasspath diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/marker/GradleDependencyConfiguration.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/marker/GradleDependencyConfiguration.java index 7afae579465..c1f0886dae3 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/marker/GradleDependencyConfiguration.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/marker/GradleDependencyConfiguration.java @@ -70,7 +70,7 @@ public List getDirectResolved() { */ public List getResolved() { List resolved = new ArrayList<>(getDirectResolved()); - Set alreadyResolved = new HashSet<>(); + Set alreadyResolved = new LinkedHashSet<>(); return resolveTransitiveDependencies(resolved, alreadyResolved); } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/GradleWrapper.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/GradleWrapper.java index 0d9a7157d6c..9efe9ed707c 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/GradleWrapper.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/GradleWrapper.java @@ -94,7 +94,11 @@ public static GradleWrapper create(@Nullable String distributionTypeName, @Nulla // Supports org.openrewrite.gradle.toolingapi.Assertions.withToolingApi(URI) // This method is provided to support recipe development at organizations which require gradle to come from // internal repositories. This is not used in contexts where services.gradle.org is accessible - @SuppressWarnings("unused") + + /** + * Construct a Gradle wrapper from a URI. + * Can be used in contexts where servcies.gradle.org, normally used for version lookups, is unavailable. + */ public static GradleWrapper create(URI fullDistributionUri, @SuppressWarnings("unused") ExecutionContext ctx) { String version = ""; Matcher matcher = GRADLE_VERSION_PATTERN.matcher(fullDistributionUri.toString()); diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/DependencyConstraintToRuleTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/DependencyConstraintToRuleTest.java new file mode 100644 index 00000000000..6385665b7d4 --- /dev/null +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/DependencyConstraintToRuleTest.java @@ -0,0 +1,171 @@ +/* + * 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.gradle; + +import org.junit.jupiter.api.Test; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.gradle.Assertions.buildGradle; + + +class DependencyConstraintToRuleTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new DependencyConstraintToRule()); + } + + @Test + void newResolutionStrategyBlock() { + rewriteRun( + buildGradle( + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + dependencies { + constraints { + implementation('com.fasterxml.jackson.core:jackson-core:2.12.5') { + because 'CVE-2024-BAD' + } + } + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """, + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + configurations.all { + resolutionStrategy.eachDependency { details -> + if (details.requested.group == 'com.fasterxml.jackson.core' && details.requested.name == 'jackson-core') { + details.useVersion('2.12.5') + details.because('CVE-2024-BAD') + } + } + } + dependencies { + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """ + ) + ); + } + + @Test + void updateExistingResolutionStrategyBlock() { + rewriteRun( + buildGradle( + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + configurations.all { + resolutionStrategy.eachDependency { d -> + if (d.requested.group == 'com.whatever' && d.requested.name == 'doesntmatter') { + d.useVersion('1.2.3') + } else { + // foo + } + } + } + dependencies { + constraints { + implementation('com.fasterxml.jackson.core:jackson-core:2.12.5') + } + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """, + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + configurations.all { + resolutionStrategy.eachDependency { d -> + if (d.requested.group == 'com.whatever' && d.requested.name == 'doesntmatter') { + d.useVersion('1.2.3') + } else if (d.requested.group == 'com.fasterxml.jackson.core' && d.requested.name == 'jackson-core') { + d.useVersion('2.12.5') + } else { + // foo + } + } + } + dependencies { + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """ + ) + ); + } + + @Test + void leaveDifficultConstraintAlone() { + rewriteRun( + buildGradle( + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + dependencies { + constraints { + implementation('com.fasterxml.jackson.core:jackson-core') { + // This is hard to convert to a resolution strategy block so better to do nothing + version { + strictly '[2.12.4, 2.12.5[' + reject '2.12.3' + } + } + implementation('org.yaml:snakeyaml:2.2') + } + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """, + """ + plugins { + id 'java' + } + repositories { mavenCentral() } + configurations.all { + resolutionStrategy.eachDependency { details -> + if (details.requested.group == 'org.yaml' && details.requested.name == 'snakeyaml') { + details.useVersion('2.2') + } + } + } + dependencies { + constraints { + implementation('com.fasterxml.jackson.core:jackson-core') { + // This is hard to convert to a resolution strategy block so better to do nothing + version { + strictly '[2.12.4, 2.12.5[' + reject '2.12.3' + } + } + } + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """ + ) + ); + } +} diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpdateGradleWrapperTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpdateGradleWrapperTest.java index e73548f85c7..1537063e75c 100755 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpdateGradleWrapperTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpdateGradleWrapperTest.java @@ -19,6 +19,7 @@ import org.openrewrite.*; import org.openrewrite.internal.StringUtils; import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.ipc.http.HttpSender; import org.openrewrite.ipc.http.HttpUrlConnectionSender; import org.openrewrite.marker.BuildTool; import org.openrewrite.properties.tree.Properties; @@ -589,6 +590,44 @@ void customDistributionUri() { ); } + @Test + void servicesGradleOrgUnavailable() { + HttpSender unhelpfulSender = request -> { + if(request.getUrl().toString().contains("services.gradle.org")) { + throw new RuntimeException("I'm sorry Dave, I'm afraid I can't do that."); + } + return new HttpUrlConnectionSender().send(request); + }; + HttpSenderExecutionContextView ctx = HttpSenderExecutionContextView.view(new InMemoryExecutionContext()) + .setHttpSender(unhelpfulSender) + .setLargeFileHttpSender(unhelpfulSender); + rewriteRun( + spec -> spec.recipe(new UpdateGradleWrapper("8.6", null, null, "https://artifactory.moderne.ninja/artifactory/gradle-distributions/gradle-${version}-bin.zip")) + .allSources(source -> source.markers(new BuildTool(Tree.randomId(), BuildTool.Type.Gradle, "7.4"))) + .executionContext(ctx), + properties( + """ + distributionBase=GRADLE_USER_HOME + distributionPath=wrapper/dists + distributionUrl=https\\://services.gradle.org/distributions/gradle-5.6.4-bin.zip + zipStoreBase=GRADLE_USER_HOME + zipStorePath=wrapper/dists + """, + """ + distributionBase=GRADLE_USER_HOME + distributionPath=wrapper/dists + distributionUrl=https\\://artifactory.moderne.ninja/artifactory/gradle-distributions/gradle-8.6-bin.zip + zipStoreBase=GRADLE_USER_HOME + zipStorePath=wrapper/dists + """, + spec -> spec.path("gradle/wrapper/gradle-wrapper.properties") + ), + gradlew, + gradlewBat, + gradleWrapperJarQuark + ); + } + @Test void updateWrapperInSubDirectory() { rewriteRun( diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersionTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersionTest.java index df094239384..80d0d357b4a 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersionTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersionTest.java @@ -228,10 +228,10 @@ void addConstraintAddsUnrelatedConfigurationsForSameArtifact() { dependencies { constraints { - implementation('com.fasterxml.jackson.core:jackson-core:2.12.5') { + earlib('com.fasterxml.jackson.core:jackson-core:2.12.5') { because 'CVE-2024-BAD' } - earlib('com.fasterxml.jackson.core:jackson-core:2.12.5') { + implementation('com.fasterxml.jackson.core:jackson-core:2.12.5') { because 'CVE-2024-BAD' } } @@ -479,7 +479,6 @@ void constraintDoesNotGetAddedToNonTransitiveNonExtendingConfiguration() { void constraintDoesNotGetAddedInsideConstraint() { rewriteRun( spec -> spec - .beforeRecipe(withToolingApi()) .recipe(new UpgradeTransitiveDependencyVersion("com.fasterxml.jackson.core", "jackson-core","2.12.5", null, "CVE-2024-BAD", null)), //language=groovy buildGradle( @@ -533,7 +532,6 @@ void constraintDoesNotGetAddedInsideConstraint() { void includedConfigurationsReceiveOnlyConfiguredConstraints() { rewriteRun( spec -> spec - .beforeRecipe(withToolingApi()) .recipe(new UpgradeTransitiveDependencyVersion( "org.apache.commons", "commons-lang3", "3.14.0", null, null, List.of("pitest"))), buildGradle( @@ -568,7 +566,6 @@ void includedConfigurationsReceiveOnlyConfiguredConstraints() { void noIncludedConfigurationsReceiveAllConstraints() { rewriteRun( spec -> spec - .beforeRecipe(withToolingApi()) .recipe(new UpgradeTransitiveDependencyVersion( "org.apache.commons", "commons-lang3", "3.14.0", null, null, null)), buildGradle( @@ -605,7 +602,6 @@ void noIncludedConfigurationsReceiveAllConstraints() { void IncludedDefaultConfigurationsReceiveRuntimeConstraints() { rewriteRun( spec -> spec - .beforeRecipe(withToolingApi()) .recipe(new UpgradeTransitiveDependencyVersion( "org.apache.commons", "commons-lang3", "3.14.0", null, null, List.of("implementation", "runtimeOnly"))), buildGradle( @@ -635,4 +631,44 @@ void IncludedDefaultConfigurationsReceiveRuntimeConstraints() { ) ); } + + @Test + void useResolutionStrategyWhenSpringDependencyManagementPluginIsPresent() { + rewriteRun( + buildGradle( + """ + plugins { + id 'java' + id 'io.spring.dependency-management' version '1.1.5' + } + repositories { mavenCentral() } + + dependencies { + + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """, + """ + plugins { + id 'java' + id 'io.spring.dependency-management' version '1.1.5' + } + repositories { mavenCentral() } + configurations.all { + resolutionStrategy.eachDependency { details -> + if (details.requested.group == 'com.fasterxml.jackson.core' && details.requested.name == 'jackson-core') { + details.useVersion('2.12.5') + details.because('CVE-2024-BAD') + } + } + } + + dependencies { + + implementation 'org.openrewrite:rewrite-java:7.0.0' + } + """ + ) + ); + } } diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java index 5b60a68e666..ae1c00c7eaf 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -15,6 +15,7 @@ */ package org.openrewrite.groovy; +import groovy.lang.GroovySystem; import groovyjarjarasm.asm.Opcodes; import lombok.Getter; import lombok.RequiredArgsConstructor; @@ -87,6 +88,9 @@ public class GroovyParserVisitor { */ private int columnOffset; + @Nullable + private static Boolean olderThanGroovy3; + @SuppressWarnings("unused") public GroovyParserVisitor(Path sourcePath, @Nullable FileAttributes fileAttributes, EncodingDetectingInputStream source, JavaTypeCache typeCache, ExecutionContext ctx) { this.sourcePath = sourcePath; @@ -97,6 +101,15 @@ public GroovyParserVisitor(Path sourcePath, @Nullable FileAttributes fileAttribu this.typeMapping = new GroovyTypeMapping(typeCache); } + private static boolean isOlderThanGroovy3() { + if (olderThanGroovy3 == null) { + String groovyVersionText = GroovySystem.getVersion(); + int majorVersion = Integer.parseInt(groovyVersionText.substring(0, groovyVersionText.indexOf('.'))); + olderThanGroovy3 = majorVersion < 3; + } + return olderThanGroovy3; + } + public G.CompilationUnit visit(SourceUnit unit, ModuleNode ast) throws GroovyParsingException { NavigableMap> sortedByPosition = new TreeMap<>(); for (org.codehaus.groovy.ast.stmt.Statement s : ast.getStatementBlock().getStatements()) { @@ -684,7 +697,7 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { .collect(Collectors.toList()); } else if (!unparsedArgs.isEmpty() && unparsedArgs.get(0) instanceof MapExpression) { // The map literal may or may not be wrapped in "[]" - // If it is wrapped in "[]" then this isn't a named arguments situation and we should not lift the parameters out of the enclosing MapExpression + // If it is wrapped in "[]" then this isn't a named arguments situation, and we should not lift the parameters out of the enclosing MapExpression saveCursor = cursor; whitespace(); boolean isOpeningBracketPresent = '[' == source.charAt(cursor); @@ -1178,7 +1191,7 @@ public void visitConstantExpression(ConstantExpression expression) { } else if (type == ClassHelper.STRING_TYPE) { jType = JavaType.Primitive.String; // String literals value returned by getValue()/getText() has already processed sequences like "\\" -> "\" - int length = sourceLengthOfNext(expression); + int length = sourceLengthOfString(expression); // this is an attribute selector if (source.startsWith("@"+value, cursor)) { length += 1; @@ -1444,7 +1457,7 @@ public void visitGStringExpression(GStringExpression gstring) { // The sub-strings within a GString have no delimiters of their own, confusing visitConstantExpression() // ConstantExpression.getValue() cannot be trusted for strings as its values don't match source code because sequences like "\\" have already been replaced with a single "\" // Use the AST element's line/column positions to figure out its extent, but those numbers need tweaks to be correct - int length = sourceLengthOfNext(cs); + int length = sourceLengthOfString(cs); if (i == 0 || i == rawExprs.size() - 1) { // The first and last constants within a GString have line/column position which incorrectly include the GString's delimiters length -= delimiter.length(); @@ -2282,10 +2295,45 @@ private Space sourceBefore(String untilDelim) { } /** - * Gets the length in characters of the AST node. + * Gets the length in characters of a String literal. + * Attempts to account for a number of different strange compiler behaviors, old bugs, and edge cases. * cursor is presumed to point at the beginning of the node. */ - private int sourceLengthOfNext(ASTNode node) { + private int sourceLengthOfString(ConstantExpression expr) { + // ConstantExpression.getValue() already has resolved escaped characters. So "\t" and a literal tab will look the same. + // Since we cannot differentiate between the two, use this alternate method only when an old version of groovy indicates risk + // and the literal doesn't contain any characters which might be from an escape sequence. e.g.: tabs, newlines, carriage returns + String value = (String) expr.getValue(); + if (isOlderThanGroovy3() && value.matches("[^\\t\\r\\n\\\\]*")) { + int delimiterLength = getDelimiterLength(); + return delimiterLength + value.length() + delimiterLength; + } + return lengthAccordingToAst(expr); + } + + private int getDelimiterLength() { + String maybeDelimiter = source.substring(cursor, Math.min(cursor + 3, source.length())); + int delimiterLength = 0; + if (maybeDelimiter.startsWith("$/")) { + delimiterLength = 2; + } else if (maybeDelimiter.startsWith("\"\"\"") || maybeDelimiter.startsWith("'''")) { + delimiterLength = 3; + } else if (maybeDelimiter.startsWith("/") || maybeDelimiter.startsWith("\"") || maybeDelimiter.startsWith("'")) { + delimiterLength = 1; + } + return delimiterLength; + } + + /** + * Gets the length according to the Groovy compiler's attestation of starting/ending line and column numbers. + * On older versions of the JDK/Groovy compiler string literals with following whitespace sometimes erroneously include + * the length of the whitespace in the length of the AST node. + * So in this method invocation: + * foo( 'a' ) + * the correct source length for the AST node representing 'a' is 3, but in affected groovy versions the length + * on the node is '4' because it is also counting the trailing whitespace. + */ + private int lengthAccordingToAst(ConstantExpression node) { if (!appearsInSource(node)) { return 0; } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java index 9a469c71c79..f6ec216903e 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java @@ -89,8 +89,8 @@ void gString() { rewriteRun( groovy( """ - "uid: ${ UUID.randomUUID() } " - """ + " uid: ${ UUID.randomUUID() } " + """ ) ); } @@ -282,13 +282,23 @@ void escapeCharacters() { rewriteRun( groovy( """ - "\\\\n\\t" - '\\\\n\\t' - ///\\\\n\\t/// - + "\\\\n\\t" + '\\\\n\\t' + ///\\\\n\\t/// """ ) ); + } + @Test + void differentiateEscapeFromLiteral() { + rewriteRun( + groovy( + """ + '\t' + ' ' + """ + ) + ); } } 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 cf6e2364688..5384f979900 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 @@ -25,6 +25,7 @@ import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.SourceSpec; import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; @@ -171,6 +172,7 @@ void changeInnerClassToOuterClass() { class Test { Entry p; Map.Entry p2; + java.util.Map.Entry p3; } """, """ @@ -179,6 +181,7 @@ class Test { class Test { List p; List p2; + java.util.List p3; } """ ) @@ -952,7 +955,7 @@ void changeTypeWithInnerClass() { public class OuterClass { public static class InnerClass { - + } } """ @@ -1776,7 +1779,7 @@ void doesNotModifyInnerClassesIfIgnoreDefinitionTrue() { public class Test { private class InnerA { } - + private class InnerB { } @@ -1789,7 +1792,7 @@ public void test(String s) { public class Test { private class InnerA { } - + private class InnerB { } @@ -1846,4 +1849,67 @@ public Test test() { ) ); } + + @Issue("https://github.com/openrewrite/rewrite-jackson/pull/6") + @Test + void changeTypeOfStaticImportOfNestedEnumValueUsed() { + rewriteRun( + recipeSpec -> recipeSpec.recipes( + new ChangeType( + "org.codehaus.jackson.map.ObjectMapper", + "com.fasterxml.jackson.databind.ObjectMapper", true), + new ChangeType( + "org.codehaus.jackson.map.SerializationConfig$Feature", + "com.fasterxml.jackson.databind.SerializationFeature", true) + ), + java( + """ + package org.codehaus.jackson.map; + public class SerializationConfig { + public static enum Feature { + WRAP_ROOT_VALUE + } + } + """, + SourceSpec::skip + ), + java( + """ + package org.codehaus.jackson.map; + public class ObjectMapper { + public ObjectMapper configure(SerializationConfig.Feature f, boolean state) { + return this; + } + } + """, + SourceSpec::skip + ), + java( + """ + import org.codehaus.jackson.map.ObjectMapper; + + import static org.codehaus.jackson.map.SerializationConfig.Feature.WRAP_ROOT_VALUE; + + class A { + void test() { + ObjectMapper mapper = new ObjectMapper(); + mapper.configure(WRAP_ROOT_VALUE, true); + } + } + """, + """ + import com.fasterxml.jackson.databind.ObjectMapper; + + import static com.fasterxml.jackson.databind.SerializationFeature.WRAP_ROOT_VALUE; + + class A { + void test() { + ObjectMapper mapper = new ObjectMapper(); + mapper.configure(WRAP_ROOT_VALUE, true); + } + } + """ + ) + ); + } } 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 87bb00ae278..fadbc756933 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java @@ -276,6 +276,10 @@ public J visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) { e = i.withType(targetType); } return e; + } else if (maybeClass.toString().equals(oldType.getFullyQualifiedName().replace('$', '.'))) { + maybeRemoveImport(oldType.getOwningClass()); + return updateOuterClassTypes(TypeTree.build(((JavaType.FullyQualified) targetType).getFullyQualifiedName()) + .withPrefix(fieldAccess.getPrefix())); } } return super.visitFieldAccess(fieldAccess, ctx); @@ -310,6 +314,22 @@ public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) { ident = ident.withSimpleName(((JavaType.Primitive) targetType).getKeyword()); } } + + // Recreate any static imports as needed + JavaSourceFile cu = getCursor().firstEnclosing(JavaSourceFile.class); + if (cu != null) { + for (J.Import anImport : cu.getImports()) { + if (anImport.isStatic() && anImport.getQualid().getTarget().getType() != null) { + JavaType.FullyQualified fqn = TypeUtils.asFullyQualified(anImport.getQualid().getTarget().getType()); + if (fqn != null && TypeUtils.isOfClassType(fqn, originalType.getFullyQualifiedName()) && + ident.getSimpleName().equals(anImport.getQualid().getSimpleName())) { + JavaType.FullyQualified targetFqn = (JavaType.FullyQualified) targetType; + maybeAddImport((targetFqn).getFullyQualifiedName(), ident.getSimpleName()); + break; + } + } + } + } } ident = ident.withType(updateType(ident.getType())); return visitAndCast(ident, ctx, super::visitIdentifier); diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java index 0096705358b..32bbf4cd240 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java @@ -29,10 +29,7 @@ import org.openrewrite.semver.Semver; import org.openrewrite.xml.tree.Xml; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.regex.Pattern; import static java.util.Objects.requireNonNull; @@ -181,14 +178,14 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if (tree instanceof JavaSourceFile) { if (onlyIfUsing == null || sourceFile != new UsesType<>(onlyIfUsing, true).visit(sourceFile, ctx)) { acc.usingType = true; - sourceFile.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> - sourceFile.getMarkers().findFirst(JavaSourceSet.class).ifPresent(sourceSet -> - acc.scopeByProject.compute(javaProject, (jp, scope) -> "compile".equals(scope) ? - scope /* a `compile` scope dependency will also be available in test source set */ : - "test".equals(sourceSet.getName()) ? "test" : "compile" - ) - ) - ); + JavaProject javaProject = sourceFile.getMarkers().findFirst(JavaProject.class).orElse(null); + JavaSourceSet javaSourceSet = sourceFile.getMarkers().findFirst(JavaSourceSet.class).orElse(null); + if(javaProject != null && javaSourceSet != null) { + acc.scopeByProject.compute(javaProject, (jp, scope) -> "compile".equals(scope) ? + scope /* a `compile` scope dependency will also be available in test source set */ : + "test".equals(javaSourceSet.getName()) ? "test" : "compile" + ); + } } } else if(tree instanceof Xml.Document) { Xml.Document doc = (Xml.Document) tree; @@ -215,7 +212,7 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) { JavaProject javaProject = document.getMarkers().findFirst(JavaProject.class).orElse(null); String maybeScope = javaProject == null ? null : acc.scopeByProject.get(javaProject); - if (maybeScope == null && !acc.scopeByProject.isEmpty()) { + if (onlyIfUsing != null && maybeScope == null && !acc.scopeByProject.isEmpty()) { return maven; } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddParentPom.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddParentPom.java new file mode 100644 index 00000000000..d495affc70f --- /dev/null +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddParentPom.java @@ -0,0 +1,163 @@ +/* + * 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.openrewrite.*; +import org.openrewrite.internal.StringUtils; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.marker.SearchResult; +import org.openrewrite.maven.table.MavenMetadataFailures; +import org.openrewrite.maven.tree.MavenMetadata; +import org.openrewrite.maven.tree.MavenRepository; +import org.openrewrite.semver.Semver; +import org.openrewrite.semver.VersionComparator; +import org.openrewrite.xml.AddToTagVisitor; +import org.openrewrite.xml.tree.Xml; + +import java.util.Collection; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; + +@Value +@EqualsAndHashCode(callSuper = false) +public class AddParentPom extends Recipe { + transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this); + + @Option(displayName = "Group ID", + description = "The group ID of the maven parent pom to be adopted.", + example = "org.springframework.boot") + String groupId; + + @Option(displayName = "Artifact ID", + description = "The artifact ID of the maven parent pom to be adopted.", + example = "spring-boot-starter-parent") + String artifactId; + + @Option(displayName = "Version", + description = "An exact version number or node-style semver selector used to select the version number.", + example = "29.X") + String version; + + @Option(displayName = "Relative path", + description = "New relative path attribute for parent lookup.", + example = "../pom.xml") + @Nullable + String relativePath; + + @Option(displayName = "Version pattern", + description = "Allows version selection to be extended beyond the original Node Semver semantics. So for example," + + "Setting 'version' to \"25-29\" can be paired with a metadata pattern of \"-jre\" to select Guava 29.0-jre", + example = "-jre", + required = false) + @Nullable + String versionPattern; + + @Override + public String getDisplayName() { + return "Add Maven parent"; + } + + @Override + public String getInstanceNameSuffix() { + return String.format("`%s:%s:%s`", groupId, artifactId, version); + } + + @Override + public String getDescription() { + return "Add a parent pom to a Maven pom.xml. Does nothing if a parent pom is already present."; + } + + @Override + public Validated validate() { + Validated validated = super.validate(); + //noinspection ConstantConditions + if (version != null) { + validated = validated.and(Semver.validate(version, versionPattern)); + } + return validated; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check(new MavenVisitor() { + @Override + public Xml visitDocument(Xml.Document document, ExecutionContext ctx) { + Xml.Tag root = document.getRoot(); + if (!root.getChild("parent").isPresent()) { + return SearchResult.found(document); + } + return document; + + } + }, new MavenIsoVisitor() { + @Nullable + private Collection availableVersions; + + @Override + public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { + Xml.Tag root = document.getRoot(); + assert !root.getChild("parent").isPresent(); + + try { + Optional targetVersion = findAcceptableVersion(groupId, artifactId, ctx); + if (targetVersion.isPresent()) { + Xml.Tag parentTag = Xml.Tag.build( + "\n" + + "" + groupId + "\n" + + "" + artifactId + "\n" + + "" + targetVersion.get() + "\n" + + (relativePath == null ? "" : StringUtils.isBlank(relativePath) ? + "" : "" + relativePath + "") + + ""); + + document = (Xml.Document) new AddToTagVisitor<>(root, parentTag, new MavenTagInsertionComparator(root.getChildren())) + .visitNonNull(document, ctx, getCursor().getParentOrThrow()); + + maybeUpdateModel(); + doAfterVisit(new RemoveRedundantDependencyVersions(null, null, + RemoveRedundantDependencyVersions.Comparator.GTE, null).getVisitor()); + } + } catch (MavenDownloadingException e) { + for (Map.Entry repositoryResponse : e.getRepositoryResponses().entrySet()) { + MavenRepository repository = repositoryResponse.getKey(); + metadataFailures.insertRow(ctx, new MavenMetadataFailures.Row(groupId, artifactId, version, + repository.getUri(), repository.getSnapshots(), repository.getReleases(), repositoryResponse.getValue())); + } + return e.warn(document); + } + + return super.visitDocument(document, ctx); + } + + private final VersionComparator versionComparator = Objects.requireNonNull(Semver.validate(version, versionPattern).getValue()); + + private Optional findAcceptableVersion(String groupId, String artifactId, ExecutionContext ctx) + throws MavenDownloadingException { + if (availableVersions == null) { + MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx)); + availableVersions = mavenMetadata.getVersioning().getVersions().stream() + .filter(v -> versionComparator.isValid(null, v)) + .collect(Collectors.toList()); + } + return availableVersions.stream().max(versionComparator); + } + }); + } +} diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java index 6114e437fb0..bc622e429ad 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java @@ -421,6 +421,7 @@ public static class ServerConfiguration { @JacksonXmlElementWrapper(localName = "httpHeaders", useWrapping = true) // wrapping is disabled by default on MavenXmlMapper @Nullable List httpHeaders; + /** * Timeout in milliseconds for establishing a connection. */ 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 a98a07dae9f..0bca82c31b5 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java @@ -270,8 +270,9 @@ public ResolvedManagedDependency findManagedDependency(Xml.Tag tag) { .orElse(getResolutionResult().getPom().getGroupId())); String artifactId = getResolutionResult().getPom().getValue(tag.getChildValue("artifactId").orElse("")); String classifier = getResolutionResult().getPom().getValue(tag.getChildValue("classifier").orElse(null)); + String type = getResolutionResult().getPom().getValue(tag.getChildValue("type").orElse(null)); if (groupId != null && artifactId != null) { - return findManagedDependency(groupId, artifactId, classifier); + return findManagedDependency(groupId, artifactId, classifier, type); } return null; } @@ -283,10 +284,16 @@ public ResolvedManagedDependency findManagedDependency(String groupId, String ar @Nullable private ResolvedManagedDependency findManagedDependency(String groupId, String artifactId, @Nullable String classifier) { + return findManagedDependency(groupId, artifactId, classifier, null); + } + + @Nullable + private ResolvedManagedDependency findManagedDependency(String groupId, String artifactId, @Nullable String classifier, @Nullable String type) { for (ResolvedManagedDependency d : getResolutionResult().getPom().getDependencyManagement()) { if (groupId.equals(d.getGroupId()) && artifactId.equals(d.getArtifactId()) && - (classifier == null || classifier.equals(d.getClassifier()))) { + (classifier == null || classifier.equals(d.getClassifier())) && + (type == null || type.equals(d.getType()))) { return d; } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RenamePropertyKey.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RenamePropertyKey.java index b516b7ca556..21cd96c45ea 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/RenamePropertyKey.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RenamePropertyKey.java @@ -66,7 +66,6 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); if (isPropertyTag() && oldKey.equals(t.getName())) { t = t.withName(newKey); - maybeUpdateModel(); } if (t.getChildren().isEmpty()) { Optional value = t.getValue(); @@ -77,6 +76,15 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } return t; } + + @Override + public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { + Xml.Document d = super.visitDocument(document, ctx); + if (d != document) { + maybeUpdateModel(); + } + return d; + } }; } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java index 58de657afd6..1288389894d 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpdateMavenModel.java @@ -40,6 +40,7 @@ public Xml visitDocument(Xml.Document document, P p) { Pom requested = resolutionResult.getPom().getRequested(); Optional properties = document.getRoot().getChild("properties"); + requested.getProperties().clear(); if (properties.isPresent()) { for (final Xml.Tag propertyTag : properties.get().getChildren()) { requested.getProperties().put(propertyTag.getName(), diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/search/EffectiveManagedDependencies.java b/rewrite-maven/src/main/java/org/openrewrite/maven/search/EffectiveManagedDependencies.java index 50f313d4c4d..e2e0ca4434c 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/search/EffectiveManagedDependencies.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/search/EffectiveManagedDependencies.java @@ -33,7 +33,7 @@ public class EffectiveManagedDependencies extends Recipe { @Override public String getDisplayName() { - return "Effective dependencies"; + return "Effective managed dependencies"; } @Override diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java index a351c9bf890..008da3f933b 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java @@ -22,7 +22,10 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.fasterxml.jackson.databind.node.ObjectNode; -import lombok.*; +import lombok.Builder; +import lombok.Getter; +import lombok.Value; +import lombok.With; import lombok.experimental.NonFinal; import org.openrewrite.ExecutionContext; import org.openrewrite.internal.ListUtils; @@ -156,7 +159,7 @@ private static class UniqueDependencyKey { @SuppressWarnings("DuplicatedCode") public ResolvedPom resolve(ExecutionContext ctx, MavenPomDownloader downloader) throws MavenDownloadingException { // If this resolved pom represents an obsolete pom format, refuse to resolve in same as Maven itself would - if(requested.getObsoletePomVersion() != null) { + if (requested.getObsoletePomVersion() != null) { return this; } @@ -421,12 +424,12 @@ private void resolveParentDependenciesRecursively(List pomAncestry) throws for (Profile profile : pom.getProfiles()) { if (profile.isActive(activeProfiles)) { - mergeDependencyManagement(profile.getDependencyManagement(), pom); + mergeDependencyManagement(profile.getDependencyManagement(), pomAncestry); mergeRequestedDependencies(profile.getDependencies()); } } - mergeDependencyManagement(pom.getDependencyManagement(), pom); + mergeDependencyManagement(pom.getDependencyManagement(), pomAncestry); mergeRequestedDependencies(pom.getDependencies()); if (pom.getParent() != null) { @@ -768,14 +771,19 @@ private void mergeProperties(Map incomingProperties, Pom pom) { } } - private void mergeDependencyManagement(List incomingDependencyManagement, Pom pom) throws MavenDownloadingException { + private void mergeDependencyManagement(List incomingDependencyManagement, List pomAncestry) throws MavenDownloadingException { + Pom pom = pomAncestry.get(0); if (!incomingDependencyManagement.isEmpty()) { if (dependencyManagement == null || dependencyManagement.isEmpty()) { dependencyManagement = new ArrayList<>(); } for (ManagedDependency d : incomingDependencyManagement) { if (d instanceof Imported) { - ResolvedPom bom = downloader.download(getValues(((Imported) d).getGav()), null, ResolvedPom.this, repositories) + GroupArtifactVersion groupArtifactVersion = getValues(((Imported) d).getGav()); + if (isAlreadyResolved(groupArtifactVersion, pomAncestry)) { + continue; + } + ResolvedPom bom = downloader.download(groupArtifactVersion, null, ResolvedPom.this, repositories) .resolve(activeProfiles, downloader, initialRepositories, ctx); MavenExecutionContextView.view(ctx) .getResolutionListener() @@ -802,6 +810,19 @@ private void mergeDependencyManagement(List incomingDependenc } } } + + private boolean isAlreadyResolved(GroupArtifactVersion groupArtifactVersion, List pomAncestry) { + for (int i = 1; i < pomAncestry.size(); i++) { // skip current pom + Pom pom = pomAncestry.get(i); + ResolvedGroupArtifactVersion alreadyResolvedGav = pom.getGav(); + if (alreadyResolvedGav.getGroupId().equals(groupArtifactVersion.getGroupId()) + && alreadyResolvedGav.getArtifactId().equals(groupArtifactVersion.getArtifactId()) + && alreadyResolvedGav.getVersion().equals(groupArtifactVersion.getVersion())) { + return true; + } + } + return false; + } } public List resolveDependencies(Scope scope, MavenPomDownloader downloader, ExecutionContext ctx) throws MavenDownloadingExceptions { diff --git a/rewrite-maven/src/main/resources/META-INF/rewrite/maven.yml b/rewrite-maven/src/main/resources/META-INF/rewrite/maven.yml index 0c9477000ab..612b3c7efd5 100644 --- a/rewrite-maven/src/main/resources/META-INF/rewrite/maven.yml +++ b/rewrite-maven/src/main/resources/META-INF/rewrite/maven.yml @@ -23,6 +23,8 @@ recipeList: - org.openrewrite.maven.cleanup.ExplicitPluginVersion - org.openrewrite.maven.cleanup.PrefixlessExpressions - org.openrewrite.maven.OrderPomElements + - org.openrewrite.maven.RemoveDuplicateDependencies + - org.openrewrite.maven.RemoveRedundantDependencyVersions --- type: specs.openrewrite.org/v1beta/recipe name: org.openrewrite.maven.cleanup.PrefixlessExpressions diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java index af55d3903b3..a5286ae8a7a 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java @@ -950,6 +950,9 @@ void preferRootPom() { ) ), mavenProject("project1", + srcMainJava( + java(usingGuavaIntMath) + ), pomXml( """ diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddParentPomTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddParentPomTest.java new file mode 100644 index 00000000000..a2575e1e656 --- /dev/null +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddParentPomTest.java @@ -0,0 +1,475 @@ +/* + * 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.DocumentExample; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.mavenProject; +import static org.openrewrite.maven.Assertions.pomXml; + +class AddParentPomTest implements RewriteTest { + + @DocumentExample + @Test + void addParent() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + "1.5.12.RELEASE", + null, + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.12.RELEASE + + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @ParameterizedTest + @ValueSource(strings = { + "1.5.99.RELEASE", + "~1.9" + }) + void shoudNotAddInvalidParentPomVersion(String version) { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + version, + null, + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @Test + void addParentWithRelativePath() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + "1.5.12.RELEASE", + "../../pom.xml", + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.12.RELEASE + ../../pom.xml + + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @Test + void addParentWithRelativePathEmptyValue() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + "1.5.12.RELEASE", + "", + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.12.RELEASE + + + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @Test + void multiModuleRelativePath() { + AddParentPom recipe = new AddParentPom("org.springframework.boot", "spring-boot-starter-parent", "1.5.12.RELEASE", "", null); + rewriteRun( + spec -> spec.recipe(recipe), + mavenProject("parent", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + module1 + module2 + + + """, + """ + + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.12.RELEASE + + + org.sample + sample + 1.0.0 + + + module1 + module2 + + + """ + ), + mavenProject("module1", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module1 + + """ + )), + mavenProject("module2", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + """ + ) + ) + ) + ); + } + + @Test + void multiModuleShouldOnlyChangeRoot() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + "1.5.12.RELEASE", + null, null)), + mavenProject("parent", + pomXml( + """ + + + 4.0.0 + org.sample + sample + 1.0.0 + + + module1 + module2 + + + """, + """ + + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.12.RELEASE + + org.sample + sample + 1.0.0 + + + module1 + module2 + + + """ + ), + mavenProject("module1", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module1 + + """ + )), + mavenProject("module2", + pomXml( + """ + + + 4.0.0 + + org.sample + sample + 1.0.0 + + module2 + + """ + ) + ) + ) + ); + } + + @Test + void shouldResolveWildcardVersion() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.springframework.boot", + "spring-boot-starter-parent", + "~1.5", + null, + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 1.5.22.RELEASE + + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @Test + void removesRedundantExplicitVersionsMatchingNewParent() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.junit", + "junit-bom", + "5.9.1", + "", + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + + org.junit.jupiter + junit-jupiter-api + 5.9.1 + + + + """, + """ + + 4.0.0 + + org.junit + junit-bom + 5.9.1 + + + + com.mycompany.app + my-app + 1 + + + + org.junit.jupiter + junit-jupiter-api + + + + """ + ) + ); + } + + @Test + void takesNewVersionFromParent() { + rewriteRun( + spec -> spec.recipe(new AddParentPom( + "org.junit", + "junit-bom", + "5.9.1", + "", + null + )), + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + + org.junit.jupiter + junit-jupiter-api + 5.8.0 + + + + """, + """ + + 4.0.0 + + org.junit + junit-bom + 5.9.1 + + + + com.mycompany.app + my-app + 1 + + + + org.junit.jupiter + junit-jupiter-api + + + + """ + ) + ); + } +} diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java index 26763316dd8..05d37789d11 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java @@ -3038,4 +3038,73 @@ void runtimeClasspathOnly() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite/issues/4093") + void circularImportDependency() { + rewriteRun( + mavenProject("root", + pomXml( + """ + + com.example + circular-example-parent + 0.0.1-SNAPSHOT + + circular-example-child + + + + + com.example + circular-example-child + 0.0.1-SNAPSHOT + import + + + + + """, + spec -> spec.afterRecipe(pomXml -> { + MavenResolutionResult resolution = pomXml.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow(); + GroupArtifactVersion gav = resolution.getPom().getDependencyManagement().get(0).getGav(); + assertThat(gav.getGroupId()).isEqualTo("junit"); + assertThat(gav.getArtifactId()).isEqualTo("junit"); + assertThat(gav.getVersion()).isEqualTo("4.13.2"); + }) + ), + mavenProject("circular-example-child", + pomXml( + """ + + + com.example + circular-example-parent + 0.0.1-SNAPSHOT + + circular-example-child + 0.0.1-SNAPSHOT + + + + junit + junit + 4.13.2 + + + + + """, + spec -> spec.afterRecipe(pomXml -> { + MavenResolutionResult resolution = pomXml.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow(); + GroupArtifactVersion gav = resolution.getPom().getDependencyManagement().get(0).getGav(); + assertThat(gav.getGroupId()).isEqualTo("junit"); + assertThat(gav.getArtifactId()).isEqualTo("junit"); + assertThat(gav.getVersion()).isEqualTo("4.13.2"); + }) + ) + ) + ) + ); + } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveDuplicateDependenciesTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveDuplicateDependenciesTest.java index 6676c747468..aaa95f3e37b 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveDuplicateDependenciesTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/RemoveDuplicateDependenciesTest.java @@ -345,6 +345,39 @@ void keepDependencyWithType() { ); } + @Test + void keepDependencyManagementWithType() { + rewriteRun( + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + + + com.acme + example-dependency + 1.0.0 + + + com.acme + example-dependency + 1.0.0 + test-jar + + + + + """ + ) + ); + } + @Test void keepDependencyWithDifferentScope() { rewriteRun( diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java index ec7da455a79..20c2e99500c 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java @@ -17,9 +17,13 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.maven.tree.MavenResolutionResult; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.maven.Assertions.pomXml; class RemovePropertyTest implements RewriteTest { @@ -60,7 +64,15 @@ void removeProperty() { a - """ + """, + sourceSpecs -> { + sourceSpecs.afterRecipe(d -> { + MavenResolutionResult resolution = d.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow(); + Map properties = resolution.getPom().getRequested().getProperties(); + assertThat(properties.get("a.version")).isEqualTo("a"); + assertThat(properties.get("bla.version")).isNull(); + }); + } ) ); } @@ -90,7 +102,14 @@ void removeOnlyProperty() { my-app 1 - """ + """, + sourceSpecs -> { + sourceSpecs.afterRecipe(d -> { + MavenResolutionResult resolution = d.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow(); + Map properties = resolution.getPom().getRequested().getProperties(); + assertThat(properties.isEmpty()).isTrue(); + }); + } ) ); } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RenamePropertyKeyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RenamePropertyKeyTest.java index 0a865ea5a93..f61ad5e5e5c 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/RenamePropertyKeyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/RenamePropertyKeyTest.java @@ -15,11 +15,16 @@ */ package org.openrewrite.maven; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.maven.tree.MavenResolutionResult; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.maven.Assertions.pomXml; class RenamePropertyKeyTest implements RewriteTest { @@ -222,4 +227,27 @@ void renamePropertyAndValue() { ) ); } + + @Test + void nothingToRename() { + rewriteRun( + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + a + b + + + """ + ) + ); + } + } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index 145c21fad5a..e01b34844f4 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -862,7 +862,7 @@ void upgradeVersionDefinedViaPropertyInLocalParent() { @Test void upgradeVersionDefinedViaImplicitPropertyInRemoteParent() { rewriteRun( - spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.x", "", true, null)), + spec -> spec.recipe(new UpgradeDependencyVersion("org.flywaydb", "flyway-core", "10.15.0", "", true, null)), pomXml( """ diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 950c74166a9..d78648e4d00 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -96,10 +96,10 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) ctx, getCursor())); } Yaml.Document d = super.visitDocument(document, ctx); - if(d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { + if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { // No matching element already exists, attempt to construct one String valueKey = maybeKeyFromJsonPath(key); - if(valueKey == null) { + if (valueKey == null) { return d; } // If there is no space between the colon and the value it will not be interpreted as a mapping @@ -137,7 +137,15 @@ public String indent(String text) { @Nullable private String maybeKeyFromJsonPath(String jsonPath) { - if(!jsonPath.startsWith("$.")) { + if (!jsonPath.startsWith("$.")) { + return null; + } + // if the key contains a jsonpath filter we cannot infer a valid key + if (jsonPath.matches(".*\\[\\s?\\?\\s?\\(\\s?@\\..*\\)\\s?].*")) { + return null; + } + // remove keys that contain wildcard or deep search + if (jsonPath.matches(".*\\*.*") || jsonPath.matches(".*\\.\\..*")) { return null; } return jsonPath.substring(2); diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index dd8693bd02c..5761ef74d9c 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.yaml; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -326,7 +325,100 @@ void insertInSequenceEntries() { } @Test - @Disabled + void insertInSequenceEntriesWithWildcard() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$.*.containers", + "imagePullPolicy: Always", + true, + null + )), + yaml( + """ + kind: Pod + spec: + containers: + - name: + """, + """ + kind: Pod + spec: + containers: + - name: + imagePullPolicy: Always + """ + ) + ); + } + + @Test + void noInsertInSequenceEntriesWithWildcard() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$.*.unknown", + "imagePullPolicy: Always", + true, + null + )), + yaml( + """ + kind: Pod + spec: + containers: + - name: + """ + ) + ); + } + + @Test + void insertInSequenceEntriesWithDeepSearch() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$..containers", + "imagePullPolicy: Always", + true, + null + )), + yaml( + """ + kind: Pod + spec: + containers: + - name: + """, + """ + kind: Pod + spec: + containers: + - name: + imagePullPolicy: Always + """ + ) + ); + } + + @Test + void noInsertInSequenceEntriesWithDeepSearch() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$..unknown", + "imagePullPolicy: Always", + true, + null + )), + yaml( + """ + kind: Pod + spec: + containers: + - name: + """ + ) + ); + } + + @Test void insertInSequenceEntriesMatchingPredicate() { rewriteRun( spec -> spec.recipe(new MergeYaml( @@ -356,6 +448,28 @@ void insertInSequenceEntriesMatchingPredicate() { ); } + @Test + void noChangeInSequenceEntriesNotMatchingPredicate() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$.spec.containers[?(@.name == 'pod-x')]", + //language=yaml + "imagePullPolicy: Always", + true, + null + )), + yaml( + """ + kind: Pod + spec: + containers: + - name: pod-0 + - name: pod-1 + """ + ) + ); + } + @Test void insertBlockInSequenceEntriesWithExistingBlock() { rewriteRun(