Fix outdated comment#2954
Merged
Merged
Conversation
MukjepScarlet
approved these changes
Nov 27, 2025
eamonnmcmanus
approved these changes
Dec 17, 2025
eamonnmcmanus
pushed a commit
to eamonnmcmanus/gson
that referenced
this pull request
Dec 19, 2025
eamonnmcmanus
added a commit
that referenced
this pull request
Dec 23, 2025
* Add type adapters for `java.time` classes. These adapters essentially freeze the JSON representation that `ReflectiveTypeAdapterFactory` established by default, based on the private fields of `java.time` classes. That's not a great representation, but it is understandable. Changing it to anything else would break compatibility with systems that are expecting the current format. If Gson had been updated with `java.time` adapters at the time the `java.time` API was added, the representation would surely have been something else, probably ISO standard formats. We can still supply non-default adapters for that, but we'll still need to have these legacy adapters by default. I've been meaning to make this change for a while, but the need to do so becomes more urgent with [this JDK commit](openjdk/jdk@cc05530) which makes a number of `java.time` fields `transient`. That means that the reflective-based adapter will no longer work with the classes that were changed. * Apply a couple of Error Prone suggestions. * Reformat a couple of source files. * Fix another EP warning. * Fix a particularly annoying EP warning/error. * Update the Android API level. I think it is time to do this, but obviously it should be a separate PR. * Substantial rework. * Move the new `TypeAdapter` implementations into a new class `JavaTimeTypeAdapters`. This allows us to omit that class from Android builds where it otherwise triggers warnings about SDK levels. * Ensure that every `java.time` class that needs a `TypeAdapter` has one. This doesn't include "subpackages" `java.time.*` like `java.time.chrono`, where it doesn't appear that there are any classes that are likely to be serialized to JSON. * Fix the handling of `ZoneId` and its subclasses. * refactor: slightly optimize ConstructorConstructor (#2950) * refactor: slightly optimize ConstructorConstructor * Update comments Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com> * spotless apply Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com> --------- Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com> * Fix outdated comment (#2954) * Compilation fixes. I copied the internal version that was running on Google's infrastructure, and some adjustments were needed. * Fix lint errors. * Fix a particularly annoying lint error. * Fix the fix. * Further fixes. This still doesn't pass with a local build, and I may need to debug an OSGi problem. * Revert to Android API 23, but with gummy-bears-api. * Annotate `JavaTimeTypeAdapters` to suppress animal-sniffer errors. * Update `OSGiManifestIT` not to depend on order. It appears that sometimes the clauses being checked for appear in the other order, possibly because of a `HashMap` or the like somewhere in the guts of OSGi. I haven't seen this on GitHub, but I do see it when running locally with Google's JDK, which has more hash randomization. * Fix use of assignment expression. * Remove the parser, which was overkill. It's enough just to swap the two clauses of a line when they are not in the expected order. Also, unrelatedly, update `protobuf-maven-plugin`. * Use a local `@IgnoreJRERequirement`. * Configure the local animal-sniffer annotation. * Suppress IdentifierName warning. * Update comments and undo an unnecessary split. * Restore Math.toIntExact. --------- Co-authored-by: 木葉 Scarlet <93977077+MukjepScarlet@users.noreply.github.com> Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.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.
Follow-up for #2950
The comment still used the old class name, before the class was renamed, see #2950 (comment).
(Sorry for the confusion the separate review comments on that previous PR might have caused.)