refactor: slightly optimize ConstructorConstructor #2950
Merged
+66
−48
Conversation
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
Contributor
Author
|
@eamonnmcmanus Please review this. Because I'm not Android developer, I'm not sure if these are benificial with Proguard/R8. |
Marcono1234
reviewed
Nov 25, 2025
Contributor
Marcono1234
left a comment
Marcono1234
left a comment
There was a problem hiding this comment.
Thanks a lot!
The idea to add an ExceptionObjectConstructor is really good to reduce the code duplication. I have just a few small suggestions.
The project members might have additional change requests though.
gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
Outdated
Show resolved
Hide resolved
gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
Outdated
Show resolved
Hide resolved
gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
eamonnmcmanus
approved these changes
Nov 26, 2025
Member
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
There was a problem hiding this comment.
Thanks for this! I checked that it passes all of Google's internal tests.
Merged
eamonnmcmanus
pushed a commit
to eamonnmcmanus/gson
that referenced
this pull request
Dec 19, 2025
* 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>
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Purpose
Part of #2864
Description
lambda -> method reference (reduce synthetic methods)
duplicated lambdas -> a class
Checklist
This is automatically checked by
mvn verify, but can also be checked on its own usingmvn spotless:check.Style violations can be fixed using
mvn spotless:apply; this can be done in a separate commit to verify that it did not cause undesired changes.null@since $next-version$(
$next-version$is a special placeholder which is automatically replaced during release)TestCase)mvn clean verify javadoc:jarpasses without errors