THRIFT-5919: Set JDK 17 as minimum Java version#6
Open
greggdonovan wants to merge 29 commits intomasterfrom
Open
THRIFT-5919: Set JDK 17 as minimum Java version#6greggdonovan wants to merge 29 commits intomasterfrom
greggdonovan wants to merge 29 commits intomasterfrom
Conversation
- Update lib/java/build.gradle to require JDK 17 minimum - Update lib/java/gradle/sourceConfiguration.gradle to target Java 17 - Update contrib/thrift-maven-plugin/pom.xml to use Java 17 - Update .github/workflows/build.yml cross-test to use JDK 17 - Update lib/kotlin/*.gradle.kts files to target JVM 17 - Add Jdk17Test.java to verify JDK 17 runtime features - Apply JDK 17 features: - Pattern matching for instanceof in TBaseHelper, TUnion - Switch expressions in TBinaryProtocol, TCompactProtocol, TJSONProtocol https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
Remove obsolete options: - java5: Java 1.5 compatibility no longer needed - android_legacy: Android 2.3 compatibility no longer needed - option_type: Always use java.util.Optional for optional fields Change defaults: - jakarta_annotations is now the default (use javax_annotations to opt out) - Optional fields are always wrapped in java.util.Optional Remove custom Option class: - Delete org.apache.thrift.Option (replaced by java.util.Optional) - Delete TestOptionType.java test - Delete JavaOptionTypeJdk8Test.thrift test resource Update build configuration: - Simplify Gradle test generation tasks - Remove option_type and jakarta_annotations from generator invocations Generated code now requires JDK 17 or later. https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
Maven artifact improvements: - Add maven.compiler.release=17 property to published POM so consumers know JDK 17 is required - Remove obsolete Java 9 compatibility check in javadoc configuration Remove legacy Android library: - Remove lib/java/android directory (unmaintained since 2015) - Android Gradle Plugin 1.5.0 and SDK 23 are incompatible with JDK 17 - Not tested in CI and not published to Maven - Update CMakeLists.txt to remove Android build support - Update Makefile.am to remove android from EXTRA_DIST - Clean up outdated Android comment in THttpClientResponseHandler https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
The Thrift-specific @nullable annotation is no longer needed since: - The code generator now uses @org.jspecify.annotations.Nullable - The library code uses JSpecify annotations - JSpecify is a standard dependency https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
- Add labels for easier PR categorization (dependencies, java, kotlin, github-actions) - Group related dependencies to reduce PR noise: - static-analysis: ErrorProne, NullAway, SpotBugs - testing: JUnit, Mockito - apache-http: HttpClient, HttpCore This ensures automated updates for ErrorProne, NullAway, JSpecify, and other dependencies added as part of the JDK 17 modernization. https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
Dependency upgrades: - SpotBugs: plugin 5.2.5→6.4.8, tool 4.5.3→4.9.8 - FindSecBugs: 1.12.0→1.14.0 - PMD: 6.0.0→7.9.0 (with updated ruleset for PMD 7) - SLF4J: 1.7.36→2.0.17 (switch to slf4j-simple for tests) - JUnit: 5.9.1→5.11.4 - Mockito: 5.3.0→5.15.2 - HttpClient: 5.2.1→5.4.1 - HttpCore: 5.2→5.3.1 - Jakarta Servlet: 5.0.0→6.1.0 - Jakarta Annotation: 2.1.1→3.0.0 - Tomcat Embed: 10.1.4→11.0.2 Dependabot enhancements: - Add jakarta group for Jakarta EE APIs - Add logging group for SLF4J dependencies - Add PMD and FindSecBugs to static-analysis group https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
- Add JSpecify, Spotless to static-analysis group - Rename apache-http to apache, add Commons Lang and Tomcat Embed https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
Reorder imports to follow Google Java Format conventions: - java.* imports first - org.apache.* imports next - org.jspecify.* imports last https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
…s-test-server - Fix potential NPE in TUnion.toString() by using String.valueOf(v) instead of v.toString() for @nullable getFieldValue() result - Use local variables after null checks so NullAway can track narrowing in toString(), StandardScheme.write(), and TupleScheme.write() - Add null check in getFieldValue(F) to satisfy NullAway since it delegates to @nullable getFieldValue() - Make deepCopyObject() accept/return @nullable to match @nullable value_ - Add JVM 17 target to Kotlin cross-test-server build to match cross-test-client and root module https://claude.ai/code/session_01EQ812qEBnt14Gq7gKCriiS
Update code formatting tooling to latest versions and apply reformatting to ensure CI passes spotlessCheck. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use assignment syntax (=) for property assignments - Add comment noting SpotBugs enum string coercion needs fixing for Gradle 10 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add test Thrift IDL and Java test for Optional-based fields * Fix sourceConfiguration.gradle to use file() for test resources dir * Fix generateTestThrift.gradle to always generate Optional-style code Co-Authored-By: Claude <noreply@anthropic.com>
- Java codegen: add ErrorProne/NullAway suppressions, use switch expressions for enums, remove JSpecify @nullable from positions incompatible with TYPE_USE annotations - Kotlin: modernize deps (kotlin-stdlib, kotlinx-coroutines-core) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Strip out ErrorProne/NullAway/JSpecify artifacts that came through from cherry-picks: - Remove jspecify @nullable imports from TApplicationException and TUnion - Restore original @SuppressWarnings in code generator (remove ErrorProne-specific entries) - Remove JSpecify comments from code generator - Clean ErrorProne/NullAway/JSpecify patterns from dependabot config - Restore org.apache.thrift.annotation.Nullable class (still used by code generator) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SpotBugs Gradle plugin 6.x requires typed enum values for effort and reportLevel properties, which don't resolve in applied script files. Keep plugin at 5.2.5 while still upgrading SpotBugs tool to 4.9.8 and FindSecBugs to 1.14.0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The code generator now always wraps optional fields in java.util.Optional, so all test files need .orElseThrow() to unwrap values: - TestDeepCopy: unwrap Optional getters for deep copy assertions - TestEnumContainers: unwrap Optional getters for enum container operations - TestPartialThriftDeserializer: unwrap Optional collection getters - ThriftStructProcessorTest: fix field lookup for new Optional struct - Remove TestOptionalsWithJdk8.java (replaced by TestOptionalsWithJavaOptional) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use Provider(String, String, String) instead of deprecated Provider(String, double, String) to avoid -Werror failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The code generator now wraps optional fields in java.util.Optional, causing ClassCastException when PartialThriftComparer tries to cast field values directly to List/Set/Map. Add unwrapOptional() to extract values before comparison and fix checkNullEquality to return NOT_EQUAL when one operand is null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Kotlin code generator emits `scope.future { }` blocks that require
`kotlinx.coroutines.future.future` from the jdk8 coroutines artifact.
Reverting the modernization from coroutines-jdk8 to coroutines-core
which removed this required API.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove ant dependency from configure.ac and CI (Java build is pure Gradle) - Replace manual Gradle download with gradle/actions/setup-gradle@v5 (uses wrapper version 9.3.1 automatically, handles caching) - Switch shadow plugin from abandoned com.github.johnrengelman.shadow 8.1.1 to com.gradleup.shadow 9.3.1 (new maintainer, requires Gradle 9+/JDK 17+) - Replace kotlinx-coroutines-jdk8 with kotlinx-coroutines-core (jdk8 module was merged into core in kotlinx.coroutines 1.7.0) - Bump kotlinx-coroutines version from 1.6.1 to 1.10.2 in gradle.properties Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove Jdk17Test.java (served its purpose verifying JDK 17 compiler) - Upgrade SLF4J 1.7.35 -> 2.0.17 (active maintained line) - Upgrade Logback 1.3.0-alpha14 -> 1.5.28 (stable release, SLF4J 2.x compatible) - Upgrade HttpCore 4.4.15 -> 4.4.16 (final 4.x patch) - Upgrade HttpClient 4.5.13 -> 4.5.14 (final 4.x patch) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f8276f6 to
a21ec12
Compare
…timization, SLF4J 2.0 - Add breaking changes and bugfix notes to CHANGES.md - Remove orphaned TUnion null check (was added for NullAway, which was removed) - Replace var with TFieldIdEnum in TUnion write methods - Optimize PartialThriftComparer: move unwrapOptional from hot dispatch path to struct field retrieval only (avoids instanceof check on every nested element in lists, maps, and sets) - Narrow SLF4J OSGi Import-Package range from [1.4,3) to [2.0,3) since SLF4J 1.x is EOL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore the option_type=jdk8 generator flag as opt-in (default false) so optional fields return raw types by default, preserving backward compatibility. Only jdk8 is supported; option_type=thrift now throws an error directing users to option_type=jdk8. - Restore use_option_type_ member and option_type flag parsing - Restore bool optional guard in generate_java_bean_boilerplate - Revert .orElseThrow() calls in 4 test files (compiled without option_type) - Move JavaOptionTypeOptionalTest.thrift to separate compilation task with java:option_type=jdk8 - Update CHANGES.md breaking changes accordingly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The workaround for GradleUp/shadow#651 is not compatible with com.gradleup.shadow 9.3.1, which does not create a shadowRuntimeElements configuration. This caused publishToMavenLocal to fail with "Variant for configuration 'shadowRuntimeElements' does not exist in component 'java'". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
|
@greptile |
Additional Comments (1)
This PR removed the |
- Use https://thrift.apache.org/ to match the published 0.22.0 POM - Replace fragile withXml appendNode with Gradle's native properties DSL to avoid risk of duplicate <properties> blocks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove @SuppressWarnings({"UnusedVariable"}) from TestPartialThriftDeserializer — was added for the since-reverted .orElseThrow() changes and is no longer needed - Revert TUnion toString/write cleanups (local variable extraction, String.valueOf) that were unrelated to JDK 17 modernization, keeping only the pattern matching instanceof changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d682cf9 to
69f3878
Compare
The thrift compiler does not generate package-info.java files, so the filter excluding them from secondary gen directories was dead code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore 6 @nullable annotation sites in t_java_generator.cc that were accidentally dropped during the JDK 17 modernization. The annotation class was preserved but the code generator stopped emitting @nullable on field declarations, getters, setters, setFieldValue/getFieldValue, and iterator accessors for nullable types. Move our Java changelog entries from 0.22.0 into a new 0.23.0 section, citing THRIFT-5919 per project conventions. THRIFT-5858 stays in 0.22.0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
THRIFT-5919: Set JDK 17 as minimum Java version
Set JDK 17 as the minimum Java version for the Thrift Java library - Java 8 reached public EOL in March 2022 and Java 11 in September 2023. JDK 17 is the current baseline LTS version.
What changed
Code generation (
t_java_generator.cc)android_legacy,java5generator flagsoption_type=thriftsupport (throws error directing users tooption_type=jdk8)option_type=jdk8remains opt-in (default behavior unchanged: raw types, null for unset optional fields)jakarta.annotation.Generated) are now the default; opt out withjavax_annotations@Nullableannotations from generated codeJava library
org.apache.thrift.Optionclass (replaced byjava.util.Optional)lib/java/android/(Android AAR build targeting SDK 23 / build tools 23.0.1)TBaseHelper,TUnion, protocol classes with pattern matching instanceof and switch expressionsPartialThriftComparernull comparison bug: when one object was null and the other non-null, returnedUNKNOWNinstead ofNOT_EQUALunwrapOptional()toPartialThriftComparerforOptional-wrapped field valuesBuild infrastructure
antdependency; Gradle is the sole Java build systemgradle/actions/setup-gradle@v5com.github.johnrengelman.shadow8.1.1 tocom.gradleup.shadow9.3.1shadowRuntimeElementsworkaround (incompatible with new Shadow plugin)tasks.register,@Inject ExecOperations)generateTestThrift.gradle: replace closure-basedthriftCompilewithThriftGeneratorTaskabstract classjakarta_annotationsfrom generator args in test thrift compilation (now default)Dependencies
[2.0,3))slf4j-log4j12withslf4j-simplefor test runtime (log4j 1.x is EOL)Kotlin
kotlinx-coroutines-jdk8tokotlinx-coroutines-core(jdk8 APIs merged into core since 1.7.0)Other
antfromconfigure.acJava detectioncontrib/thrift-maven-plugin/pom.xmlcompiler source/target from 1.8 to 17lib/java/android/fromMakefile.amlib/java/CMakeLists.txt(removeif(ANDROID)branch)Breaking Changes
See
CHANGES.mdfor full details. Summary:org.apache.thrift.Optiondeletedjava.util.Optionaloption_type=thriftremovedoption_type=jdk8insteadjavax_annotationsflag to opt outandroid_legacy,java5flags removed--gen java:argsPOM diff vs published 0.22.0
All differences are intentional (version bump, dependency upgrades, new
maven.compiler.releaseproperty):No structural changes — same
<groupId>,<artifactId>,<name>,<description>,<url>,<licenses>,<developers>,<scm>, and<dependencies>layout.Related JIRA Issues
Test plan
--gen java:javax_annotationsopt-out works--gen java:option_type=jdk8works (opt-in Optional wrapping)--gen java:option_type=thriftfails with clear error message