Skip to content

Expand star imports in ChangePackage and related recipes when they would create ambiguity #7202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
steve-aom-elliott merged 7 commits into from
Apr 10, 2026
Merged

Expand star imports in ChangePackage and related recipes when they would create ambiguity #7202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bc0a29b
Add star import ambiguity detection to ChangePackage
steve-aom-elliott Mar 30, 2026
f2449d6
Update JavaSourceSet marker from dependency-modifying recipes
steve-aom-elliott Apr 6, 2026
9930b9d
Add star import ambiguity detection to ChangeType
steve-aom-elliott Apr 7, 2026
edf0c15
Extract withSourceTypesOnClasspath to shared Assertions helper, add i…
steve-aom-elliott Apr 7, 2026
18ffbda
Address review feedback: warning markers, remove G/K guard, cache sou…
steve-aom-elliott Apr 8, 2026
32bf4cd
Use project repositories instead of hardcoded Maven Central in Gradle…
steve-aom-elliott Apr 8, 2026
a5abf62
Move JavaSourceSet updating logic into JavaSourceSet, simplify recipes
steve-aom-elliott Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 95 additions & 4 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,15 @@
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaSourceFile;
import org.openrewrite.marker.Markup;
import org.openrewrite.maven.MavenDownloadingException;
import org.openrewrite.maven.MavenExecutionContextView;
import org.openrewrite.maven.internal.MavenPomDownloader;
import org.openrewrite.maven.table.MavenMetadataFailures;
import org.openrewrite.maven.tree.GroupArtifact;
import org.openrewrite.maven.tree.*;
import org.openrewrite.maven.utilities.JavaSourceSetUpdater;
import org.openrewrite.semver.Semver;
import org.openrewrite.semver.VersionComparator;

import java.util.*;

Expand Down Expand Up @@ -138,6 +144,11 @@ public Validated<Object> validate() {
public static class Scanned {
Map<JavaProject, Boolean> usingType = new HashMap<>();
Map<JavaProject, Set<String>> configurationsByProject = new HashMap<>();
@Nullable
String resolvedVersion;
List<MavenRepository> repositories = new ArrayList<>();
@Nullable
Exception versionResolutionFailure;
}

@Override
Expand Down Expand Up @@ -176,7 +187,9 @@ private boolean usesType(SourceFile sourceFile, ExecutionContext ctx) {
sourceFile == hasTestSourceSet.visit(sourceFile, ctx)) {
return tree;
}
sourceFile.getMarkers().findFirst(JavaProject.class).ifPresent(javaProject -> {
Optional<JavaProject> maybeJavaProject = sourceFile.getMarkers().findFirst(JavaProject.class);
if (maybeJavaProject.isPresent()) {
JavaProject javaProject = maybeJavaProject.get();
boolean uses = usesType(sourceFile, ctx);
acc.usingType.compute(javaProject, (jp, usingType) -> Boolean.TRUE.equals(usingType) || uses);

Expand All @@ -185,7 +198,25 @@ private boolean usesType(SourceFile sourceFile, ExecutionContext ctx) {
sourceFile.getMarkers().findFirst(JavaSourceSet.class).ifPresent(sourceSet ->
configurations.add("main".equals(sourceSet.getName()) ? "implementation" : sourceSet.getName() + "Implementation"));
}
});

// Resolve version once for JavaSourceSet updates
if (acc.resolvedVersion == null && version != null) {
Optional<GradleProject> maybeGp = sourceFile.getMarkers().findFirst(GradleProject.class);
if (maybeGp.isPresent()) {
try {
DependencyVersionSelector selector = new DependencyVersionSelector(metadataFailures, maybeGp.get(), null);
acc.resolvedVersion = selector.select(
new GroupArtifact(groupId, artifactId), "implementation",
version, versionPattern, ctx);
if (acc.resolvedVersion != null) {
acc.repositories = maybeGp.get().getMavenRepositories();
}
} catch (MavenDownloadingException e) {
acc.versionResolutionFailure = e;
}
}
}
}
return tree;
}
};
Expand All @@ -196,7 +227,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(Scanned acc) {
// Allow when configuration is explicitly provided, when onlyIfUsing is not set (default to "implementation"),
// or when source files were scanned
boolean hasExplicitConfiguration = !StringUtils.isBlank(configuration);
return Preconditions.check(hasExplicitConfiguration || onlyIfUsing == null || !acc.configurationsByProject.isEmpty(),
TreeVisitor<?, ExecutionContext> gradleVisitor = Preconditions.check(hasExplicitConfiguration || onlyIfUsing == null || !acc.configurationsByProject.isEmpty(),
Preconditions.check(new IsBuildGradle<>(true), new JavaIsoVisitor<ExecutionContext>() {

@Override
Expand Down Expand Up @@ -280,5 +311,65 @@ private boolean isTopLevel(Cursor cursor) {
}
})
);

if (acc.configurationsByProject.isEmpty() || acc.resolvedVersion == null) {
if (acc.versionResolutionFailure != null) {
Exception failure = acc.versionResolutionFailure;
return new TreeVisitor<Tree, ExecutionContext>() {
@Override
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
return gradleVisitor.isAcceptable(sourceFile, ctx);
}

@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
Tree result = gradleVisitor.visit(tree, ctx);
if (result != tree) {
result = Markup.warn(result, failure);
}
return result;
}
};
}
return gradleVisitor;
}

return new TreeVisitor<Tree, ExecutionContext>() {
@Nullable
private JavaSourceSetUpdater updater;

@Override
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
return gradleVisitor.isAcceptable(sourceFile, ctx) || sourceFile instanceof JavaSourceFile;
}

@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
if (!(tree instanceof SourceFile)) {
return tree;
}
SourceFile sf = (SourceFile) tree;
if (gradleVisitor.isAcceptable(sf, ctx)) {
return gradleVisitor.visit(tree, ctx);
}
if (sf instanceof JavaSourceFile) {
return updateJavaSourceSet(sf, ctx);
}
return tree;
}

private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
Optional<JavaProject> maybeJp = sf.getMarkers().findFirst(JavaProject.class);
if (!maybeJp.isPresent() || !acc.configurationsByProject.containsKey(maybeJp.get())) {
return sf;
}
if (updater == null) {
updater = new JavaSourceSetUpdater(ctx);
}
return JavaSourceSet.updateOnSourceFile(sf, sourceSet ->
sourceSet.getGavToTypes().isEmpty() ? sourceSet :
updater.addDependency(sourceSet, groupId, artifactId, acc.resolvedVersion, acc.repositories));
}
};
}
}
76 changes: 74 additions & 2 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@
import org.openrewrite.internal.StringUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.marker.JavaProject;
import org.openrewrite.java.marker.JavaSourceSet;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaSourceFile;
import org.openrewrite.kotlin.tree.K;
import org.openrewrite.marker.Markup;
import org.openrewrite.maven.MavenDownloadingException;
import org.openrewrite.maven.tree.*;
import org.openrewrite.maven.table.MavenMetadataFailures;
import org.openrewrite.maven.utilities.JavaSourceSetUpdater;
import org.openrewrite.properties.PropertiesVisitor;
import org.openrewrite.properties.tree.Properties;
import org.openrewrite.semver.DependencyMatcher;
Expand Down Expand Up @@ -171,6 +174,8 @@ public static class Accumulator {
Map<String, Object> versionVariableUpdates = new HashMap<>();
Map<String, Set<GroupArtifact>> versionVariableUsages = new HashMap<>();
Set<GroupArtifact> failedResolutions = new HashSet<>();
Map<JavaProject, ResolvedDependency> modulesWithOldDependency = new HashMap<>();
Map<JavaProject, List<MavenRepository>> moduleRepositories = new HashMap<>();
}

@Override
Expand All @@ -197,6 +202,21 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
if (gradleProject == null) {
return (J) tree;
}
// Record this module if it has the old dependency
Optional<JavaProject> maybeJp = tree.getMarkers().findFirst(JavaProject.class);
if (maybeJp.isPresent() && !acc.modulesWithOldDependency.containsKey(maybeJp.get())) {
outer:
for (GradleDependencyConfiguration config : gradleProject.getConfigurations()) {
for (ResolvedDependency resolved : config.getDirectResolved()) {
if (StringUtils.matchesGlob(resolved.getGroupId(), oldGroupId) &&
StringUtils.matchesGlob(resolved.getArtifactId(), oldArtifactId)) {
acc.modulesWithOldDependency.put(maybeJp.get(), resolved);
acc.moduleRepositories.put(maybeJp.get(), gradleProject.getMavenRepositories());
break outer;
}
}
}
}
}
return super.visit(tree, ctx);
}
Expand Down Expand Up @@ -480,14 +500,23 @@ private GradleProject updateGradleModel(GradleProject gp, ExecutionContext ctx)
});

DependencyMatcher propsMatcher = requireNonNull(DependencyMatcher.build(oldGroupId + ":" + oldArtifactId).getValue());
boolean hasModulesWithOldDep = !acc.modulesWithOldDependency.isEmpty();
return new TreeVisitor<Tree, ExecutionContext>() {
@Nullable
private JavaSourceSetUpdater updater;
private final Map<String, JavaSourceSet> updatedSourceSets = new HashMap<>();

@Override
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
if (sourceFile instanceof Properties.File) {
return sourceFile.getSourcePath().endsWith(GRADLE_PROPERTIES_FILE_NAME);
}
return (sourceFile instanceof G.CompilationUnit || sourceFile instanceof K.CompilationUnit)
&& sourceFile.getMarkers().findFirst(GradleProject.class).isPresent();
if ((sourceFile instanceof G.CompilationUnit || sourceFile instanceof K.CompilationUnit)
&& sourceFile.getMarkers().findFirst(GradleProject.class).isPresent()) {
return true;
}
// Accept Java source files for JavaSourceSet updates
return hasModulesWithOldDep && sourceFile instanceof JavaSourceFile;
}

@Override
Expand Down Expand Up @@ -517,8 +546,51 @@ public Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) {
}
return tree;
}
// For Java source files, update JavaSourceSet marker
if (hasModulesWithOldDep && tree instanceof JavaSourceFile) {
return updateJavaSourceSet((SourceFile) tree, ctx);
}
return gradleVisitor.visit(tree, ctx);
}

private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
Optional<JavaProject> maybeJp = sf.getMarkers().findFirst(JavaProject.class);
if (!maybeJp.isPresent()) {
return sf;
}
ResolvedDependency oldDep = acc.modulesWithOldDependency.get(maybeJp.get());
if (oldDep == null) {
return sf;
}
if (updater == null) {
updater = new JavaSourceSetUpdater(ctx);
}
JavaProject jp = maybeJp.get();
return JavaSourceSet.updateOnSourceFile(sf, updatedSourceSets, sourceSet -> {
String effectiveNewGroupId = newGroupId != null ? newGroupId : oldDep.getGroupId();
String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldDep.getArtifactId();
ResolvedGroupArtifactVersion newGav = new ResolvedGroupArtifactVersion(
oldDep.getGav().getRepository(),
effectiveNewGroupId, effectiveNewArtifactId, oldDep.getVersion(), null);
ResolvedDependency newDep = oldDep
.withGav(newGav)
.withRepository(findRemoteRepository(jp));
return updater.changeDependency(sourceSet, oldDep, newDep);
});
}

private MavenRepository findRemoteRepository(JavaProject jp) {
List<MavenRepository> repos = acc.moduleRepositories.get(jp);
if (repos != null) {
for (MavenRepository repo : repos) {
String uri = repo.getUri();
if (uri != null && (uri.startsWith("http://") || uri.startsWith("https://"))) {
return repo;
}
}
}
return MavenRepository.MAVEN_CENTRAL;
}
};
}

Expand Down
26 changes: 25 additions & 1 deletion rewrite-gradle/src/main/java/org/openrewrite/gradle/RemoveDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.openrewrite.internal.ListUtils;
import org.openrewrite.internal.StringUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.marker.JavaSourceSet;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaSourceFile;
import org.openrewrite.kotlin.tree.K;
Expand Down Expand Up @@ -69,7 +70,7 @@ public String getInstanceNameSuffix() {

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(new IsBuildGradle<>(), new JavaIsoVisitor<ExecutionContext>() {
TreeVisitor<?, ExecutionContext> gradleVisitor = Preconditions.check(new IsBuildGradle<>(), new JavaIsoVisitor<ExecutionContext>() {
final GradleDependency.Matcher gradleDependencyMatcher = new GradleDependency.Matcher()
.configuration(configuration)
.groupId(groupId)
Expand Down Expand Up @@ -157,5 +158,28 @@ private GradleProject updateGradleModel(GradleProject gp) {
return gp;
}
});

return new TreeVisitor<Tree, ExecutionContext>() {
@Override
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
return gradleVisitor.isAcceptable(sourceFile, ctx) || sourceFile instanceof JavaSourceFile;
}

@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
if (!(tree instanceof SourceFile)) {
return tree;
}
SourceFile sf = (SourceFile) tree;
if (gradleVisitor.isAcceptable(sf, ctx)) {
return gradleVisitor.visit(tree, ctx);
}
if (sf instanceof JavaSourceFile) {
return JavaSourceSet.updateOnSourceFile(sf,
sourceSet -> sourceSet.removeTypesMatching(groupId, artifactId));
}
return tree;
}
};
}
}
Loading
Loading
⚡