diff --git a/.github/workflows/receive-pr.yml b/.github/workflows/receive-pr.yml index 63e17f40a67..67d695322ed 100644 --- a/.github/workflows/receive-pr.yml +++ b/.github/workflows/receive-pr.yml @@ -17,64 +17,3 @@ jobs: uses: openrewrite/gh-automation/.github/workflows/receive-pr.yml@main with: recipe: 'org.openrewrite.recipes.OpenRewriteBestPracticesSubset' - rewrite_yml: | - --- - type: specs.openrewrite.org/v1beta/recipe - name: org.openrewrite.recipes.OpenRewriteBestPracticesSubset - displayName: OpenRewrite best practices - description: Best practices for OpenRewrite recipe development. - recipeList: - - org.openrewrite.recipes.JavaRecipeBestPracticesSubset - - org.openrewrite.recipes.RecipeTestingBestPracticesSubset - - org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset - #- org.openrewrite.java.OrderImports - - org.openrewrite.java.format.EmptyNewlineAtEndOfFile - - org.openrewrite.staticanalysis.InlineVariable - - org.openrewrite.staticanalysis.MissingOverrideAnnotation - - org.openrewrite.staticanalysis.UseDiamondOperator - --- - type: specs.openrewrite.org/v1beta/recipe - name: org.openrewrite.recipes.JavaRecipeBestPracticesSubset - displayName: Java Recipe best practices - description: Best practices for Java recipe development. - preconditions: - - org.openrewrite.java.search.FindTypes: - fullyQualifiedTypeName: org.openrewrite.Recipe - checkAssignability: true - recipeList: - - org.openrewrite.java.recipes.BlankLinesAroundFieldsWithAnnotations - - org.openrewrite.java.recipes.ExecutionContextParameterName - - org.openrewrite.java.recipes.MissingOptionExample - - org.openrewrite.java.recipes.RecipeEqualsAndHashCodeCallSuper - - org.openrewrite.java.recipes.UseTreeRandomId - - org.openrewrite.staticanalysis.NeedBraces - #- org.openrewrite.staticanalysis.RemoveSystemOutPrintln - --- - type: specs.openrewrite.org/v1beta/recipe - name: org.openrewrite.recipes.RecipeTestingBestPracticesSubset - displayName: Recipe testing best practices - description: Best practices for testing recipes. - preconditions: - - org.openrewrite.java.search.FindTypes: - fullyQualifiedTypeName: org.openrewrite.test.RewriteTest - checkAssignability: true - recipeList: - - org.openrewrite.java.recipes.RewriteTestClassesShouldNotBePublic - #- org.openrewrite.java.recipes.SelectRecipeExamples - - org.openrewrite.java.recipes.SourceSpecTextBlockIndentation - - org.openrewrite.java.testing.cleanup.RemoveTestPrefix - - org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic - - org.openrewrite.staticanalysis.NeedBraces - - org.openrewrite.staticanalysis.RemoveSystemOutPrintln - --- - type: specs.openrewrite.org/v1beta/recipe - name: org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset - displayName: Recipe nullability best practices - description: Use OpenRewrite internal nullability annotations; drop JetBrains annotations; use `package-info.java` instead. - recipeList: - - org.openrewrite.staticanalysis.NullableOnMethodReturnType - - org.openrewrite.java.RemoveAnnotation: - annotationPattern: '@org.jetbrains.annotations.NotNull' - - org.openrewrite.java.RemoveAnnotation: - annotationPattern: '@jakarta.annotation.Nonnull' - #- org.openrewrite.java.jspecify.MigrateToJspecify diff --git a/rewrite-core/src/main/java/org/openrewrite/GitRemote.java b/rewrite-core/src/main/java/org/openrewrite/GitRemote.java index 6816c8e9c8b..fd42a076e68 100644 --- a/rewrite-core/src/main/java/org/openrewrite/GitRemote.java +++ b/rewrite-core/src/main/java/org/openrewrite/GitRemote.java @@ -113,51 +113,64 @@ public Parser() { * @return the clone url */ public URI toUri(GitRemote remote, String protocol) { + return buildUri(remote.service, remote.origin, remote.path, protocol); + } + + /** + * Build a {@link URI} clone url from components, if that protocol is supported (configured) by the matched server + * + * @param service the type of SCM service + * @param origin the origin of the SCM service, any protocol will be stripped (and not used for matching) + * @param path the path to the repository + * @param protocol the protocol to use. Supported protocols: ssh, http, https + * @return the clone URL if it could be created. + * @throws IllegalArgumentException if the protocol is not supported by the server. + */ + public URI buildUri(Service service, String origin, String path, String protocol) { if (!ALLOWED_PROTOCOLS.contains(protocol)) { throw new IllegalArgumentException("Invalid protocol: " + protocol + ". Must be one of: " + ALLOWED_PROTOCOLS); } URI selectedBaseUrl; - - if (remote.service == Service.Unknown) { - if (PORT_PATTERN.matcher(remote.origin).find()) { - throw new IllegalArgumentException("Unable to determine protocol/port combination for an unregistered origin with a port: " + remote.origin); + if (service == Service.Unknown) { + if (PORT_PATTERN.matcher(origin).find()) { + throw new IllegalArgumentException("Unable to determine protocol/port combination for an unregistered origin with a port: " + origin); } - selectedBaseUrl = URI.create(protocol + "://" + stripProtocol(remote.origin)); + selectedBaseUrl = URI.create(protocol + "://" + stripProtocol(origin)); } else { selectedBaseUrl = servers.stream() .filter(server -> server.allOrigins() .stream() - .anyMatch(origin -> origin.equalsIgnoreCase(stripProtocol(remote.origin))) + .anyMatch(o -> o.equalsIgnoreCase(stripProtocol(origin))) ) .flatMap(server -> server.getUris().stream()) - .filter(uri -> uri.getScheme().equals(protocol)) + .filter(uri -> Parser.normalize(uri).getScheme().equals(protocol)) .findFirst() .orElseGet(() -> { - URI normalizedUri = Parser.normalize(remote.origin); + URI normalizedUri = Parser.normalize(origin); if (!normalizedUri.getScheme().equals(protocol)) { - throw new IllegalStateException("No matching server found that supports ssh for origin: " + remote.origin); + throw new IllegalArgumentException("Unable to build clone URL. No matching server found that supports " + protocol + " for origin: " + origin); } return normalizedUri; }); } - String path = remote.path.replaceFirst("^/", ""); + path = path.replaceFirst("^/", ""); boolean ssh = protocol.equals("ssh"); - switch (remote.service) { + switch (service) { case Bitbucket: if (!ssh) { - path = "scm/" + remote.path; + path = "scm/" + path; } break; case AzureDevOps: if (ssh) { - path = "v3/" + remote.path; + path = "v3/" + path; } else { - path = remote.path.replaceFirst("([^/]+)/([^/]+)/(.*)", "$1/$2/_git/$3"); + path = path.replaceFirst("([^/]+)/([^/]+)/(.*)", "$1/$2/_git/$3"); } break; } - if (remote.service != Service.AzureDevOps) { + if (service != Service.AzureDevOps) { path += ".git"; } String maybeSlash = selectedBaseUrl.toString().endsWith("/") ? "" : "/"; @@ -203,11 +216,18 @@ public Parser registerRemote(Service service, String origin) { return this; } + /** + * Find a registered remote server by an origin. + * + * @param origin the origin of the server. Any protocol will be stripped (and not used to match) + * @return The server if found, or an unknown type server with a normalized url/origin if not found. + */ public RemoteServer findRemoteServer(String origin) { - return servers.stream().filter(server -> server.origin.equalsIgnoreCase(origin)) + String strippedOrigin = stripProtocol(origin); + return servers.stream().filter(server -> server.origin.equalsIgnoreCase(strippedOrigin)) .findFirst() .orElseGet(() -> { - URI normalizedUri = normalize(origin); + URI normalizedUri = normalize(strippedOrigin); String normalizedOrigin = normalizedUri.getHost() + maybePort(normalizedUri.getPort(), normalizedUri.getScheme()); return new RemoteServer(Service.Unknown, normalizedOrigin, normalizedUri); }); @@ -281,6 +301,10 @@ private String repositoryPath(RemoteServerMatch match, URI normalizedUri) { private static final Pattern PORT_PATTERN = Pattern.compile(":\\d+(/.+)(/.+)+"); + static URI normalize(URI url) { + return normalize(url.toString()); + } + static URI normalize(String url) { try { URIish uri = new URIish(url); @@ -374,10 +398,11 @@ public Set allOrigins() { Set origins = new LinkedHashSet<>(); origins.add(origin); for (URI uri : uris) { - URI normalized = Parser.normalize(uri.toString()); + URI normalized = Parser.normalize(uri); origins.add(Parser.stripProtocol(normalized.toString())); } return origins; } + } } diff --git a/rewrite-core/src/main/java/org/openrewrite/text/Find.java b/rewrite-core/src/main/java/org/openrewrite/text/Find.java index 90450be6d64..e0503c6d73a 100644 --- a/rewrite-core/src/main/java/org/openrewrite/text/Find.java +++ b/rewrite-core/src/main/java/org/openrewrite/text/Find.java @@ -26,9 +26,7 @@ import org.openrewrite.remote.Remote; import org.openrewrite.table.TextMatches; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -91,6 +89,22 @@ public String getDescription() { @Nullable String filePattern; + private static Deque findAllNewLineIndexes(String input, int offset) { + ArrayDeque indexes = new ArrayDeque<>(); + int index = input.lastIndexOf('\n', offset); // Find the first occurrence + if (index != -1) { + indexes.add(index); + } + + index = input.indexOf('\n', offset); // Find occurrence after the offset + while (index != -1) { + indexes.add(index); // Add the index to the list + index = input.indexOf('\n', index + 1); // Find the next occurrence + } + + return indexes; + } + @Override public TreeVisitor getVisitor() { @@ -123,24 +137,42 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { return sourceFile; } matcher.reset(); + + String sourceFilePath = sourceFile.getSourcePath().toString(); + List snippets = new ArrayList<>(); int previousEnd = 0; + + Deque newlineIndexes = null; + int lastNewLineIndex = -1; + while (matcher.find()) { + if (newlineIndexes == null) { + newlineIndexes = findAllNewLineIndexes(rawText, matcher.start()); + } + int matchStart = matcher.start(); snippets.add(snippet(rawText.substring(previousEnd, matchStart))); snippets.add(SearchResult.found(snippet(rawText.substring(matchStart, matcher.end())))); previousEnd = matcher.end(); - int startLine = Math.max(0, rawText.substring(0, matchStart).lastIndexOf('\n') + 1); + while (!newlineIndexes.isEmpty() && newlineIndexes.peek() < matchStart) { + lastNewLineIndex = newlineIndexes.pop(); + } + int startLine = Math.max(0, lastNewLineIndex + 1); + int endLine = rawText.indexOf('\n', matcher.end()); if (endLine == -1) { endLine = rawText.length(); } textMatches.insertRow(ctx, new TextMatches.Row( - sourceFile.getSourcePath().toString(), - rawText.substring(startLine, matcher.start()) + "~~>" + - rawText.substring(matcher.start(), endLine) + sourceFilePath, + new StringBuilder(endLine - startLine + 3) + .append(rawText, startLine, matcher.start()) + .append("~~>") + .append(rawText, matcher.start(), endLine) + .toString() )); } snippets.add(snippet(rawText.substring(previousEnd))); @@ -160,8 +192,8 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { return visitor; } - private static PlainText.Snippet snippet(String text) { return new PlainText.Snippet(Tree.randomId(), Markers.EMPTY, text); } + } diff --git a/rewrite-core/src/test/java/org/openrewrite/GitRemoteTest.java b/rewrite-core/src/test/java/org/openrewrite/GitRemoteTest.java index 410eb17df9b..37581c2f934 100644 --- a/rewrite-core/src/test/java/org/openrewrite/GitRemoteTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/GitRemoteTest.java @@ -232,11 +232,21 @@ void shouldNotStripJgit() { assertThat(remote.getPath()).isEqualTo("openrewrite/jgit"); } + @Test + void shouldNotReplaceExistingWellKnownServer(){ + GitRemote.Parser parser = new GitRemote.Parser() + .registerRemote(GitRemote.Service.GitHub, URI.create("https://github.com"), List.of(URI.create("ssh://notgithub.com"))); + + assertThat(parser.findRemoteServer("github.com").getUris()) + .containsExactlyInAnyOrder(URI.create("https://github.com"), URI.create("ssh://git@github.com")); + } + @Test void findRemote() { GitRemote.Parser parser = new GitRemote.Parser() .registerRemote(GitRemote.Service.Bitbucket, URI.create("scm.company.com/stash"), Collections.emptyList()); assertThat(parser.findRemoteServer("github.com").getService()).isEqualTo(GitRemote.Service.GitHub); + assertThat(parser.findRemoteServer("https://github.com").getService()).isEqualTo(GitRemote.Service.GitHub); assertThat(parser.findRemoteServer("gitlab.com").getService()).isEqualTo(GitRemote.Service.GitLab); assertThat(parser.findRemoteServer("bitbucket.org").getService()).isEqualTo(GitRemote.Service.BitbucketCloud); assertThat(parser.findRemoteServer("dev.azure.com").getService()).isEqualTo(GitRemote.Service.AzureDevOps); @@ -265,18 +275,18 @@ void parseOriginCaseInsensitive(String cloneUrl, String expectedOrigin, String e @ParameterizedTest @CsvSource(textBlock = """ - GitHub, GitHub - GITLAB, GitLab - bitbucket, Bitbucket - BitbucketCloud, BitbucketCloud - Bitbucket Cloud, BitbucketCloud - BITBUCKET_CLOUD, BitbucketCloud - AzureDevOps, AzureDevOps - AZURE_DEVOPS, AzureDevOps - Azure DevOps, AzureDevOps - idontknow, Unknown - """) - void findServiceForName(String name, GitRemote.Service service){ + GitHub, GitHub + GITLAB, GitLab + bitbucket, Bitbucket + BitbucketCloud, BitbucketCloud + Bitbucket Cloud, BitbucketCloud + BITBUCKET_CLOUD, BitbucketCloud + AzureDevOps, AzureDevOps + AZURE_DEVOPS, AzureDevOps + Azure DevOps, AzureDevOps + idontknow, Unknown + """) + void findServiceForName(String name, GitRemote.Service service) { assertThat(GitRemote.Service.forName(name)).isEqualTo(service); } diff --git a/rewrite-core/src/test/java/org/openrewrite/text/FindTest.java b/rewrite-core/src/test/java/org/openrewrite/text/FindTest.java index 0bfae8cadbb..b887986efb1 100644 --- a/rewrite-core/src/test/java/org/openrewrite/text/FindTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/text/FindTest.java @@ -111,4 +111,121 @@ void caseInsensitive() { ) ); } + + @Test + void regexBasicMultiLine() { + rewriteRun( + spec -> spec.recipe(new Find("[T\\s]", true, true, true, null, null)), + text( + """ + This is\ttext. + This is\ttext. + """, + """ + ~~>This~~> is~~>\ttext.~~> + ~~>This~~> is~~>\ttext. + """ + ) + ); + } + + @Test + void regexWithoutMultilineAndDotall() { + rewriteRun( + spec -> spec.recipe(new Find("^This.*below\\.$", true, true, false, false, null)), + text( + """ + This is text. + This is a line below. + This is a line above. + This is text. + This is a line below. + """ + ) + ); + } + + @Test + void regexMatchingWhitespaceWithoutMultilineWithDotall() { + rewriteRun( + spec -> spec.recipe(new Find("One.Two$", true, true, false, true, null)), + //language=csv + text( // the `.` above matches the space character on the same line + """ + Zero + One Two + Three + """ + ) + ); + } + + @Test + void regexWithoutMultilineAndWithDotAll() { + rewriteRun( + spec -> spec.recipe(new Find("^This.*below\\.$", true, true, false, true, null)), + text( + """ + This is text. + This is a line below. + This is a line above. + This is text. + This is a line below. + """, + """ + ~~>This is text. + This is a line below. + This is a line above. + This is text. + This is a line below. + """ + ) + ); + } + + @Test + void regexWithMultilineAndWithoutDotall() { + rewriteRun( + spec -> spec.recipe(new Find("^This.*below\\.$", true, true, true, false, null)), + text( + """ + This is text. + This is a line below. + This is a line above. + This is text. + This is a line below. + """, + """ + This is text. + ~~>This is a line below. + This is a line above. + This is text. + ~~>This is a line below. + """ + ) + ); + } + + @Test + void regexWithBothMultilineAndDotAll() { + rewriteRun( + spec -> spec.recipe(new Find("^This.*below\\.$", true, true, true, true, null)), + text( + """ + The first line. + This is a line below. + This is a line above. + This is text. + This is a line below. + """, + """ + The first line. + ~~>This is a line below. + This is a line above. + This is text. + This is a line below. + """ + ) + ); + } } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java index 0ff50885dee..9312fa96c3b 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java @@ -180,14 +180,16 @@ public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionCon public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .groupId(oldGroupId) + .artifactId(oldArtifactId); if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } List depArgs = m.getArguments(); - if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry) { + if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry || depArgs.get(0) instanceof G.MapLiteral) { m = updateDependency(m, ctx); } else if (depArgs.get(0) instanceof J.MethodInvocation && (((J.MethodInvocation) depArgs.get(0)).getSimpleName().equals("platform") || @@ -204,7 +206,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte String gav = (String) ((J.Literal) depArgs.get(0)).getValue(); if (gav != null) { Dependency original = DependencyStringNotationConverter.parse(gav); - if (original != null && depMatcher.matches(original.getGroupId(), original.getArtifactId())) { + if (original != null) { Dependency updated = original; if (!StringUtils.isBlank(newGroupId) && !updated.getGroupId().equals(newGroupId)) { updated = updated.withGroupId(newGroupId); @@ -238,7 +240,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte J.Literal literal = (J.Literal) strings.get(0); Dependency original = DependencyStringNotationConverter.parse((String) requireNonNull(literal.getValue())); - if (original != null && depMatcher.matches(original.getGroupId(), original.getArtifactId())) { + if (original != null) { Dependency updated = original; if (!StringUtils.isBlank(newGroupId) && !updated.getGroupId().equals(newGroupId)) { updated = updated.withGroupId(newGroupId); @@ -352,6 +354,92 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte return arg; })); } + } else if (m.getArguments().get(0) instanceof G.MapLiteral) { + G.MapLiteral map = (G.MapLiteral) depArgs.get(0); + G.MapEntry groupEntry = null; + G.MapEntry artifactEntry = null; + G.MapEntry versionEntry = null; + String groupId = null; + String artifactId = null; + String version = null; + + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + switch (keyValue) { + case "group": + groupEntry = arg; + groupId = valueValue; + break; + case "name": + artifactEntry = arg; + artifactId = valueValue; + break; + case "version": + versionEntry = arg; + version = valueValue; + break; + } + } + if (groupId == null || artifactId == null) { + return m; + } + if (!depMatcher.matches(groupId, artifactId)) { + return m; + } + String updatedGroupId = groupId; + if (!StringUtils.isBlank(newGroupId) && !updatedGroupId.equals(newGroupId)) { + updatedGroupId = newGroupId; + } + String updatedArtifactId = artifactId; + if (!StringUtils.isBlank(newArtifactId) && !updatedArtifactId.equals(newArtifactId)) { + updatedArtifactId = newArtifactId; + } + String updatedVersion = version; + if (!StringUtils.isBlank(newVersion) && (!StringUtils.isBlank(version) || Boolean.TRUE.equals(overrideManagedVersion))) { + String resolvedVersion; + try { + resolvedVersion = new DependencyVersionSelector(null, gradleProject, null) + .select(new GroupArtifact(updatedGroupId, updatedArtifactId), m.getSimpleName(), newVersion, versionPattern, ctx); + } catch (MavenDownloadingException e) { + return e.warn(m); + } + if (resolvedVersion != null && !resolvedVersion.equals(updatedVersion)) { + updatedVersion = resolvedVersion; + } + } + + if (!updatedGroupId.equals(groupId) || !updatedArtifactId.equals(artifactId) || updatedVersion != null && !updatedVersion.equals(version)) { + G.MapEntry finalGroup = groupEntry; + String finalGroupIdValue = updatedGroupId; + G.MapEntry finalArtifact = artifactEntry; + String finalArtifactIdValue = updatedArtifactId; + G.MapEntry finalVersion = versionEntry; + String finalVersionValue = updatedVersion; + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> { + if (e == finalGroup) { + return finalGroup.withValue(ChangeStringLiteral.withStringValue((J.Literal) finalGroup.getValue(), finalGroupIdValue)); + } + if (e == finalArtifact) { + return finalArtifact.withValue(ChangeStringLiteral.withStringValue((J.Literal) finalArtifact.getValue(), finalArtifactIdValue)); + } + if (e == finalVersion) { + return finalVersion.withValue(ChangeStringLiteral.withStringValue((J.Literal) finalVersion.getValue(), finalVersionValue)); + } + return e; + })); + })); + } } return m; diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java index ba98f950ca9..02e028fd305 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java @@ -25,21 +25,18 @@ import org.openrewrite.gradle.util.ChangeStringLiteral; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.StringUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; -import org.openrewrite.maven.tree.GroupArtifact; -import org.openrewrite.maven.tree.ResolvedDependency; import org.openrewrite.semver.DependencyMatcher; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Optional; import static java.util.Objects.requireNonNull; @@ -90,38 +87,42 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - final Map updatedDependencies = new HashMap<>(); + GradleProject gradleProject; @Override - public G visitCompilationUnit(G.CompilationUnit compilationUnit, ExecutionContext ctx) { - G.CompilationUnit cu = (G.CompilationUnit) super.visitCompilationUnit(compilationUnit, ctx); - if(cu != compilationUnit) { - cu = cu.withMarkers(cu.getMarkers().withMarkers(ListUtils.map(cu.getMarkers().getMarkers(), m -> { - if (m instanceof GradleProject) { - return updateModel((GradleProject) m, updatedDependencies); - } - return m; - }))); + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { + Optional maybeGp = cu.getMarkers().findFirst(GradleProject.class); + if (!maybeGp.isPresent()) { + return cu; + } + + gradleProject = maybeGp.get(); + + G.CompilationUnit g = super.visitCompilationUnit(cu, ctx); + if (g != cu) { + g = g.withMarkers(g.getMarkers().setByType(updateGradleModel(gradleProject))); } - return cu; + return g; } @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration) + .groupId(groupId) + .artifactId(artifactId); - if (!((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isBlank(configuration) || m.getSimpleName().equals(configuration)))) { + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } List depArgs = m.getArguments(); - if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry) { + if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry || depArgs.get(0) instanceof G.MapLiteral) { m = updateDependency(m); } else if (depArgs.get(0) instanceof J.MethodInvocation && (((J.MethodInvocation) depArgs.get(0)).getSimpleName().equals("platform") || @@ -138,11 +139,8 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { String gav = (String) ((J.Literal) depArgs.get(0)).getValue(); if (gav != null) { Dependency dependency = DependencyStringNotationConverter.parse(gav); - if (dependency != null && !newArtifactId.equals(dependency.getArtifactId()) && - ((dependency.getVersion() == null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) || - (dependency.getVersion() != null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())))) { + if (dependency != null && !newArtifactId.equals(dependency.getArtifactId())) { Dependency newDependency = dependency.withArtifactId(newArtifactId); - updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, newDependency.toStringNotation()))); } } @@ -151,10 +149,8 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { if (strings.size() >= 2 && strings.get(0) instanceof J.Literal) { Dependency dependency = DependencyStringNotationConverter.parse((String) requireNonNull(((J.Literal) strings.get(0)).getValue())); - if (dependency != null && !newArtifactId.equals(dependency.getArtifactId()) && - depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + if (dependency != null && !newArtifactId.equals(dependency.getArtifactId())) { Dependency newDependency = dependency.withArtifactId(newArtifactId); - updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); String replacement = newDependency.toStringNotation(); m = m.withArguments(ListUtils.mapFirst(depArgs, arg -> { G.GString gString = (G.GString) arg; @@ -166,7 +162,6 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { G.MapEntry artifactEntry = null; String groupId = null; String artifactId = null; - String version = null; String versionStringDelimiter = "'"; for (Expression e : depArgs) { @@ -192,13 +187,9 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { } artifactEntry = arg; artifactId = valueValue; - } else if ("version".equals(keyValue)) { - version = valueValue; } } - if (groupId == null || artifactId == null || - (version == null && !depMatcher.matches(groupId, artifactId)) || - (version != null && !depMatcher.matches(groupId, artifactId, version))) { + if (groupId == null || artifactId == null) { return m; } String delimiter = versionStringDelimiter; @@ -211,35 +202,86 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { } return arg; })); + } else if (depArgs.get(0) instanceof G.MapLiteral) { + G.MapLiteral map = (G.MapLiteral) depArgs.get(0); + G.MapEntry artifactEntry = null; + String groupId = null; + String artifactId = null; + + String versionStringDelimiter = "'"; + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue)) { + groupId = valueValue; + } else if ("name".equals(keyValue) && !newArtifactId.equals(valueValue)) { + if (value.getValueSource() != null) { + versionStringDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); + } + artifactEntry = arg; + artifactId = valueValue; + } + } + if (groupId == null || artifactId == null) { + return m; + } + String delimiter = versionStringDelimiter; + G.MapEntry finalArtifact = artifactEntry; + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> { + if (e == finalArtifact) { + return finalArtifact.withValue(((J.Literal) finalArtifact.getValue()) + .withValue(newArtifactId) + .withValueSource(delimiter + newArtifactId + delimiter)); + } + return e; + })); + })); } return m; } - }); - } - private GradleProject updateModel(GradleProject gp, Map updatedDependencies) { - Map nameToConfigurations = gp.getNameToConfiguration(); - Map updatedNameToConfigurations = new HashMap<>(); - for (Map.Entry nameToConfiguration : nameToConfigurations.entrySet()) { - String configurationName = nameToConfiguration.getKey(); - GradleDependencyConfiguration configuration = nameToConfiguration.getValue(); - - List newRequested = configuration.getRequested() - .stream() - .map(requested -> requested.withGav(requested.getGav() - .withGroupArtifact(updatedDependencies.getOrDefault(requested.getGav().asGroupArtifact(), requested.getGav().asGroupArtifact())))) - .collect(Collectors.toList()); - - List newResolved = configuration.getResolved().stream() - .map(resolved -> - resolved.withGav(resolved.getGav() - .withGroupArtifact(updatedDependencies.getOrDefault(resolved.getGav().asGroupArtifact(), resolved.getGav().asGroupArtifact())))) - .collect(Collectors.toList()); - - updatedNameToConfigurations.put(configurationName, configuration.withRequested(newRequested).withDirectResolved(newResolved)); - } - - return gp.withNameToConfiguration(updatedNameToConfigurations); + private GradleProject updateGradleModel(GradleProject gp) { + Map nameToConfiguration = gp.getNameToConfiguration(); + Map newNameToConfiguration = new HashMap<>(nameToConfiguration.size()); + boolean anyChanged = false; + for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { + if (!StringUtils.isBlank(configuration) && configuration.equals(gdc.getName())) { + newNameToConfiguration.put(gdc.getName(), gdc); + continue; + } + + GradleDependencyConfiguration newGdc = gdc; + newGdc = newGdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { + if (depMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { + return requested.withGav(requested.getGav().withArtifactId(newArtifactId)); + } + return requested; + })); + newGdc = newGdc.withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> { + if (depMatcher.matches(resolved.getGroupId(), resolved.getArtifactId())) { + return resolved.withGav(resolved.getGav().withArtifactId(newArtifactId)); + } + return resolved; + })); + anyChanged |= newGdc != gdc; + newNameToConfiguration.put(newGdc.getName(), newGdc); + } + if (anyChanged) { + gp = gp.withNameToConfiguration(newNameToConfiguration); + } + return gp; + } + }); } } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java index c11f7ccd68d..007f0c45ef4 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java @@ -19,21 +19,21 @@ import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; +import org.openrewrite.gradle.marker.GradleDependencyConfiguration; +import org.openrewrite.gradle.marker.GradleProject; import org.openrewrite.gradle.trait.GradleDependency; import org.openrewrite.gradle.util.ChangeStringLiteral; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.StringUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.semver.DependencyMatcher; -import java.util.List; -import java.util.Objects; +import java.util.*; import static java.util.Objects.requireNonNull; @@ -86,16 +86,36 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); + + GradleProject gradleProject; @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { + Optional maybeGp = cu.getMarkers().findFirst(GradleProject.class); + if (!maybeGp.isPresent()) { + return cu; + } + + gradleProject = maybeGp.get(); - if (!((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isBlank(configuration) || m.getSimpleName().equals(configuration)))) { + G.CompilationUnit g = super.visitCompilationUnit(cu, ctx); + if (g != cu) { + g = g.withMarkers(g.getMarkers().setByType(updateGradleModel(gradleProject))); + } + return g; + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration) + .groupId(groupId) + .artifactId(artifactId); + + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } @@ -104,8 +124,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) String gav = (String) ((J.Literal) depArgs.get(0)).getValue(); if (gav != null) { Dependency dependency = DependencyStringNotationConverter.parse(gav); - if (dependency != null && dependency.getVersion() != null && !Objects.equals(newClassifier, dependency.getClassifier()) && - depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())) { + if (dependency != null && dependency.getVersion() != null && !Objects.equals(newClassifier, dependency.getClassifier())) { Dependency newDependency = dependency.withClassifier(newClassifier); m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, newDependency.toStringNotation()))); } @@ -157,10 +176,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) } index++; } - if (groupId == null || artifactId == null || - (version == null && !depMatcher.matches(groupId, artifactId)) || - (version != null && !depMatcher.matches(groupId, artifactId, version)) || - Objects.equals(newClassifier, classifier)) { + if (groupId == null || artifactId == null || Objects.equals(newClassifier, classifier)) { return m; } @@ -187,12 +203,118 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) })); } } + } else if (depArgs.get(0) instanceof G.MapLiteral) { + G.MapLiteral map = (G.MapLiteral) depArgs.get(0); + G.MapEntry classifierEntry = null; + String groupId = null; + String artifactId = null; + String classifier = null; + + String groupDelimiter = "'"; + G.MapEntry mapEntry = null; + String classifierStringDelimiter = null; + int index = 0; + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue)) { + groupId = valueValue; + if (value.getValueSource() != null) { + groupDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); + } + } else if ("name".equals(keyValue)) { + if (index > 0 && mapEntry == null) { + mapEntry = arg; + } + artifactId = valueValue; + } else if ("classifier".equals(keyValue)) { + if (value.getValueSource() != null) { + classifierStringDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); + } + classifierEntry = arg; + classifier = valueValue; + } + index++; + } + if (groupId == null || artifactId == null || Objects.equals(newClassifier, classifier)) { + return m; + } + if (classifier == null) { + String delimiter = groupDelimiter; + G.MapEntry finalMapEntry = mapEntry; + J.Literal keyLiteral = new J.Literal(Tree.randomId(), mapEntry == null ? Space.EMPTY : mapEntry.getKey().getPrefix(), Markers.EMPTY, "classifier", "classifier", null, JavaType.Primitive.String); + J.Literal valueLiteral = new J.Literal(Tree.randomId(), mapEntry == null ? Space.EMPTY : mapEntry.getValue().getPrefix(), Markers.EMPTY, newClassifier, delimiter + newClassifier + delimiter, null, JavaType.Primitive.String); + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.concat(mapLiteral.getElements(), new G.MapEntry(Tree.randomId(), finalMapEntry == null ? Space.EMPTY : finalMapEntry.getPrefix(), Markers.EMPTY, JRightPadded.build(keyLiteral), valueLiteral, null))); + })); + } else { + G.MapEntry finalClassifier = classifierEntry; + if (newClassifier == null) { + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> e == finalClassifier ? null : e)); + })); + } else { + String delimiter = classifierStringDelimiter; // `classifierStringDelimiter` cannot be null + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> { + if (e == finalClassifier) { + return finalClassifier.withValue(((J.Literal) finalClassifier.getValue()) + .withValue(newClassifier) + .withValueSource(delimiter + newClassifier + delimiter)); + } + return e; + })); + })); + } + } } return m; } + + private GradleProject updateGradleModel(GradleProject gp) { + Map nameToConfiguration = gp.getNameToConfiguration(); + Map newNameToConfiguration = new HashMap<>(nameToConfiguration.size()); + boolean anyChanged = false; + for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { + if (!StringUtils.isBlank(configuration) && !configuration.equals(gdc.getName())) { + newNameToConfiguration.put(gdc.getName(), gdc); + continue; + } + + GradleDependencyConfiguration newGdc = gdc; + newGdc = newGdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { + if (depMatcher.matches(requested.getGroupId(), requested.getArtifactId()) && !Objects.equals(requested.getClassifier(), newClassifier)) { + return requested.withClassifier(newClassifier); + } + return requested; + })); + newGdc = newGdc.withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> { + if (depMatcher.matches(resolved.getGroupId(), resolved.getArtifactId()) && !Objects.equals(resolved.getClassifier(), newClassifier)) { + return resolved.withClassifier(newClassifier); + } + return resolved; + })); + anyChanged |= newGdc != gdc; + newNameToConfiguration.put(newGdc.getName(), newGdc); + } + if (anyChanged) { + gp = gp.withNameToConfiguration(newNameToConfiguration); + } + return gp; + } }); } - } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java index dd2e0275e5c..ad6747d3a41 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java @@ -22,9 +22,8 @@ import org.openrewrite.gradle.trait.GradleDependency; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; -import org.openrewrite.internal.StringUtils; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; @@ -86,16 +85,18 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { + // Still need to be able to change the configuration for project dependencies which are not yet supported by the `GradleDependency.Matcher` final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration); - if (!((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isBlank(configuration) || m.getSimpleName().equals(configuration)))) { + if (!(gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m))) { return m; } @@ -127,19 +128,65 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) if (dependency == null || !dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { return m; } - } else if (args.get(0) instanceof G.MapEntry && args.size() >= 2) { - Expression groupValue = ((G.MapEntry) args.get(0)).getValue(); - Expression artifactValue = ((G.MapEntry) args.get(1)).getValue(); - if (!(groupValue instanceof J.Literal) || !(artifactValue instanceof J.Literal)) { + } else if (args.get(0) instanceof G.MapEntry) { + if (args.size() < 2) { return m; } - J.Literal groupLiteral = (J.Literal) groupValue; - J.Literal artifactLiteral = (J.Literal) artifactValue; - if (!(groupLiteral.getValue() instanceof String) || !(artifactLiteral.getValue() instanceof String)) { + + String groupId = null; + String artifactId = null; + for (Expression e : args) { + if (!(e instanceof G.MapEntry)) { + continue; + } + G.MapEntry arg = (G.MapEntry) e; + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue)) { + groupId = valueValue; + } else if ("name".equals(keyValue)) { + artifactId = valueValue; + } + } + + if (artifactId == null || !dependencyMatcher.matches(groupId, artifactId)) { return m; } + } else if (args.get(0) instanceof G.MapLiteral) { + if (args.size() < 2) { + return m; + } + + G.MapLiteral map = (G.MapLiteral) args.get(0); + String groupId = null; + String artifactId = null; + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue)) { + groupId = valueValue; + } else if ("name".equals(keyValue)) { + artifactId = valueValue; + } + } - if (!dependencyMatcher.matches((String) groupLiteral.getValue(), (String) artifactLiteral.getValue())) { + if (artifactId == null || !dependencyMatcher.matches(groupId, artifactId)) { return m; } } else if (args.get(0) instanceof J.MethodInvocation) { diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java index a6a617f627b..dc103c00f75 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java @@ -23,19 +23,15 @@ import org.openrewrite.gradle.util.ChangeStringLiteral; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; -import org.openrewrite.internal.StringUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.semver.DependencyMatcher; import java.util.List; -import static java.util.Objects.requireNonNull; - @Value @EqualsAndHashCode(callSuper = false) public class ChangeDependencyExtension extends Recipe { @@ -83,17 +79,17 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { - final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration) + .groupId(groupId) + .artifactId(artifactId); - if (!((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isBlank(configuration) || m.getSimpleName().equals(configuration)))) { + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } @@ -102,9 +98,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) String gav = (String) ((J.Literal) depArgs.get(0)).getValue(); if (gav != null) { Dependency dependency = DependencyStringNotationConverter.parse(gav); - if (dependency != null && !newExtension.equals(dependency.getExt()) && - ((dependency.getVersion() == null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) || - (dependency.getVersion() != null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())))) { + if (dependency != null && !newExtension.equals(dependency.getExt())) { Dependency newDependency = dependency.withExt(newExtension); m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, newDependency.toStringNotation()))); } @@ -113,7 +107,6 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) G.MapEntry extensionEntry = null; String groupId = null; String artifactId = null; - String version = null; String extension = null; String extensionStringDelimiter = "'"; @@ -136,8 +129,6 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) groupId = valueValue; } else if ("name".equals(keyValue)) { artifactId = valueValue; - } else if ("version".equals(keyValue)) { - version = valueValue; } else if ("ext".equals(keyValue) && !newExtension.equals(valueValue)) { if (value.getValueSource() != null) { extensionStringDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); @@ -146,10 +137,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) extension = valueValue; } } - if (groupId == null || artifactId == null || - (version == null && !depMatcher.matches(groupId, artifactId)) || - (version != null && !depMatcher.matches(groupId, artifactId, version)) || - extension == null) { + if (groupId == null || artifactId == null || extension == null) { return m; } String delimiter = extensionStringDelimiter; @@ -162,6 +150,53 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) } return arg; })); + } else if (depArgs.get(0) instanceof G.MapLiteral) { + G.MapLiteral map = (G.MapLiteral) depArgs.get(0); + G.MapEntry extensionEntry = null; + String groupId = null; + String artifactId = null; + String extension = null; + + String extensionStringDelimiter = "'"; + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue)) { + groupId = valueValue; + } else if ("name".equals(keyValue)) { + artifactId = valueValue; + } else if ("ext".equals(keyValue) && !newExtension.equals(valueValue)) { + if (value.getValueSource() != null) { + extensionStringDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); + } + extensionEntry = arg; + extension = valueValue; + } + } + if (groupId == null || artifactId == null || extension == null) { + return m; + } + String delimiter = extensionStringDelimiter; + G.MapEntry finalExtension = extensionEntry; + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> { + if (e == finalExtension) { + return finalExtension.withValue(((J.Literal) finalExtension.getValue()) + .withValue(newExtension) + .withValueSource(delimiter + newExtension + delimiter)); + } + return e; + })); + })); } return m; diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java index 550b71f1492..ade503f67d1 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java @@ -25,21 +25,18 @@ import org.openrewrite.gradle.util.ChangeStringLiteral; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.StringUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; -import org.openrewrite.maven.tree.GroupArtifact; -import org.openrewrite.maven.tree.ResolvedDependency; import org.openrewrite.semver.DependencyMatcher; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Optional; import static java.util.Objects.requireNonNull; @@ -90,38 +87,42 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - final Map updatedDependencies = new HashMap<>(); + GradleProject gradleProject; @Override - public G visitCompilationUnit(G.CompilationUnit compilationUnit, ExecutionContext ctx) { - G.CompilationUnit cu = (G.CompilationUnit) super.visitCompilationUnit(compilationUnit, ctx); - if(cu != compilationUnit) { - cu = cu.withMarkers(cu.getMarkers().withMarkers(ListUtils.map(cu.getMarkers().getMarkers(), m -> { - if (m instanceof GradleProject) { - return updateModel((GradleProject) m, updatedDependencies); - } - return m; - }))); + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { + Optional maybeGp = cu.getMarkers().findFirst(GradleProject.class); + if (!maybeGp.isPresent()) { + return cu; + } + + gradleProject = maybeGp.get(); + + G.CompilationUnit g = super.visitCompilationUnit(cu, ctx); + if (g != cu) { + g = g.withMarkers(g.getMarkers().setByType(updateGradleModel(gradleProject))); } - return cu; + return g; } @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); + GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration) + .groupId(groupId) + .artifactId(artifactId); - if (!((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isBlank(configuration) || m.getSimpleName().equals(configuration)))) { + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } List depArgs = m.getArguments(); - if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry) { + if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry || depArgs.get(0) instanceof G.MapLiteral) { m = updateDependency(m); } else if (depArgs.get(0) instanceof J.MethodInvocation && (((J.MethodInvocation) depArgs.get(0)).getSimpleName().equals("platform") || @@ -138,11 +139,8 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { String gav = (String) ((J.Literal) depArgs.get(0)).getValue(); if (gav != null) { Dependency dependency = DependencyStringNotationConverter.parse(gav); - if (dependency != null && !newGroupId.equals(dependency.getGroupId()) && - ((dependency.getVersion() == null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) || - (dependency.getVersion() != null && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())))) { + if (dependency != null && !newGroupId.equals(dependency.getGroupId())) { Dependency newDependency = dependency.withGroupId(newGroupId); - updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, newDependency.toStringNotation()))); } } @@ -151,10 +149,8 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { if (strings.size() >= 2 && strings.get(0) instanceof J.Literal) { Dependency dependency = DependencyStringNotationConverter.parse((String) requireNonNull(((J.Literal) strings.get(0)).getValue())); - if (dependency != null && !newGroupId.equals(dependency.getGroupId()) && - depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + if (dependency != null && !newGroupId.equals(dependency.getGroupId())) { Dependency newDependency = dependency.withGroupId(newGroupId); - updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); String replacement = newDependency.toStringNotation(); m = m.withArguments(ListUtils.mapFirst(depArgs, arg -> { G.GString gString = (G.GString) arg; @@ -166,7 +162,6 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { G.MapEntry groupEntry = null; String groupId = null; String artifactId = null; - String version = null; String versionStringDelimiter = "'"; for (Expression e : depArgs) { @@ -192,13 +187,9 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { groupId = valueValue; } else if ("name".equals(keyValue)) { artifactId = valueValue; - } else if ("version".equals(keyValue)) { - version = valueValue; } } - if (groupId == null || artifactId == null || - (version == null && !depMatcher.matches(groupId, artifactId)) || - (version != null && !depMatcher.matches(groupId, artifactId, version))) { + if (groupId == null || artifactId == null) { return m; } String delimiter = versionStringDelimiter; @@ -211,35 +202,86 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m) { } return arg; })); + } else if (depArgs.get(0) instanceof G.MapLiteral) { + G.MapLiteral map = (G.MapLiteral) depArgs.get(0); + G.MapEntry groupEntry = null; + String groupId = null; + String artifactId = null; + + String versionStringDelimiter = "'"; + for (G.MapEntry arg : map.getElements()) { + if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { + continue; + } + J.Literal key = (J.Literal) arg.getKey(); + J.Literal value = (J.Literal) arg.getValue(); + if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { + continue; + } + String keyValue = (String) key.getValue(); + String valueValue = (String) value.getValue(); + if ("group".equals(keyValue) && !newGroupId.equals(valueValue)) { + if (value.getValueSource() != null) { + versionStringDelimiter = value.getValueSource().substring(0, value.getValueSource().indexOf(valueValue)); + } + groupEntry = arg; + groupId = valueValue; + } else if ("name".equals(keyValue)) { + artifactId = valueValue; + } + } + if (groupId == null || artifactId == null) { + return m; + } + String delimiter = versionStringDelimiter; + G.MapEntry finalGroup = groupEntry; + m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), e -> { + if (e == finalGroup) { + return finalGroup.withValue(((J.Literal) finalGroup.getValue()) + .withValue(newGroupId) + .withValueSource(delimiter + newGroupId + delimiter)); + } + return e; + })); + })); } return m; } - }); - } - private GradleProject updateModel(GradleProject gp, Map updatedDependencies) { - Map nameToConfigurations = gp.getNameToConfiguration(); - Map updatedNameToConfigurations = new HashMap<>(); - for (Map.Entry nameToConfiguration : nameToConfigurations.entrySet()) { - String configurationName = nameToConfiguration.getKey(); - GradleDependencyConfiguration configuration = nameToConfiguration.getValue(); - - List newRequested = configuration.getRequested() - .stream() - .map(requested -> requested.withGav(requested.getGav() - .withGroupArtifact(updatedDependencies.getOrDefault(requested.getGav().asGroupArtifact(), requested.getGav().asGroupArtifact())))) - .collect(Collectors.toList()); - - List newResolved = configuration.getResolved().stream() - .map(resolved -> - resolved.withGav(resolved.getGav() - .withGroupArtifact(updatedDependencies.getOrDefault(resolved.getGav().asGroupArtifact(), resolved.getGav().asGroupArtifact())))) - .collect(Collectors.toList()); - - updatedNameToConfigurations.put(configurationName, configuration.withRequested(newRequested).withDirectResolved(newResolved)); - } - - return gp.withNameToConfiguration(updatedNameToConfigurations); + private GradleProject updateGradleModel(GradleProject gp) { + Map nameToConfiguration = gp.getNameToConfiguration(); + Map newNameToConfiguration = new HashMap<>(nameToConfiguration.size()); + boolean anyChanged = false; + for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { + if (!StringUtils.isBlank(configuration) && configuration.equals(gdc.getName())) { + newNameToConfiguration.put(gdc.getName(), gdc); + continue; + } + + GradleDependencyConfiguration newGdc = gdc; + newGdc = newGdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { + if (depMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { + return requested.withGav(requested.getGav().withGroupId(newGroupId)); + } + return requested; + })); + newGdc = newGdc.withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> { + if (depMatcher.matches(resolved.getGroupId(), resolved.getArtifactId())) { + return resolved.withGav(resolved.getGav().withGroupId(newGroupId)); + } + return resolved; + })); + anyChanged |= newGdc != gdc; + newNameToConfiguration.put(newGdc.getName(), newGdc); + } + if (anyChanged) { + gp = gp.withNameToConfiguration(newNameToConfiguration); + } + return gp; + } + }); } } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java index 984523ff5d8..301fb375987 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java @@ -25,7 +25,6 @@ import org.openrewrite.groovy.GroovyVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; @@ -53,15 +52,13 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - @Override public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); - if (!(gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m))) { + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } m = forBasicString(m); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseStringNotation.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseStringNotation.java index b3c17260f90..17f8b300a05 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseStringNotation.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseStringNotation.java @@ -23,7 +23,6 @@ import org.openrewrite.gradle.trait.GradleDependency; import org.openrewrite.groovy.GroovyVisitor; import org.openrewrite.groovy.tree.G; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; @@ -50,15 +49,13 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - @Override public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); - if (!(gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m))) { + if (!gradleDependencyMatcher.get(getCursor()).isPresent()) { return m; } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java index f38e853fbe6..b05ac29701b 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java @@ -15,7 +15,6 @@ */ package org.openrewrite.gradle; -import io.micrometer.core.instrument.util.StringUtils; import lombok.EqualsAndHashCode; import lombok.Value; import org.jspecify.annotations.Nullable; @@ -23,17 +22,14 @@ import org.openrewrite.gradle.marker.GradleDependencyConfiguration; import org.openrewrite.gradle.marker.GradleProject; import org.openrewrite.gradle.trait.GradleDependency; -import org.openrewrite.gradle.util.Dependency; -import org.openrewrite.gradle.util.DependencyStringNotationConverter; -import org.openrewrite.groovy.GroovyVisitor; +import org.openrewrite.groovy.GroovyIsoVisitor; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; -import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.tree.Expression; +import org.openrewrite.internal.StringUtils; import org.openrewrite.java.tree.J; import org.openrewrite.semver.DependencyMatcher; -import java.util.List; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -77,55 +73,35 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new IsBuildGradle<>(), new GroovyVisitor() { - final MethodMatcher dependencyDsl = new MethodMatcher("DependencyHandlerSpec *(..)"); - final DependencyMatcher dependencyMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); + return Preconditions.check(new IsBuildGradle<>(), new GroovyIsoVisitor() { + final GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher() + .configuration(configuration) + .groupId(groupId) + .artifactId(artifactId); + final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(groupId + ":" + artifactId).getValue()); + + GradleProject gradleProject; @Override - public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { - G.CompilationUnit g = (G.CompilationUnit) super.visitCompilationUnit(cu, ctx); - if (g == cu) { + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { + Optional maybeGp = cu.getMarkers().findFirst(GradleProject.class); + if (!maybeGp.isPresent()) { return cu; } - Optional maybeGp = g.getMarkers().findFirst(GradleProject.class); - if (!maybeGp.isPresent()) { - // Allow modification of freestanding scripts which do not carry a GradleProject marker - return g; - } + gradleProject = maybeGp.get(); - GradleProject gp = maybeGp.get(); - Map nameToConfiguration = gp.getNameToConfiguration(); - boolean anyChanged = false; - for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { - GradleDependencyConfiguration newGdc = gdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { - if (requested.getGroupId() != null && dependencyMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { - return null; - } - return requested; - })); - newGdc = newGdc.withDirectResolved(ListUtils.map(newGdc.getDirectResolved(), resolved -> { - if (dependencyMatcher.matches(resolved.getGroupId(), resolved.getArtifactId())) { - return null; - } - return resolved; - })); - nameToConfiguration.put(newGdc.getName(), newGdc); - anyChanged |= newGdc != gdc; - } - - if (!anyChanged) { - // instance was changed, but no marker update is needed - return g; + G.CompilationUnit g = super.visitCompilationUnit(cu, ctx); + if (g != cu) { + g = g.withMarkers(g.getMarkers().setByType(updateGradleModel(gradleProject))); } - - return g.withMarkers(g.getMarkers().setByType(gp.withNameToConfiguration(nameToConfiguration))); + return g; } @Override - public @Nullable J visitReturn(J.Return return_, ExecutionContext ctx) { - boolean dependencyInvocation = return_.getExpression() instanceof J.MethodInvocation && dependencyDsl.matches((J.MethodInvocation) return_.getExpression()); - J.Return r = (J.Return) super.visitReturn(return_, ctx); + public J.@Nullable Return visitReturn(J.Return return_, ExecutionContext ctx) { + boolean dependencyInvocation = return_.getExpression() instanceof J.MethodInvocation && gradleDependencyMatcher.get(return_.getExpression(), getCursor()).isPresent(); + J.Return r = super.visitReturn(return_, ctx); if (dependencyInvocation && r.getExpression() == null) { //noinspection DataFlowIssue return null; @@ -134,86 +110,46 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { } @Override - public @Nullable J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); - - GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); - - if ((gradleDependencyMatcher.get(getCursor()).isPresent() || dependencyDsl.matches(m)) && (StringUtils.isEmpty(configuration) || configuration.equals(m.getSimpleName()))) { - Expression firstArgument = m.getArguments().get(0); - if (firstArgument instanceof J.Literal || firstArgument instanceof G.GString || firstArgument instanceof G.MapEntry) { - //noinspection DataFlowIssue - return maybeRemoveDependency(m); - } else if (firstArgument instanceof J.MethodInvocation && - (((J.MethodInvocation) firstArgument).getSimpleName().equals("platform") || - ((J.MethodInvocation) firstArgument).getSimpleName().equals("enforcedPlatform"))) { - J after = maybeRemoveDependency((J.MethodInvocation) firstArgument); - if (after == null) { - //noinspection DataFlowIssue - return null; - } - } + public J.@Nullable MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); + + if (gradleDependencyMatcher.get(getCursor()).isPresent()) { + return null; } return m; } - private @Nullable J maybeRemoveDependency(J.MethodInvocation m) { - if (m.getArguments().get(0) instanceof G.GString) { - G.GString gString = (G.GString) m.getArguments().get(0); - List strings = gString.getStrings(); - if (strings.size() != 2 || !(strings.get(0) instanceof J.Literal) || !(strings.get(1) instanceof G.GString.Value)) { - return m; - } - J.Literal groupArtifact = (J.Literal) strings.get(0); - if (!(groupArtifact.getValue() instanceof String)) { - return m; - } - Dependency dependency = DependencyStringNotationConverter.parse((String) groupArtifact.getValue()); - if (dependency != null && dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { - return null; - } - } else if (m.getArguments().get(0) instanceof J.Literal) { - Object value = ((J.Literal) m.getArguments().get(0)).getValue(); - if(!(value instanceof String)) { - return null; - } - Dependency dependency = DependencyStringNotationConverter.parse((String) value); - if (dependency != null && dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { - return null; + private GradleProject updateGradleModel(GradleProject gp) { + Map nameToConfiguration = gp.getNameToConfiguration(); + Map newNameToConfiguration = new HashMap<>(nameToConfiguration.size()); + boolean anyChanged = false; + for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { + if (!StringUtils.isBlank(configuration) && configuration.equals(gdc.getName())) { + newNameToConfiguration.put(gdc.getName(), gdc); + continue; } - } else if (m.getArguments().get(0) instanceof G.MapEntry) { - String groupId = null; - String artifactId = null; - for (Expression e : m.getArguments()) { - if (!(e instanceof G.MapEntry)) { - continue; - } - G.MapEntry arg = (G.MapEntry) e; - if (!(arg.getKey() instanceof J.Literal) || !(arg.getValue() instanceof J.Literal)) { - continue; - } - J.Literal key = (J.Literal) arg.getKey(); - J.Literal value = (J.Literal) arg.getValue(); - if (!(key.getValue() instanceof String) || !(value.getValue() instanceof String)) { - continue; + GradleDependencyConfiguration newGdc = gdc; + newGdc = newGdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { + if (depMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { + return null; } - String keyValue = (String) key.getValue(); - String valueValue = (String) value.getValue(); - if ("group".equals(keyValue)) { - groupId = valueValue; - } else if ("name".equals(keyValue)) { - artifactId = valueValue; + return requested; + })); + newGdc = newGdc.withDirectResolved(ListUtils.map(gdc.getDirectResolved(), resolved -> { + if (depMatcher.matches(resolved.getGroupId(), resolved.getArtifactId())) { + return null; } - } - - if (groupId != null && artifactId != null && dependencyMatcher.matches(groupId, artifactId)) { - return null; - } + return resolved; + })); + anyChanged |= newGdc != gdc; + newNameToConfiguration.put(newGdc.getName(), newGdc); } - - return m; + if (anyChanged) { + gp = gp.withNameToConfiguration(newNameToConfiguration); + } + return gp; } }); } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveRedundantDependencyVersions.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveRedundantDependencyVersions.java new file mode 100644 index 00000000000..3aaa09f6a4e --- /dev/null +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveRedundantDependencyVersions.java @@ -0,0 +1,316 @@ +/* + * 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.jspecify.annotations.Nullable; +import org.openrewrite.*; +import org.openrewrite.gradle.marker.GradleDependencyConfiguration; +import org.openrewrite.gradle.marker.GradleProject; +import org.openrewrite.gradle.trait.GradleDependency; +import org.openrewrite.gradle.util.ChangeStringLiteral; +import org.openrewrite.gradle.util.Dependency; +import org.openrewrite.gradle.util.DependencyStringNotationConverter; +import org.openrewrite.groovy.GroovyIsoVisitor; +import org.openrewrite.groovy.tree.G; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.internal.StringUtils; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.maven.MavenDownloadingException; +import org.openrewrite.maven.internal.MavenPomDownloader; +import org.openrewrite.maven.tree.GroupArtifactVersion; +import org.openrewrite.maven.tree.ResolvedDependency; +import org.openrewrite.maven.tree.ResolvedPom; +import org.openrewrite.semver.ExactVersion; +import org.openrewrite.semver.LatestIntegration; +import org.openrewrite.semver.Semver; +import org.openrewrite.semver.VersionComparator; + +import java.util.*; + +@Value +@EqualsAndHashCode(callSuper = false) +public class RemoveRedundantDependencyVersions extends Recipe { + @Option(displayName = "Group", + description = "Group glob expression pattern used to match dependencies that should be managed." + + "Group is the first part of a dependency coordinate `com.google.guava:guava:VERSION`.", + example = "com.google.*", + required = false) + @Nullable + String groupPattern; + + @Option(displayName = "Artifact", + description = "Artifact glob expression pattern used to match dependencies that should be managed." + + "Artifact is the second part of a dependency coordinate `com.google.guava:guava:VERSION`.", + example = "guava*", + required = false) + @Nullable + String artifactPattern; + + @Option(displayName = "Only if managed version is ...", + description = "Only remove the explicit version if the managed version has the specified comparative relationship to the explicit version. " + + "For example, `gte` will only remove the explicit version if the managed version is the same or newer. " + + "Default `eq`.", + valid = {"any", "eq", "lt", "lte", "gt", "gte"}, + required = false) + @Nullable + Comparator onlyIfManagedVersionIs; + + @Option(displayName = "Except", + description = "Accepts a list of GAVs. Dependencies matching a GAV will be ignored by this recipe." + + " GAV versions are ignored if provided.", + example = "com.jcraft:jsch", + required = false) + @Nullable + List except; + + public enum Comparator { + ANY, + EQ, + LT, + LTE, + GT, + GTE + } + + @Override + public String getDisplayName() { + return "Remove redundant explicit dependency versions"; + } + + @Override + public String getDescription() { + return "Remove explicitly-specified dependency versions that are managed by a Gradle `platform`/`enforcedPlatform`."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + new IsBuildGradle<>(), + new GroovyIsoVisitor() { + GradleProject gp; + final Map> platforms = new HashMap<>(); + + @Override + public G.CompilationUnit visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { + Optional maybeGp = cu.getMarkers().findFirst(GradleProject.class); + if (!maybeGp.isPresent()) { + return cu; + } + + gp = maybeGp.get(); + new GroovyIsoVisitor() { + final MethodMatcher platformMatcher = new MethodMatcher("org.gradle.api.artifacts.dsl.DependencyHandler platform(..)"); + final MethodMatcher enforcedPlatformMatcher = new MethodMatcher("org.gradle.api.artifacts.dsl.DependencyHandler enforcedPlatform(..)"); + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); + if (!platformMatcher.matches(m) && !enforcedPlatformMatcher.matches(m)) { + return m; + } + + if (m.getArguments().get(0) instanceof J.Literal) { + J.Literal l = (J.Literal) m.getArguments().get(0); + if (l.getType() != JavaType.Primitive.String) { + return m; + } + + Dependency dependency = DependencyStringNotationConverter.parse((String) l.getValue()); + MavenPomDownloader mpd = new MavenPomDownloader(ctx); + try { + ResolvedPom platformPom = mpd.download(new GroupArtifactVersion(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion()), null, null, gp.getMavenRepositories()) + .resolve(Collections.emptyList(), mpd, ctx); + platforms.computeIfAbsent(getCursor().getParent(1).firstEnclosing(J.MethodInvocation.class).getSimpleName(), k -> new ArrayList<>()).add(platformPom); + } catch (MavenDownloadingException e) { + return m; + } + } else if (m.getArguments().get(0) instanceof G.MapEntry) { + String groupId = null; + String artifactId = null; + String version = null; + + for (Expression arg : m.getArguments()) { + if (!(arg instanceof G.MapEntry)) { + continue; + } + + G.MapEntry entry = (G.MapEntry) arg; + if (!(entry.getKey() instanceof J.Literal) || !(entry.getValue() instanceof J.Literal)) { + continue; + } + + J.Literal key = (J.Literal) entry.getKey(); + J.Literal value = (J.Literal) entry.getValue(); + if (key.getType() != JavaType.Primitive.String || value.getType() != JavaType.Primitive.String) { + continue; + } + + switch ((String) key.getValue()) { + case "group": + groupId = (String) value.getValue(); + break; + case "name": + artifactId = (String) value.getValue(); + break; + case "version": + version = (String) value.getValue(); + break; + } + } + + if (groupId == null || artifactId == null || version == null) { + return m; + } + + MavenPomDownloader mpd = new MavenPomDownloader(ctx); + try { + ResolvedPom platformPom = mpd.download(new GroupArtifactVersion(groupId, artifactId, version), null, null, gp.getMavenRepositories()) + .resolve(Collections.emptyList(), mpd, ctx); + platforms.computeIfAbsent(getCursor().getParent(1).firstEnclosing(J.MethodInvocation.class).getSimpleName(), k -> new ArrayList<>()).add(platformPom); + } catch (MavenDownloadingException e) { + return m; + } + } + return m; + } + }.visit(cu, ctx); + + return super.visitCompilationUnit(cu, ctx); + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation m = super.visitMethodInvocation(method, ctx); + + Optional maybeGradleDependency = new GradleDependency.Matcher() + .groupId(groupPattern) + .artifactId(artifactPattern) + .get(getCursor()); + if (!maybeGradleDependency.isPresent()) { + return m; + } + + GradleDependency gradleDependency = maybeGradleDependency.get(); + ResolvedDependency d = gradleDependency.getResolvedDependency(); + if (StringUtils.isBlank(d.getVersion())) { + return m; + } + + if (platforms.containsKey(m.getSimpleName())) { + for (ResolvedPom platform : platforms.get(m.getSimpleName())) { + String managedVersion = platform.getManagedVersion(d.getGroupId(), d.getArtifactId(), null, d.getRequested().getClassifier()); + if (matchesComparator(managedVersion, d.getVersion())) { + return maybeRemoveVersion(m); + } + } + } + GradleDependencyConfiguration gdc = gp.getConfiguration(m.getSimpleName()); + if (gdc != null) { + for (GradleDependencyConfiguration configuration : gdc.allExtendsFrom()) { + if (platforms.containsKey(configuration.getName())) { + for (ResolvedPom platform : platforms.get(configuration.getName())) { + String managedVersion = platform.getManagedVersion(d.getGroupId(), d.getArtifactId(), null, d.getRequested().getClassifier()); + if (matchesComparator(managedVersion, d.getVersion())) { + return maybeRemoveVersion(m); + } + } + } + } + } + + return m; + } + + private J.MethodInvocation maybeRemoveVersion(J.MethodInvocation m) { + if (m.getArguments().get(0) instanceof J.Literal) { + J.Literal l = (J.Literal) m.getArguments().get(0); + if (l.getType() != JavaType.Primitive.String) { + return m; + } + + Dependency dep = DependencyStringNotationConverter.parse((String) l.getValue()) + .withVersion(null); + if (dep.getClassifier() != null || dep.getExt() != null) { + return m; + } + + return m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue(l, dep.toStringNotation()))); + } else if (m.getArguments().get(0) instanceof G.MapLiteral) { + return m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> { + G.MapLiteral mapLiteral = (G.MapLiteral) arg; + return mapLiteral.withElements(ListUtils.map(mapLiteral.getElements(), entry -> { + if (entry.getKey() instanceof J.Literal && + "version".equals(((J.Literal) entry.getKey()).getValue())) { + return null; + } + return entry; + })); + })); + } else if (m.getArguments().get(0) instanceof G.MapEntry) { + return m.withArguments(ListUtils.map(m.getArguments(), arg -> { + G.MapEntry entry = (G.MapEntry) arg; + if (entry.getKey() instanceof J.Literal && + "version".equals(((J.Literal) entry.getKey()).getValue())) { + return null; + } + return entry; + })); + } + return m; + } + } + ); + } + + private Comparator determineComparator() { + if (onlyIfManagedVersionIs != null) { + return onlyIfManagedVersionIs; + } + return Comparator.EQ; + } + + private boolean matchesComparator(@Nullable String managedVersion, String requestedVersion) { + Comparator comparator = determineComparator(); + if (managedVersion == null) { + return false; + } + if (comparator.equals(Comparator.ANY)) { + return true; + } + if (!isExact(managedVersion)) { + return false; + } + int comparison = new LatestIntegration(null) + .compare(null, managedVersion, requestedVersion); + if (comparison < 0) { + return comparator.equals(Comparator.LT) || comparator.equals(Comparator.LTE); + } else if (comparison > 0) { + return comparator.equals(Comparator.GT) || comparator.equals(Comparator.GTE); + } else { + return comparator.equals(Comparator.EQ) || comparator.equals(Comparator.LTE) || comparator.equals(Comparator.GTE); + } + } + + private boolean isExact(String managedVersion) { + Validated maybeVersionComparator = Semver.validate(managedVersion, null); + return maybeVersionComparator.isValid() && maybeVersionComparator.getValue() instanceof ExactVersion; + } +} 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 d008c4ae3f1..d8c32b7cd84 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java @@ -32,7 +32,6 @@ import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.StringUtils; -import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaSourceFile; @@ -140,8 +139,6 @@ public DependencyVersionState getInitialValue(ExecutionContext ctx) { return new DependencyVersionState(); } - private static final MethodMatcher DEPENDENCY_DSL_MATCHER = new MethodMatcher("DependencyHandlerSpec *(..)"); - @Override public TreeVisitor getScanner(DependencyVersionState acc) { @@ -163,7 +160,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); - if (gradleDependencyMatcher.get(getCursor()).isPresent() || DEPENDENCY_DSL_MATCHER.matches(m)) { + if (gradleDependencyMatcher.get(getCursor()).isPresent()) { if (m.getArguments().get(0) instanceof G.MapEntry) { String groupId = null; String artifactId = null; @@ -374,15 +371,10 @@ public J postVisit(J tree, ExecutionContext ctx) { @Override public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - if ("constraints".equals(method.getSimpleName()) || "project".equals(method.getSimpleName())) { - // don't mess with anything inside a constraints block, leave that to UpgradeTransitiveDependency version recipe - // `project` dependencies should also be skipped - return method; - } J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher(); - if (gradleDependencyMatcher.get(getCursor()).isPresent() || DEPENDENCY_DSL_MATCHER.matches(m)) { + if (gradleDependencyMatcher.get(getCursor()).isPresent()) { List depArgs = m.getArguments(); if (depArgs.get(0) instanceof J.Literal || depArgs.get(0) instanceof G.GString || depArgs.get(0) instanceof G.MapEntry) { m = updateDependency(m, ctx); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java index 1f20c8e92e2..e4a806cdd25 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java @@ -24,10 +24,13 @@ import org.openrewrite.gradle.util.DependencyStringNotationConverter; import org.openrewrite.groovy.tree.G; import org.openrewrite.internal.StringUtils; +import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.maven.tree.Dependency; +import org.openrewrite.maven.tree.GroupArtifactVersion; import org.openrewrite.maven.tree.ResolvedDependency; +import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion; import org.openrewrite.trait.Trait; import java.util.List; @@ -43,6 +46,8 @@ public class GradleDependency implements Trait { ResolvedDependency resolvedDependency; public static class Matcher extends GradleTraitMatcher { + private static final MethodMatcher DEPENDENCY_DSL_MATCHER = new MethodMatcher("DependencyHandlerSpec *(..)"); + @Nullable protected String configuration; @@ -77,13 +82,14 @@ public Matcher artifactId(@Nullable String artifactId) { return null; } - GradleProject gradleProject = getGradleProject(cursor); - if (gradleProject == null) { + if (withinDependencyConstraintsBlock(cursor)) { + // A dependency constraint is different from an actual dependency return null; } + GradleProject gradleProject = getGradleProject(cursor); GradleDependencyConfiguration gdc = getConfiguration(gradleProject, methodInvocation); - if (gdc == null) { + if (gdc == null && !(DEPENDENCY_DSL_MATCHER.matches(methodInvocation) && !"project".equals(methodInvocation.getSimpleName()))) { return null; } @@ -95,49 +101,77 @@ public Matcher artifactId(@Nullable String artifactId) { Expression argument = methodInvocation.getArguments().get(0); if (argument instanceof J.Literal || argument instanceof G.GString || argument instanceof G.MapEntry || argument instanceof G.MapLiteral) { dependency = parseDependency(methodInvocation.getArguments()); - } else if (argument instanceof J.MethodInvocation && - (((J.MethodInvocation) argument).getSimpleName().equals("platform") || - ((J.MethodInvocation) argument).getSimpleName().equals("enforcedPlatform"))) { - dependency = parseDependency(((J.MethodInvocation) argument).getArguments()); + } else if (argument instanceof J.MethodInvocation) { + if (((J.MethodInvocation) argument).getSimpleName().equals("platform") || + ((J.MethodInvocation) argument).getSimpleName().equals("enforcedPlatform")) { + dependency = parseDependency(((J.MethodInvocation) argument).getArguments()); + } else if (((J.MethodInvocation) argument).getSimpleName().equals("project")) { + // project dependencies are not yet supported + return null; + } } if (dependency == null) { return null; } - if (gdc.isCanBeResolved()) { - for (ResolvedDependency resolvedDependency : gdc.getResolved()) { - if ((groupId == null || matchesGlob(resolvedDependency.getGroupId(), groupId)) && - (artifactId == null || matchesGlob(resolvedDependency.getArtifactId(), artifactId))) { - Dependency req = resolvedDependency.getRequested(); - if ((req.getGroupId() == null || req.getGroupId().equals(dependency.getGroupId())) && - req.getArtifactId().equals(dependency.getArtifactId())) { - return new GradleDependency(cursor, resolvedDependency); + if (gdc != null) { + if (gdc.isCanBeResolved()) { + for (ResolvedDependency resolvedDependency : gdc.getResolved()) { + if ((groupId == null || matchesGlob(resolvedDependency.getGroupId(), groupId)) && + (artifactId == null || matchesGlob(resolvedDependency.getArtifactId(), artifactId))) { + Dependency req = resolvedDependency.getRequested(); + if ((req.getGroupId() == null || req.getGroupId().equals(dependency.getGroupId())) && + req.getArtifactId().equals(dependency.getArtifactId())) { + return new GradleDependency(cursor, resolvedDependency); + } } } - } - } else { - for (GradleDependencyConfiguration transitiveConfiguration : gradleProject.configurationsExtendingFrom(gdc, true)) { - if (transitiveConfiguration.isCanBeResolved()) { - for (ResolvedDependency resolvedDependency : transitiveConfiguration.getResolved()) { - if ((groupId == null || matchesGlob(resolvedDependency.getGroupId(), groupId)) && - (artifactId == null || matchesGlob(resolvedDependency.getArtifactId(), artifactId))) { - Dependency req = resolvedDependency.getRequested(); - if ((req.getGroupId() == null || req.getGroupId().equals(dependency.getGroupId())) && - req.getArtifactId().equals(dependency.getArtifactId())) { - return new GradleDependency(cursor, resolvedDependency); + } else { + for (GradleDependencyConfiguration transitiveConfiguration : gradleProject.configurationsExtendingFrom(gdc, true)) { + if (transitiveConfiguration.isCanBeResolved()) { + for (ResolvedDependency resolvedDependency : transitiveConfiguration.getResolved()) { + if ((groupId == null || matchesGlob(resolvedDependency.getGroupId(), groupId)) && + (artifactId == null || matchesGlob(resolvedDependency.getArtifactId(), artifactId))) { + Dependency req = resolvedDependency.getRequested(); + if ((req.getGroupId() == null || req.getGroupId().equals(dependency.getGroupId())) && + req.getArtifactId().equals(dependency.getArtifactId())) { + return new GradleDependency(cursor, resolvedDependency); + } } } } } } } + + if ((groupId == null || matchesGlob(dependency.getGroupId(), groupId)) && + (artifactId == null || matchesGlob(dependency.getArtifactId(), artifactId))) { + // Couldn't find the actual resolved dependency, return a virtualized one instead + ResolvedDependency resolvedDependency = ResolvedDependency.builder() + .depth(-1) + .gav(new ResolvedGroupArtifactVersion(null, dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion() != null ? dependency.getVersion() : "", null)) + .classifier(dependency.getClassifier()) + .type(dependency.getExt()) + .requested(Dependency.builder() + .scope(methodInvocation.getSimpleName()) + .type(dependency.getExt()) + .gav(new GroupArtifactVersion(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())) + .classifier(dependency.getClassifier()) + .build()) + .build(); + return new GradleDependency(cursor, resolvedDependency); + } } return null; } - private static @Nullable GradleDependencyConfiguration getConfiguration(GradleProject gradleProject, J.MethodInvocation methodInvocation) { + private static @Nullable GradleDependencyConfiguration getConfiguration(@Nullable GradleProject gradleProject, J.MethodInvocation methodInvocation) { + if (gradleProject == null) { + return null; + } + String methodName = methodInvocation.getSimpleName(); if (methodName.equals("classpath")) { return gradleProject.getBuildscript().getConfiguration(methodName); @@ -146,12 +180,12 @@ public Matcher artifactId(@Nullable String artifactId) { } } - private boolean withinDependenciesBlock(Cursor cursor) { + private boolean withinBlock(Cursor cursor, String name) { Cursor parentCursor = cursor.getParent(); while (parentCursor != null) { if (parentCursor.getValue() instanceof J.MethodInvocation) { J.MethodInvocation m = parentCursor.getValue(); - if (m.getSimpleName().equals("dependencies")) { + if (m.getSimpleName().equals(name)) { return true; } } @@ -161,6 +195,14 @@ private boolean withinDependenciesBlock(Cursor cursor) { return false; } + private boolean withinDependenciesBlock(Cursor cursor) { + return withinBlock(cursor, "dependencies"); + } + + private boolean withinDependencyConstraintsBlock(Cursor cursor) { + return withinBlock(cursor, "constraints") && withinDependenciesBlock(cursor); + } + private org.openrewrite.gradle.util.@Nullable Dependency parseDependency(List arguments) { Expression argument = arguments.get(0); if (argument instanceof J.Literal) { diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveRedundantDependencyVersionsTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveRedundantDependencyVersionsTest.java new file mode 100644 index 00000000000..379b0381291 --- /dev/null +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveRedundantDependencyVersionsTest.java @@ -0,0 +1,310 @@ +/* + * 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.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.gradle.Assertions.buildGradle; +import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; + +class RemoveRedundantDependencyVersionsTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.beforeRecipe(withToolingApi()) + .recipe(new RemoveRedundantDependencyVersions(null, null, null, null)); + } + + @DocumentExample + @Test + void literal() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3:3.14.0") + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3") + } + """ + ) + ); + } + + @Test + void mapEntry() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation(group: "org.apache.commons", name: "commons-lang3", version: "3.14.0") + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation(group: "org.apache.commons", name: "commons-lang3") + } + """ + ) + ); + } + + @Test + void mapLiteral() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation([group: "org.apache.commons", name: "commons-lang3", version: "3.14.0"]) + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation([group: "org.apache.commons", name: "commons-lang3"]) + } + """ + ) + ); + } + + @Test + void enforcedPlatform() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(enforcedPlatform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3:3.14.0") + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(enforcedPlatform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3") + } + """ + ) + ); + } + + @Test + void platformUsingMapEntry() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(enforcedPlatform(group: "org.springframework.boot", name: "spring-boot-dependencies", version: "3.3.3")) + implementation("org.apache.commons:commons-lang3:3.14.0") + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation(enforcedPlatform(group: "org.springframework.boot", name: "spring-boot-dependencies", version: "3.3.3")) + implementation("org.apache.commons:commons-lang3") + } + """ + ) + ); + } + + @Test + void freestandingScript() { + rewriteRun( + buildGradle( + """ + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3:3.14.0") + } + """, + """ + repositories { + mavenCentral() + } + + dependencies { + implementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3") + } + """, + spec -> spec.path("dependencies.gradle") + ), + buildGradle( + """ + plugins { + id("java") + } + + apply from: 'dependencies.gradle' + """ + ) + ); + } + + @Test + void transitiveConfiguration() { + rewriteRun( + buildGradle( + """ + plugins { + id "java-library" + } + + repositories { + mavenCentral() + } + + dependencies { + api(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3:3.14.0") + } + """, + """ + plugins { + id "java-library" + } + + repositories { + mavenCentral() + } + + dependencies { + api(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + implementation("org.apache.commons:commons-lang3") + } + """ + ) + ); + } + + @Test + void unmanagedDependency() { + rewriteRun( + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation("org.apache.commons:commons-lang3:3.14.0") + + testImplementation(platform("org.springframework.boot:spring-boot-dependencies:3.3.3")) + } + """ + ) + ); + } +} 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 47b65310df4..e14a122df1f 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -36,7 +36,9 @@ import org.openrewrite.internal.ListUtils; import org.openrewrite.java.internal.JavaTypeCache; import org.openrewrite.java.marker.ImplicitReturn; +import org.openrewrite.java.marker.OmitParentheses; import org.openrewrite.java.marker.Semicolon; +import org.openrewrite.java.marker.TrailingComma; import org.openrewrite.java.tree.*; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.Statement; @@ -110,6 +112,35 @@ private static boolean isOlderThanGroovy3() { return olderThanGroovy3; } + /** + * Groovy methods can be declared with "def" AND a return type + * In these cases the "def" is semantically meaningless but needs to be preserved for source code accuracy + * If there is both a def and a return type, this method returns a RedundantDef object and advances the cursor + * position past the "def" keyword, leaving the return type to be parsed as normal. + * In any other situation an empty Optional is returned and the cursor is not advanced. + */ + private Optional maybeRedundantDef(ClassNode type, String name) { + int saveCursor = cursor; + Space defPrefix = whitespace(); + if(source.startsWith("def", cursor)) { + skip("def"); + // The def is redundant only when it is followed by the method's return type + // I hope no one puts an annotation between "def" and the return type + int cursorBeforeReturnType = cursor; + int lengthOfTypePrefix = indexOfNextNonWhitespace(cursorBeforeReturnType, source) - cursorBeforeReturnType; + // Differentiate between the next token being the method/variable name and it being the return type + if (!source.startsWith(name, cursorBeforeReturnType + lengthOfTypePrefix)) { + TypeTree returnType = visitTypeTree(type); + if (source.startsWith(returnType.toString(), cursorBeforeReturnType + lengthOfTypePrefix)) { + cursor = cursorBeforeReturnType; + return Optional.of(new RedundantDef(randomId(), defPrefix)); + } + } + } + cursor = saveCursor; + return Optional.empty(); + } + 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()) { @@ -204,7 +235,7 @@ public G.CompilationUnit visit(SourceUnit unit, ModuleNode ast) throws GroovyPar null, pkg, statements, - format(source.substring(cursor)) + format(source, cursor, source.length()) ); } @@ -218,16 +249,7 @@ private class RewriteGroovyClassVisitor extends ClassCodeVisitorSupport { @Override public void visitClass(ClassNode clazz) { Space fmt = whitespace(); - List leadingAnnotations; - if (clazz.getAnnotations().isEmpty()) { - leadingAnnotations = emptyList(); - } else { - leadingAnnotations = new ArrayList<>(clazz.getAnnotations().size()); - for (AnnotationNode annotation : clazz.getAnnotations()) { - visitAnnotation(annotation); - leadingAnnotations.add(pollQueue()); - } - } + List leadingAnnotations = visitAndGetAnnotations(clazz); List modifiers = visitModifiers(clazz.getModifiers()); Space kindPrefix = whitespace(); @@ -315,13 +337,13 @@ J.Block visitClassBlock(ClassNode clazz) { } } for (org.codehaus.groovy.ast.stmt.Statement objectInitializer : clazz.getObjectInitializerStatements()) { - if(!(objectInitializer instanceof BlockStatement)) { + if (!(objectInitializer instanceof BlockStatement)) { continue; } // A class initializer BlockStatement will be wrapped in an otherwise empty BlockStatement with the same source positions // No idea why, except speculation that it is for consistency with static intiializers, but we can skip the wrapper and just visit its contents BlockStatement s = (BlockStatement) objectInitializer; - if(s.getStatements().size() == 1 && pos(s).equals(pos(s.getStatements().get(0)))) { + if (s.getStatements().size() == 1 && pos(s).equals(pos(s.getStatements().get(0)))) { s = (BlockStatement) s.getStatements().get(0); } sortedByPosition.computeIfAbsent(pos(s), i -> new ArrayList<>()) @@ -412,13 +434,7 @@ private void visitEnumField(@SuppressWarnings("unused") FieldNode fieldNode) { private void visitVariableField(FieldNode field) { RewriteGroovyVisitor visitor = new RewriteGroovyVisitor(field, this); - List annotations = field.getAnnotations().stream() - .map(a -> { - visitAnnotation(a); - return (J.Annotation) pollQueue(); - }) - .collect(Collectors.toList()); - + List annotations = visitAndGetAnnotations(field); List modifiers = visitModifiers(field.getModifiers()); TypeTree typeExpr = visitTypeTree(field.getOriginType()); @@ -502,15 +518,9 @@ argName, padLeft(sourceBefore("="), bodyVisitor.visit(arg.getValue())), public void visitMethod(MethodNode method) { Space fmt = whitespace(); - List annotations = method.getAnnotations().stream() - .map(a -> { - visitAnnotation(a); - return (J.Annotation) pollQueue(); - }) - .collect(Collectors.toList()); - + List annotations = visitAndGetAnnotations(method); List modifiers = visitModifiers(method.getModifiers()); - + Optional redundantDef = maybeRedundantDef(method.getReturnType(), method.getName()); TypeTree returnType = visitTypeTree(method.getReturnType()); // Method name might be in quotes @@ -539,12 +549,7 @@ public void visitMethod(MethodNode method) { for (int i = 0; i < unparsedParams.length; i++) { Parameter param = unparsedParams[i]; - List paramAnnotations = param.getAnnotations().stream() - .map(a -> { - visitAnnotation(a); - return (J.Annotation) pollQueue(); - }) - .collect(Collectors.toList()); + List paramAnnotations = visitAndGetAnnotations(param); TypeTree paramType; if (param.isDynamicTyped()) { @@ -589,7 +594,8 @@ null, emptyList(), bodyVisitor.visit(method.getCode()); queue.add(new J.MethodDeclaration( - randomId(), fmt, Markers.EMPTY, + randomId(), fmt, + redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY), annotations, modifiers, null, @@ -604,50 +610,39 @@ null, emptyList(), } @Override - public void visitConstructor(ConstructorNode ctor) { + public void visitConstructor(ConstructorNode constructor) { Space fmt = whitespace(); - List annotations = ctor.getAnnotations().stream() - .map(a -> { - visitAnnotation(a); - return (J.Annotation) pollQueue(); - }) - .collect(Collectors.toList()); - - List modifiers = visitModifiers(ctor.getModifiers()); + List annotations = visitAndGetAnnotations(constructor); + List modifiers = visitModifiers(constructor.getModifiers()); // Constructor name might be in quotes Space namePrefix = whitespace(); - String ctorName; - if (source.startsWith(ctor.getDeclaringClass().getName(), cursor)) { - ctorName = ctor.getDeclaringClass().getName(); + String constructorName; + if (source.startsWith(constructor.getDeclaringClass().getName(), cursor)) { + constructorName = constructor.getDeclaringClass().getName(); } else { char openingQuote = source.charAt(cursor); - ctorName = openingQuote + ctor.getName() + openingQuote; + constructorName = openingQuote + constructor.getName() + openingQuote; } - cursor += ctorName.length(); + cursor += constructorName.length(); J.Identifier name = new J.Identifier(randomId(), namePrefix, Markers.EMPTY, emptyList(), - ctorName, + constructorName, null, null); - RewriteGroovyVisitor bodyVisitor = new RewriteGroovyVisitor(ctor, this); + RewriteGroovyVisitor bodyVisitor = new RewriteGroovyVisitor(constructor, this); // Parameter has no visit implementation, so we've got to do this by hand Space beforeParen = sourceBefore("("); - List> params = new ArrayList<>(ctor.getParameters().length); - Parameter[] unparsedParams = ctor.getParameters(); + List> params = new ArrayList<>(constructor.getParameters().length); + Parameter[] unparsedParams = constructor.getParameters(); for (int i = 0; i < unparsedParams.length; i++) { Parameter param = unparsedParams[i]; - List paramAnnotations = param.getAnnotations().stream() - .map(a -> { - visitAnnotation(a); - return (J.Annotation) pollQueue(); - }) - .collect(Collectors.toList()); + List paramAnnotations = visitAndGetAnnotations(param); TypeTree paramType; if (param.isDynamicTyped()) { @@ -682,14 +677,14 @@ null, emptyList(), params.add(JRightPadded.build(new J.Empty(randomId(), sourceBefore(")"), Markers.EMPTY))); } - JContainer throws_ = ctor.getExceptions().length == 0 ? null : JContainer.build( + JContainer throws_ = constructor.getExceptions().length == 0 ? null : JContainer.build( sourceBefore("throws"), - bodyVisitor.visitRightPadded(ctor.getExceptions(), null), + bodyVisitor.visitRightPadded(constructor.getExceptions(), null), Markers.EMPTY ); - J.Block body = ctor.getCode() == null ? null : - bodyVisitor.visit(ctor.getCode()); + J.Block body = constructor.getCode() == null ? null : + bodyVisitor.visit(constructor.getCode()); queue.add(new J.MethodDeclaration( randomId(), fmt, Markers.EMPTY, @@ -702,10 +697,23 @@ null, emptyList(), throws_, body, null, - typeMapping.methodType(ctor) + typeMapping.methodType(constructor) )); } + public List visitAndGetAnnotations(AnnotatedNode node) { + if (node.getAnnotations().isEmpty()) { + return emptyList(); + } + + List paramAnnotations = new ArrayList<>(node.getAnnotations().size()); + for (AnnotationNode annotationNode : node.getAnnotations()) { + visitAnnotation(annotationNode); + paramAnnotations.add(pollQueue()); + } + return paramAnnotations; + } + @SuppressWarnings({"ConstantConditions", "unchecked"}) private T pollQueue() { return (T) queue.poll(); @@ -733,14 +741,14 @@ private List> visitRightPadded(ASTNode[] nodes, @Nullable St List> ts = new ArrayList<>(nodes.length); for (int i = 0; i < nodes.length; i++) { ASTNode node = nodes[i]; - @SuppressWarnings("unchecked") JRightPadded converted = JRightPadded.build( - node instanceof ClassNode ? (T) visitTypeTree((ClassNode) node) : visit(node)); + @SuppressWarnings("unchecked") + JRightPadded converted = JRightPadded.build(node instanceof ClassNode ? (T) visitTypeTree((ClassNode) node) : visit(node)); if (i == nodes.length - 1) { converted = converted.withAfter(whitespace()); if (',' == source.charAt(cursor)) { // In Groovy trailing "," are allowed cursor += 1; - converted = converted.withMarkers(Markers.EMPTY.add(new org.openrewrite.java.marker.TrailingComma(randomId(), whitespace()))); + converted = converted.withMarkers(Markers.EMPTY.add(new TrailingComma(randomId(), whitespace()))); } ts.add(converted); if (afterLast != null && source.startsWith(afterLast, cursor)) { @@ -797,11 +805,11 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { int saveCursor = cursor; Space beforeOpenParen = whitespace(); - org.openrewrite.java.marker.OmitParentheses omitParentheses = null; + boolean hasParentheses = true; if (source.charAt(cursor) == '(') { cursor++; } else { - omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId()); + hasParentheses = false; beforeOpenParen = EMPTY; cursor = saveCursor; } @@ -831,9 +839,8 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { // 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); cursor = saveCursor; - if (!isOpeningBracketPresent) { + if ('[' != source.charAt(cursor)) { // Bring named parameters out of their containing MapExpression so that they can be parsed correctly MapExpression namedArgExpressions = (MapExpression) unparsedArgs.get(0); unparsedArgs = @@ -846,25 +853,26 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { if (unparsedArgs.isEmpty()) { args.add(JRightPadded.build((Expression) new J.Empty(randomId(), whitespace(), Markers.EMPTY)) - .withAfter(omitParentheses == null ? sourceBefore(")") : EMPTY)); + .withAfter(hasParentheses ? sourceBefore(")") : EMPTY)); } else { + boolean lastArgumentsAreAllClosures = endsWithClosures(expression.getExpressions()); for (int i = 0; i < unparsedArgs.size(); i++) { org.codehaus.groovy.ast.expr.Expression rawArg = unparsedArgs.get(i); Expression arg = visit(rawArg); - if (omitParentheses != null) { - arg = arg.withMarkers(arg.getMarkers().add(omitParentheses)); + if (!hasParentheses) { + arg = arg.withMarkers(arg.getMarkers().add(new OmitParentheses(randomId()))); } Space after = EMPTY; if (i == unparsedArgs.size() - 1) { - if (omitParentheses == null) { + if (hasParentheses) { after = sourceBefore(")"); } - } else { + } else if (!(arg instanceof J.Lambda && lastArgumentsAreAllClosures && !hasParentheses)) { after = whitespace(); if (source.charAt(cursor) == ')') { - // the next argument will have an OmitParentheses marker - omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId()); + // next argument(s), if they exists, are trailing closures and will have an OmitParentheses marker + hasParentheses = false; } cursor++; } @@ -876,6 +884,25 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { queue.add(JContainer.build(beforeOpenParen, args, Markers.EMPTY)); } + public boolean endsWithClosures(List list) { + if (!(list.get(list.size() - 1) instanceof ClosureExpression)) { + return false; + } + + boolean foundNonClosure = false; + for (int i = list.size() - 2; i >= 0; i--) { + if (list.get(i) instanceof ClosureExpression) { + if (foundNonClosure) { + return false; + } + } else { + foundNonClosure = true; + } + } + + return true; + } + @Override public void visitClassExpression(ClassExpression clazz) { String unresolvedName = clazz.getType().getUnresolvedName().replace('$', '.'); @@ -1072,7 +1099,7 @@ public void visitBlockStatement(BlockStatement block) { J expr = visit(statement); if (i == blockStatements.size() - 1 && (expr instanceof Expression)) { if (parent instanceof ClosureExpression || (parent instanceof MethodNode && - !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) { + !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) { expr = new J.Return(randomId(), expr.getPrefix(), Markers.EMPTY, expr.withPrefix(EMPTY)); expr = expr.withMarkers(expr.getMarkers().add(new ImplicitReturn(randomId()))); @@ -1374,7 +1401,7 @@ public void visitConstantExpression(ConstantExpression expression) { public void visitConstructorCallExpression(ConstructorCallExpression ctor) { queue.add(insideParentheses(ctor, fmt -> { cursor += 3; // skip "new" - TypeTree clazz = visitTypeTree(ctor.getType(), ctor.getMetaDataMap().containsKey(StaticTypesMarker.INFERRED_TYPE)); + TypeTree clazz = visitTypeTree(ctor.getType(), ctor.getNodeMetaData().containsKey(StaticTypesMarker.INFERRED_TYPE)); JContainer args = visit(ctor.getArguments()); J.Block body = null; if (ctor.isUsingAnonymousInnerClass() && ctor.getType() instanceof InnerClassNode) { @@ -1411,12 +1438,13 @@ public void visitNotExpression(NotExpression expression) { @Override public void visitDeclarationExpression(DeclarationExpression expression) { + Optional redundantDef = maybeRedundantDef(expression.getVariableExpression().getType(), expression.getVariableExpression().getName()); TypeTree typeExpr = visitVariableExpressionType(expression.getVariableExpression()); J.VariableDeclarations.NamedVariable namedVariable; if (expression.isMultipleAssignmentDeclaration()) { // def (a, b) = [1, 2] - throw new UnsupportedOperationException("FIXME"); + throw new UnsupportedOperationException("Parsing multiple assignment (e.g.: def (a, b) = [1, 2]) is not implemented"); } else { J.Identifier name = visit(expression.getVariableExpression()); namedVariable = new J.VariableDeclarations.NamedVariable( @@ -1439,7 +1467,7 @@ public void visitDeclarationExpression(DeclarationExpression expression) { J.VariableDeclarations variableDeclarations = new J.VariableDeclarations( randomId(), EMPTY, - Markers.EMPTY, + redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY), emptyList(), emptyList(), typeExpr, @@ -1657,7 +1685,6 @@ public void visitMapExpression(MapExpression map) { @Override public void visitMethodCallExpression(MethodCallExpression call) { queue.add(insideParentheses(call, fmt -> { - ImplicitDot implicitDot = null; JRightPadded select = null; if (!call.isImplicitThis()) { @@ -1732,30 +1759,14 @@ public void visitMethodCallExpression(MethodCallExpression call) { } } } - // handle the obscure case where there are empty parens ahead of a closure - if (args.getExpressions().size() == 1 && args.getExpressions().get(0) instanceof ClosureExpression) { - int saveCursor = cursor; - Space argPrefix = whitespace(); - if (source.charAt(cursor) == '(') { - cursor += 1; - Space infix = whitespace(); - if (source.charAt(cursor) == ')') { - cursor += 1; - markers = markers.add(new EmptyArgumentListPrecedesArgument(randomId(), argPrefix, infix)); - } else { - cursor = saveCursor; - } - } else { - cursor = saveCursor; - } - } + markers = handlesCaseWhereEmptyParensAheadOfClosure(args, markers); } JContainer args = visit(call.getArguments()); MethodNode methodNode = (MethodNode) call.getNodeMetaData().get(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET); JavaType.Method methodType = null; if (methodNode == null && call.getObjectExpression() instanceof VariableExpression && - ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) { + ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) { // Groovy doesn't know what kind of object this method is being invoked on // But if this invocation is inside a Closure we may have already enriched its parameters with types from the static type checker // Use any such type information to attempt to find a matching method @@ -1836,23 +1847,7 @@ public void visitStaticMethodCallExpression(StaticMethodCallExpression call) { } } } - // handle the obscure case where there are empty parens ahead of a closure - if (args.getExpressions().size() == 1 && args.getExpressions().get(0) instanceof ClosureExpression) { - int saveCursor = cursor; - Space prefix = whitespace(); - if (source.charAt(cursor) == '(') { - cursor += 1; - Space infix = whitespace(); - if (source.charAt(cursor) == ')') { - cursor += 1; - markers = markers.add(new EmptyArgumentListPrecedesArgument(randomId(), prefix, infix)); - } else { - cursor = saveCursor; - } - } else { - cursor = saveCursor; - } - } + markers = handlesCaseWhereEmptyParensAheadOfClosure(args, markers); } JContainer args = visit(call.getArguments()); @@ -1983,11 +1978,11 @@ public void visitTupleExpression(TupleExpression tuple) { int saveCursor = cursor; Space beforeOpenParen = whitespace(); - org.openrewrite.java.marker.OmitParentheses omitParentheses = null; + OmitParentheses omitParentheses = null; if (source.charAt(cursor) == '(') { cursor++; } else { - omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId()); + omitParentheses = new OmitParentheses(randomId()); beforeOpenParen = EMPTY; cursor = saveCursor; } @@ -2011,7 +2006,7 @@ public void visitTupleExpression(TupleExpression tuple) { after = whitespace(); if (source.charAt(cursor) == ')') { // the next argument will have an OmitParentheses marker - omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId()); + omitParentheses = new OmitParentheses(randomId()); } cursor++; } @@ -2047,7 +2042,6 @@ public void visitTryCatchFinally(TryCatchStatement node) { //noinspection ConstantConditions queue.add(new J.Try(randomId(), prefix, Markers.EMPTY, resources, body, catches, finally_)); - } @Override @@ -2136,19 +2130,21 @@ public TypeTree visitVariableExpressionType(VariableExpression expression) { @Override public void visitVariableExpression(VariableExpression expression) { - JavaType type; - if (expression.isDynamicTyped() && expression.getAccessedVariable() != null && expression.getAccessedVariable().getType() != expression.getOriginType()) { - type = typeMapping.type(staticType(expression.getAccessedVariable())); - } else { - type = typeMapping.type(staticType((org.codehaus.groovy.ast.expr.Expression) expression)); - } - queue.add(new J.Identifier(randomId(), - sourceBefore(expression.getName()), - Markers.EMPTY, - emptyList(), - expression.getName(), - type, null) - ); + queue.add(insideParentheses(expression, fmt -> { + JavaType type; + if (expression.isDynamicTyped() && expression.getAccessedVariable() != null && expression.getAccessedVariable().getType() != expression.getOriginType()) { + type = typeMapping.type(staticType(expression.getAccessedVariable())); + } else { + type = typeMapping.type(staticType((org.codehaus.groovy.ast.expr.Expression) expression)); + } + + return new J.Identifier(randomId(), + fmt.withWhitespace(fmt.getWhitespace() + sourceBefore(expression.getName()).getWhitespace()), + Markers.EMPTY, + emptyList(), + expression.getName(), + type, null); + })); } @Override @@ -2200,6 +2196,27 @@ private T pollQueue() { } } + // handle the obscure case where there are empty parens ahead of a closure + private Markers handlesCaseWhereEmptyParensAheadOfClosure(ArgumentListExpression args, Markers markers) { + if (args.getExpressions().size() == 1 && args.getExpressions().get(0) instanceof ClosureExpression) { + int saveCursor = cursor; + Space argPrefix = whitespace(); + if (source.charAt(cursor) == '(') { + cursor += 1; + Space infix = whitespace(); + if (source.charAt(cursor) == ')') { + cursor += 1; + markers = markers.add(new EmptyArgumentListPrecedesArgument(randomId(), argPrefix, infix)); + } else { + cursor = saveCursor; + } + } else { + cursor = saveCursor; + } + } + return markers; + } + private JRightPadded convertTopLevelStatement(SourceUnit unit, ASTNode node) { if (node instanceof ClassNode) { ClassNode classNode = (ClassNode) node; @@ -2428,15 +2445,24 @@ private TypeTree mapDimensions(TypeTree baseType, ClassNode classNode) { return baseType; } + /** + * Get all characters of the source file between the cursor and the given delimiter. + * The cursor will be moved past the delimiter. + */ private Space sourceBefore(String untilDelim) { int delimIndex = positionOfNext(untilDelim); if (delimIndex < 0) { return EMPTY; // unable to find this delimiter } - String prefix = source.substring(cursor, delimIndex); - cursor += prefix.length() + untilDelim.length(); // advance past the delimiter - return Space.format(prefix); + if (delimIndex == cursor) { + cursor += untilDelim.length(); + return EMPTY; + } + + Space space = format(source, cursor, delimIndex); + cursor = delimIndex + untilDelim.length(); // advance past the delimiter + return space; } /** diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java index 223036f95c2..17e0a466a6c 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java @@ -25,6 +25,7 @@ import org.openrewrite.groovy.tree.GRightPadded; import org.openrewrite.groovy.tree.GSpace; import org.openrewrite.java.JavaPrinter; +import org.openrewrite.java.marker.CompactConstructor; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Marker; import org.openrewrite.marker.Markers; @@ -258,22 +259,52 @@ public J visitTypeCast(J.TypeCast t, PrintOutputCapture

p) { return t; } + + @Override + public J visitVariableDeclarations(J.VariableDeclarations multiVariable, PrintOutputCapture

p) { + beforeSyntax(multiVariable, Space.Location.VARIABLE_DECLARATIONS_PREFIX, p); + visitSpace(Space.EMPTY, Space.Location.ANNOTATIONS, p); + visit(multiVariable.getLeadingAnnotations(), p); + for (J.Modifier m : multiVariable.getModifiers()) { + visitModifier(m, p); + } + multiVariable.getMarkers().findFirst(RedundantDef.class).ifPresent(def -> { + visitSpace(def.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p); + p.append("def"); + }); + visit(multiVariable.getTypeExpression(), p); + // For backwards compatibility. + for (JLeftPadded dim : multiVariable.getDimensionsBeforeName()) { + visitSpace(dim.getBefore(), Space.Location.DIMENSION_PREFIX, p); + p.append('['); + visitSpace(dim.getElement(), Space.Location.DIMENSION, p); + p.append(']'); + } + if (multiVariable.getVarargs() != null) { + visitSpace(multiVariable.getVarargs(), Space.Location.VARARGS, p); + p.append("..."); + } + visitRightPadded(multiVariable.getPadding().getVariables(), JRightPadded.Location.NAMED_VARIABLE, ",", p); + afterSyntax(multiVariable, p); + return multiVariable; + } + @Override public J visitLambda(J.Lambda lambda, PrintOutputCapture

p) { beforeSyntax(lambda, Space.Location.LAMBDA_PREFIX, p); LambdaStyle ls = lambda.getMarkers().findFirst(LambdaStyle.class) .orElse(new LambdaStyle(null, false, !lambda.getParameters().getParameters().isEmpty())); boolean parenthesized = lambda.getParameters().isParenthesized(); - if(!ls.isJavaStyle()) { + if (!ls.isJavaStyle()) { p.append('{'); } visitMarkers(lambda.getParameters().getMarkers(), p); visitSpace(lambda.getParameters().getPrefix(), Space.Location.LAMBDA_PARAMETERS_PREFIX, p); - if(parenthesized) { + if (parenthesized) { p.append('('); } visitRightPadded(lambda.getParameters().getPadding().getParameters(), JRightPadded.Location.LAMBDA_PARAM, ",", p); - if(parenthesized) { + if (parenthesized) { p.append(')'); } if (ls.isArrow()) { @@ -287,7 +318,7 @@ public J visitLambda(J.Lambda lambda, PrintOutputCapture

p) { } else { visit(lambda.getBody(), p); } - if(!ls.isJavaStyle()) { + if (!ls.isJavaStyle()) { p.append('}'); } afterSyntax(lambda, p); @@ -328,6 +359,40 @@ public J visitForEachLoop(J.ForEachLoop forEachLoop, PrintOutputCapture

p) { return forEachLoop; } + @Override + public J visitMethodDeclaration(J.MethodDeclaration method, PrintOutputCapture

p) { + beforeSyntax(method, Space.Location.METHOD_DECLARATION_PREFIX, p); + visitSpace(Space.EMPTY, Space.Location.ANNOTATIONS, p); + visit(method.getLeadingAnnotations(), p); + for (J.Modifier m : method.getModifiers()) { + visitModifier(m, p); + } + J.TypeParameters typeParameters = method.getAnnotations().getTypeParameters(); + if (typeParameters != null) { + visit(typeParameters.getAnnotations(), p); + visitSpace(typeParameters.getPrefix(), Space.Location.TYPE_PARAMETERS, p); + visitMarkers(typeParameters.getMarkers(), p); + p.append('<'); + visitRightPadded(typeParameters.getPadding().getTypeParameters(), JRightPadded.Location.TYPE_PARAMETER, ",", p); + p.append('>'); + } + method.getMarkers().findFirst(RedundantDef.class).ifPresent(def -> { + visitSpace(def.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p); + p.append("def"); + }); + visit(method.getReturnTypeExpression(), p); + visit(method.getAnnotations().getName().getAnnotations(), p); + visit(method.getName(), p); + if (!method.getMarkers().findFirst(CompactConstructor.class).isPresent()) { + visitContainer("(", method.getPadding().getParameters(), JContainer.Location.METHOD_DECLARATION_PARAMETERS, ",", ")", p); + } + visitContainer("throws", method.getPadding().getThrows(), JContainer.Location.THROWS, ",", null, p); + visit(method.getBody(), p); + visitLeftPadded("default", method.getPadding().getDefaultValue(), JLeftPadded.Location.METHOD_DECLARATION_DEFAULT_VALUE, p); + afterSyntax(method, p); + return method; + } + @Override public J visitMethodInvocation(J.MethodInvocation method, PrintOutputCapture

p) { beforeSyntax(method, Space.Location.METHOD_INVOCATION_PREFIX, p); @@ -353,29 +418,32 @@ public J visitMethodInvocation(J.MethodInvocation method, PrintOutputCapture

visitSpace(argContainer.getBefore(), Space.Location.METHOD_INVOCATION_ARGUMENTS, p); List> args = argContainer.getPadding().getElements(); + boolean argsAreAllClosures = args.stream().allMatch(it -> it.getElement() instanceof J.Lambda); + boolean hasParentheses = true; + boolean applyTrailingLambdaParenthese = true; for (int i = 0; i < args.size(); i++) { JRightPadded arg = args.get(i); - boolean omitParens = arg.getElement().getMarkers() - .findFirst(OmitParentheses.class) - .isPresent() || - arg.getElement().getMarkers() - .findFirst(org.openrewrite.java.marker.OmitParentheses.class) - .isPresent(); - - if (i == 0 && !omitParens) { - p.append('('); - } else if (i > 0 && omitParens && ( - !args.get(0).getElement().getMarkers().findFirst(OmitParentheses.class).isPresent() && - !args.get(0).getElement().getMarkers().findFirst(org.openrewrite.java.marker.OmitParentheses.class).isPresent() - )) { - p.append(')'); - } else if (i > 0) { + boolean omitParensCurrElem = arg.getElement().getMarkers().findFirst(OmitParentheses.class).isPresent() || + arg.getElement().getMarkers().findFirst(org.openrewrite.java.marker.OmitParentheses.class).isPresent(); + + if (i == 0) { + if (omitParensCurrElem) { + hasParentheses = false; + } else { + p.append('('); + } + } else if (hasParentheses && omitParensCurrElem) { // first trailing lambda, eg: `stage('Build..') {}`, should close the method + if (applyTrailingLambdaParenthese) { // apply once, to support multiple closures: `foo("baz") {} {} + p.append(')'); + applyTrailingLambdaParenthese = false; + } + } else if (hasParentheses || !argsAreAllClosures) { p.append(','); } visitRightPadded(arg, JRightPadded.Location.METHOD_INVOCATION_ARGUMENT, p); - if (i == args.size() - 1 && !omitParens) { + if (i == args.size() - 1 && !omitParensCurrElem) { p.append(')'); } } diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/RedundantDef.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/RedundantDef.java new file mode 100644 index 00000000000..34157a68d97 --- /dev/null +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/RedundantDef.java @@ -0,0 +1,34 @@ +/* + * 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.groovy.marker; + +import lombok.Value; +import lombok.With; +import org.openrewrite.java.tree.Space; +import org.openrewrite.marker.Marker; + +import java.util.UUID; + +/** + * In Groovy methods can be declared with a return type and also a redundant 'def' keyword. + * This captures the extra def keyword. + */ +@Value +@With +public class RedundantDef implements Marker { + UUID id; + Space prefix; +} diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java index 2d2675bfe72..13239c0fc76 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java @@ -34,7 +34,9 @@ void insideParentheses() { // NOT inside parentheses, but verifies the parser's // test for "inside parentheses" condition - groovy("(1) + 1") + groovy("(1) + 1"), + // And combine the two cases + groovy("((1) + 1)") ); } @@ -62,6 +64,19 @@ void in() { ); } + @Test + void withVariable() { + rewriteRun( + groovy( + """ + def foo(int a) { + 60 + a + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/1531") @Test void regexFindOperator() { @@ -201,17 +216,12 @@ void stringMultipliedInParentheses() { ); } - @Disabled @Issue("https://github.com/openrewrite/rewrite/issues/4703") @Test void extraParensAroundInfixOperator() { rewriteRun( groovy( """ - def foo(Map map) { - ((map.containsKey("foo")) - && ((map.get("foo")).equals("bar"))) - } def timestamp(int hours, int minutes, int seconds) { (hours) * 60 * 60 + (minutes * 60) + seconds } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java index e5fc6a0cb95..5cf82ae85be 100755 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java @@ -16,6 +16,7 @@ package org.openrewrite.groovy.tree; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; @@ -79,6 +80,18 @@ void groovyCastAndInvokeMethod() { ); } + @Test + @ExpectedToFail("Parentheses with method invocation is not yet supported") + void groovyCastAndInvokeMethodWithParentheses() { + rewriteRun( + groovy( + """ + (((((( "" as String ))))).toString()) + """ + ) + ); + } + @Test void javaCastAndInvokeMethod() { rewriteRun( @@ -89,4 +102,16 @@ void javaCastAndInvokeMethod() { ) ); } + + @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Test + void javaCastAndInvokeMethodWithParentheses() { + rewriteRun( + groovy( + """ + (((((( (String) "" )).toString())))) + """ + ) + ); + } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodDeclarationTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodDeclarationTest.java index 1963ef2db08..ff2bf380095 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodDeclarationTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodDeclarationTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.groovy.tree; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.Issue; import org.openrewrite.java.JavaIsoVisitor; @@ -182,16 +181,16 @@ void escapedMethodNameWithSpacesTest() { } @Issue("https://github.com/openrewrite/rewrite/issues/4705") - @Disabled @Test void functionWithDefAndExplicitReturnType() { rewriteRun( groovy( - """ - class A { - def int one() { 1 } - } """ + class A { + def /*int*/ int one() { 1 } + def /*Object*/ Object two() { 2 } + } + """ ) ); } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java index e22c1e08d46..a0714126901 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java @@ -16,6 +16,7 @@ package org.openrewrite.groovy.tree; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; @@ -45,6 +46,22 @@ void gradle() { ); } + @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Test + @Issue("https://github.com/openrewrite/rewrite/issues/4615") + void gradleWithParentheses() { + rewriteRun( + groovy( + """ + plugins { + id 'java-library' + } + def version = (rootProject.jobName.startsWith('a')) ? "latest.release" : "3.0" + """ + ) + ); + } + @Test void emptyArgsWithParens() { rewriteRun( @@ -52,6 +69,19 @@ void emptyArgsWithParens() { ); } + @Test + void noParentheses() { + rewriteRun( + groovy( + """ + class SomeObject {} + def foo(String a, int b, SomeObject c, String d) {} + foo "a", 3, new SomeObject(), "d" + """ + ) + ); + } + @Test @SuppressWarnings("GroovyVariableNotAssigned") void nullSafeDereference() { @@ -163,6 +193,72 @@ def acceptsClosure(Closure cl) {} ); } + @Issue("https://github.com/openrewrite/rewrite/issues/4766") + @Test + void gradleFileWithMultipleClosuresWithoutParentheses() { + rewriteRun( + groovy( + """ + copySpec { + from { 'src/main/webapp' } { exclude "**/*.jpg" } + rename '(.+)-staging(.+)', '$1$2' + } + """ + ) + ); + } + + @Test + void multipleClosureArgumentsWithoutParentheses() { + rewriteRun( + groovy( + """ + def foo(Closure a, Closure b, Closure c) {} + foo { } { } { + } + """ + ) + ); + } + + @Test + void multipleClosureArgumentsWithParentheses() { + rewriteRun( + groovy( + """ + def foo(Closure a, Closure b, Closure c) {} + foo({ }, { }, { + }) + """ + ) + ); + } + + @Test + void multipleArgumentsWithClosuresAndNonClosuresWithoutParentheses() { + rewriteRun( + groovy( + """ + def foo(String a, Closure b, Closure c, String d) {} + foo "a", { }, { + }, "d" + """ + ) + ); + } + + @Test + void trailingClosures() { + rewriteRun( + groovy( + """ + def foo(String a, int b, String c, Closure d, Closure e, Closure f) {} + foo("bar", 3, "baz") { } { } {} + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/1236") @Test @SuppressWarnings("GroovyAssignabilityCheck") @@ -262,4 +358,33 @@ static void main(String[] args) { ) ); } + + @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Issue("https://github.com/openrewrite/rewrite/issues/4703") + @Test + void insideParentheses() { + rewriteRun( + groovy( + """ + static def foo(Map map) { + ((map.containsKey("foo")) + && ((map.get("foo")).equals("bar"))) + } + """ + ) + ); + } + + @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Issue("https://github.com/openrewrite/rewrite/issues/4703") + @Test + void insideParenthesesWithoutNewLineAndEscapedMethodName() { + rewriteRun( + groovy( + """ + static def foo(Map someMap) {((((((someMap.get("(bar")))) ).'equals' "baz" ) ) } + """ + ) + ); + } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java index bdea313eb33..ac839e34d88 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java @@ -16,6 +16,7 @@ package org.openrewrite.groovy.tree; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.test.RewriteTest; import static org.openrewrite.groovy.Assertions.groovy; @@ -46,4 +47,18 @@ void parenthesized() { ) ); } + + @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Test + void parenthesizedAndInvokeMethodWithParentheses() { + rewriteRun( + groovy( + """ + (((( 8..19 ))).each { majorVersion -> + if (majorVersion == 9) return + }) + """ + ) + ); + } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/TernaryTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/TernaryTest.java index 09ef3f92b64..bb6bfb5fb4a 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/TernaryTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/TernaryTest.java @@ -31,7 +31,9 @@ void insideParentheses() { // NOT inside parentheses, but verifies the parser's // test for "inside parentheses" condition - groovy("(true) ? 1 : 2") + groovy("(true) ? 1 : 2"), + // And combine the two cases + groovy("((true) ? 1 : 2)") ); } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/VariableDeclarationsTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/VariableDeclarationsTest.java index 3915c6c39f8..9a2efcf03bd 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/VariableDeclarationsTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/VariableDeclarationsTest.java @@ -32,7 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.groovy.Assertions.groovy; -@SuppressWarnings({"GroovyUnusedAssignment", "DataFlowIssue"}) +@SuppressWarnings({"GroovyUnusedAssignment", "DataFlowIssue", "GrUnnecessaryDefModifier"}) class VariableDeclarationsTest implements RewriteTest { @Test @@ -203,4 +203,17 @@ class A { ) ); } + + @Issue("https://github.com/openrewrite/rewrite/issues/4705") + @Test + void defAndExplicitReturnType() { + rewriteRun( + groovy( + """ + def /*int*/ int one = 1 + def /*Object*/ Object two = 2 + """ + ) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java index e5b6c404e5f..d82981f8863 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java @@ -37,9 +37,7 @@ import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; -import static org.openrewrite.java.Assertions.addTypesToSourceSet; -import static org.openrewrite.java.Assertions.java; -import static org.openrewrite.java.Assertions.srcMainJava; +import static org.openrewrite.java.Assertions.*; import static org.openrewrite.test.RewriteTest.toRecipe; @SuppressWarnings("rawtypes") @@ -113,7 +111,7 @@ void dontDuplicateImports() { """ import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus.Series; - + class A {} """ ) @@ -139,7 +137,7 @@ class A {} import org.junit.jupiter.api.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - + class A {} """ ) @@ -155,16 +153,16 @@ void dontDuplicateImports3() { """ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; - + import java.util.List; class A {} """, """ import static org.junit.jupiter.api.Assertions.*; - + import java.util.List; - + class A {} """ ) @@ -178,7 +176,7 @@ void dontImportYourself() { java( """ package com.myorg; - + class A { } """ @@ -241,7 +239,7 @@ void dontImportFromSamePackage() { java( """ package com.myorg; - + class B { } """ @@ -249,7 +247,7 @@ class B { java( """ package com.myorg; - + class A { } """ @@ -309,7 +307,7 @@ void addNamedImport() { java("class A {}", """ import java.util.List; - + class A {} """ ) @@ -323,7 +321,7 @@ void doNotAddImportIfNotReferenced() { java( """ package a; - + class A {} """ ) @@ -337,22 +335,22 @@ void addImportInsertsNewMiddleBlock() { java( """ package a; - + import com.sun.naming.*; - + import static java.util.Collections.*; - + class A {} """, """ package a; - + import com.sun.naming.*; - + import java.util.List; - + import static java.util.Collections.*; - + class A {} """ ) @@ -366,14 +364,14 @@ void addFirstImport() { java( """ package a; - + class A {} """, """ package a; - + import java.util.List; - + class A {} """ ) @@ -385,22 +383,22 @@ class A {} void addImportIfReferenced() { rewriteRun( spec -> spec.recipe(toRecipe(() -> - new JavaIsoVisitor<>() { - @Override - public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { - J.ClassDeclaration c = super.visitClassDeclaration(classDecl, ctx); - maybeAddImport("java.math.BigDecimal"); - maybeAddImport("java.math.RoundingMode"); - return JavaTemplate.builder("BigDecimal d = BigDecimal.valueOf(1).setScale(1, RoundingMode.HALF_EVEN);") - .imports("java.math.BigDecimal", "java.math.RoundingMode") - .build() - .apply( - updateCursor(c), - c.getBody().getCoordinates().lastStatement() - ); - } - } - ).withMaxCycles(1)), + new JavaIsoVisitor<>() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration c = super.visitClassDeclaration(classDecl, ctx); + maybeAddImport("java.math.BigDecimal"); + maybeAddImport("java.math.RoundingMode"); + return JavaTemplate.builder("BigDecimal d = BigDecimal.valueOf(1).setScale(1, RoundingMode.HALF_EVEN);") + .imports("java.math.BigDecimal", "java.math.RoundingMode") + .build() + .apply( + updateCursor(c), + c.getBody().getCoordinates().lastStatement() + ); + } + } + ).withMaxCycles(1)), java( """ package a; @@ -410,10 +408,10 @@ class A { """, """ package a; - + import java.math.BigDecimal; import java.math.RoundingMode; - + class A { BigDecimal d = BigDecimal.valueOf(1).setScale(1, RoundingMode.HALF_EVEN); } @@ -429,7 +427,7 @@ void doNotAddWildcardImportIfNotReferenced() { java( """ package a; - + class A {} """ ) @@ -443,7 +441,7 @@ void lastImportWhenFirstClassDeclarationHasJavadoc() { java( """ import java.util.List; - + /** * My type */ @@ -451,9 +449,9 @@ class A {} """, """ import java.util.List; - + import static java.util.Collections.*; - + /** * My type */ @@ -474,9 +472,9 @@ class A {} """, """ package a; - + import java.util.List; - + class A {} """ ) @@ -516,18 +514,18 @@ public class B {} java( """ package a; - + import c.C0; import c.c.C1; import c.c.c.C2; - + class A {} """, String.format(""" package a; - + %s - + class A {} """, expectedImports.stream().map(i -> "import " + i + ";").collect(Collectors.joining("\n")) @@ -549,7 +547,7 @@ void doNotAddImportIfAlreadyExists() { java( """ package a; - + import java.util.List; class A {} """ @@ -564,7 +562,7 @@ void doNotAddImportIfCoveredByStarImport() { java( """ package a; - + import java.util.*; class A {} """ @@ -595,17 +593,17 @@ void addNamedImportIfStarStaticImportExists() { java( """ package a; - + import static java.util.List.*; class A {} """, """ package a; - + import java.util.List; - + import static java.util.List.*; - + class A {} """ ) @@ -623,9 +621,9 @@ class A {} """, """ import java.util.*; - + import static java.util.Collections.emptyList; - + class A {} """ ) @@ -640,7 +638,7 @@ void addStaticImportForUnreferencedField() { java( """ package mycompany; - + public class Type { public static String FIELD; } @@ -650,7 +648,7 @@ public class Type { "class A {}", """ import static mycompany.Type.FIELD; - + class A {} """ ) @@ -683,17 +681,17 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex java( """ public class A { - + } """, """ import java.time.temporal.ChronoUnit; - + import static java.time.temporal.ChronoUnit.MILLIS; - + public class A { ChronoUnit unit = MILLIS; - + } """ ) @@ -708,9 +706,9 @@ void dontAddImportToStaticFieldWithNamespaceConflict() { java( """ package a; - + import java.time.temporal.ChronoUnit; - + class A { static final int MILLIS = 1; ChronoUnit unit = ChronoUnit.MILLIS; @@ -727,7 +725,7 @@ void dontAddStaticWildcardImportIfNotReferenced() { java( """ package a; - + class A {} """ ) @@ -749,9 +747,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu java( """ package a; - + import java.util.List; - + class A { public A() { List list = java.util.Collections.emptyList(); @@ -760,11 +758,11 @@ public A() { """, """ package a; - + import java.util.List; - + import static java.util.Collections.emptyList; - + class A { public A() { List list = emptyList(); @@ -775,6 +773,49 @@ public A() { ); } + @Test + void addNamedStaticImportWhenReferenced2() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext executionContext) { + method = super.visitMethodDeclaration(method, executionContext); + method = JavaTemplate.builder("List list = new ArrayList<>();") + .imports("java.util.ArrayList", "java.util.List") + .staticImports("java.util.Calendar.Builder") + .build() + .apply(getCursor(), method.getBody().getCoordinates().firstStatement()); + maybeAddImport("java.util.ArrayList"); + maybeAddImport("java.util.List"); + maybeAddImport("java.util.Calendar", "Builder"); + return method; + } + }).withMaxCycles(1)), + java( + """ + import static java.util.Calendar.Builder; + + class A { + public A() { + } + } + """, + """ + import java.util.ArrayList; + import java.util.List; + + import static java.util.Calendar.Builder; + + class A { + public A() { + List list = new ArrayList<>(); + } + } + """ + ) + ); + } + @Test void doNotAddNamedStaticImportIfNotReferenced() { rewriteRun( @@ -782,7 +823,7 @@ void doNotAddNamedStaticImportIfNotReferenced() { java( """ package a; - + class A {} """ ) @@ -807,9 +848,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu java( """ package a; - + import java.util.List; - + class A { public A() { List list = java.util.Collections.emptyList(); @@ -818,11 +859,11 @@ public A() { """, """ package a; - + import java.util.List; - + import static java.util.Collections.*; - + class A { public A() { List list = emptyList(); @@ -883,14 +924,14 @@ public class C { """ import foo.B; import foo.C; - + import java.util.Collections; import java.util.List; import java.util.HashSet; import java.util.HashMap; import java.util.Map; import java.util.Set; - + class A { B b = new B(); C c = new C(); @@ -903,7 +944,7 @@ class A { """ import foo.B; import foo.C; - + import java.util.*; class A { @@ -928,15 +969,15 @@ void addImportWhenDuplicatesExist() { """ import javax.ws.rs.Path; import javax.ws.rs.Path; - + class A {} """, """ import org.springframework.http.MediaType; - + import javax.ws.rs.Path; import javax.ws.rs.Path; - + class A {} """ ) @@ -952,15 +993,15 @@ void unorderedImportsWithNewBlock() { """ import org.foo.B; import org.foo.A; - + class A {} """, """ import org.foo.B; import org.foo.A; - + import java.time.Duration; - + class A {} """ ) @@ -981,7 +1022,7 @@ void doNotFoldNormalImportWithNamespaceConflict() { import java.util.Collections; import java.util.Map; import java.util.Set; - + @SuppressWarnings("ALL") class Test { List list; @@ -994,7 +1035,7 @@ class Test { import java.util.List; import java.util.Map; import java.util.Set; - + @SuppressWarnings("ALL") class Test { List list; @@ -1136,7 +1177,7 @@ void noImportLayout() { """, """ import java.util.List; - + import static java.util.Collections.*; """ ) @@ -1154,9 +1195,9 @@ class A {} """.replace("\n", "\r\n"), """ package a; - + import java.util.List; - + class A {} """.replace("\n", "\r\n") ) @@ -1170,17 +1211,17 @@ void crlfNewLinesWithPreviousImports() { java( """ package a; - + import java.util.Set; - + class A {} """.replace("\n", "\r\n"), """ package a; - + import java.util.List; import java.util.Set; - + class A {} """.replace("\n", "\r\n") ) @@ -1194,13 +1235,13 @@ void crlfNewLinesWithPreviousImportsNoPackage() { java( """ import java.util.Set; - + class A {} """.replace("\n", "\r\n"), """ import java.util.List; import java.util.Set; - + class A {} """.replace("\n", "\r\n") ) @@ -1214,13 +1255,13 @@ void crlfNewLinesWithPreviousImportsNoClass() { java( """ package a; - + import java.util.Arrays; import java.util.Set; """.replace("\n", "\r\n"), """ package a; - + import java.util.Arrays; import java.util.List; import java.util.Set; @@ -1269,10 +1310,10 @@ void crlfNewLinesInComments() { * limitations under the License. */ """.replace("\n", "\r\n") + - """ - import java.util.Arrays; - import java.util.Set; - """, + """ + import java.util.Arrays; + import java.util.Set; + """, """ /* * Copyright 2023 the original author or authors. @@ -1305,31 +1346,31 @@ void crlfNewLinesInJavadoc() { """ import java.util.Arrays; import java.util.Set; - + """ + - """ - /** - * 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. - */ - """.replace("\n", "\r\n") + - "class Foo {}", + """ + /** + * 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. + */ + """.replace("\n", "\r\n") + + "class Foo {}", """ import java.util.Arrays; import java.util.List; import java.util.Set; - + /** * Copyright 2023 the original author or authors. *

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 39ca3ed049e..5419f7c5be6 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 @@ -32,6 +32,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.properties.Assertions.properties; import static org.openrewrite.xml.Assertions.xml; +import static org.openrewrite.yaml.Assertions.yaml; @SuppressWarnings("ConstantConditions") class ChangePackageTest implements RewriteTest { @@ -430,11 +431,11 @@ class A { } """, """ - import org.openrewrite.test.other.Test; - class A { - Test test = null; - } - """, + import org.openrewrite.test.other.Test; + class A { + Test test = null; + } + """, spec -> spec.afterRecipe(cu -> { assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty(); assertThat(cu.findType("org.openrewrite.test.other.Test")).isNotEmpty(); @@ -488,11 +489,11 @@ class A { } """, """ - import org.openrewrite.test.other.Test; - class A { - Test test = null; - } - """, + import org.openrewrite.test.other.Test; + class A { + Test test = null; + } + """, spec -> spec.afterRecipe(cu -> { assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty(); assertThat(cu.findType("org.openrewrite.test.other.Test")).isNotEmpty(); @@ -546,11 +547,11 @@ class A { } """, """ - import org.openrewrite.test.other.Test; - class A { - Test test = null; - } - """, + import org.openrewrite.test.other.Test; + class A { + Test test = null; + } + """, spec -> spec.afterRecipe(cu -> { assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty(); assertThat(cu.findType("org.openrewrite.test.other.Test")).isNotEmpty(); @@ -705,7 +706,7 @@ void method() {} void annotationArgument() { rewriteRun( java( - """ + """ package org.openrewrite; public class Argument {} """, @@ -757,7 +758,7 @@ void method() {} void annotationArgumentNamed() { rewriteRun( java( - """ + """ package org.openrewrite; public class Argument {} """, @@ -807,7 +808,7 @@ void method() {} void annotationArgumentFullyQualified() { rewriteRun( java( - """ + """ package org.openrewrite; public class Argument {} """, @@ -855,7 +856,7 @@ void method() {} void annotationArgumentNamedFullyQualified() { rewriteRun( java( - """ + """ package org.openrewrite; public class Argument {} """, @@ -1439,7 +1440,7 @@ void staticImport() { java( """ import static org.openrewrite.Test.stat; - + public class B { public void test() { stat(); @@ -1448,7 +1449,7 @@ public void test() { """, """ import static org.openrewrite.test.Test.stat; - + public class B { public void test() { stat(); @@ -1724,7 +1725,6 @@ void changePackageInSpringXml() { """ ) ); - } @Test @@ -1741,7 +1741,30 @@ void changeTypeInPropertiesFile() { a.property=java.cool.String b.property=java.cool.test.String c.property=String - """, spec -> spec.path("application.properties")) + """, + spec -> spec.path("application.properties")) + ); + } + + @Test + void changePackageInYaml() { + rewriteRun( + spec -> spec.recipe(new ChangePackage("java.lang", "java.cool", true)), + yaml( + """ + root: + a: java.lang.String + b: java.lang.test.String + c: String + """, + """ + root: + a: java.cool.String + b: java.cool.test.String + c: String + """, + spec -> spec.path("application.yaml") + ) ); } } 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 487e56e4a8e..c453a2246fb 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 @@ -31,6 +31,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.properties.Assertions.properties; import static org.openrewrite.xml.Assertions.xml; +import static org.openrewrite.yaml.Assertions.yaml; @SuppressWarnings("ConstantConditions") class ChangeTypeTest implements RewriteTest { @@ -207,7 +208,7 @@ class Test { class Test { List p; - List p2; + java.util.List p2; java.util.List p3; } """ @@ -2071,12 +2072,150 @@ void changeTypeInPropertiesFile() { properties( """ a.property=java.lang.String + c.property=java.lang.StringBuilder b.property=String """, """ a.property=java.lang.Integer + c.property=java.lang.StringBuilder b.property=String """, spec -> spec.path("application.properties")) ); } + + @Test + void changeTypeInYaml() { + rewriteRun( + spec -> spec.recipe(new ChangeType("java.lang.String", "java.lang.Integer", true)), + yaml( + """ + root: + a: java.lang.String + b: java.lang.StringBuilder + c: java.lang.test.String + d: String + """, + """ + root: + a: java.lang.Integer + b: java.lang.StringBuilder + c: java.lang.test.String + d: String + """, + spec -> spec.path("application.yaml") + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite/issues/4773") + void noRenameOfTypeWithMatchingPrefix() { + rewriteRun( + spec -> spec.recipe(new ChangeType("org.codehaus.jackson.annotate.JsonIgnoreProperties", "com.fasterxml.jackson.annotation.JsonIgnoreProperties", false)) + .parser(JavaParser.fromJavaVersion() + .dependsOn( + """ + package org.codehaus.jackson.annotate; + public @interface JsonIgnoreProperties { + boolean ignoreUnknown() default false; + } + """, + """ + package org.codehaus.jackson.annotate; + public @interface JsonIgnore { + } + """ + ) + ), + java( + """ + import org.codehaus.jackson.annotate.JsonIgnore; + import org.codehaus.jackson.annotate.JsonIgnoreProperties; + + @JsonIgnoreProperties(ignoreUnknown = true) + public class myClass { + @JsonIgnore + public boolean isDirty() { + return false; + } + } + """, + """ + import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + import org.codehaus.jackson.annotate.JsonIgnore; + + @JsonIgnoreProperties(ignoreUnknown = true) + public class myClass { + @JsonIgnore + public boolean isDirty() { + return false; + } + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite/issues/4764") + void changeTypeOfInnerClass() { + rewriteRun( + spec -> spec.recipe(new ChangeType("foo.A$Builder", "bar.A$Builder", true)) + .parser(JavaParser.fromJavaVersion().dependsOn( + """ + package foo; + + public class A { + public A.Builder builder() { + return new A.Builder(); + } + + public static class Builder { + public A build() { + return new A(); + } + } + } + """, + """ + package bar; + + public class A { + public A.Builder builder() { + return new A.Builder(); + } + + public static class Builder { + public A build() { + return new A(); + } + } + } + """ + ) + ), + java( + """ + import foo.A; + import foo.A.Builder; + + class Test { + A test() { + A.Builder b = A.builder(); + return b.build(); + } + } + """, """ + import foo.A; + + class Test { + A test() { + bar.A.Builder b = A.builder(); + return b.build(); + } + } + """ + ) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java index 03c6a38ee89..3115386c7d7 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java @@ -29,8 +29,10 @@ import org.openrewrite.test.RewriteTest; import java.io.IOException; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.stream.Stream; @@ -105,6 +107,16 @@ void dependenciesFromResources(@TempDir Path temp) throws Exception { "directory contains guava-30.0-jre.jar which has the same prefix"); } + @Test + void getParserClasspathDownloadCreateRequiredFolder(@TempDir Path temp) throws Exception { + Path updatedTemp = Path.of(temp.toString(), "someFolder"); + assertThat(updatedTemp.toFile().exists()).isFalse(); + JavaParserExecutionContextView ctx = JavaParserExecutionContextView.view(new InMemoryExecutionContext()); + ctx.setParserClasspathDownloadTarget(updatedTemp.toFile()); + ctx.getParserClasspathDownloadTarget(); + assertThat(updatedTemp.toFile().exists()).isTrue(); + } + @Test @Issue("https://github.com/openrewrite/rewrite/issues/3222") void parseFromByteArray() { @@ -214,4 +226,22 @@ class A { ) ); } + + @Test + void filterArtifacts() { + List classpath = List.of( + URI.create("file:/.m2/repository/com/google/guava/guava-24.1.1/com_google_guava_guava-24.1.1.jar"), + URI.create("file:/.m2/repository/org/threeten/threeten-extra-1.5.0/org_threeten_threeten_extra-1.5.0.jar"), + URI.create("file:/.m2/repository/com/amazonaws/aws-java-sdk-s3-1.11.546/com_amazonaws_aws_java_sdk_s3-1.11.546.jar"), + URI.create("file:/.m2/repository/org/openrewrite/rewrite-java/8.41.1/rewrite-java-8.41.1.jar") + ); + assertThat(JavaParser.filterArtifacts("threeten-extra", classpath)) + .containsOnly(Paths.get("/.m2/repository/org/threeten/threeten-extra-1.5.0/org_threeten_threeten_extra-1.5.0.jar")); + assertThat(JavaParser.filterArtifacts("guava", classpath)) + .containsOnly(Paths.get("/.m2/repository/com/google/guava/guava-24.1.1/com_google_guava_guava-24.1.1.jar")); + assertThat(JavaParser.filterArtifacts("aws-java-sdk-s3", classpath)) + .containsOnly(Paths.get("/.m2/repository/com/amazonaws/aws-java-sdk-s3-1.11.546/com_amazonaws_aws_java_sdk_s3-1.11.546.jar")); + assertThat(JavaParser.filterArtifacts("rewrite-java", classpath)) + .containsOnly(Paths.get("/.m2/repository/org/openrewrite/rewrite-java/8.41.1/rewrite-java-8.41.1.jar")); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java index 6b8d5b62381..4264fcf0986 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java @@ -64,7 +64,7 @@ public class A { @DocumentExample @Test - void foo() { + void skipMissingTypeAttribution() { rewriteRun( spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), java( diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/search/FindTypesTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/search/FindTypesTest.java index d85bd134a47..321f560b763 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/search/FindTypesTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/search/FindTypesTest.java @@ -47,6 +47,10 @@ public static void stat() {} @Test void simpleName() { rewriteRun( + spec -> spec.dataTable(TypeUses.Row.class, rows -> assertThat(rows) + .containsExactly( + new TypeUses.Row("B.java", "A1", "a.A1") + )), java( """ import a.A1; diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java b/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java index 3feff736d1c..9aecd66f89f 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java @@ -107,7 +107,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String // No need to add imports if the class to import is in java.lang, or if the classes are within the same package if (("java.lang".equals(packageName) && StringUtils.isBlank(member)) || (cu.getPackageDeclaration() != null && - packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor())))) { + packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor())))) { return cu; } @@ -119,10 +119,10 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String String ending = i.getQualid().getSimpleName(); if (member == null) { return !i.isStatic() && i.getPackageName().equals(packageName) && - (ending.equals(typeName) || "*".equals(ending)); + (ending.equals(typeName) || "*".equals(ending)); } return i.isStatic() && i.getTypeName().equals(fullyQualifiedName) && - (ending.equals(member) || "*".equals(ending)); + (ending.equals(member) || "*".equals(ending)); })) { return cu; } @@ -133,7 +133,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String new JLeftPadded<>(member == null ? Space.EMPTY : Space.SINGLE_SPACE, member != null, Markers.EMPTY), TypeTree.build(fullyQualifiedName + - (member == null ? "" : "." + member)).withPrefix(Space.SINGLE_SPACE), + (member == null ? "" : "." + member)).withPrefix(Space.SINGLE_SPACE), null); List> imports = new ArrayList<>(cu.getPadding().getImports()); @@ -214,7 +214,7 @@ private boolean hasReference(JavaSourceFile compilationUnit) { //Non-static imports, we just look for field accesses. for (NameTree t : FindTypes.find(compilationUnit, fullyQualifiedName)) { if ((!(t instanceof J.FieldAccess) || !((J.FieldAccess) t).isFullyQualifiedClassReference(fullyQualifiedName)) && - isTypeReference(t)) { + isTypeReference(t)) { return true; } } @@ -226,7 +226,7 @@ private boolean hasReference(JavaSourceFile compilationUnit) { if (invocation instanceof J.MethodInvocation) { J.MethodInvocation mi = (J.MethodInvocation) invocation; if (mi.getSelect() == null && - ("*".equals(member) || mi.getName().getSimpleName().equals(member))) { + ("*".equals(member) || mi.getName().getSimpleName().equals(member))) { return true; } } @@ -239,11 +239,18 @@ private boolean hasReference(JavaSourceFile compilationUnit) { } private class FindStaticFieldAccess extends JavaIsoVisitor> { + private boolean checkIsOfClassType(@Nullable JavaType type, String fullyQualifiedName) { + if (isOfClassType(type, fullyQualifiedName)) { + return true; + } + return type instanceof JavaType.Class && isOfClassType(((JavaType.Class) type).getOwningClass(), fullyQualifiedName); + } + @Override public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, AtomicReference found) { // If the type isn't used there's no need to proceed further for (JavaType.Variable varType : cu.getTypesInUse().getVariables()) { - if (varType.getName().equals(member) && isOfClassType(varType.getType(), fullyQualifiedName)) { + if (checkIsOfClassType(varType.getType(), fullyQualifiedName)) { return super.visitCompilationUnit(cu, found); } } @@ -253,8 +260,9 @@ public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, AtomicRefere @Override public J.Identifier visitIdentifier(J.Identifier identifier, AtomicReference found) { assert getCursor().getParent() != null; - if (identifier.getSimpleName().equals(member) && isOfClassType(identifier.getType(), fullyQualifiedName) && - !(getCursor().getParent().firstEnclosingOrThrow(J.class) instanceof J.FieldAccess)) { + if (identifier.getSimpleName().equals(member) && + checkIsOfClassType(identifier.getType(), fullyQualifiedName) && + !(getCursor().getParent().firstEnclosingOrThrow(J.class) instanceof J.FieldAccess)) { found.set(true); } return identifier; 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 be098009e16..a62f0be4967 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -85,7 +85,7 @@ public TreeVisitor getVisitor() { return Preconditions.check(new UsesType<>(annotationType, false), new JavaIsoVisitor() { @Override public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { - J.Annotation original = a; + J.Annotation original = super.visitAnnotation(a, ctx); if (!TypeUtils.isOfClassType(a.getType(), annotationType)) { return a; } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java index 98913cb04a7..066da57e0fb 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java @@ -74,26 +74,11 @@ static List dependenciesFromClasspath(String... artifactNames) { List artifacts = new ArrayList<>(artifactNames.length); List missingArtifactNames = new ArrayList<>(artifactNames.length); for (String artifactName : artifactNames) { - Pattern jarPattern = Pattern.compile(artifactName + "(?:" + "-.*?" + ")?" + "\\.jar$"); - // In a multi-project IDE classpath, some classpath entries aren't jars - Pattern explodedPattern = Pattern.compile("/" + artifactName + "/"); - boolean lacking = true; - for (URI cpEntry : runtimeClasspath) { - if (!"file".equals(cpEntry.getScheme())) { - // exclude any `jar` entries which could result from `Bundle-ClassPath` in `MANIFEST.MF` - continue; - } - String cpEntryString = cpEntry.toString(); - Path path = Paths.get(cpEntry); - if (jarPattern.matcher(cpEntryString).find() || - explodedPattern.matcher(cpEntryString).find() && path.toFile().isDirectory()) { - artifacts.add(path); - lacking = false; - // Do not break because jarPattern matches "foo-bar-1.0.jar" and "foo-1.0.jar" to "foo" - } - } - if (lacking) { + List matchedArtifacts = filterArtifacts(artifactName, runtimeClasspath); + if (matchedArtifacts.isEmpty()) { missingArtifactNames.add(artifactName); + } else { + artifacts.addAll(matchedArtifacts); } } @@ -107,6 +92,37 @@ static List dependenciesFromClasspath(String... artifactNames) { return artifacts; } + /** + * Filters the classpath entries to find paths that match the given artifact name. + * + * @param artifactName The artifact name to search for. + * @param runtimeClasspath The list of classpath URIs to search within. + * @return List of Paths that match the artifact name. + */ + // VisibleForTesting + static List filterArtifacts(String artifactName, List runtimeClasspath) { + List artifacts = new ArrayList<>(); + // Bazel automatically replaces '-' with '_' when generating jar files. + String normalizedArtifactName = artifactName.replace('-', '_'); + Pattern jarPattern = Pattern.compile(String.format("(%s|%s)(?:-.*?)?\\.jar$", artifactName, normalizedArtifactName)); + // In a multi-project IDE classpath, some classpath entries aren't jars + Pattern explodedPattern = Pattern.compile("/" + artifactName + "/"); + for (URI cpEntry : runtimeClasspath) { + if (!"file".equals(cpEntry.getScheme())) { + // exclude any `jar` entries which could result from `Bundle-ClassPath` in `MANIFEST.MF` + continue; + } + String cpEntryString = cpEntry.toString(); + Path path = Paths.get(cpEntry); + if (jarPattern.matcher(cpEntryString).find() || + explodedPattern.matcher(cpEntryString).find() && path.toFile().isDirectory()) { + artifacts.add(path); + // Do not break because jarPattern matches "foo-bar-1.0.jar" and "foo-1.0.jar" to "foo" + } + } + return artifacts; + } + static List dependenciesFromResources(ExecutionContext ctx, String... artifactNamesWithVersions) { if (artifactNamesWithVersions.length == 0) { return Collections.emptyList(); diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaParserExecutionContextView.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaParserExecutionContextView.java index 884104913ab..266ee8928a8 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/JavaParserExecutionContextView.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/JavaParserExecutionContextView.java @@ -45,11 +45,10 @@ public JavaParserExecutionContextView setParserClasspathDownloadTarget(File fold public File getParserClasspathDownloadTarget() { File target = getMessage(PARSER_CLASSPATH_DOWNLOAD_LOCATION); if (target == null) { - File defaultTarget = new File(System.getProperty("user.home") + "/.rewrite/classpath"); - if (!defaultTarget.mkdirs() && !defaultTarget.exists()) { - throw new UncheckedIOException(new IOException("Failed to create directory " + defaultTarget.getAbsolutePath())); - } - return defaultTarget; + target = new File(System.getProperty("user.home") + "/.rewrite/classpath"); + } + if (!target.mkdirs() && !target.exists()) { + throw new UncheckedIOException(new IOException("Failed to create directory " + target.getAbsolutePath())); } return target; } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java index 7cf86789aeb..4682c999ffa 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/RemoveMethodInvocationsVisitor.java @@ -69,26 +69,32 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) return j; } - private @Nullable J removeMethods(Expression expression, int depth, boolean isLambdaBody, Stack selectAfter) { - if (!(expression instanceof J.MethodInvocation)) { + private @Nullable J removeMethods(@Nullable Expression expression, int depth, boolean isLambdaBody, Stack selectAfter) { + if (!(expression instanceof J.MethodInvocation) || ((J.MethodInvocation) expression).getMethodType() == null) { return expression; } - boolean isStatement = isStatement(); J.MethodInvocation m = (J.MethodInvocation) expression; + boolean isStatic = m.getMethodType().hasFlags(Flag.Static); - if (m.getMethodType() == null || m.getSelect() == null) { + if (isStatic && !isStatementInParentBlock(m)) { return expression; } + boolean isStatement = isStatement(); + 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; + boolean hasSameReturnType = m.getSelect() != null && TypeUtils.isAssignableTo(m.getMethodType().getReturnType(), m.getSelect().getType()); + boolean removable = ((isStatement || isStatic) && depth == 0) || hasSameReturnType; if (!removable) { return expression; } - if (m.getSelect() instanceof J.Identifier || m.getSelect() instanceof J.NewClass) { + maybeRemoveImport(m.getMethodType().getDeclaringType()); + + if (m.getSelect() == null) { + return null; + } else if (m.getSelect() instanceof J.Identifier || m.getSelect() instanceof J.NewClass) { boolean keepSelect = depth != 0; if (keepSelect) { selectAfter.add(getSelectAfter(m)); @@ -131,6 +137,11 @@ private boolean isStatement() { ).getValue() instanceof J.Block; } + private boolean isStatementInParentBlock(Statement method) { + J.Block parentBlock = getCursor().firstEnclosing(J.Block.class); + return parentBlock == null || parentBlock.getStatements().contains(method); + } + private boolean isLambdaBody() { if (getCursor().getParent() == null) { return false; @@ -241,7 +252,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { @Value @With - static class ToBeRemoved implements Marker { + private static class ToBeRemoved implements Marker { UUID id; static J2 withMarker(J2 j) { return j.withMarkers(j.getMarkers().addIfAbsent(new ToBeRemoved(randomId()))); 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 1cf2f2ff53f..1a1b5aa6f40 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/TypeMatcher.java @@ -119,5 +119,4 @@ public boolean matchesReference(Reference reference) { public Reference.Renamer createRenamer(String newName) { return reference -> newName; } - } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/BlockStatementTemplateGenerator.java b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/BlockStatementTemplateGenerator.java index 3be2c6e2b0b..e39d37b01ea 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/BlockStatementTemplateGenerator.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/BlockStatementTemplateGenerator.java @@ -463,16 +463,17 @@ private void contextTemplate(Cursor cursor, J prior, StringBuilder before, Strin // If prior is a type parameter, wrap in __M__.anyT() // For anything else, ignore the invocation J.MethodInvocation m = (J.MethodInvocation) j; + J firstEnclosing = cursor.getParentOrThrow().firstEnclosing(J.class); if (m.getArguments().stream().anyMatch(arg -> referToSameElement(prior, arg))) { before.insert(0, "__M__.any("); - if (cursor.getParentOrThrow().firstEnclosing(J.class) instanceof J.Block) { + if (firstEnclosing instanceof J.Block || firstEnclosing instanceof J.Case) { after.append(");"); } else { after.append(")"); } } else if (m.getTypeParameters() != null && m.getTypeParameters().stream().anyMatch(tp -> referToSameElement(prior, tp))) { before.insert(0, "__M__.anyT<"); - if (cursor.getParentOrThrow().firstEnclosing(J.class) instanceof J.Block) { + if (firstEnclosing instanceof J.Block || firstEnclosing instanceof J.Case) { after.append(">();"); } else { after.append(">()"); @@ -481,7 +482,7 @@ private void contextTemplate(Cursor cursor, J prior, StringBuilder before, Strin List comments = new ArrayList<>(1); comments.add(new TextComment(true, STOP_COMMENT, "", Markers.EMPTY)); after.append(".").append(m.withSelect(null).withComments(comments).printTrimmed(cursor.getParentOrThrow())); - if (cursor.getParentOrThrow().firstEnclosing(J.class) instanceof J.Block) { + if (firstEnclosing instanceof J.Block || firstEnclosing instanceof J.Case) { after.append(";"); } } @@ -561,6 +562,8 @@ private void contextTemplate(Cursor cursor, J prior, StringBuilder before, Strin before.insert(0, ev.getName()); } else if (j instanceof J.EnumValueSet) { after.append(";"); + } else if (j instanceof J.Case) { + after.append(";"); } contextTemplate(next(cursor), j, before, after, insertionPoint, REPLACEMENT); } 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 3b9c7f4d254..f9e4a051cf7 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 @@ -206,11 +206,14 @@ public J visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) { private J2 found(J2 j, ExecutionContext ctx) { JavaType.FullyQualified fqn = TypeUtils.asFullyQualified(j.getType()); - typeUses.insertRow(ctx, new TypeUses.Row( - getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), - j.printTrimmed(getCursor().getParentTreeCursor()), - fqn == null ? j.getType().toString() : fqn.getFullyQualifiedName() - )); + if (!j.getMarkers().findFirst(SearchResult.class).isPresent()) { + // Avoid double-counting results in the data table + typeUses.insertRow(ctx, new TypeUses.Row( + getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), + j.printTrimmed(getCursor().getParentTreeCursor()), + fqn == null ? j.getType().toString() : fqn.getFullyQualifiedName() + )); + } return SearchResult.found(j); } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/service/JavaNamingService.java b/rewrite-java/src/main/java/org/openrewrite/java/service/JavaNamingService.java index 5f2e84e9c04..f424ce8d9de 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/service/JavaNamingService.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/service/JavaNamingService.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 the original author or authors. + * 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. 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 7afe8698797..1ad1cee2193 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 @@ -1979,10 +1979,14 @@ public List getSideEffects() { } public boolean isFullyQualifiedClassReference(String className) { - if (!className.contains(".")) { + if (getName().getFieldType() == null && getName().getType() instanceof JavaType.FullyQualified && + !(getName().getType() instanceof JavaType.Unknown) && + TypeUtils.fullyQualifiedNamesAreEqual(((JavaType.FullyQualified) getName().getType()).getFullyQualifiedName(), className)) { + return true; + } else if (!className.contains(".")) { return false; } - return isFullyQualifiedClassReference(this, className, className.length()); + return isFullyQualifiedClassReference(this, TypeUtils.toFullyQualifiedName(className), className.length()); } private boolean isFullyQualifiedClassReference(J.FieldAccess fieldAccess, String className, int prevDotIndex) { @@ -1991,7 +1995,7 @@ private boolean isFullyQualifiedClassReference(J.FieldAccess fieldAccess, String return false; } String simpleName = fieldAccess.getName().getSimpleName(); - if (!simpleName.regionMatches(0, className, dotIndex + 1, simpleName.length())) { + if (!simpleName.regionMatches(0, className, dotIndex + 1, Math.max(simpleName.length(), prevDotIndex - dotIndex - 1))) { return false; } if (fieldAccess.getTarget() instanceof J.FieldAccess) { diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java index 112861469e8..2fa596187f4 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java @@ -15,7 +15,10 @@ */ package org.openrewrite.java.tree; -import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIdentityInfo; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.ObjectIdGenerators; import lombok.AccessLevel; import lombok.Getter; import lombok.With; @@ -102,7 +105,7 @@ public MultiCatch(@Nullable List throwableTypes) { this.throwableTypes = arrayOrNullIfEmpty(throwableTypes, EMPTY_JAVA_TYPE_ARRAY); } - MultiCatch(JavaType @Nullable[] throwableTypes) { + MultiCatch(JavaType @Nullable [] throwableTypes) { this.throwableTypes = nullIfEmpty(throwableTypes); } @@ -110,7 +113,7 @@ public MultiCatch(@Nullable List throwableTypes) { MultiCatch() { } - private JavaType[] throwableTypes; + private JavaType @Nullable [] throwableTypes; public List getThrowableTypes() { if (throwableTypes == null) { @@ -143,7 +146,7 @@ public Intersection(@Nullable List bounds) { this.bounds = arrayOrNullIfEmpty(bounds, EMPTY_JAVA_TYPE_ARRAY); } - Intersection(JavaType @Nullable[] throwableTypes) { + Intersection(JavaType @Nullable [] throwableTypes) { this.bounds = nullIfEmpty(throwableTypes); } @@ -151,7 +154,7 @@ public Intersection(@Nullable List bounds) { Intersection() { } - private JavaType[] bounds; + private JavaType @Nullable [] bounds; public List getBounds() { if (bounds == null) { @@ -320,16 +323,16 @@ public String getPackageName() { public boolean isAssignableTo(String fullyQualifiedName) { return TypeUtils.fullyQualifiedNamesAreEqual(getFullyQualifiedName(), fullyQualifiedName) || - getInterfaces().stream().anyMatch(anInterface -> anInterface.isAssignableTo(fullyQualifiedName)) || - (getSupertype() != null && getSupertype().isAssignableTo(fullyQualifiedName)); + getInterfaces().stream().anyMatch(anInterface -> anInterface.isAssignableTo(fullyQualifiedName)) || + (getSupertype() != null && getSupertype().isAssignableTo(fullyQualifiedName)); } public boolean isAssignableFrom(@Nullable JavaType type) { if (type instanceof FullyQualified) { FullyQualified clazz = (FullyQualified) type; return TypeUtils.fullyQualifiedNamesAreEqual(getFullyQualifiedName(), clazz.getFullyQualifiedName()) || - isAssignableFrom(clazz.getSupertype()) || - clazz.getInterfaces().stream().anyMatch(this::isAssignableFrom); + isAssignableFrom(clazz.getSupertype()) || + clazz.getInterfaces().stream().anyMatch(this::isAssignableFrom); } else if (type instanceof GenericTypeVariable) { GenericTypeVariable generic = (GenericTypeVariable) type; for (JavaType bound : generic.getBounds()) { @@ -372,7 +375,7 @@ class Class extends FullyQualified { Kind kind; @NonFinal - JavaType @Nullable[] typeParameters; + JavaType @Nullable [] typeParameters; @With @Nullable @@ -386,7 +389,7 @@ class Class extends FullyQualified { @NonFinal - FullyQualified @Nullable[] annotations; + FullyQualified @Nullable [] annotations; public Class(@Nullable Integer managedReference, long flagsBitMap, String fullyQualifiedName, Kind kind, @Nullable List typeParameters, @Nullable FullyQualified supertype, @Nullable FullyQualified owningClass, @@ -408,9 +411,9 @@ public Class(@Nullable Integer managedReference, long flagsBitMap, String fullyQ } Class(@Nullable Integer managedReference, long flagsBitMap, String fullyQualifiedName, - Kind kind, JavaType @Nullable[] typeParameters, @Nullable FullyQualified supertype, @Nullable FullyQualified owningClass, - FullyQualified @Nullable[] annotations, FullyQualified @Nullable[] interfaces, - Variable @Nullable[] members, Method @Nullable[] methods) { + Kind kind, JavaType @Nullable [] typeParameters, @Nullable FullyQualified supertype, @Nullable FullyQualified owningClass, + FullyQualified @Nullable [] annotations, FullyQualified @Nullable [] interfaces, + Variable @Nullable [] members, Method @Nullable [] methods) { this.managedReference = managedReference; this.flagsBitMap = flagsBitMap & Flag.VALID_CLASS_FLAGS; this.fullyQualifiedName = fullyQualifiedName; @@ -445,7 +448,7 @@ public Class withAnnotations(@Nullable List annotations) { @NonFinal - FullyQualified @Nullable[] interfaces; + FullyQualified @Nullable [] interfaces; @Override public List getInterfaces() { @@ -463,7 +466,7 @@ public Class withInterfaces(@Nullable List interfaces) { @NonFinal - Variable @Nullable[] members; + Variable @Nullable [] members; @Override public List getMembers() { @@ -481,7 +484,7 @@ public Class withMembers(@Nullable List members) { @NonFinal - Method @Nullable[] methods; + Method @Nullable [] methods; @Override public List getMethods() { @@ -554,9 +557,9 @@ public Class unsafeSet(@Nullable List typeParameters, @Nullable FullyQ return this; } - public Class unsafeSet(JavaType @Nullable[] typeParameters, @Nullable FullyQualified supertype, @Nullable FullyQualified owningClass, - FullyQualified @Nullable[] annotations, FullyQualified @Nullable[] interfaces, - Variable @Nullable[] members, Method @Nullable[] methods) { + public Class unsafeSet(JavaType @Nullable [] typeParameters, @Nullable FullyQualified supertype, @Nullable FullyQualified owningClass, + FullyQualified @Nullable [] annotations, FullyQualified @Nullable [] interfaces, + Variable @Nullable [] members, Method @Nullable [] methods) { //noinspection DuplicatedCode this.typeParameters = ListUtils.nullIfEmpty(typeParameters); this.supertype = supertype; @@ -574,7 +577,7 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; Class aClass = (Class) o; return TypeUtils.fullyQualifiedNamesAreEqual(fullyQualifiedName, aClass.fullyQualifiedName) && - (typeParameters == null && aClass.typeParameters == null || typeParameters != null && Arrays.equals(typeParameters, aClass.typeParameters)); + (typeParameters == null && aClass.typeParameters == null || typeParameters != null && Arrays.equals(typeParameters, aClass.typeParameters)); } @Override @@ -639,7 +642,7 @@ class Parameterized extends FullyQualified { FullyQualified type; @NonFinal - JavaType @Nullable[] typeParameters; + JavaType @Nullable [] typeParameters; public Parameterized(@Nullable Integer managedReference, @Nullable FullyQualified type, @Nullable List typeParameters) { @@ -651,7 +654,7 @@ public Parameterized(@Nullable Integer managedReference, @Nullable FullyQualifie } Parameterized(@Nullable Integer managedReference, @Nullable FullyQualified type, - JavaType @Nullable[] typeParameters) { + JavaType @Nullable [] typeParameters) { this.managedReference = managedReference; this.type = unknownIfNull(type); this.typeParameters = nullIfEmpty(typeParameters); @@ -691,7 +694,7 @@ public Parameterized unsafeSet(@Nullable FullyQualified type, @Nullable List bounds) { this( @@ -802,7 +805,7 @@ public GenericTypeVariable(@Nullable Integer managedReference, String name, Vari ); } - GenericTypeVariable(@Nullable Integer managedReference, String name, Variance variance, JavaType @Nullable[] bounds) { + GenericTypeVariable(@Nullable Integer managedReference, String name, Variance variance, JavaType @Nullable [] bounds) { this.managedReference = managedReference; this.name = name; this.variance = variance; @@ -838,7 +841,7 @@ public GenericTypeVariable unsafeSet(String name, Variance variance, @Nullable L return this; } - public GenericTypeVariable unsafeSet(String name, Variance variance, JavaType @Nullable[] bounds) { + public GenericTypeVariable unsafeSet(String name, Variance variance, JavaType @Nullable [] bounds) { this.name = name; this.variance = variance; this.bounds = ListUtils.nullIfEmpty(bounds); @@ -851,7 +854,7 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; GenericTypeVariable that = (GenericTypeVariable) o; return name.equals(that.name) && variance == that.variance && - (variance == Variance.INVARIANT && bounds == null && that.bounds == null || bounds != null && Arrays.equals(bounds, that.bounds)); + (variance == Variance.INVARIANT && bounds == null && that.bounds == null || bounds != null && Arrays.equals(bounds, that.bounds)); } @Override @@ -879,9 +882,9 @@ class Array implements JavaType { @NonFinal - FullyQualified @Nullable[] annotations; + FullyQualified @Nullable [] annotations; - public Array(@Nullable Integer managedReference, @Nullable JavaType elemType, FullyQualified @Nullable[] annotations) { + public Array(@Nullable Integer managedReference, @Nullable JavaType elemType, FullyQualified @Nullable [] annotations) { this.managedReference = managedReference; this.elemType = unknownIfNull(elemType); this.annotations = ListUtils.nullIfEmpty(annotations); @@ -913,7 +916,7 @@ public Array unsafeSetManagedReference(Integer id) { return this; } - public Array unsafeSet(JavaType elemType, FullyQualified @Nullable[] annotations) { + public Array unsafeSet(JavaType elemType, FullyQualified @Nullable [] annotations) { this.elemType = unknownIfNull(elemType); this.annotations = ListUtils.nullIfEmpty(annotations); return this; @@ -1099,16 +1102,16 @@ class Method implements JavaType { @NonFinal - String @Nullable[] parameterNames; + String @Nullable [] parameterNames; @NonFinal - JavaType @Nullable[] parameterTypes; + JavaType @Nullable [] parameterTypes; @NonFinal - FullyQualified @Nullable[] thrownExceptions; + FullyQualified @Nullable [] thrownExceptions; @NonFinal - FullyQualified @Nullable[] annotations; + FullyQualified @Nullable [] annotations; @Incubating(since = "7.34.0") @Nullable @@ -1142,9 +1145,9 @@ public Method(@Nullable Integer managedReference, long flagsBitMap, @Nullable Fu } public Method(@Nullable Integer managedReference, long flagsBitMap, @Nullable FullyQualified declaringType, String name, - @Nullable JavaType returnType, String @Nullable[] parameterNames, - JavaType @Nullable[] parameterTypes, FullyQualified @Nullable[] thrownExceptions, - FullyQualified @Nullable[] annotations, @Nullable List defaultValue) { + @Nullable JavaType returnType, String @Nullable [] parameterNames, + JavaType @Nullable [] parameterTypes, FullyQualified @Nullable [] thrownExceptions, + FullyQualified @Nullable [] annotations, @Nullable List defaultValue) { this.managedReference = managedReference; this.flagsBitMap = flagsBitMap & Flag.VALID_FLAGS; this.declaringType = unknownIfNull(declaringType); @@ -1182,9 +1185,9 @@ public Method unsafeSet(@Nullable FullyQualified declaringType, public Method unsafeSet(@Nullable FullyQualified declaringType, @Nullable JavaType returnType, - JavaType @Nullable[] parameterTypes, - FullyQualified @Nullable[] thrownExceptions, - FullyQualified @Nullable[] annotations) { + JavaType @Nullable [] parameterTypes, + FullyQualified @Nullable [] thrownExceptions, + FullyQualified @Nullable [] annotations) { this.declaringType = unknownIfNull(declaringType); this.returnType = unknownIfNull(returnType); this.parameterTypes = ListUtils.nullIfEmpty(parameterTypes); @@ -1355,9 +1358,9 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; Method method = (Method) o; return Objects.equals(declaringType, method.declaringType) && - name.equals(method.name) && - Objects.equals(returnType, method.returnType) && - Arrays.equals(parameterTypes, method.parameterTypes); + name.equals(method.name) && + Objects.equals(returnType, method.returnType) && + Arrays.equals(parameterTypes, method.parameterTypes); } @Override @@ -1393,7 +1396,7 @@ class Variable implements JavaType { JavaType type; @NonFinal - FullyQualified @Nullable[] annotations; + FullyQualified @Nullable [] annotations; public Variable(@Nullable Integer managedReference, long flagsBitMap, String name, @Nullable JavaType owner, @Nullable JavaType type, @Nullable List annotations) { @@ -1408,7 +1411,7 @@ public Variable(@Nullable Integer managedReference, long flagsBitMap, String nam } Variable(@Nullable Integer managedReference, long flagsBitMap, String name, @Nullable JavaType owner, - @Nullable JavaType type, FullyQualified @Nullable[] annotations) { + @Nullable JavaType type, FullyQualified @Nullable [] annotations) { this.managedReference = managedReference; this.flagsBitMap = flagsBitMap & Flag.VALID_FLAGS; this.name = name; diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java index 129a0eee381..834fb6b6800 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java @@ -286,13 +286,17 @@ public String toString() { printedWs.append(spaces[(i - lastNewline) % 10]); } else if (c == '\t') { printedWs.append(tabs[(i - lastNewline) % 10]); + } else { + // should never happen (probably a bug in the parser) + printedWs.append(c); } } } + String whitespaces = printedWs.toString(); return "Space(" + - "comments=<" + (comments.size() == 1 ? "1 comment" : comments.size() + " comments") + - ">, whitespace='" + printedWs + "')"; + "comments=<" + (comments.size() == 1 ? "1 comment" : comments.size() + " comments") + ">, " + + "whitespace=" + (whitespaces.isEmpty() ? "" : "'" + whitespaces + "'") + ")"; } public enum Location { diff --git a/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java b/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java index c94f455eb16..279e70c1a8b 100644 --- a/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java +++ b/rewrite-java/src/test/java/org/openrewrite/java/RemoveMethodInvocationsVisitorTest.java @@ -15,13 +15,14 @@ */ package org.openrewrite.java; +import java.util.List; + 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; @@ -172,7 +173,6 @@ void method() { } @Test - @ExpectedToFail void removeWithoutSelect() { rewriteRun( spec -> spec.recipe(createRemoveMethodsRecipe("Test foo()")), @@ -492,4 +492,140 @@ public void method() { ) ); } + + @Test + void removeStaticMethodFromImport() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("org.junit.jupiter.api.Assertions assertTrue(..)")), + // language=java + java( + """ + import static org.junit.jupiter.api.Assertions.assertTrue; + + class Test { + void method() { + assertTrue(true); + } + } + """, + """ + class Test { + void method() { + } + } + """ + ) + ); + } + + @Test + void keepStaticMethodFromImportWithAssignment() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")), + // language=java + java( + """ + import java.util.List; + + import static java.util.Collections.emptyList; + + class Test { + void method() { + List emptyList = emptyList(); + emptyList.isEmpty(); + } + } + """ + ) + ); + } + + @Test + void keepStaticMethodFromImportClassField() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")), + // language=java + java( + """ + import java.util.List; + + import static java.util.Collections.emptyList; + + class Test { + List emptyList = emptyList(); + void method() { + emptyList.isEmpty(); + } + } + """ + ) + ); + } + + @Test + void removeStaticMethod() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("org.junit.jupiter.api.Assertions assertTrue(..)")), + // language=java + java( + """ + import org.junit.jupiter.api.Assertions; + + class Test { + void method() { + Assertions.assertTrue(true); + } + } + """, + """ + class Test { + void method() { + } + } + """ + ) + ); + } + + @Test + void keepStaticMethodWithAssignment() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")), + // language=java + java( + """ + import java.util.List; + import java.util.Collections; + + class Test { + void method() { + List emptyList = Collections.emptyList(); + emptyList.size(); + } + } + """ + ) + ); + } + + @Test + void keepStaticMethodClassField() { + rewriteRun( + spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")), + // language=java + java( + """ + import java.util.List; + import java.util.Collections; + + class Test { + List emptyList = Collections.emptyList(); + void method() { + emptyList.size(); + } + } + """ + ) + ); + } } diff --git a/rewrite-maven/build.gradle.kts b/rewrite-maven/build.gradle.kts index 64b4be8bef4..d6efe37c616 100755 --- a/rewrite-maven/build.gradle.kts +++ b/rewrite-maven/build.gradle.kts @@ -26,8 +26,6 @@ dependencies { // needed by AddDependency implementation(project(":rewrite-java")) - compileOnly("guru.nidi:graphviz-java:latest.release") - compileOnly("org.rocksdb:rocksdbjni:latest.release") compileOnly(project(":rewrite-yaml")) implementation(project(":rewrite-properties")) @@ -41,7 +39,6 @@ dependencies { testImplementation("com.squareup.okhttp3:okhttp-tls:4.+") testImplementation("com.squareup.okio:okio-jvm:3.9.1") testImplementation("org.mapdb:mapdb:latest.release") - testImplementation("guru.nidi:graphviz-java:latest.release") testRuntimeOnly("org.mapdb:mapdb:latest.release") testRuntimeOnly(project(":rewrite-java-17")) diff --git a/rewrite-maven/dependency-viz.kt b/rewrite-maven/dependency-viz.kt deleted file mode 100644 index b9ee2bd1a84..00000000000 --- a/rewrite-maven/dependency-viz.kt +++ /dev/null @@ -1,125 +0,0 @@ -/* - * 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.maven - -import guru.nidi.graphviz.engine.Format -import org.openrewrite.ExecutionContext -import org.openrewrite.InMemoryExecutionContext -import org.openrewrite.maven.tree.GraphvizResolutionEventListener -import org.openrewrite.maven.tree.MavenResolutionResult -import org.openrewrite.maven.tree.Scope -import java.net.URL -import java.nio.file.Path -import java.nio.file.Paths -import kotlin.io.path.exists - -fun visualize( - ctx: ExecutionContext = InMemoryExecutionContext { t -> throw t }, - scope: Scope = Scope.Compile, - outputFileLocation: Path = Paths.get("diagrams"), - outputFilePrefix: String = "maven", - showProperties: Boolean = false, - showManagedDependencies: Boolean = false, - runnable: () -> T -) { - val viz = GraphvizResolutionEventListener(scope, showProperties, showManagedDependencies) - MavenExecutionContextView(ctx).setResolutionListener(viz) - try { - runnable() - } finally { - viz.graphviz().render(Format.DOT).toFile(outputFileLocation.resolve("${outputFilePrefix}.dot").toFile()) - viz.graphviz() - .postProcessor { result, _, _ -> - result.mapString { svg -> - svg.replace("font-family=\"Times,serif\" ", "") - .replace("a xlink:href", "a target=\"_blank\" xlink:href") - } - } - .render(Format.SVG) - .toFile(outputFileLocation.resolve("${outputFilePrefix}.svg").toFile()) - - val panZoom = outputFileLocation.resolve("svg-pan-zoom.min.js") - if (!panZoom.exists()) { - panZoom.toFile().writeText( - URL("https://raw.githubusercontent.com/bumbu/svg-pan-zoom/master/dist/svg-pan-zoom.min.js") - .openStream().bufferedReader().readText() - ) - } - - outputFileLocation.resolve("${outputFilePrefix}.html").toFile().writeText( - //language=html - """ - - - - - - - - - - - """.trimIndent() - ) - } -} - -fun visualize( - gav: String, - ctx: ExecutionContext = InMemoryExecutionContext { t -> throw t }, - scope: Scope = Scope.Compile, - showProperties: Boolean = false, - showManagedDependencies: Boolean = false -) { - visualize(ctx, scope, Paths.get("diagrams"), gav.replace(':', '_'), showProperties, showManagedDependencies) { - parse(gav, ctx) - } -} - -fun parse(gav: String, ctx: ExecutionContext = InMemoryExecutionContext { t -> throw t }): MavenResolutionResult { - val (group, artifact, version) = gav.split(":") - val maven = MavenParser.builder().build().parse( - ctx, """ - - org.openrewrite - dependency-viz - 0.0.1 - - - ${group} - ${artifact} - ${version} - - - - """ - )[0] - - return maven.mavenResolutionResult() -} 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 780127619be..af1691941b7 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/IncrementProjectVersion.java @@ -165,13 +165,15 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { return t; } String newVersion = acc.get(new GroupArtifact( - t.getChildValue("groupId").orElse(null), t.getChildValue("artifactId").orElse(null))); - if (newVersion == null || newVersion.equals(t.getChildValue("version").orElse(null))) { + t.getChildValue("groupId").orElse(null), + t.getChildValue("artifactId").orElse(null))); + String oldVersion = t.getChildValue("version").orElse(null); + if (newVersion == null || newVersion.equals(oldVersion)) { return t; } t = t.withMarkers(t.getMarkers().add(new AlreadyIncremented(randomId()))); - return (Xml.Tag) new ChangeTagValue("version", null, newVersion).getVisitor() - .visitNonNull(t, ctx); + return (Xml.Tag) new ChangeTagValue("version", oldVersion, newVersion, null) + .getVisitor().visitNonNull(t, ctx); } }; } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSecuritySettings.java b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSecuritySettings.java new file mode 100644 index 00000000000..f5006c9c612 --- /dev/null +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSecuritySettings.java @@ -0,0 +1,214 @@ +/* + * 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 com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement; +import lombok.*; +import lombok.experimental.FieldDefaults; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Parser; +import org.openrewrite.internal.PropertyPlaceholderHelper; +import org.openrewrite.maven.internal.MavenXmlMapper; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.Base64; +import java.util.Optional; +import java.util.function.UnaryOperator; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static java.util.Collections.emptyList; + +@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) +@ToString(onlyExplicitlyIncluded = true) +@EqualsAndHashCode(onlyExplicitlyIncluded = true) +@Data +@AllArgsConstructor +@JacksonXmlRootElement(localName = "settingsSecurity") +public class MavenSecuritySettings { + + @Nullable + String master; + + @Nullable + String relocation; + + private static @Nullable MavenSecuritySettings parse(Parser.Input source, ExecutionContext ctx) { + try { + return new Interpolator().interpolate( + MavenXmlMapper.readMapper().readValue(source.getSource(ctx), MavenSecuritySettings.class)); + } catch (IOException e) { + ctx.getOnError().accept(new IOException("Failed to parse " + source.getPath(), e)); + return null; + } + } + + private static @Nullable MavenSecuritySettings parse(Path settingsPath, ExecutionContext ctx) { + return parse(new Parser.Input(settingsPath, () -> { + try { + return Files.newInputStream(settingsPath); + } catch (IOException e) { + ctx.getOnError().accept(new IOException("Failed to read settings-security.xml at " + settingsPath, e)); + return null; + } + }), ctx); + } + + public static @Nullable MavenSecuritySettings readMavenSecuritySettingsFromDisk(ExecutionContext ctx) { + Optional userSettings = Optional.of(userSecuritySettingsPath()) + .filter(MavenSecuritySettings::exists) + .map(path -> parse(path, ctx)); + MavenSecuritySettings installSettings = findMavenHomeSettings().map(path -> parse(path, ctx)).orElse(null); + MavenSecuritySettings mergedSettings = userSettings + .map(mavenSecuritySettings -> mavenSecuritySettings.merge(installSettings)) + .orElse(installSettings); + if (mergedSettings != null && mergedSettings.relocation != null) { + return mergedSettings.merge(parse(Paths.get(mergedSettings.relocation), ctx)); + } + return mergedSettings; + } + + private static Path userSecuritySettingsPath() { + return Paths.get(System.getProperty("user.home")).resolve(".m2/settings-security.xml"); + } + + private static Optional findMavenHomeSettings() { + for (String envVariable : Arrays.asList("MVN_HOME", "M2_HOME", "MAVEN_HOME")) { + for (String s : Optional.ofNullable(System.getenv(envVariable)).map(Arrays::asList).orElse(emptyList())) { + Path resolve = Paths.get(s).resolve("conf/settings-security.xml"); + if (exists(resolve)) { + return Optional.of(resolve); + } + } + } + return Optional.empty(); + } + + private static boolean exists(Path path) { + try { + return path.toFile().exists(); + } catch (SecurityException e) { + return false; + } + } + + private MavenSecuritySettings merge(@Nullable MavenSecuritySettings installSettings) { + return installSettings == null ? this : new MavenSecuritySettings( + master == null ? installSettings.master : master, + relocation == null ? installSettings.relocation : relocation + ); + } + + /** + * Resolve all properties EXCEPT in the profiles section, which can be affected by + * the POM using the settings. + */ + private static class Interpolator { + private static final PropertyPlaceholderHelper propertyPlaceholders = new PropertyPlaceholderHelper( + "${", "}", null); + + private static final UnaryOperator propertyResolver = key -> { + String property = System.getProperty(key); + if (property != null) { + return property; + } + if (key.startsWith("env.")) { + return System.getenv().get(key.substring(4)); + } + return System.getenv().get(key); + }; + + public MavenSecuritySettings interpolate(MavenSecuritySettings mavenSecuritySettings) { + return new MavenSecuritySettings( + interpolate(mavenSecuritySettings.master), + interpolate(mavenSecuritySettings.relocation) + ); + } + + private @Nullable String interpolate(@Nullable String s) { + return s == null ? null : propertyPlaceholders.replacePlaceholders(s, propertyResolver); + } + } + + @Nullable + String decrypt(@Nullable String fieldValue, @Nullable String password) { + if (fieldValue == null || fieldValue.isEmpty() || password == null) { + return null; + } + + try { + byte[] encryptedText = extractPassword(fieldValue); + + byte[] salt = new byte[8]; + System.arraycopy(encryptedText, 0, salt, 0, 8); + + int padLength = encryptedText[8]; + byte[] encryptedBytes = new byte[encryptedText.length - 9 - padLength]; + System.arraycopy(encryptedText, 9, encryptedBytes, 0, encryptedBytes.length); + + byte[] keyAndIV = new byte[32]; + byte[] pwdBytes = extractPassword(password); + int offset = 0; + while (offset < 32) { + java.security.MessageDigest digest = java.security.MessageDigest.getInstance("SHA-256"); + digest.update(pwdBytes); + digest.update(salt); + byte[] hash = digest.digest(); + System.arraycopy(hash, 0, keyAndIV, offset, Math.min(hash.length, 32 - offset)); + offset += hash.length; + } + + Key key = new SecretKeySpec(keyAndIV, 0, 16, "AES"); + IvParameterSpec iv = new IvParameterSpec(keyAndIV, 16, 16); + Cipher cipher = Cipher.getInstance("AES/CBC/NoPadding"); + cipher.init(Cipher.DECRYPT_MODE, key, iv); + byte[] clearBytes = cipher.doFinal(encryptedBytes); + + int paddingLength = clearBytes[clearBytes.length - 1]; + byte[] decryptedBytes = new byte[clearBytes.length - paddingLength]; + System.arraycopy(clearBytes, 0, decryptedBytes, 0, decryptedBytes.length); + return new String(decryptedBytes, StandardCharsets.UTF_8); + } catch (NoSuchPaddingException | NoSuchAlgorithmException | BadPaddingException | IllegalBlockSizeException | + InvalidKeyException | InvalidAlgorithmParameterException | IllegalArgumentException e) { + return null; + } + } + + private byte[] extractPassword(String pwd) throws IllegalArgumentException { + Pattern pattern = Pattern.compile(".*?[^\\\\]?\\{(.*?)}.*"); + Matcher matcher = pattern.matcher(pwd); + if (matcher.find()) { + return Base64.getDecoder().decode(matcher.group(1)); + } + return pwd.getBytes(StandardCharsets.UTF_8); + } +} 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 ae23c9ff06b..2c56bd6ddee 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java @@ -34,12 +34,25 @@ import org.openrewrite.maven.tree.MavenRepository; import org.openrewrite.maven.tree.ProfileActivation; +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; import java.util.*; import java.util.function.UnaryOperator; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static java.util.Collections.emptyList; import static org.openrewrite.maven.tree.MavenRepository.MAVEN_LOCAL_DEFAULT; @@ -109,10 +122,40 @@ public MavenSettings(@Nullable String localRepository, @Nullable Profiles profil .filter(MavenSettings::exists) .map(path -> parse(path, ctx)); final MavenSettings installSettings = findMavenHomeSettings().map(path -> parse(path, ctx)).orElse(null); - return userSettings.map(mavenSettings -> mavenSettings.merge(installSettings)) + MavenSettings settings = userSettings.map(mavenSettings -> mavenSettings.merge(installSettings)) .orElse(installSettings); + + if (settings != null) { + settings.maybeDecryptPasswords(ctx); + } + + return settings; } + void maybeDecryptPasswords(ExecutionContext ctx) { + MavenSecuritySettings security = MavenSecuritySettings.readMavenSecuritySettingsFromDisk(ctx); + if (security == null) { + return; + } + + String decryptedMasterPassword = security.decrypt(security.getMaster(), "settings.security"); + if (decryptedMasterPassword != null) { + if (mavenLocal != null) { + String password = security.decrypt(mavenLocal.getPassword(), decryptedMasterPassword); + if (password != null) { + mavenLocal = mavenLocal.withPassword(password); + } + } + if (servers != null) { + servers.servers = ListUtils.map(servers.servers, server -> { + String password = security.decrypt(server.getPassword(), decryptedMasterPassword); + return password == null ? server : server.withPassword(password); + }); + } + } + } + + public static boolean readFromDiskEnabled() { final String propertyValue = System.getProperty("org.openrewrite.test.readMavenSettingsFromDisk"); return propertyValue != null && !propertyValue.equalsIgnoreCase("false"); @@ -158,7 +201,7 @@ public List getActiveRepositories(Iterable a if (profiles != null) { for (Profile profile : profiles.getProfiles()) { if (profile.isActive(activeProfiles) || (this.activeProfiles != null && - profile.isActive(this.activeProfiles.getActiveProfiles()))) { + profile.isActive(this.activeProfiles.getActiveProfiles()))) { if (profile.repositories != null) { for (RawRepositories.Repository repository : profile.repositories.getRepositories()) { activeRepositories.put(repository.getId(), repository); @@ -409,7 +452,8 @@ public static class Server { @JsonIgnoreProperties(value = "httpHeaders") public static class ServerConfiguration { @JacksonXmlProperty(localName = "property") - @JacksonXmlElementWrapper(localName = "httpHeaders", useWrapping = true) // wrapping is disabled by default on MavenXmlMapper + @JacksonXmlElementWrapper(localName = "httpHeaders", useWrapping = true) + // wrapping is disabled by default on MavenXmlMapper @Nullable List httpHeaders; diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java index 364078042d7..26e59770b35 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java @@ -561,51 +561,75 @@ public Pom download(GroupArtifactVersion gav, Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion()); try { - File f = new File(uri); + File pomFile = new File(uri); + File jarFile = pomFile.toPath().resolveSibling(gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".jar").toFile(); - //NOTE: The pom may exist without a .jar artifact if the pom packaging is "pom" - if (!f.exists()) { + //NOTE: + // - The pom may exist without a .jar artifact if the pom packaging is "pom" + // - The jar may exist without a pom, if manually installed + if (!pomFile.exists() && !jarFile.exists()) { continue; } - try (FileInputStream fis = new FileInputStream(f)) { - RawPom rawPom = RawPom.parse(fis, Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot); - Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav); - - if (pom.getPackaging() == null || pom.hasJarPackaging()) { - File jar = f.toPath().resolveSibling(gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".jar").toFile(); - if (!jar.exists() || jar.length() == 0) { - // The jar has not been downloaded, making this dependency unusable. - continue; - } + RawPom rawPom; + if (pomFile.exists()) { + try (FileInputStream fis = new FileInputStream(pomFile)) { + rawPom = RawPom.parse(fis, Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot); } + } else { + // Record the absense of the pom file + ctx.getResolutionListener().downloadError(gav, uris, (containingPom == null) ? null : containingPom.getRequested()); + // infer rawPom from jar + rawPom = rawPomFromGav(gav); + } - if (repo.getUri().equals(MavenRepository.MAVEN_LOCAL_DEFAULT.getUri())) { - // so that the repository path is the same regardless of username - pom = pom.withRepository(MavenRepository.MAVEN_LOCAL_USER_NEUTRAL); - } + Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav); - if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { - pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); + if (pom.getPackaging() == null || pom.hasJarPackaging()) { + if (!jarFile.exists() || jarFile.length() == 0) { + // The jar has not been downloaded, making this dependency unusable. + continue; } - mavenCache.putPom(resolvedGav, pom); - ctx.getResolutionListener().downloadSuccess(resolvedGav, containingPom); - sample.stop(timer.tags("outcome", "from maven local").register(Metrics.globalRegistry)); - return pom; } + + if (repo.getUri().equals(MavenRepository.MAVEN_LOCAL_DEFAULT.getUri())) { + // so that the repository path is the same regardless of username + pom = pom.withRepository(MavenRepository.MAVEN_LOCAL_USER_NEUTRAL); + } + + if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { + pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); + } + mavenCache.putPom(resolvedGav, pom); + ctx.getResolutionListener().downloadSuccess(resolvedGav, containingPom); + sample.stop(timer.tags("outcome", "from maven local").register(Metrics.globalRegistry)); + return pom; } catch (IOException e) { // unable to read the pom from a file-based repository. repositoryResponses.put(repo, e.getMessage()); } } else { try { - byte[] responseBody = requestAsAuthenticatedOrAnonymous(repo, uri.toString()); + RawPom rawPom; + try { + byte[] responseBody = requestAsAuthenticatedOrAnonymous(repo, uri.toString()); + rawPom = RawPom.parse( + new ByteArrayInputStream(responseBody), + Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot + ); + } catch (HttpSenderResponseException e) { + repositoryResponses.put(repo, e.getMessage()); + // When `pom` is not found, try to see if `jar` exists for the same GAV + if (!e.isClientSideException() || !jarExistsForPomUri(repo, uri.toString())) { + throw e; + } + // Record the absense of the pom file + ctx.getResolutionListener().downloadError(gav, uris, (containingPom == null) ? null : containingPom.getRequested()); + // Continue with a recreated pom + rawPom = rawPomFromGav(gav); + } Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion()); - RawPom rawPom = RawPom.parse( - new ByteArrayInputStream(responseBody), - Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot - ); Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav); if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); @@ -637,6 +661,12 @@ public Pom download(GroupArtifactVersion gav, .setRepositoryResponses(repositoryResponses); } + private RawPom rawPomFromGav(GroupArtifactVersion gav) { + return new RawPom(null, null, gav.getGroupId(), gav.getArtifactId(), gav.getVersion(), null, + null, null, null, "jar", null, null, null, + null, null, null, null, null, null); + } + /** * Gets the base version from snapshot timestamp version. */ @@ -850,6 +880,34 @@ private ReachabilityResult reachable(HttpSender.Request.Builder request) { } } + private boolean jarExistsForPomUri(MavenRepository repo, String pomUrl) { + String jarUrl = pomUrl.replaceAll("\\.pom$", ".jar"); + try { + try { + return Failsafe.with(retryPolicy).get(() -> { + HttpSender.Request authenticated = applyAuthenticationAndTimeoutToRequest(repo, httpSender.get(jarUrl)).build(); + try (HttpSender.Response response = httpSender.send(authenticated)) { + return response.isSuccessful(); + } + }); + } catch (FailsafeException failsafeException) { + Throwable cause = failsafeException.getCause(); + if (cause instanceof HttpSenderResponseException && hasCredentials(repo) && + ((HttpSenderResponseException) cause).isClientSideException()) { + return Failsafe.with(retryPolicy).get(() -> { + HttpSender.Request unauthenticated = httpSender.get(jarUrl).build(); + try (HttpSender.Response response = httpSender.send(unauthenticated)) { + return response.isSuccessful(); + } + }); + } + } + } catch (Throwable e) { + // Not interested in exceptions downloading the jar; we'll throw the original exception for the pom + } + return false; + } + /** * Replicates Apache Maven's behavior to attempt anonymous download if repository credentials prove invalid diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/VersionRequirement.java b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/VersionRequirement.java index f488d30f79b..ee67ff652f4 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/VersionRequirement.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/VersionRequirement.java @@ -30,8 +30,6 @@ import org.openrewrite.maven.tree.MavenMetadata; import org.openrewrite.maven.tree.MavenRepository; import org.openrewrite.maven.tree.Version; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.Iterator; import java.util.List; @@ -41,7 +39,6 @@ @RequiredArgsConstructor public class VersionRequirement { - private static final Logger logger = LoggerFactory.getLogger(VersionRequirement.class); @Nullable private final VersionRequirement nearer; @@ -100,7 +97,7 @@ static VersionSpec build(String requested, boolean direct) { CharStreams.fromString(requested)))); parser.removeErrorListeners(); - parser.addErrorListener(new LoggingErrorListener()); + parser.addErrorListener(new PrintingErrorListener()); return new VersionRangeParserBaseVisitor() { @Override @@ -287,11 +284,11 @@ public String toString() { }); } - private static class LoggingErrorListener extends BaseErrorListener { + private static class PrintingErrorListener extends BaseErrorListener { @Override public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, String msg, RecognitionException e) { - logger.warn("Syntax error at line {}:{} {}", line, charPositionInLine, msg); + System.out.printf("Syntax error at line %d:%d %s%n", line, charPositionInLine, msg); } } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/GraphvizResolutionEventListener.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/GraphvizResolutionEventListener.java deleted file mode 100644 index cf94887334e..00000000000 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/GraphvizResolutionEventListener.java +++ /dev/null @@ -1,216 +0,0 @@ -/* - * 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.maven.tree; - -import guru.nidi.graphviz.attribute.Color; -import guru.nidi.graphviz.attribute.Label; -import guru.nidi.graphviz.attribute.Shape; -import guru.nidi.graphviz.attribute.Style; -import guru.nidi.graphviz.engine.Graphviz; -import guru.nidi.graphviz.model.Link; -import guru.nidi.graphviz.model.MutableGraph; -import guru.nidi.graphviz.model.MutableNode; -import lombok.RequiredArgsConstructor; -import org.jspecify.annotations.Nullable; - -import java.io.UnsupportedEncodingException; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; -import java.util.*; -import java.util.stream.Collectors; - -import static guru.nidi.graphviz.model.Factory.mutGraph; -import static guru.nidi.graphviz.model.Factory.mutNode; -import static guru.nidi.graphviz.model.Link.to; -import static java.util.Collections.emptySet; - -@RequiredArgsConstructor -public class GraphvizResolutionEventListener implements ResolutionEventListener { - public static final String GRAPH_NAME = "resolution"; - - private final Map propToNode = new HashMap<>(); - private final Map pomToNode = new HashMap<>(); - private final Map> alreadySeen = new HashMap<>(); - - private MutableGraph g = mutGraph(GRAPH_NAME).setDirected(true); - - private final Scope scope; - private final boolean showProperties; - private final boolean showManagedDependencies; - - private int parentLinks; - private int dependencyLinks; - private int managedDependencies; - - @Override - public void clear() { - g = mutGraph(GRAPH_NAME).setDirected(true); - propToNode.clear(); - pomToNode.clear(); - alreadySeen.clear(); - parentLinks = 0; - dependencyLinks = 0; - managedDependencies = 0; - } - - @Override - public void parent(Pom parent, Pom containing) { - if (alreadySeen.getOrDefault(simplifyGav(containing.getGav()), emptySet()).contains(simplifyGav(parent.getGav()))) { - return; - } - alreadySeen.computeIfAbsent(simplifyGav(containing.getGav()), g -> new HashSet<>()).add(simplifyGav(parent.getGav())); - - Link link = to(gavNode(parent.getGav()).add(Style.FILLED, Color.rgb("deefee"))).with(Style.DASHED); - if (parentLinks++ == 0) { - link = link.with(Label.of("parent")); - } - gavNode(containing.getGav()).addLink(link); - } - - @Override - public void dependency(Scope scope, ResolvedDependency resolvedDependency, ResolvedPom containing) { - if (scope != this.scope) { - return; - } - - Link link = to(gavNode(resolvedDependency.getGav()).add(Style.FILLED, Color.rgb("e6eaff"))); - if (dependencyLinks++ == 0) { - link = link.with(Label.of("dependency")); - } - gavNode(containing.getGav()).addLink(link); - } - - @Override - public void property(String key, String value, Pom containing) { - if (!showProperties) { - return; - } - gavNode(containing.getGav()).addLink(propNode(key, value)); - } - - @Override - public void dependencyManagement(ManagedDependency dependencyManagement, Pom containing) { - if (!showManagedDependencies) { - return; - } - GroupArtifactVersion gav = new GroupArtifactVersion(dependencyManagement.getGroupId(), dependencyManagement.getArtifactId(), dependencyManagement.getVersion()); - - if (alreadySeen.getOrDefault(simplifyGav(containing.getGav()), emptySet()).contains(gav)) { - return; - } - alreadySeen.computeIfAbsent(simplifyGav(containing.getGav()), g -> new HashSet<>()).add(gav); - - Link link = to(dmNode(gav).add(Style.FILLED, Color.rgb("d6d6de"))); - if (managedDependencies++ == 0) { - link = link.with(Label.of("dependencyManagement")); - } - gavNode(containing.getGav()).addLink(link); - } - - @Override - public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { - Link link = to(gavNode(gav).add(Style.FILLED, Color.rgb("ff1947"))) - .with(Label.of("error")); - if(containing != null) { - gavNode(containing.getGav()) - .addLink(link); - } - } - - @Override - public void bomImport(ResolvedGroupArtifactVersion gav, Pom containing) { - if (alreadySeen.getOrDefault(simplifyGav(containing.getGav()), emptySet()).contains(simplifyGav(gav))) { - return; - } - alreadySeen.computeIfAbsent(simplifyGav(containing.getGav()), g -> new HashSet<>()).add(simplifyGav(gav)); - - Link link = to(gavNode(gav).add(Style.FILLED, Color.rgb("e6eaff"))); - if (dependencyLinks++ == 0) { - link = link.with(Label.of("dependency")); - } - gavNode(containing.getGav()).addLink(link); - } - - @SuppressWarnings("unused") - public Graphviz graphviz() { - return Graphviz.fromGraph(g); - } - - private GroupArtifactVersion simplifyGav(ResolvedGroupArtifactVersion gav) { - return new GroupArtifactVersion(gav.getGroupId(), gav.getArtifactId(), - gav.getDatedSnapshotVersion() == null ? gav.getVersion() : gav.getDatedSnapshotVersion()); - } - - private MutableNode gavNode(ResolvedGroupArtifactVersion gav) { - return gavNode(simplifyGav(gav)); - } - - private MutableNode gavNode(GroupArtifactVersion gav) { - return pomToNode.computeIfAbsent(gav, ignored -> { - MutableNode node = mutNode(Label.lines(gav.getGroupId(), gav.getArtifactId(), gav.getVersion())) - .add(Shape.RECTANGLE, Style.FILLED, Color.rgb("f9a91b")); - String url = gavUrl(gav); - if (url != null) { - node = node.add("URL", url); - } - g.add(node); - return node; - }); - } - - private MutableNode dmNode(GroupArtifactVersion gav) { - return pomToNode.computeIfAbsent(gav, ignored -> { - MutableNode node = mutNode(Label.lines(gav.getGroupId(), gav.getArtifactId(), gav.getVersion())).add(Shape.RECTANGLE); - String url = gavUrl(gav); - if (url != null) { - node = node.add("URL", url); - } - g.add(node); - return node; - }); - } - - private MutableNode propNode(String key, String value) { - return propToNode.computeIfAbsent(key + "=" + value, ignored -> { - MutableNode node = mutNode(Label.lines(key, value)); - g.add(node); - return node; - }); - } - - private @Nullable String gavUrl(GroupArtifactVersion gav) { - if (gav.getGroupId() == null || gav.getArtifactId() == null || gav.getVersion() == null) { - return null; - } - try { - return "https://repo1.maven.org/maven2/" + - Arrays.stream(gav.getGroupId().split("\\.")) - .map(g -> { - try { - return URLEncoder.encode(g, StandardCharsets.UTF_8.name()); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } - }) - .collect(Collectors.joining("/")) + '/' + - URLEncoder.encode(gav.getArtifactId(), StandardCharsets.UTF_8.name()) + '/' + - URLEncoder.encode(gav.getVersion(), StandardCharsets.UTF_8.name()) + '/' + - URLEncoder.encode(gav.getArtifactId() + '-' + gav.getVersion() + ".pom", StandardCharsets.UTF_8.name()); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } - } -} diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/IncrementProjectVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/IncrementProjectVersionTest.java index 9e68f1f8cea..4198418ee22 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/IncrementProjectVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/IncrementProjectVersionTest.java @@ -35,18 +35,54 @@ public void defaults(RecipeSpec spec) { void changeProjectVersion() { rewriteRun( pomXml( - """ + """ + + org.openrewrite + rewrite-maven + 8.40.1-SNAPSHOT + + """, + """ org.openrewrite rewrite-maven - 8.40.1-SNAPSHOT + 8.41.0-SNAPSHOT - """, + """ + ) + ); + } + + @Test + void changeProjectVersionShouldNotUpdateDependencyVersion() { + rewriteRun( + pomXml( + """ + + org.openrewrite + rewrite-maven + 8.40.1-SNAPSHOT + + + com.google.guava + guava + 29.0-jre + + + + """, """ org.openrewrite rewrite-maven 8.41.0-SNAPSHOT + + + com.google.guava + guava + 29.0-jre + + """ ) diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/MavenSecuritySettingsTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenSecuritySettingsTest.java new file mode 100644 index 00000000000..1166ba41b81 --- /dev/null +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenSecuritySettingsTest.java @@ -0,0 +1,297 @@ +/* + * 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.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.openrewrite.InMemoryExecutionContext; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +class MavenSecuritySettingsTest { + + private static final String MASTER_PASS_ENCRYPTED = "FyoLIiN2Fx8HpT8O0aBsTn2/s3pYmtLRRCpoWPzhN4A="; // "master" + private static final String USER_PASS_ENCRYPTED = "ERozWEamSJoHRBT+wVx51V2Emr9PazZR9txMntZPlJc="; // "testpass" + private static final String USER_PASS_DECRYPTED = "testpass"; + + private String originalUserHome; + + @TempDir + Path tempDir; + + @BeforeEach + void setUp() throws IOException { + originalUserHome = System.getProperty("user.home"); + System.setProperty("user.home", tempDir.toString()); + Files.createDirectories(tempDir.resolve(".m2")); + } + + @AfterEach + void tearDown() { + System.setProperty("user.home", originalUserHome); + } + + @Test + void decryptCredentials() throws IOException { + // Create settings-security.xml with master password + Files.writeString(tempDir.resolve(".m2/settings-security.xml"), + //language=xml + """ + + + {%s} + + """.formatted(MASTER_PASS_ENCRYPTED)); + + // Create settings.xml with encrypted password + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + {%s} + + + + """.formatted(USER_PASS_ENCRYPTED)); + + // Use the public API to read settings + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()) + .hasSize(1) + .first() + .satisfies(server -> { + assertThat(server.getId()).isEqualTo("test-server"); + assertThat(server.getUsername()).isEqualTo("admin"); + assertThat(server.getPassword()).isEqualTo(USER_PASS_DECRYPTED); + }); + } + + @Test + void relocatedCredentials() throws IOException { + // Create settings-security.xml with relocation + Path relocated = tempDir.resolve(".m2/relocation-settings-security.xml"); + Files.writeString(tempDir.resolve(".m2/settings-security.xml"), + //language=xml + """ + + + %s + + """.formatted(relocated)); + // Create relocation-settings-security.xml with master password + Files.writeString(relocated, + //language=xml + """ + + + {%s} + + """.formatted(MASTER_PASS_ENCRYPTED)); + + // Create settings.xml with encrypted password + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + {%s} + + + + """.formatted(USER_PASS_ENCRYPTED)); + + // Use the public API to read settings + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()) + .hasSize(1) + .first() + .satisfies(server -> { + assertThat(server.getId()).isEqualTo("test-server"); + assertThat(server.getUsername()).isEqualTo("admin"); + assertThat(server.getPassword()).isEqualTo(USER_PASS_DECRYPTED); + }); + } + + @Test + void handleInvalidEncryptedPassword() throws IOException { + // Create settings-security.xml with master password + Files.writeString(tempDir.resolve(".m2/settings-security.xml"), + //language=xml + """ + + + {jSMOWnoPFgsHVpMvz5VrIt5kRbzGpI8u+9EF1iFQyJQ=} + + """); + + // Create settings.xml with invalid encrypted password + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + {invalid_format} + + + + """); + + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()).hasSize(1) + .first() + .satisfies(server -> assertThat(server.getPassword()).isEqualTo("{invalid_format}")); + } + + @Test + void noSecuritySettingsNoDecryption() throws IOException { + // Only create settings.xml without settings-security.xml + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + {encrypted_password} + + + + """); + + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()) + .hasSize(1) + .first() + .satisfies(server -> assertThat(server.getPassword()).isEqualTo("{encrypted_password}")); + } + + @Test + void decryptPasswordWithComments() throws IOException { + // Create settings-security.xml with master password + Files.writeString(tempDir.resolve(".m2/settings-security.xml"), + //language=xml + """ + + + {%s} + + """.formatted(MASTER_PASS_ENCRYPTED)); + + // Create settings.xml with password containing comments + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + Oleg reset this password on 2009-03-11, expires on 2009-04-11 {%s} + + + + """.formatted(USER_PASS_ENCRYPTED)); + + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()) + .hasSize(1) + .first() + .satisfies(server -> assertThat(server.getPassword()).isEqualTo(USER_PASS_DECRYPTED)); + } + + @Test + void invalidMasterPasswordButValidPasswordFormat() throws IOException { + // Create settings-security.xml with invalid master password + Files.writeString(tempDir.resolve(".m2/settings-security.xml"), + //language=xml + """ + + + {invalid_master_password} + + """); + + // Create settings.xml with valid encrypted password + Files.writeString(tempDir.resolve(".m2/settings.xml"), + //language=xml + """ + + + + + test-server + admin + {%s} + + + + """.formatted(USER_PASS_ENCRYPTED)); + + var ctx = new InMemoryExecutionContext(t -> { + throw new RuntimeException(t); + }); + MavenSettings settings = MavenSettings.readMavenSettingsFromDisk(ctx); + assert settings != null && settings.getServers() != null; + assertThat(settings.getServers().getServers()) + .hasSize(1) + .first() + .satisfies(server -> + // Password should remain in encrypted form since master password is invalid + assertThat(server.getPassword()).isEqualTo("{%s}".formatted(USER_PASS_ENCRYPTED))); + } +} diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java index f16810183a6..152265c02fd 100755 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java @@ -573,6 +573,42 @@ void skipsLocalInvalidArtifactsEmptyJar(@TempDir Path localRepository) throws IO .download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal))); } + @Test + void dontAllowPomDowloadFailureWithoutJar(@TempDir Path localRepository) throws IOException, MavenDownloadingException { + MavenRepository mavenLocal = MavenRepository.builder() + .id("local") + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + + // Do not return invalid dependency + assertThrows(MavenDownloadingException.class, () -> new MavenPomDownloader(emptyMap(), ctx) + .download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal))); + } + + @Test + void allowPomDowloadFailureWithJar(@TempDir Path localRepository) throws IOException, MavenDownloadingException { + MavenRepository mavenLocal = MavenRepository.builder() + .id("local") + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + + // Create a valid jar + Path localJar = localRepository.resolve("com/some/some-artifact/1/some-artifact-1.jar"); + assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); + Files.writeString(localJar, "some content not to be empty"); + + // Do not throw exception since we have a jar + var result = new MavenPomDownloader(emptyMap(), ctx) + .download(new GroupArtifactVersion("com.some", "some-artifact", "1"), null, null, List.of(mavenLocal)); + assertThat(result.getGav().getGroupId()).isEqualTo("com.some"); + assertThat(result.getGav().getArtifactId()).isEqualTo("some-artifact"); + assertThat(result.getGav().getVersion()).isEqualTo("1"); + } + @Test void doNotRenameRepoForCustomMavenLocal(@TempDir Path tempDir) throws MavenDownloadingException, IOException { GroupArtifactVersion gav = createArtifact(tempDir); @@ -1023,5 +1059,62 @@ public MockResponse dispatch(RecordedRequest recordedRequest) { throw new RuntimeException(e); } } + + @Test + @DisplayName("Throw exception if there is no pom and no jar for the artifact") + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + void pomNotFoundWithNoJarShouldThrow() throws Exception { + try (MockWebServer mockRepo = getMockServer()) { + mockRepo.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest recordedRequest) { + assert recordedRequest.getPath() != null; + return new MockResponse().setResponseCode(404).setBody(""); + } + }); + mockRepo.start(); + var repositories = List.of(MavenRepository.builder() + .id("id") + .uri("http://%s:%d/maven".formatted(mockRepo.getHostName(), mockRepo.getPort())) + .username("user") + .password("pass") + .build()); + + var downloader = new MavenPomDownloader(emptyMap(), ctx); + var gav = new GroupArtifactVersion("fred", "fred", "1"); + assertThrows(MavenDownloadingException.class, () -> downloader.download(gav, null, null, repositories)); + } + } + + @Test + @DisplayName("Don't throw exception if there is no pom and but there is a jar for the artifact") + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + void pomNotFoundWithJarFoundShouldntThrow() throws Exception { + try (MockWebServer mockRepo = getMockServer()) { + mockRepo.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest recordedRequest) { + assert recordedRequest.getPath() != null; + if (recordedRequest.getPath().endsWith("fred/fred/1/fred-1.pom")) + return new MockResponse().setResponseCode(404).setBody(""); + return new MockResponse().setResponseCode(200).setBody("some bytes so the jar isn't empty"); + } + }); + mockRepo.start(); + var repositories = List.of(MavenRepository.builder() + .id("id") + .uri("http://%s:%d/maven".formatted(mockRepo.getHostName(), mockRepo.getPort())) + .username("user") + .password("pass") + .build()); + + var gav = new GroupArtifactVersion("fred", "fred", "1"); + var downloader = new MavenPomDownloader(emptyMap(), ctx); + Pom downloaded = downloader.download(gav, null, null, repositories); + assertThat(downloaded.getGav().getGroupId()).isEqualTo("fred"); + assertThat(downloaded.getGav().getArtifactId()).isEqualTo("fred"); + assertThat(downloaded.getGav().getVersion()).isEqualTo("1"); + } + } } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java index 0d2714390ab..b1c0e12f1bd 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java @@ -16,9 +16,20 @@ package org.openrewrite.maven.tree; import com.fasterxml.jackson.databind.ObjectMapper; +import org.intellij.lang.annotations.Language; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.Issue; +import org.openrewrite.maven.MavenExecutionContextView; import org.openrewrite.test.RewriteTest; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -278,4 +289,97 @@ void resolveExecutionsFromDifferentParents() { ) ); } + + @Nested + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + class TolerateMissingPom { + + @Language("xml") + private static final String POM_WITH_DEPENDENCY = """ + + 4.0.0 + foo + bar + 1.0-SNAPSHOT + + + com.some + some-artifact + 1 + + + + """; + + @TempDir + Path localRepository; + @TempDir + Path localRepository2; + + @Test + void singleRepositoryContainingJar() throws IOException { + MavenRepository mavenLocal = createMavenRepository(localRepository, "local"); + createJarFile(localRepository); + + List> downloadErrorArgs = new ArrayList<>(); + MavenExecutionContextView ctx = MavenExecutionContextView.view(new InMemoryExecutionContext(Throwable::printStackTrace)); + ctx.setRepositories(List.of(mavenLocal)); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { + List list = new ArrayList<>(); + list.add(gav); + list.add(attemptedUris); + list.add(containing); + downloadErrorArgs.add(list); + } + }); + rewriteRun( + spec -> spec.executionContext(ctx), + pomXml(POM_WITH_DEPENDENCY) + ); + assertThat(downloadErrorArgs).hasSize(1); + } + + @Test + void twoRepositoriesSecondContainingJar() throws IOException { + MavenRepository mavenLocal = createMavenRepository(localRepository, "local"); + MavenRepository mavenLocal2 = createMavenRepository(localRepository2, "local2"); + createJarFile(localRepository2); + + List> downloadErrorArgs = new ArrayList<>(); + MavenExecutionContextView ctx = MavenExecutionContextView.view(new InMemoryExecutionContext(Throwable::printStackTrace)); + ctx.setRepositories(List.of(mavenLocal, mavenLocal2)); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { + List list = new ArrayList<>(); + list.add(gav); + list.add(attemptedUris); + list.add(containing); + downloadErrorArgs.add(list); + } + }); + rewriteRun( + spec -> spec.executionContext(ctx), + pomXml(POM_WITH_DEPENDENCY) + ); + assertThat(downloadErrorArgs).hasSize(1); + } + + private static void createJarFile(Path localRepository1) throws IOException { + Path localJar = localRepository1.resolve("com/some/some-artifact/1/some-artifact-1.jar"); + assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); + Files.writeString(localJar, "some content not to be empty"); + } + + private static MavenRepository createMavenRepository(Path localRepository, String name) { + return MavenRepository.builder() + .id(name) + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + } + } } diff --git a/rewrite-properties/src/main/java/org/openrewrite/properties/trait/PropertiesReference.java b/rewrite-properties/src/main/java/org/openrewrite/properties/trait/PropertiesReference.java index 038aafccfd6..2d5fd55846a 100644 --- a/rewrite-properties/src/main/java/org/openrewrite/properties/trait/PropertiesReference.java +++ b/rewrite-properties/src/main/java/org/openrewrite/properties/trait/PropertiesReference.java @@ -53,9 +53,6 @@ public boolean supportsRename() { return true; } - /** - * {@inheritDoc} - */ @Override public Tree rename(Renamer renamer, Cursor cursor, ExecutionContext ctx) { Tree tree = cursor.getValue(); @@ -67,7 +64,7 @@ public Tree rename(Renamer renamer, Cursor cursor, ExecutionContext ctx) { return tree; } - public static class Matcher extends SimpleTraitMatcher { + private static class Matcher extends SimpleTraitMatcher { private static final Predicate javaFullyQualifiedTypeMatcher = Pattern.compile( "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(?:\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*").asPredicate(); @@ -75,7 +72,7 @@ public static class Matcher extends SimpleTraitMatcher { protected @Nullable PropertiesReference test(Cursor cursor) { Object value = cursor.getValue(); if (value instanceof Properties.Entry && - javaFullyQualifiedTypeMatcher.test(((Properties.Entry) value).getValue().getText())) { + javaFullyQualifiedTypeMatcher.test(((Properties.Entry) value).getValue().getText())) { return new PropertiesReference(cursor, determineKind(((Properties.Entry) value).getValue().getText())); } return null; @@ -88,8 +85,14 @@ private Kind determineKind(String value) { @SuppressWarnings("unused") public static class Provider implements Reference.Provider { + private static final Predicate applicationPropertiesMatcher = Pattern.compile("^application(-\\w+)?\\.properties$").asPredicate(); + @Override + public boolean isAcceptable(SourceFile sourceFile) { + return sourceFile instanceof Properties.File && applicationPropertiesMatcher.test(sourceFile.getSourcePath().getFileName().toString()); + } + @Override public Set getReferences(SourceFile sourceFile) { Set references = new HashSet<>(); @@ -99,10 +102,5 @@ public Set getReferences(SourceFile sourceFile) { }).visit(sourceFile, 0); return references; } - - @Override - public boolean isAcceptable(SourceFile sourceFile) { - return sourceFile instanceof Properties.File && applicationPropertiesMatcher.test(sourceFile.getSourcePath().getFileName().toString()); - } } } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/ChangeTagValue.java b/rewrite-xml/src/main/java/org/openrewrite/xml/ChangeTagValue.java index 0773c33abba..254fb705ed7 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/ChangeTagValue.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/ChangeTagValue.java @@ -18,12 +18,16 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Option; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; +import org.openrewrite.marker.AlreadyReplaced; +import org.openrewrite.marker.Markers; import org.openrewrite.xml.tree.Xml; +import java.util.regex.Pattern; + +import static java.util.Collections.singletonList; +import static org.openrewrite.Tree.randomId; + @Value @EqualsAndHashCode(callSuper = false) public class ChangeTagValue extends Recipe { @@ -34,17 +38,24 @@ public class ChangeTagValue extends Recipe { String elementName; @Option(displayName = "Old value", - description = "The old value of the tag.", + description = "The old value of the tag. Interpreted as pattern if regex is enabled.", required = false, example = "user") @Nullable String oldValue; @Option(displayName = "New value", - description = "The new value for the tag.", + description = "The new value for the tag. Supports capture groups when regex is enabled. " + + "If literal $,\\ characters are needed in newValue, with regex true, then it should be escaped.", example = "user") String newValue; + @Option(displayName = "Regex", + description = "Default false. If true, `oldValue` will be interpreted as a [Regular Expression](https://en.wikipedia.org/wiki/Regular_expression), and capture group contents will be available in `newValue`.", + required = false) + @Nullable + Boolean regex; + @Override public String getDisplayName() { return "Change XML tag value"; @@ -52,7 +63,15 @@ public String getDisplayName() { @Override public String getDescription() { - return "Alters the value of XML tags matching the provided expression."; + return "Alters the value of XML tags matching the provided expression. " + + "When regex is enabled the replacement happens only for text nodes provided the pattern matches."; + } + + @Override + public Validated validate(ExecutionContext ctx) { + //noinspection ConstantValue + return super.validate(ctx).and(Validated.test("regex", "Regex usage requires an `oldValue`", regex, + value -> value == null || oldValue != null && !oldValue.equals(newValue))); } @Override @@ -63,9 +82,9 @@ public TreeVisitor getVisitor() { @Override public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { if (xPathMatcher.matches(getCursor())) { - // if either no oldValue is supplied OR oldValue equals the value of this tag - // then change the value to the newValue supplied - if (oldValue == null || oldValue.equals(tag.getValue().orElse(null))) { + if (Boolean.TRUE.equals(regex) && oldValue != null) { + doAfterVisit(new RegexReplaceVisitor<>(tag, oldValue, newValue)); + } else if (oldValue == null || oldValue.equals(tag.getValue().orElse(null))) { doAfterVisit(new ChangeTagValueVisitor<>(tag, newValue)); } } @@ -73,4 +92,48 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { } }; } + + /** + * This visitor finds and replaces text within a specified XML tag. + * It supports both literal and regular expression replacements. + * Use {@link ChangeTagValueVisitor} if you only wish change the value, irrespective of current data. + * + * @param

+ */ + @Value + @EqualsAndHashCode(callSuper = false) + static class RegexReplaceVisitor

extends XmlVisitor

{ + + Xml.Tag scope; + String oldValue; + String newValue; + + @Override + public Xml visitTag(Xml.Tag tag, P p) { + Xml.Tag t = (Xml.Tag) super.visitTag(tag, p); + if (scope.isScope(t) && + t.getContent() != null && + t.getContent().size() == 1 && + t.getContent().get(0) instanceof Xml.CharData) { + return updateUsingRegex(t, (Xml.CharData) t.getContent().get(0)); + } + return t; + } + + private Xml.Tag updateUsingRegex(Xml.Tag t, Xml.CharData content) { + String text = content.getText(); + if (Pattern.compile(oldValue).matcher(text).find()) { + Markers oldMarkers = content.getMarkers(); + if (oldMarkers + .findAll(AlreadyReplaced.class) + .stream() + .noneMatch(m -> m.getFind().equals(oldValue) && newValue.equals(m.getReplace()))) { + return t.withContent(singletonList(content + .withText(text.replaceAll(oldValue, newValue)) + .withMarkers(oldMarkers.add(new AlreadyReplaced(randomId(), oldValue, newValue))))); + } + } + return t; + } + } } diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueTest.java new file mode 100755 index 00000000000..cf7f8dda3df --- /dev/null +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueTest.java @@ -0,0 +1,314 @@ +/* + * 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.xml; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.ExecutionContext; +import org.openrewrite.test.RewriteTest; +import org.openrewrite.xml.tree.Xml; + +import static java.util.Objects.requireNonNull; +import static org.openrewrite.test.RewriteTest.toRecipe; +import static org.openrewrite.xml.Assertions.xml; + +class ChangeTagValueTest implements RewriteTest { + + @DocumentExample + @Test + void rewriteEmptyTagValue() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + null, "2.0", null)), + xml( + """ + + + + """, + """ + + 2.0 + + """) + ); + } + + @Test + void matchExistingTagValue() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + "1.0", "2.0", null)), + xml( + """ + + 1.0 + + """, + """ + + 2.0 + + """) + ); + } + + @Test + void noMatchExistingTagValue() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + "1.0", "2.0", null)), + xml( + """ + + 3.0 + + """) + ); + } + + @Test + void rewriteTagValueSubstring() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + "SNAPSHOT", "RELEASE", true) + ), + xml( + """ + + com.company.project + artifact + 1.2.3-SNAPSHOT + + """, + """ + + com.company.project + artifact + 1.2.3-RELEASE + + """ + ) + ); + } + + + @Test + void appendTagValue() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + "$", "-RELEASE", true) + ), + xml( + """ + + com.company.project + artifact + 1.2.3 + + """, + """ + + com.company.project + artifact + 1.2.3-RELEASE + + """ + ) + ); + } + + + @Test + void replaceWithCapturingGroups() { + rewriteRun( + spec -> spec.recipe( + new ChangeTagValue("/dependency/version", + "(\\d).(\\d).(\\d)", "$1.$3.4", true) + ), + xml( + """ + + com.company.project + artifact + 1.2.3 + + """, + """ + + com.company.project + artifact + 1.3.4 + + """ + ) + ); + } + + @Test + void replacedExactlyOnce() { + rewriteRun( + spec -> spec.recipe(new ChangeTagValue( + "/tag", + "(aaa)", + "$1-aaa", + true)), + xml( + "aaa", + "aaa-aaa" + ) + ); + } + + @Nested + class FindAndReplaceTagTextVisitorTest { + + @Test + void findAndReplace() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValue.RegexReplaceVisitor<>( + (Xml.Tag) requireNonNull(x.getRoot().getContent()).get(0), + "2.0", + "3.0") + ); + return super.visitDocument(x, ctx); + } + })), + xml( + """ + + + 2.0 + + + """, + """ + + + 3.0 + + + """ + ) + ); + } + + @Test + void changeContentSubstring() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValue.RegexReplaceVisitor<>( + requireNonNull(x.getRoot()), + "SNAPSHOT", + "RELEASE") + ); + return super.visitDocument(x, ctx); + } + })), + xml( + "1.2.3-SNAPSHOT", + "1.2.3-RELEASE" + ) + ); + } + + @Test + void doNothingIfPatternDoesNotMatch() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValue.RegexReplaceVisitor<>( + (Xml.Tag) requireNonNull(x.getRoot().getContent()).get(0), + "7.0", + "8.0") + ); + return super.visitDocument(x, ctx); + } + })), + xml( + """ + + + + + + + """ + ) + ); + } + + @Test + void doNothingForNonTextNotFound() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValue.RegexReplaceVisitor<>( + (Xml.Tag) requireNonNull(x.getRoot().getContent()).get(0), + "7.0", + "8.0") + ); + return super.visitDocument(x, ctx); + } + })), + xml( + """ + + 2.0 + + """ + ) + ); + } + + @Test + void skipsChildIfNotText() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValue.RegexReplaceVisitor<>( + (Xml.Tag) requireNonNull(x.getRoot().getContent()).get(0), + "invalid", + "2.0") + ); + return super.visitDocument(x, ctx); + } + })), + xml( + """ + + + + """ + ) + ); + } + } +} diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueVisitorTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueVisitorTest.java index 8d844e515bc..808c19e7eb2 100755 --- a/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueVisitorTest.java +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/ChangeTagValueVisitorTest.java @@ -53,6 +53,26 @@ public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { ); } + @Test + void noChangeIfAlreadyPresent() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { + @Override + public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { + doAfterVisit(new ChangeTagValueVisitor<>((Xml.Tag) requireNonNull(x.getRoot().getContent()).get(0), "2.0")); + return super.visitDocument(x, ctx); + } + })), + xml( + """ + + 2.0 + + """ + ) + ); + } + @Test void preserveOriginalFormatting() { rewriteRun( diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/CommentOutProperty.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/CommentOutProperty.java index 83d5563bc10..8d94aa04c3b 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/CommentOutProperty.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/CommentOutProperty.java @@ -17,7 +17,11 @@ import lombok.EqualsAndHashCode; import lombok.Value; -import org.openrewrite.*; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; import org.openrewrite.yaml.tree.Yaml; import java.util.ArrayDeque; @@ -40,6 +44,12 @@ public class CommentOutProperty extends Recipe { example = "The `foo` property is deprecated, please migrate") String commentText; + @Option(example = "true", displayName = "Comment out property", + description = "If false, property wouldn't be commented out, only comment will be added. By default, set to true", + required = false) + @Nullable + Boolean commentOutProperty; + @Override public String getDisplayName() { return "Comment out property"; @@ -61,6 +71,7 @@ public TreeVisitor getVisitor() { private boolean nextDocNeedsNewline; private String comment = ""; private String indentation = ""; + private boolean isBlockCommentExists; @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { @@ -72,7 +83,7 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) } // Add any leftover comment to the end of document - if (!comment.isEmpty()) { + if (!comment.isEmpty() && !Boolean.FALSE.equals(commentOutProperty)) { String newPrefix = String.format("%s# %s%s%s", indentation, commentText, @@ -89,7 +100,9 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) public Yaml.Sequence.Entry visitSequenceEntry(Yaml.Sequence.Entry entry, ExecutionContext ctx) { indentation = entry.getPrefix(); - if (!comment.isEmpty()) { + if (Boolean.FALSE.equals(commentOutProperty)) { + return addBockCommentIfNecessary(entry, ctx); + } else if (!comment.isEmpty()) { // add comment and return String newPrefix = entry.getPrefix() + "# " + commentText + comment + entry.getPrefix(); comment = ""; @@ -103,20 +116,13 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC String lastIndentation = indentation; indentation = entry.getPrefix(); - if (!comment.isEmpty()) { + if (!comment.isEmpty() && !Boolean.FALSE.equals(commentOutProperty)) { String newPrefix = entry.getPrefix() + "# " + commentText + comment + entry.getPrefix(); comment = ""; return entry.withPrefix(newPrefix); } - Deque propertyEntries = getCursor().getPathAsStream() - .filter(Yaml.Mapping.Entry.class::isInstance) - .map(Yaml.Mapping.Entry.class::cast) - .collect(Collectors.toCollection(ArrayDeque::new)); - - String prop = stream(spliteratorUnknownSize(propertyEntries.descendingIterator(), 0), false) - .map(e2 -> e2.getKey().getValue()) - .collect(Collectors.joining(".")); + String prop = calculateCurrentKeyPath(); if (prop.equals(propertyKey)) { String prefix = entry.getPrefix(); @@ -128,12 +134,50 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC comment = lastIndentation + "#" + entry.print(getCursor().getParentTreeCursor()); } - doAfterVisit(new DeleteProperty(propertyKey, null, null, null).getVisitor()); + if (Boolean.FALSE.equals(commentOutProperty)) { + if (!entry.getPrefix().contains(commentText) && !isBlockCommentExists) { + return entry.withPrefix(entry.getPrefix() + "# " + commentText + (entry.getPrefix().contains("\n") ? entry.getPrefix() : "\n" + entry.getPrefix())); + } + } else { + doAfterVisit(new DeleteProperty(propertyKey, null, null, null).getVisitor()); + } return entry; } return super.visitMappingEntry(entry, ctx); } + + private Yaml.Sequence.Entry addBockCommentIfNecessary(Yaml.Sequence.Entry entry, ExecutionContext ctx) { + boolean propertyExistsInSequence = isPropertyExistsInSequence(entry); + if (propertyExistsInSequence) { + isBlockCommentExists = true; + if (!entry.getPrefix().contains(commentText)) { + return entry.withPrefix(entry.getPrefix() + "# " + commentText + (entry.getPrefix().contains("\n") ? entry.getPrefix() : "\n" + entry.getPrefix())); + } + } + return super.visitSequenceEntry(entry, ctx); + } + + private boolean isPropertyExistsInSequence(Yaml.Sequence.Entry entry) { + if (!(entry.getBlock() instanceof Yaml.Mapping)) { + return false; + } + Yaml.Mapping mapping = (Yaml.Mapping) entry.getBlock(); + String prop = calculateCurrentKeyPath(); + return mapping.getEntries().stream() + .anyMatch(e -> propertyKey.equals(prop + "." + e.getKey().getValue())); + } + + private String calculateCurrentKeyPath() { + Deque propertyEntries = getCursor().getPathAsStream() + .filter(Yaml.Mapping.Entry.class::isInstance) + .map(Yaml.Mapping.Entry.class::cast) + .collect(Collectors.toCollection(ArrayDeque::new)); + + return stream(spliteratorUnknownSize(propertyEntries.descendingIterator(), 0), false) + .map(e2 -> e2.getKey().getValue()) + .collect(Collectors.joining(".")); + } }; } } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/CreateYamlFile.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/CreateYamlFile.java index 157cb2f7db3..adbcf45ecd0 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/CreateYamlFile.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/CreateYamlFile.java @@ -46,7 +46,10 @@ public class CreateYamlFile extends ScanningRecipe { @Language("yml") @Option(displayName = "File contents", description = "Multiline text content for the file.", - example = "a:\nproperty: value\nanother:\nproperty: value", + example = "a:\n" + + " property: value\n" + + "another:\n" + + " property: value", required = false) @Nullable String fileContents; 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 3b1bf9c1355..d2fe5d563ac 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -33,7 +33,7 @@ public class MergeYaml extends Recipe { @Option(displayName = "YAML snippet", description = "The YAML snippet to insert. The snippet will be indented to match the style of its surroundings.", - example = "labels: \n\tlabel-one: \"value-one\"") + example = "labels:\n label-one: \"value-one\"") @Language("yml") String yaml; diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/YamlReference.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/YamlReference.java new file mode 100644 index 00000000000..23705fdae96 --- /dev/null +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/YamlReference.java @@ -0,0 +1,105 @@ +/* + * 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.yaml.trait; + +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +import org.openrewrite.SourceFile; +import org.openrewrite.Tree; +import org.openrewrite.trait.Reference; +import org.openrewrite.trait.SimpleTraitMatcher; +import org.openrewrite.yaml.tree.Yaml; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +@Value +public class YamlReference implements Reference { + Cursor cursor; + Kind kind; + + @Override + public Kind getKind() { + return kind; + } + + @Override + public String getValue() { + if (getTree() instanceof Yaml.Scalar) { + return ((Yaml.Scalar) getTree()).getValue(); + } + throw new IllegalArgumentException("getTree() must be an Yaml.Scalar: " + getTree().getClass()); + } + + @Override + public boolean supportsRename() { + return true; + } + + @Override + public Tree rename(Renamer renamer, Cursor cursor, ExecutionContext ctx) { + Tree tree = cursor.getValue(); + if (tree instanceof Yaml.Scalar) { + return ((Yaml.Scalar) tree).withValue(renamer.rename(this)); + } + throw new IllegalArgumentException("cursor.getValue() must be an Yaml.Scalar but is: " + tree.getClass()); + } + + private static class Matcher extends SimpleTraitMatcher { + private static final Predicate javaFullyQualifiedTypePattern = Pattern.compile( + "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(?:\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*") + .asPredicate(); + + @Override + protected @Nullable YamlReference test(Cursor cursor) { + Object value = cursor.getValue(); + if (value instanceof Yaml.Scalar && + javaFullyQualifiedTypePattern.test(((Yaml.Scalar) value).getValue())) { + return new YamlReference(cursor, determineKind(((Yaml.Scalar) value).getValue())); + } + return null; + } + + private Kind determineKind(String value) { + return Character.isUpperCase(value.charAt(value.lastIndexOf('.') + 1)) ? Kind.TYPE : Kind.PACKAGE; + } + } + + @SuppressWarnings("unused") + public static class Provider implements Reference.Provider { + + private static final Predicate applicationPropertiesMatcher = Pattern.compile("^application(-\\w+)?\\.(yaml|yml)$").asPredicate(); + + @Override + public boolean isAcceptable(SourceFile sourceFile) { + return sourceFile instanceof Yaml.Documents && applicationPropertiesMatcher.test(sourceFile.getSourcePath().getFileName().toString()); + } + + @Override + public Set getReferences(SourceFile sourceFile) { + Set references = new HashSet<>(); + new Matcher().asVisitor(reference -> { + references.add(reference); + return reference.getTree(); + }).visit(sourceFile, 0); + return references; + } + } +} diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/package-info.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/package-info.java new file mode 100644 index 00000000000..b2cf0ce8763 --- /dev/null +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/trait/package-info.java @@ -0,0 +1,21 @@ +/* + * 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. + */ +@NullMarked +@NonNullFields +package org.openrewrite.yaml.trait; + +import org.jspecify.annotations.NullMarked; +import org.openrewrite.internal.lang.NonNullFields; diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java index 5245d9ab397..4639bfbf40b 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java @@ -17,12 +17,15 @@ import lombok.*; import lombok.experimental.FieldDefaults; +import lombok.experimental.NonFinal; import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.marker.Markers; import org.openrewrite.yaml.YamlVisitor; import org.openrewrite.yaml.internal.YamlPrinter; +import java.beans.Transient; +import java.lang.ref.SoftReference; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Path; @@ -60,8 +63,10 @@ default

boolean isAcceptable(TreeVisitor v, P p) { @Value @EqualsAndHashCode(callSuper = false, onlyExplicitlyIncluded = true) + @RequiredArgsConstructor + @AllArgsConstructor(access = AccessLevel.PRIVATE) @With - class Documents implements Yaml, SourceFile { + class Documents implements Yaml, SourceFileWithReferences { @EqualsAndHashCode.Include UUID id; @@ -125,6 +130,27 @@ public Documents withPrefix(String prefix) { public

TreeVisitor> printer(Cursor cursor) { return new YamlPrinter<>(); } + + @Nullable + @NonFinal + transient SoftReference references; + + @Transient + @Override + public References getReferences() { + References cache; + if (this.references == null) { + cache = References.build(this); + this.references = new SoftReference<>(cache); + } else { + cache = this.references.get(); + if (cache == null || cache.getSourceFile() != this) { + cache = References.build(this); + this.references = new SoftReference<>(cache); + } + } + return cache; + } } @Value diff --git a/rewrite-yaml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider b/rewrite-yaml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider new file mode 100644 index 00000000000..dd857df5564 --- /dev/null +++ b/rewrite-yaml/src/main/resources/META-INF/services/org.openrewrite.trait.Reference$Provider @@ -0,0 +1 @@ +org.openrewrite.yaml.trait.YamlReference$Provider \ No newline at end of file diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/CommentOutPropertyTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/CommentOutPropertyTest.java index 57334b24734..9f5d7dcf239 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/CommentOutPropertyTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/CommentOutPropertyTest.java @@ -17,24 +17,17 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; -import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.yaml.Assertions.yaml; class CommentOutPropertyTest implements RewriteTest { - @Override - public void defaults(RecipeSpec spec) { - spec.recipe(new CommentOutProperty("management.metrics.binders.files.enabled", "some comments")); - } - @DocumentExample("comment out a map entry") @Test void regular() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence.propertyA", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence.propertyA", "Some comments", null)), yaml( """ foo: @@ -63,8 +56,7 @@ void regular() { @Test void commentSequence() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence", "Some comments", null)), yaml( """ foo: @@ -92,8 +84,7 @@ void commentSequence() { @Test void commentSingleProperty() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("with.java-version", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", null)), yaml( """ with: @@ -115,8 +106,7 @@ void commentSingleProperty() { @Test void commentLastPropertyWithIndent() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("with.java-version", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", null)), yaml( """ with: @@ -136,8 +126,7 @@ void commentLastPropertyWithIndent() { @Test void commentLastPropertyOfFirstDocument() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("with.java-version", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", null)), yaml( """ with: @@ -159,8 +148,7 @@ void commentLastPropertyOfFirstDocument() { @Test void commentLastProperty() { rewriteRun( - spec -> spec.recipe(new CommentOutProperty("test", - "Some comments")), + spec -> spec.recipe(new CommentOutProperty("test", "Some comments", null)), yaml( """ test: foo @@ -172,4 +160,172 @@ void commentLastProperty() { ) ); } + + @DocumentExample("comment out a map entry") + @Test + void sequenceKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence.propertyA", + "Some comments", false)), + yaml( + """ + foo: + bar: + sequence: + - name: name + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """, + """ + foo: + bar: + sequence: + - name: name + # Some comments + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """ + ) + ); + } + + @DocumentExample("comment out a map entry") + @Test + void sequenceFirstKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence.name", "Some comments", false)), + yaml( + """ + foo: + bar: + sequence: + - name: name + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """, + """ + foo: + bar: + sequence: + # Some comments + - name: name + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """ + ) + ); + } + + @DocumentExample("comment out entire sequence") + @Test + void commentSequenceKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("foo.bar.sequence", "Some comments", false)), + yaml( + """ + foo: + bar: + sequence: + - name: name + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """, + """ + foo: + bar: + # Some comments + sequence: + - name: name + - propertyA: fieldA + - propertyB: fieldB + scalar: value + """ + ) + ); + } + + @Test + void commentSinglePropertyKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", false)), + yaml( + """ + with: + java-cache: 'maven' + java-version: 11 + test: 'bar' + """, + """ + with: + java-cache: 'maven' + # Some comments + java-version: 11 + test: 'bar' + """ + ) + ); + } + + @Test + void commentLastPropertyWithIndentKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", false)), + yaml( + """ + with: + java-cache: 'maven' + java-version: 11 + """, + """ + with: + java-cache: 'maven' + # Some comments + java-version: 11 + """ + ) + ); + } + + @Test + void commentLastPropertyOfFirstDocumentKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("with.java-version", "Some comments", false)), + yaml( + """ + with: + java-cache: 'maven' + java-version: 11 + --- + """, + """ + with: + java-cache: 'maven' + # Some comments + java-version: 11 + --- + """ + ) + ); + } + + @Test + void commentLastPropertyKeepProperty() { + rewriteRun( + spec -> spec.recipe(new CommentOutProperty("test", "Some comments", false)), + yaml( + """ + test: foo + """, + """ + # Some comments + test: foo + """ + ) + ); + } } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/trait/YamlReferenceTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/trait/YamlReferenceTest.java new file mode 100644 index 00000000000..a9bd71b35f4 --- /dev/null +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/trait/YamlReferenceTest.java @@ -0,0 +1,96 @@ +/* + * 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.yaml.trait; + +import org.intellij.lang.annotations.Language; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.openrewrite.test.RewriteTest; +import org.openrewrite.trait.Reference; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.openrewrite.yaml.Assertions.yaml; + +class YamlReferenceTest implements RewriteTest { + @Language("yml") + private static final String YAML = """ + root: + a: java.lang.String + b: java.lang + c: String + recipelist: + - org.openrewrite.java.DoSomething: + option: 'org.foo.Bar' + """; + + + @ParameterizedTest + @CsvSource({ + "application.yaml", + "application.yml", + "application-test.yaml", + "application-test.yml", + "/foo/bar/application-test.yaml", + "/foo/bar/application-test.yml", + }) + void findJavaReferencesInYamlProperties(String filename) { + rewriteRun( + yaml( + YAML, + spec -> spec.path(filename).afterRecipe(doc -> { + assertThat(doc.getReferences().getReferences()).satisfiesExactlyInAnyOrder( + ref -> { + assertThat(ref.getKind()).isEqualTo(Reference.Kind.TYPE); + assertThat(ref.getValue()).isEqualTo("java.lang.String"); + }, + ref -> { + assertThat(ref.getKind()).isEqualTo(Reference.Kind.PACKAGE); + assertThat(ref.getValue()).isEqualTo("java.lang"); + }, + ref -> { + assertThat(ref.getKind()).isEqualTo(Reference.Kind.TYPE); + assertThat(ref.getValue()).isEqualTo("org.openrewrite.java.DoSomething"); + }, + ref -> { + assertThat(ref.getKind()).isEqualTo(Reference.Kind.TYPE); + assertThat(ref.getValue()).isEqualTo("org.foo.Bar"); + }); + })) + ); + } + + @ParameterizedTest + @CsvSource({ + "application-.yaml", + "application-.yml", + "application.test.yaml", + "application.test.yml", + "other-application.yaml", + "other-application.yml", + "other.yaml", + "other.yml", + "/foo/bar/other.yaml", + "/foo/bar/other.yml" + }) + void noReferencesInMismatchedFilenames(String filename) { + rewriteRun( + yaml( + YAML, + spec -> spec.path(filename).afterRecipe(doc -> assertThat(doc.getReferences().getReferences()).isEmpty()) + ) + ); + } +} diff --git a/rewrite.yml b/rewrite.yml new file mode 100644 index 00000000000..6ed6b2ee876 --- /dev/null +++ b/rewrite.yml @@ -0,0 +1,78 @@ +# +# 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. +# +--- +# Apply a subset of best practices to OpenRewrite recipes; typically run before committing changes. +# Any differences produced by this recipe will result in code suggestion comments on pull requests. +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.openrewrite.recipes.OpenRewriteBestPracticesSubset +displayName: OpenRewrite best practices +description: Best practices for OpenRewrite recipe development. +recipeList: + - org.openrewrite.recipes.JavaRecipeBestPracticesSubset + - org.openrewrite.recipes.RecipeTestingBestPracticesSubset + - org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset + #- org.openrewrite.java.OrderImports + - org.openrewrite.java.format.EmptyNewlineAtEndOfFile + - org.openrewrite.staticanalysis.InlineVariable + - org.openrewrite.staticanalysis.MissingOverrideAnnotation + - org.openrewrite.staticanalysis.UseDiamondOperator +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.openrewrite.recipes.JavaRecipeBestPracticesSubset +displayName: Java Recipe best practices +description: Best practices for Java recipe development. +preconditions: + - org.openrewrite.java.search.FindTypes: + fullyQualifiedTypeName: org.openrewrite.Recipe + checkAssignability: true +recipeList: + - org.openrewrite.java.recipes.BlankLinesAroundFieldsWithAnnotations + - org.openrewrite.java.recipes.ExecutionContextParameterName + - org.openrewrite.java.recipes.MissingOptionExample + - org.openrewrite.java.recipes.RecipeEqualsAndHashCodeCallSuper + - org.openrewrite.java.recipes.UseTreeRandomId + - org.openrewrite.staticanalysis.NeedBraces + #- org.openrewrite.staticanalysis.RemoveSystemOutPrintln +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.openrewrite.recipes.RecipeTestingBestPracticesSubset +displayName: Recipe testing best practices +description: Best practices for testing recipes. +preconditions: + - org.openrewrite.java.search.FindTypes: + fullyQualifiedTypeName: org.openrewrite.test.RewriteTest + checkAssignability: true +recipeList: + - org.openrewrite.java.recipes.RewriteTestClassesShouldNotBePublic + #- org.openrewrite.java.recipes.SelectRecipeExamples + - org.openrewrite.java.recipes.SourceSpecTextBlockIndentation + - org.openrewrite.java.testing.cleanup.RemoveTestPrefix + - org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic + - org.openrewrite.staticanalysis.NeedBraces + - org.openrewrite.staticanalysis.RemoveSystemOutPrintln +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset +displayName: Recipe nullability best practices +description: Use OpenRewrite internal nullability annotations; drop JetBrains annotations; use `package-info.java` instead. +recipeList: + - org.openrewrite.staticanalysis.NullableOnMethodReturnType + - org.openrewrite.java.RemoveAnnotation: + annotationPattern: '@org.jetbrains.annotations.NotNull' + - org.openrewrite.java.RemoveAnnotation: + annotationPattern: '@jakarta.annotation.Nonnull' + #- org.openrewrite.java.jspecify.MigrateToJspecify \ No newline at end of file diff --git a/suppressions.xml b/suppressions.xml index 0376a3d984e..adba34b3922 100644 --- a/suppressions.xml +++ b/suppressions.xml @@ -9,7 +9,7 @@ ^pkg:maven/com\.squareup\.okio/okio@.*$ CVE-2023-3635 - + CVE-2023-45163 CVE-2023-5964 - + @@ -39,4 +39,4 @@ ^pkg:maven/org\.glassfish/javax\.json@.*$ CVE-2023-7272 - \ No newline at end of file +