Skip to content

Commit

Permalink
Change DependencyStringNotationConverter to not throw an exception wh…
Browse files Browse the repository at this point in the history 
…en presented with something that isn't a GAV coordinate.
  • Loading branch information
@sambsnyd
sambsnyd committed Jun 3, 2024
1 parent 54f8209 commit eed3a5d
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 35 deletions.
4 changes: 2 additions & 2 deletions rewrite-core/src/main/java/org/openrewrite/semver/DependencyMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyArtifactId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyClassifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
Expand Down
6 changes: 3 additions & 3 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependencyGroupId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down
3 changes: 3 additions & 0 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/DependencyUseMapNotation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
21 changes: 14 additions & 7 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
Expand All @@ -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) {
Expand Down
11 changes: 7 additions & 4 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand All @@ -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()));
Expand Down
23 changes: 13 additions & 10 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/internal/InsertDependencyComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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)) {
Expand Down Expand Up @@ -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());
Expand Down
Loading

0 comments on commit eed3a5d

Please sign in to comment.