From eed3a5d6f6007286d3d04a20391e5d70f7017ae1 Mon Sep 17 00:00:00 2001 From: Sam Snyder <sam@moderne.io> Date: Mon, 3 Jun 2024 13:25:31 -0700 Subject: [PATCH] Change DependencyStringNotationConverter to not throw an exception when presented with something that isn't a GAV coordinate. --- .../openrewrite/semver/DependencyMatcher.java | 4 ++-- .../openrewrite/gradle/ChangeDependency.java | 4 ++-- .../gradle/ChangeDependencyArtifactId.java | 4 ++-- .../gradle/ChangeDependencyClassifier.java | 2 +- .../gradle/ChangeDependencyConfiguration.java | 6 ++--- .../gradle/ChangeDependencyExtension.java | 2 +- .../gradle/ChangeDependencyGroupId.java | 4 ++-- .../gradle/DependencyUseMapNotation.java | 3 +++ .../openrewrite/gradle/RemoveDependency.java | 21 +++++++++++------ .../gradle/UpgradeDependencyVersion.java | 11 +++++---- .../internal/InsertDependencyComparator.java | 23 +++++++++++-------- .../DependencyStringNotationConverter.java | 8 ++++++- 12 files changed, 57 insertions(+), 35 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/semver/DependencyMatcher.java b/rewrite-core/src/main/java/org/openrewrite/semver/DependencyMatcher.java index 470f6999d4c..60c669b2896 100755 --- a/rewrite-core/src/main/java/org/openrewrite/semver/DependencyMatcher.java +++ b/rewrite-core/src/main/java/org/openrewrite/semver/DependencyMatcher.java @@ -74,13 +74,13 @@ public static Validated<DependencyMatcher> build(String pattern) { return Validated.valid("pattern", new DependencyMatcher(patternPieces[0], patternPieces[1], validatedVersion.getValue())); } - public boolean matches(String groupId, String artifactId, String version) { + public boolean matches(@Nullable String groupId, String artifactId, String version) { return StringUtils.matchesGlob(groupId, groupPattern) && StringUtils.matchesGlob(artifactId, artifactPattern) && (versionComparator == null || versionComparator.isValid(null, version)); } - public boolean matches(String groupId, String artifactId) { + public boolean matches(@Nullable String groupId, String artifactId) { return StringUtils.matchesGlob(groupId, groupPattern) && StringUtils.matchesGlob(artifactId, artifactPattern); } 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 35faf48bdb3..83aa7ddc9a9 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java @@ -202,7 +202,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 (depMatcher.matches(original.getGroupId(), original.getArtifactId())) { + if (original != null && depMatcher.matches(original.getGroupId(), original.getArtifactId())) { Dependency updated = original; if (!StringUtils.isBlank(newGroupId) && !updated.getGroupId().equals(newGroupId)) { updated = updated.withGroupId(newGroupId); @@ -236,7 +236,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 (depMatcher.matches(original.getGroupId(), original.getArtifactId())) { + if (original != null && depMatcher.matches(original.getGroupId(), original.getArtifactId())) { Dependency updated = original; if (!StringUtils.isBlank(newGroupId) && !updated.getGroupId().equals(newGroupId)) { updated = updated.withGroupId(newGroupId); 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 77b28b59275..16997e2db3e 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java @@ -134,7 +134,7 @@ 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 (!newArtifactId.equals(dependency.getArtifactId()) && + 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())))) { Dependency newDependency = dependency.withArtifactId(newArtifactId); @@ -147,7 +147,7 @@ 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 (!newArtifactId.equals(dependency.getArtifactId()) + if (dependency != null && !newArtifactId.equals(dependency.getArtifactId()) && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { Dependency newDependency = dependency.withArtifactId(newArtifactId); updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); 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 383b7062c82..baec86def0d 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java @@ -98,7 +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.getVersion() != null && dependency.getClassifier() != null && !newClassifier.equals(dependency.getClassifier()) && + if (dependency != null && dependency.getVersion() != null && dependency.getClassifier() != null && !newClassifier.equals(dependency.getClassifier()) && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())) { Dependency newDependency = dependency.withClassifier(newClassifier); m = m.withArguments(ListUtils.mapFirst(m.getArguments(), arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, newDependency.toStringNotation()))); 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 dd445c58ae3..fa817da83be 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java @@ -104,7 +104,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) } Dependency dependency = DependencyStringNotationConverter.parse((String) arg.getValue()); - if (!dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + if (dependency == null || !dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { return m; } } else if (args.get(0) instanceof G.GString) { @@ -119,7 +119,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) } Dependency dependency = DependencyStringNotationConverter.parse((String) groupArtifact.getValue()); - if (!dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + if (dependency == null || !dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { return m; } } else if (args.get(0) instanceof G.MapEntry && args.size() >= 2) { @@ -158,7 +158,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) dependency = DependencyStringNotationConverter.parse((String) value.getValue()); } - if (!dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + if (dependency == null || !dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { return m; } } else { 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 d4880ff5b29..d96fbfb9e4d 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java @@ -98,7 +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 (!newExtension.equals(dependency.getExt()) && + 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())))) { Dependency newDependency = dependency.withExt(newExtension); 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 b0ef8892f07..11e34143230 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java @@ -134,7 +134,7 @@ 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 (!newGroupId.equals(dependency.getGroupId()) && + 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())))) { Dependency newDependency = dependency.withGroupId(newGroupId); @@ -147,7 +147,7 @@ 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 (!newGroupId.equals(dependency.getGroupId()) + if (dependency != null && !newGroupId.equals(dependency.getGroupId()) && depMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { Dependency newDependency = dependency.withGroupId(newGroupId); updatedDependencies.put(dependency.getGav().asGroupArtifact(), newDependency.getGav().asGroupArtifact()); 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 c3bcf665ff7..9f2eb8b146c 100755 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java @@ -78,6 +78,9 @@ private J.MethodInvocation forBasicString(J.MethodInvocation m) { return m; } Dependency dependency = DependencyStringNotationConverter.parse(dependencyString); + if (dependency == null) { + return m; + } List<Expression> arguments = new ArrayList<>(); arguments.add(mapEntry("group", dependency.getGroupId()) .withMarkers(arg.getMarkers()) 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 eda9b041253..787823c0971 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java @@ -97,7 +97,7 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { boolean anyChanged = false; for (GradleDependencyConfiguration gdc : nameToConfiguration.values()) { GradleDependencyConfiguration newGdc = gdc.withRequested(ListUtils.map(gdc.getRequested(), requested -> { - if (dependencyMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { + if (requested.getGroupId() != null && dependencyMatcher.matches(requested.getGroupId(), requested.getArtifactId())) { return null; } return requested; @@ -121,28 +121,31 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { } @Override - public @Nullable J visitReturn(J.Return return_, ExecutionContext ctx) { + public 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); if (dependencyInvocation && r.getExpression() == null) { + //noinspection DataFlowIssue return null; } return r; } @Override - public @Nullable J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); if (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; } } @@ -162,13 +165,17 @@ public J visitCompilationUnit(G.CompilationUnit cu, ExecutionContext ctx) { if (!(groupArtifact.getValue() instanceof String)) { return m; } - Dependency dep = DependencyStringNotationConverter.parse((String) groupArtifact.getValue()); - if (dependencyMatcher.matches(dep.getGroupId(), dep.getArtifactId())) { + 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) { - Dependency dependency = DependencyStringNotationConverter.parse((String) ((J.Literal) m.getArguments().get(0)).getValue()); - if (dependencyMatcher.matches(dependency.getGroupId(), dependency.getArtifactId())) { + 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; } } else if (m.getArguments().get(0) instanceof G.MapEntry) { 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 14e39322551..737e767abac 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java @@ -159,7 +159,8 @@ private static boolean isLikelyDependencyConfiguration(Cursor cursor) { if (cursor.getValue() instanceof J.MethodInvocation) { m = cursor.getValue(); String methodName = m.getSimpleName(); - if ("constraints".equals(methodName)) { + if ("constraints".equals(methodName) || "project".equals(methodName) || "modules".equals(methodName) + || "module".equals(methodName) ||"file".equals(methodName) || "files".equals(methodName)) { return false; } if (DEPENDENCY_DSL_MATCHER.matches(m)) { @@ -271,7 +272,9 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) continue; } Dependency dep = DependencyStringNotationConverter.parse((String) groupArtifact.getValue()); - + if (dep == null) { + continue; + } String versionVariableName = ((J.Identifier) versionValue.getTree()).getSimpleName(); GroupArtifact ga = new GroupArtifact(dep.getGroupId(), dep.getArtifactId()); if (acc.gaToNewVersion.containsKey(ga)) { @@ -416,7 +419,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation method, Execution return arg; } Dependency dep = DependencyStringNotationConverter.parse((String) groupArtifact.getValue()); - if (dependencyMatcher.matches(dep.getGroupId(), dep.getArtifactId())) { + if (dep != null && dependencyMatcher.matches(dep.getGroupId(), dep.getArtifactId())) { Object scanResult = acc.gaToNewVersion.get(new GroupArtifact(dep.getGroupId(), dep.getArtifactId())); if (scanResult instanceof Exception) { getCursor().putMessage(UPDATE_VERSION_ERROR_KEY, scanResult); @@ -438,7 +441,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation method, Execution return arg; } Dependency dep = DependencyStringNotationConverter.parse(gav); - if (dependencyMatcher.matches(dep.getGroupId(), dep.getArtifactId()) + if (dep != null && dependencyMatcher.matches(dep.getGroupId(), dep.getArtifactId()) && dep.getVersion() != null && !dep.getVersion().startsWith("$")) { Object scanResult = acc.gaToNewVersion.get(new GroupArtifact(dep.getGroupId(), dep.getArtifactId())); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/internal/InsertDependencyComparator.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/internal/InsertDependencyComparator.java index 874df057e4a..7622e609377 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/internal/InsertDependencyComparator.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/internal/InsertDependencyComparator.java @@ -15,6 +15,7 @@ */ package org.openrewrite.gradle.internal; +import lombok.Getter; import org.openrewrite.gradle.util.Dependency; import org.openrewrite.gradle.util.DependencyStringNotationConverter; import org.openrewrite.groovy.tree.G; @@ -28,7 +29,9 @@ public class InsertDependencyComparator implements Comparator<Statement> { private final Map<Statement, Float> positions = new LinkedHashMap<>(); + @Getter private Statement afterDependency; + @Getter private Statement beforeDependency; public InsertDependencyComparator(List<Statement> existingStatements, J.MethodInvocation dependencyToAdd) { @@ -73,15 +76,7 @@ public int compare(Statement o1, Statement o2) { return positions.get(o1).compareTo(positions.get(o2)); } - public Statement getAfterDependency() { - return afterDependency; - } - - public Statement getBeforeDependency() { - return beforeDependency; - } - - private static Comparator<Statement> dependenciesComparator = (s1, s2) -> { + private static final Comparator<Statement> dependenciesComparator = (s1, s2) -> { J.MethodInvocation d1; if (s1 instanceof J.Return) { d1 = (J.MethodInvocation) ((J.Return) s1).getExpression(); @@ -96,6 +91,7 @@ public Statement getBeforeDependency() { d2 = (J.MethodInvocation) s2; } + assert d1 != null && d2 != null; String configuration1 = d1.getSimpleName(); String configuration2 = d2.getSimpleName(); if (!configuration1.equals(configuration2)) { @@ -135,7 +131,14 @@ public Statement getBeforeDependency() { private static Optional<String> getEntry(String entry, J.MethodInvocation invocation) { if (invocation.getArguments().get(0) instanceof J.Literal) { - Dependency dependency = DependencyStringNotationConverter.parse((String) ((J.Literal) invocation.getArguments().get(0)).getValue()); + Object value = ((J.Literal) invocation.getArguments().get(0)).getValue(); + if(value == null) { + return Optional.empty(); + } + Dependency dependency = DependencyStringNotationConverter.parse((String) value); + if(dependency == null) { + return Optional.empty(); + } switch (entry) { case "group": return Optional.ofNullable(dependency.getGroupId()); diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/DependencyStringNotationConverter.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/DependencyStringNotationConverter.java index 6d352a23940..29bcdd4f326 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/DependencyStringNotationConverter.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/util/DependencyStringNotationConverter.java @@ -19,6 +19,11 @@ public class DependencyStringNotationConverter { + /** + * @param notation a String in the format group:artifact:version + * @return A corresponding Dependency or null if the notation could not be parsed + */ + @Nullable public static Dependency parse(String notation) { int idx = notation.lastIndexOf('@'); if (idx == -1) { @@ -33,6 +38,7 @@ public static Dependency parse(String notation) { return parse(notation, null); } + @Nullable private static Dependency parse(String notation, @Nullable String ext) { Dependency dependency = new Dependency(null, null, null, null, ext); @@ -51,7 +57,7 @@ private static Dependency parse(String notation, @Nullable String ext) { count++; if (count < 2 || count > 4) { - throw new IllegalArgumentException("Supplied String notation '" + notation + "' is invalid."); + return null; } return dependency;