refactor+build: OpenRewrite cleanups, Java 17 target, and JDK 25 build on Gradle 9 - #3288
refactor+build: OpenRewrite cleanups, Java 17 target, and JDK 25 build on Gradle 9#3288juherr wants to merge 21 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 422 files, which is 122 over the limit of 300. To get a review, narrow the scope: Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (422)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Apply org.openrewrite.staticanalysis.CommonStaticAnalysis across all modules: explicit-initialization removal, final on private fields, needed braces, final classes (private-ctor only), equals-avoids-null, unnecessary parentheses, lambda/method-reference simplifications, and related safe cleanups. Wildcard-import folding was disabled to keep the single-class import convention; final formatting re-applied via Autostyle (google-java-format). Recipe changes that produced invalid code were dropped (unnecessary-explicit- type-args on inference-sensitive data providers, finalize-calls-super without a throws clause).
Apply org.openrewrite.java.migrate.Java8toJava11 (source-only subset): - Paths.get(...) -> Path.of(...) - Files.newBufferedWriter(path, UTF_8) -> newBufferedWriter(path) (UTF-8 default) - redundant UTF-8 charset argument removal Build-configuration version bumps from the composite recipe were dropped to keep the Java 11 target and avoid touching build-logic. The try-with-resources rewrite was dropped where it silently removed a required IOException handler.
35a7e9e to
ab91c85
Compare
Bump the compiled bytecode target from Java 11 to Java 17. Define jdkBuildVersion and targetJavaVersion once in the root gradle.properties as the single source of truth. The main build reads them through the build-parameters plugin; build-logic-commons (a nested included build that does not inherit the root gradle.properties) now reads them from disk instead of hard-coding the versions.
Update the JDK requirements section to reflect the Java 17 bytecode target and add mise as a recommended JDK version manager alongside SDKMAN! and Adoptium.
Pin actions/checkout and peaceiris/actions-label-commenter to a full commit SHA with the version as a trailing comment, following GitHub's hardening guidance.
Replace the remaining hard-coded listOf(21, 11) with the jdkBuildVersion / targetJavaVersion read from the root gradle.properties, matching build-logic-commons and completing the single source of truth (target is now 17, not 11).
Pin every third-party and first-party action across all workflows to a full commit SHA with the version as a trailing comment, following GitHub's hardening guidance.
From 7.13.0 onward, structure entries per Keep a Changelog 1.1.0 (versioned heading + typed sections). Older releases are left untouched.
Set jdkBuildVersion=25 (the default build toolchain). Error Prone 2.20.0 crashes on JDK 25 (NoSuchFieldError: TypeTag.UNKNOWN), so upgrade error_prone_core to 2.50.0 and gradle-errorprone-plugin to 4.1.0 (the latter passes the now-required --should-stop=ifError=FLOW). Disable the SelfAssertion check, which only flags intentional trivial assertions in TestNG's sample/fixture test classes. Gradle 8.11 cannot run on JDK 25, so Gradle keeps running on JDK 21 while the JDK 25 toolchain compiles TestNG. Bytecode target stays Java 17.
Provision JDK 25 (Temurin, the distribution recommended in CONTRIBUTING) as the build toolchain in the publish and test workflows, keeping JDK 21 as the Gradle launcher (last in the list becomes JAVA_HOME). Refresh the stale JDK 21 / targetJavaVersion=11 comments.
Building now uses a JDK 25 toolchain while Gradle runs on JDK 21; the bytecode target stays Java 17.
Gradle 9 runs on JDK 25, so Gradle itself now runs on the JDK 25 build toolchain (no more JDK 21 launcher split). Migration changes: - Wrapper 8.11.1 -> 9.6.1. - build-logic/settings: include build-logic-commons via pluginManagement so its precompiled convention plugin resolves under Gradle 9. - reproducible-builds: dirMode/fileMode -> dirPermissions/filePermissions. - BuildIdentifier.isCurrentBuild() -> buildPath == ":" (removed in Gradle 9). - Manifest attributes: non-null values (attributes() now requires Pair<String, Any>). - vlsi gradle-extensions 1.90 -> 3.0.2 (Gradle.addBuildListener removed). - Shadow plugin com.github.johnrengelman 8.1.1 -> com.gradleup.shadow 9.6.1.
Now that Gradle 9 runs on JDK 25, the publish and test workflows provision a single JDK 25 (Temurin) that runs Gradle and builds TestNG, and CONTRIBUTING no longer mentions a separate JDK 21 launcher.
TestNG now targets Java 17, so its classes no longer run on Java 11; remove 11 from the tested JDK matrix and trim the now-verbose JDK-setup comments.
…che note Align the build-logic-commons jdkBuildVersion fallback with the default (25). Update the configuration-cache note: vlsi gradle-extensions 3.0.2 is now config-cache compatible, leaving com.github.autostyle as the only blocker (unmaintained, no upstream fix); link the Gradle build-listeners requirement.
Autostyle (last release 4.0.1, effectively unmaintained) is replaced by com.diffplug.spotless 8.8.0, keeping the same steps (import order, remove unused imports, trim trailing whitespace, end with newline, google-java-format). - Apply Spotless once at the root with apply false so all subprojects share a single build service (precompiled convention plugins load it per-project). - Use the JavaParser-based removeUnusedImports engine; the default google-java-format engine throws NoClassDefFoundError for Guava in this build's classloader. - Rename the skipAutostyle build parameter to skipSpotless. - Reformat 86 files to Spotless's newer google-java-format, and update the docs. Configuration cache stays disabled: Spotless's removeUnusedImports step is not yet config-cache compatible (the old Autostyle blocker, Gradle.buildFinished, is gone).
Java 26 is released, so add it to the supported test matrix; the early-access axis becomes 27 and 28 (both downloadable from jdk.java.net). Validated by running matrix.mjs locally.
false is Gradle's default (the configuration cache is opt-in), so the explicit setting and its comment only restated the convention.
…ending Gradle 9 runs on JDK 25, so the separate JDK 21 launcher is no longer needed; the setup-java list is the ascending test version + JDK 25 (kept last as JAVA_HOME).
The Tier 1 static-analysis recipe stripped the deprecated finalize() method from MyTestClassWithGlobalReferenceCounterSample. That method is the whole point of the fixture: it decrements the global instance counter when the object is garbage-collected, which is how MemoryLeakTestNg observes that instances are reclaimed. Without it the counter never returns to zero, so testMemoryLeak spins until the 10s timeout and fails on every JDK/OS in the matrix. Restore the method so the leak-reproduction fixture works again.
|
@vlsi Could you please review the Gradle changes and let me know if everything looks good? |
|
@krmahadevan Sorry for the large diff. No rush—take the time you need. |
|
@juherr - Are we bumping up the minimum requirement for running TestNG from JDK11 to JDK17 in this PR? If yes, does it mean that this will be a major release or something of that sort? Historically upgrading JDK has always been a problem for us with respect to our users. |
@juherr - If we could split this PR into different small ones each of which targets one aspect, it would be useful. The below commits if they were independent PRs, we could merge them directly because they are purely mechanical in nature. |
|
@krmahadevan @vlsi You're right. The Gradle changes became larger than expected. I'll split this into separate PRs:
|
What
OpenRewrite refactors (regenerated on top of the latest
master)refactor: apply OpenRewrite common static analysis cleanupsrefactor: modernize Java 8 idioms to Java 11 APIsJava target 17 + single version source
build: target Java 17 with a single shared version source— bytecode target 11 → 17;jdkBuildVersion/targetJavaVersiondefined once in the rootgradle.properties.build: read shared Java versions in the kotlin-dsl convention plugin.Build on JDK 25, via Gradle 9
build: build with JDK 25 by default and upgrade Error Prone for JDK 25—jdkBuildVersion21 → 25;error_prone_core2.20 → 2.50 andgradle-errorprone-plugin3.1.0 → 4.1.0 (EP 2.20 crashes on JDK 25); the newSelfAssertioncheck is disabled (it only flags intentional trivial assertions in sample/fixture tests).build: migrate to Gradle 9.6.1 (runs on JDK 25)— Gradle 9 runs on JDK 25 directly. Migration: wrapper 8.11.1 → 9.6.1;pluginManagementinclude of build-logic-commons;dirMode/fileMode→dirPermissions/filePermissions;BuildIdentifier.isCurrentBuild()→buildPath; non-null manifest attributes; vlsigradle-extensions1.90 → 3.0.2; Shadow pluginjohnrengelman:8.1.1→com.gradleup.shadow:9.6.1.Docs & CI
docs(contributing)— Java 17 target, JDK 25 build, recommendmise.ci: pin ... workflow actions to their commit SHA— every action pinned to a full SHA with a version comment.ci,docs: run Gradle on JDK 25— publish + test workflows provision a single JDK 25 (Temurin).docs: adopt Keep a Changelog format for CHANGES.txt— from 7.13.0 onward.Verification
clean build -x test(all modules; compile, jars, javadoc, Autostyle, Error Prone, the shaded-all.jar) is green on Gradle 9.6.1 running on JDK 25 — 177 tasks, 0 errors.gradle.properties.Notes
SelfAssertionwas disabled rather than rewriting fixture assertions; revisit if preferred.-x test); a full suite run before merge is recommended — CI exercises it.