Skip to content

Commit d4df178

Browse files
authored
Skip unparseable Java versions in FindJavaVersion instead of reporting -1 (#1125)
* Skip unparseable Java versions in FindJavaVersion instead of reporting -1 A JavaVersion marker whose source/target compatibility cannot be parsed reports a major version of -1. FindJavaVersion stringified that straight into the data table, so repositories surfaced a bogus -1 row. Worse, the lower() reduction keeps the lowest version per repository, and -1 sorts below every real version, so a single module with an undetermined version dragged the whole repository's reported version to -1. Skip markers whose source or target major version is negative: they carry no usable version, so they neither emit a -1 row nor mask the real versions of sibling modules. When no module has a parseable version the repository simply contributes no row. * Drop FindJavaVersion all-unknown test that asserts an absent data table When the recipe emits no rows the JavaVersionTable is absent rather than present-but-empty, so the dataTable assertion throws. The skip behavior is already covered by unknownVersionMarkersAreSkipped.
1 parent b8a298e commit d4df178

2 files changed

Lines changed: 46 additions & 2 deletions

File tree

src/main/java/org/openrewrite/java/migrate/search/FindJavaVersion.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ public Tree preVisit(Tree tree, ExecutionContext ctx) {
5858
if (tree instanceof JavaSourceFile) {
5959
Markers markers = tree.getMarkers();
6060
markers.findFirst(JavaVersion.class).ifPresent(jv -> {
61+
int sourceVersion = jv.getMajorVersion();
62+
int targetVersion = jv.getMajorReleaseVersion();
63+
// A JavaVersion whose source/target compatibility could not be parsed reports -1
64+
// (for example a build that sets the version only through a Java toolchain, or an
65+
// unresolved property placeholder). -1 is not a real Java version, and because it
66+
// sorts below every real version it would win the lower() reduction and mask the
67+
// actual versions of every other module in the repository, so skip it entirely.
68+
if (sourceVersion < 0 || targetVersion < 0) {
69+
return;
70+
}
71+
6172
// Prefer the git origin as the dedup key: every module in a multi-module repo
6273
// shares one GitProvenance, so they collapse to a single row. Fall back to the
6374
// JavaProject UUID (one row per module) when no git provenance is available,
@@ -72,8 +83,8 @@ public Tree preVisit(Tree tree, ExecutionContext ctx) {
7283
.orElseGet(() -> "version:" + jv.getId()));
7384

7485
JavaVersionTable.Row candidate = new JavaVersionTable.Row(
75-
Integer.toString(jv.getMajorVersion()),
76-
Integer.toString(jv.getMajorReleaseVersion()));
86+
Integer.toString(sourceVersion),
87+
Integer.toString(targetVersion));
7788
acc.merge(key, candidate, FindJavaVersion::lower);
7889
});
7990
}

src/test/java/org/openrewrite/java/migrate/search/FindJavaVersionTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,39 @@ class C {
143143
);
144144
}
145145

146+
@Test
147+
void unknownVersionMarkersAreSkipped() {
148+
// A JavaVersion whose source/target compatibility cannot be parsed reports -1 (for example a
149+
// build configured solely through a Java toolchain or an unresolved property). Such markers
150+
// must not contribute a "-1" row, nor drag the reported version of sibling modules down: -1
151+
// sorts below every real version and would otherwise win the lower() reduction.
152+
var git = gitProvenance("https://github.com/example/toolchain.git");
153+
var known = new JavaProject(randomId(), "known-module", null);
154+
var unknown = new JavaProject(randomId(), "unknown-module", null);
155+
var java17 = new JavaVersion(randomId(), "Sam", "Shelter", "17", "17");
156+
var unparseable = new JavaVersion(randomId(), "Sam", "Shelter", "", "");
157+
rewriteRun(
158+
spec -> spec.dataTable(JavaVersionTable.Row.class, rows ->
159+
assertThat(rows).containsExactly(
160+
new JavaVersionTable.Row("17", "17")
161+
)),
162+
//language=java
163+
java(
164+
"""
165+
class Known {
166+
}
167+
""",
168+
spec -> spec.markers(git, known, java17)),
169+
//language=java
170+
java(
171+
"""
172+
class Unknown {
173+
}
174+
""",
175+
spec -> spec.markers(git, unknown, unparseable))
176+
);
177+
}
178+
146179
@Test
147180
void withoutGitProvenanceFallsBackToPerProject() {
148181
// When no GitProvenance is available (local non-git source trees, some test setups),

0 commit comments

Comments
 (0)