From e5a310b0f7ae24c5f0a306d5720c981014d089c4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 May 2026 20:00:02 +0200 Subject: [PATCH 1/3] Record one row per project in FindJavaVersion The recipe used a `transient Set seen` HashSet on the recipe instance to deduplicate rows. Because `JavaVersion` excludes its UUID from `equals`, two compilation units from different projects built with the same JDK compare equal, and recipe instances are reused across repositories within a single run, so identical Java versions in separate projects collapsed into a single data-table row. Drop the deduplication and add a `projectName` column derived from the `JavaProject` marker so that each project contributes its own row. Fixes moderneinc/customer-requests#2409 --- .../java/migrate/search/FindJavaVersion.java | 19 +++---- .../java/migrate/table/JavaVersionTable.java | 4 ++ .../migrate/search/FindJavaVersionTest.java | 53 +++++++++++++++++-- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java index 4047ac6ed6..85c77ee397 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java @@ -21,19 +21,16 @@ import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.marker.JavaVersion; import org.openrewrite.java.migrate.table.JavaVersionTable; import org.openrewrite.java.tree.J; -import java.util.HashSet; -import java.util.Set; - @EqualsAndHashCode(callSuper = false) @Value public class FindJavaVersion extends Recipe { transient JavaVersionTable table = new JavaVersionTable(this); - transient Set seen = new HashSet<>(); String displayName = "Find Java versions in use"; @@ -45,11 +42,15 @@ public TreeVisitor getVisitor() { @Override public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { cu.getMarkers().findFirst(JavaVersion.class) - .filter(seen::add) - .map(jv -> new JavaVersionTable.Row( - Integer.toString(jv.getMajorVersion()), - Integer.toString(jv.getMajorReleaseVersion()))) - .ifPresent(row -> table.insertRow(ctx, row)); + .ifPresent(jv -> { + String projectName = cu.getMarkers().findFirst(JavaProject.class) + .map(JavaProject::getProjectName) + .orElse(""); + table.insertRow(ctx, new JavaVersionTable.Row( + projectName, + Integer.toString(jv.getMajorVersion()), + Integer.toString(jv.getMajorReleaseVersion()))); + }); return cu; } }; diff --git a/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java b/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java index 5e94dc1ca2..79e5ae9707 100644 --- a/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java +++ b/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java @@ -28,6 +28,10 @@ public JavaVersionTable(Recipe recipe) { @Value public static class Row { + @Column(displayName = "Project name", + description = "The name of the project that produced the LST.") + String projectName; + @Column(displayName = "Source compatibility", description = "The major version of Java used to compile the source code") String sourceVersion; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java index dc670cd529..6133792510 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.marker.JavaVersion; import org.openrewrite.java.migrate.table.JavaVersionTable; import org.openrewrite.test.RecipeSpec; @@ -35,12 +36,14 @@ public void defaults(RecipeSpec spec) { @DocumentExample @Test - void twoClassesWithSameMarkerLeadToOneRow() { + void twoClassesInSameProjectLeadToTwoRows() { + var project = new JavaProject(randomId(), "demo", null); var jv = new JavaVersion(randomId(), "Sam", "Shelter", "17", "8"); rewriteRun( spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> { assertThat(rows).containsExactly( - new JavaVersionTable.Row("17", "8") + new JavaVersionTable.Row("demo", "17", "8"), + new JavaVersionTable.Row("demo", "17", "8") ); }), //language=java @@ -49,14 +52,56 @@ void twoClassesWithSameMarkerLeadToOneRow() { class A { } """, - spec -> spec.markers(jv)), + spec -> spec.markers(project, jv)), //language=java java( """ class B { } """, - spec -> spec.markers(jv)) + spec -> spec.markers(project, jv)) + ); + } + + @Test + void identicalJavaVersionMarkersAcrossProjectsAreEachReported() { + // Reproduces customer-requests#2409: across multiple projects with the same JDK, + // every project must appear as its own row in the data table. Previously a + // recipe-instance HashSet deduplicated by JavaVersion content, so identical + // markers in different projects were silently dropped. + var projectA = new JavaProject(randomId(), "project-a", null); + var projectB = new JavaProject(randomId(), "project-b", null); + var projectC = new JavaProject(randomId(), "project-c", null); + var jv = new JavaVersion(randomId(), "Sam", "Shelter", "17", "8"); + rewriteRun( + spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> { + assertThat(rows).containsExactlyInAnyOrder( + new JavaVersionTable.Row("project-a", "17", "8"), + new JavaVersionTable.Row("project-b", "17", "8"), + new JavaVersionTable.Row("project-c", "17", "8") + ); + }), + //language=java + java( + """ + class A { + } + """, + spec -> spec.markers(projectA, jv)), + //language=java + java( + """ + class B { + } + """, + spec -> spec.markers(projectB, jv)), + //language=java + java( + """ + class C { + } + """, + spec -> spec.markers(projectC, jv)) ); } } From 4fff0c426f721c8195a1b258e53f224ef5de3038 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 May 2026 20:26:07 +0200 Subject: [PATCH 2/3] Aggregate FindJavaVersion to one row per git repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert FindJavaVersion to a ScanningRecipe keyed on GitProvenance.origin, so each git repository contributes exactly one row regardless of how many modules or compilation units it contains. When modules in a repository disagree on JDK, retain the row with the lowest target compatibility (tiebreak: lowest source) — the migration floor for the repository. Fallback key order: GitProvenance.origin -> JavaProject.getId() -> JavaVersion.getId(), so disconnected sources never silently merge. Revert the projectName column added in e5a310b0; JavaVersionTable.Row is back to its original two columns (sourceVersion, targetVersion). Fixes moderneinc/customer-requests#2409 --- .../java/migrate/search/FindJavaVersion.java | 98 ++++++++++---- .../java/migrate/table/JavaVersionTable.java | 4 - .../migrate/search/FindJavaVersionTest.java | 124 ++++++++++++++---- 3 files changed, 170 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java index 85c77ee397..5ea8de9e62 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java @@ -15,44 +15,94 @@ */ package org.openrewrite.java.migrate.search; -import lombok.EqualsAndHashCode; -import lombok.Value; +import lombok.Getter; import org.openrewrite.ExecutionContext; -import org.openrewrite.Recipe; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.SourceFile; +import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; -import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.marker.JavaVersion; import org.openrewrite.java.migrate.table.JavaVersionTable; -import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaSourceFile; +import org.openrewrite.marker.GitProvenance; +import org.openrewrite.marker.Markers; -@EqualsAndHashCode(callSuper = false) -@Value -public class FindJavaVersion extends Recipe { +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Optional; + +import static java.util.Collections.emptyList; + +public class FindJavaVersion extends ScanningRecipe> { transient JavaVersionTable table = new JavaVersionTable(this); - String displayName = "Find Java versions in use"; + @Getter + final String displayName = "Find Java versions in use"; + + @Getter + final String description = "Finds Java versions in use, emitting one row per git repository " + + "(the lowest source/target compatibility across modules in that repository)."; - String description = "Finds Java versions in use."; + @Override + public Map getInitialValue(ExecutionContext ctx) { + return new LinkedHashMap<>(); + } @Override - public TreeVisitor getVisitor() { - return new JavaVisitor() { + public TreeVisitor getScanner(Map acc) { + return new TreeVisitor() { @Override - public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - cu.getMarkers().findFirst(JavaVersion.class) - .ifPresent(jv -> { - String projectName = cu.getMarkers().findFirst(JavaProject.class) - .map(JavaProject::getProjectName) - .orElse(""); - table.insertRow(ctx, new JavaVersionTable.Row( - projectName, - Integer.toString(jv.getMajorVersion()), - Integer.toString(jv.getMajorReleaseVersion()))); - }); - return cu; + public Tree preVisit(Tree tree, ExecutionContext ctx) { + if (tree instanceof JavaSourceFile) { + Markers markers = tree.getMarkers(); + markers.findFirst(JavaVersion.class).ifPresent(jv -> { + // Prefer the git origin as the dedup key: every module in a multi-module repo + // shares one GitProvenance, so they collapse to a single row. Fall back to the + // JavaProject UUID (one row per module) when no git provenance is available, + // and finally to the JavaVersion UUID so disconnected source files still + // produce one row per file rather than silently merging. + Optional gp = markers.findFirst(GitProvenance.class); + Object key; + if (gp.isPresent() && gp.get().getOrigin() != null) { + key = gp.get().getOrigin(); + } else { + key = markers.findFirst(JavaProject.class) + .map(JavaProject::getId) + .orElseGet(jv::getId); + } + + JavaVersionTable.Row candidate = new JavaVersionTable.Row( + Integer.toString(jv.getMajorVersion()), + Integer.toString(jv.getMajorReleaseVersion())); + acc.merge(key, candidate, FindJavaVersion::lower); + }); + } + return tree; } }; } + + @Override + public Collection generate(Map acc, ExecutionContext ctx) { + for (JavaVersionTable.Row row : acc.values()) { + table.insertRow(ctx, row); + } + return emptyList(); + } + + // Lower target compatibility wins; tiebreak on lower source compatibility. + // The retained row is the migration floor for the repository. + private static JavaVersionTable.Row lower(JavaVersionTable.Row a, JavaVersionTable.Row b) { + int aTarget = Integer.parseInt(a.getTargetVersion()); + int bTarget = Integer.parseInt(b.getTargetVersion()); + if (aTarget != bTarget) { + return aTarget < bTarget ? a : b; + } + int aSource = Integer.parseInt(a.getSourceVersion()); + int bSource = Integer.parseInt(b.getSourceVersion()); + return aSource <= bSource ? a : b; + } } diff --git a/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java b/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java index 79e5ae9707..5e94dc1ca2 100644 --- a/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java +++ b/src/main/java/org/openrewrite/java/migrate/table/JavaVersionTable.java @@ -28,10 +28,6 @@ public JavaVersionTable(Recipe recipe) { @Value public static class Row { - @Column(displayName = "Project name", - description = "The name of the project that produced the LST.") - String projectName; - @Column(displayName = "Source compatibility", description = "The major version of Java used to compile the source code") String sourceVersion; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java index 6133792510..8a70c32b95 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java @@ -20,6 +20,7 @@ import org.openrewrite.java.marker.JavaProject; import org.openrewrite.java.marker.JavaVersion; import org.openrewrite.java.migrate.table.JavaVersionTable; +import org.openrewrite.marker.GitProvenance; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -34,74 +35,141 @@ public void defaults(RecipeSpec spec) { spec.recipe(new FindJavaVersion()); } + private static GitProvenance gitProvenance(String origin) { + return new GitProvenance(randomId(), origin, "main", "abc123", null, null, null); + } + @DocumentExample @Test - void twoClassesInSameProjectLeadToTwoRows() { - var project = new JavaProject(randomId(), "demo", null); + void multiModuleRepositoryCollapsesToOneRow() { + // All modules in a multi-module repository share one GitProvenance, so the recipe + // emits a single row identifying the repository as a whole. + var git = gitProvenance("https://github.com/example/demo.git"); + var moduleA = new JavaProject(randomId(), "module-a", null); + var moduleB = new JavaProject(randomId(), "module-b", null); var jv = new JavaVersion(randomId(), "Sam", "Shelter", "17", "8"); rewriteRun( - spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> { - assertThat(rows).containsExactly( - new JavaVersionTable.Row("demo", "17", "8"), - new JavaVersionTable.Row("demo", "17", "8") - ); - }), + spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> + assertThat(rows).containsExactly( + new JavaVersionTable.Row("17", "8") + )), //language=java java( """ class A { } """, - spec -> spec.markers(project, jv)), + spec -> spec.markers(git, moduleA, jv)), //language=java java( """ class B { } """, - spec -> spec.markers(project, jv)) + spec -> spec.markers(git, moduleB, jv)) + ); + } + + @Test + void heterogeneousVersionsInOneRepoPickLowestTarget() { + // When modules in the same repo target different JDKs, the row reports the lowest + // target — the migration floor for the repository. + var git = gitProvenance("https://github.com/example/mixed.git"); + var legacy = new JavaProject(randomId(), "legacy-module", null); + var modern = new JavaProject(randomId(), "modern-module", null); + var java8 = new JavaVersion(randomId(), "Sam", "Shelter", "8", "8"); + var java17 = new JavaVersion(randomId(), "Sam", "Shelter", "17", "17"); + rewriteRun( + spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> + assertThat(rows).containsExactly( + new JavaVersionTable.Row("8", "8") + )), + //language=java + java( + """ + class Legacy { + } + """, + spec -> spec.markers(git, legacy, java8)), + //language=java + java( + """ + class Modern { + } + """, + spec -> spec.markers(git, modern, java17)) ); } @Test - void identicalJavaVersionMarkersAcrossProjectsAreEachReported() { - // Reproduces customer-requests#2409: across multiple projects with the same JDK, - // every project must appear as its own row in the data table. Previously a - // recipe-instance HashSet deduplicated by JavaVersion content, so identical - // markers in different projects were silently dropped. - var projectA = new JavaProject(randomId(), "project-a", null); - var projectB = new JavaProject(randomId(), "project-b", null); - var projectC = new JavaProject(randomId(), "project-c", null); + void identicalJavaVersionMarkersAcrossRepositoriesAreEachReported() { + // Reproduces customer-requests#2409: across multiple repositories with the same JDK, + // every repository must contribute its own row in the data table. Previously a + // recipe-instance HashSet deduplicated by JavaVersion content, so identical markers + // in different repositories were silently dropped. + var gitA = gitProvenance("https://github.com/example/repo-a.git"); + var gitB = gitProvenance("https://github.com/example/repo-b.git"); + var gitC = gitProvenance("https://github.com/example/repo-c.git"); + var project = new JavaProject(randomId(), "service", null); var jv = new JavaVersion(randomId(), "Sam", "Shelter", "17", "8"); rewriteRun( - spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> { - assertThat(rows).containsExactlyInAnyOrder( - new JavaVersionTable.Row("project-a", "17", "8"), - new JavaVersionTable.Row("project-b", "17", "8"), - new JavaVersionTable.Row("project-c", "17", "8") - ); - }), + spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> + assertThat(rows).containsExactly( + new JavaVersionTable.Row("17", "8"), + new JavaVersionTable.Row("17", "8"), + new JavaVersionTable.Row("17", "8") + )), //language=java java( """ class A { } """, - spec -> spec.markers(projectA, jv)), + spec -> spec.markers(gitA, project, jv)), //language=java java( """ class B { } """, - spec -> spec.markers(projectB, jv)), + spec -> spec.markers(gitB, project, jv)), //language=java java( """ class C { } """, - spec -> spec.markers(projectC, jv)) + spec -> spec.markers(gitC, project, jv)) + ); + } + + @Test + void withoutGitProvenanceFallsBackToPerProject() { + // When no GitProvenance is available (local non-git source trees, some test setups), + // the recipe falls back to one row per JavaProject so distinct modules are not silently merged. + var projectOne = new JavaProject(randomId(), "module-a", null); + var projectTwo = new JavaProject(randomId(), "module-b", null); + var jv = new JavaVersion(randomId(), "Sam", "Shelter", "17", "8"); + rewriteRun( + spec -> spec.dataTable(JavaVersionTable.Row.class, rows -> + assertThat(rows).containsExactly( + new JavaVersionTable.Row("17", "8"), + new JavaVersionTable.Row("17", "8") + )), + //language=java + java( + """ + class A { + } + """, + spec -> spec.markers(projectOne, jv)), + //language=java + java( + """ + class B { + } + """, + spec -> spec.markers(projectTwo, jv)) ); } } From 5add8f52f5a13dea93ca87990badf3e43afff889 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 May 2026 20:38:24 +0200 Subject: [PATCH 3/3] Tighten FindJavaVersion accumulator key handling Address PR review feedback: - Replace `Map` with `Map`. The three key namespaces (git origin URL, JavaProject UUID, JavaVersion UUID) now get explicit `origin:` / `project:` / `version:` prefixes so they can never collide with each other. - Collapse the if/else key-derivation into a single Optional chain. --- .../java/migrate/search/FindJavaVersion.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java index 5ea8de9e62..7a314f7249 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java @@ -31,11 +31,10 @@ import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Optional; import static java.util.Collections.emptyList; -public class FindJavaVersion extends ScanningRecipe> { +public class FindJavaVersion extends ScanningRecipe> { transient JavaVersionTable table = new JavaVersionTable(this); @@ -47,12 +46,12 @@ public class FindJavaVersion extends ScanningRecipe getInitialValue(ExecutionContext ctx) { + public Map getInitialValue(ExecutionContext ctx) { return new LinkedHashMap<>(); } @Override - public TreeVisitor getScanner(Map acc) { + public TreeVisitor getScanner(Map acc) { return new TreeVisitor() { @Override public Tree preVisit(Tree tree, ExecutionContext ctx) { @@ -64,15 +63,13 @@ public Tree preVisit(Tree tree, ExecutionContext ctx) { // JavaProject UUID (one row per module) when no git provenance is available, // and finally to the JavaVersion UUID so disconnected source files still // produce one row per file rather than silently merging. - Optional gp = markers.findFirst(GitProvenance.class); - Object key; - if (gp.isPresent() && gp.get().getOrigin() != null) { - key = gp.get().getOrigin(); - } else { - key = markers.findFirst(JavaProject.class) - .map(JavaProject::getId) - .orElseGet(jv::getId); - } + // Prefixes keep the three key namespaces from colliding. + String key = markers.findFirst(GitProvenance.class) + .map(GitProvenance::getOrigin) + .map(origin -> "origin:" + origin) + .orElseGet(() -> markers.findFirst(JavaProject.class) + .map(jp -> "project:" + jp.getId()) + .orElseGet(() -> "version:" + jv.getId())); JavaVersionTable.Row candidate = new JavaVersionTable.Row( Integer.toString(jv.getMajorVersion()), @@ -86,7 +83,7 @@ public Tree preVisit(Tree tree, ExecutionContext ctx) { } @Override - public Collection generate(Map acc, ExecutionContext ctx) { + public Collection generate(Map acc, ExecutionContext ctx) { for (JavaVersionTable.Row row : acc.values()) { table.insertRow(ctx, row); }