Add type adapters for `java.time` classes #2948

eamonnmcmanus · 2025-11-25T00:30:06Z

Add type adapters for java.time classes.

These adapters essentially freeze the JSON representation thatReflectiveTypeAdapterFactory 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 which makes a number of java.time fields transient. That means that, in JDK 26, the reflective-based adapter will no longer work with the classes that were changed.

The ZonedDateTime adapter does not work properly at the moment. More work is needed on that.

This makes progress on addressing #1059 and #739. They are not completely addressed yet because not all java.time types are covered.

(Google internal bug reference: b/463237567.)

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.

I think it is time to do this, but obviously it should be a separate PR.

cpovirk · 2025-11-25T17:03:28Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+        }
+
+        @Override
+        @SuppressWarnings("JavaDurationGetSecondsGetNano")


Huh, I would have hoped that the getSeconds and getNano calls below were close enough to avoid this warning :(

cpovirk · 2025-11-25T17:11:23Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+
+    @Override
+    public T read(JsonReader in) throws IOException {
+      if (in.peek() == JsonToken.NULL) {


Not any sort of blocker for this PR, but: Has there been discussion of a common TypeAdapter subclass (even if only a private one for our own use) that passes through nulls, leaving subclasses free to implement readNonNull and writeNonNull methods without the null-handling ceremony? Maybe the common class could even call beginObject and endObject (if it's not dangerous that that delays the call to endObject until after the user subclass has done its thing)? (I mean, it's always going to be possible to go further down this road, like with automatic exception handling and even a callback-style API that invokes your callback with a name and JsonReader inside the while (!end) loop..... And if only we could assume that records are available, we could declare a record class for each as a sort of serialization proxy, maybe? Anyway, I know we're not in the mode of adding features, so I'm just acting in my role as a reviewer who sees the same pattern a few times in this file :))

cpovirk · 2025-11-25T17:31:42Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+        String name = in.nextName();
+        int index = fields.indexOf(name);
+        if (index >= 0) {
+          values[index] = in.nextLong();


In general, if we get an unrecognized field name, we're going to blow up, just probably when we request the next name, right (since we'll have a value left over, rather than a name)? I don't think it would be worth going far out of our way to fail moments earlier; I just didn't pick up on the behavior at first read.

update: And now I see a "Ignore unknown JSON property" comment in the original code below. I think that's what it was doing (well, at least for integral-valued properties) but not what it's doing anymore. We could preserve the old behavior easily enough, but I'm not sure if we care to? Plus, GitHub is matching that comment up with your new "Ignore other fields" ZDT comment, and my guess is that we don't actually ignore other fields in that implementation.

cpovirk · 2025-11-25T17:41:24Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+        }
+      };
+
+  // TODO: this does not currently work correctly.


I can't say I've figured out why, but I'm flagging this in case you still need to fix anything here or you want to leave it private (not that it "should" matter in internal code).

cpovirk · 2025-11-25T17:48:04Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+        @Override
+        public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
+          Class<? super T> rawType = typeToken.getRawType();
+          if (!rawType.getName().startsWith("java.time.")) {


If you want this to work with Android library desugaring, you might need to recognize j$.time—or maybe even obfuscated versions, which would be even more of a pain? (I recently ran into that in unsubmitted guava-testlib CL 827531714.)

Maybe we don't care unless the desugared versions of java.time start using transient internally or something? I bring this all up mostly as something to watch out for. I could try to think more about it if you'd like. (Maybe you could also get some information from testing this internally if you haven't already. My guess is that it would work fine for at least a while, especially with desugaring enabled in our internal builds with minSdkVersion 23.)

cpovirk · 2025-11-25T17:51:38Z

pom.xml

                  <groupId>net.sf.androidscents.signature</groupId>
-                  <artifactId>android-api-level-21</artifactId>
-                  <version>5.0.1_r2</version>
+                  <artifactId>android-api-level-26</artifactId>


I saw that your commit description mentions that you were thinking about putting this in a separate PR.

Alternatively, you can introduce your own annotation for Animal Sniffer suppressions (likely copying the standard name, "IgnoreJRERequirement"), configure your build to respect it, and then suppress at the narrow scope so that we continue to test that we're compatible with 21 (or 23, since that's the typical Google default nowadays) except in code that we're careful to run only under newer versions.

Marcono1234 · 2025-11-27T12:06:35Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+        }
+      }
+      in.endObject();
+      return create(values);


For better troubleshooting, would it make sense to wrap the create call in a try-catch(RuntimeException) and wrap the exceptions in a JsonSyntaxException which includes in.getPreviousPath() (and includes the original exception as cause)?

Marcono1234 · 2025-11-27T12:10:21Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+          return requireNonNull(localTime, "Missing time field")
+              .atDate(requireNonNull(localDate, "Missing date field"));


Here and below for ZONED_DATE_TIME, would it make sense to have a custom requireNonNull method like this?

private static <T> T requireNonNullField(T obj, String fieldName, JsonReader jsonReader) { if (obj == null) { throw new JsonSyntaxException("Missing " + fieldName + " field; at path " + jsonReader.getPreviousPath()); } return obj; }

Could also use it for the JsonSyntaxException thrown by ZONE_REGION then.

Marcono1234 · 2025-11-27T12:15:17Z

gson/src/test/java/com/google/gson/functional/DefaultTypeAdaptersTest.java

+      Instant.class.getDeclaredField("seconds").setAccessible(true);
+      accessible = true;


Can this ever succeed with the current Maven build config?

I think it could only succeed for the JDK 11 CI builds, but even there it won't work because we have --illegal-access=deny in gson/pom.xml.

So maybe we need a custom maven-surefire-plugin execution which just runs DefaultTypeAdaptersTest and has --add-opens.

(Side note: Would it make sense to move these java.time tests to a dedicated test class?)

Marcono1234 · 2025-11-27T12:23:18Z

gson/src/test/java/com/google/gson/functional/DefaultTypeAdaptersTest.java

+        "{\"dateTime\":{\"date\":{\"year\":2021,\"month\":12,\"day\":2},"
+            + "\"time\":{\"hour\":12,\"minute\":34,\"second\":56,\"nano\":789012345}},"
+            + "\"offset\":{\"totalSeconds\":0},"
+            + "\"zone\":{\"id\":\"Z\"}}";


Not sure if that is the reason why you consider this adapter to not be working correctly, but it seems for the reflection roundtrip this fails due to this diff:

Suggested change

+ "\"zone\":{\"id\":\"Z\"}}";

+ "\"zone\":{"totalSeconds":0}}";

It seems the reason is that because ZoneOffset.UTC is a ZoneOffset, it is used as both offset and zone, see JDK code.

But with the above change the non-reflection-based adapter fails.

So maybe you need a dedicated ZoneId adapter which delegates to the ZoneOffset / ZoneRegion:

for serialization check zone instanceof ZoneOffset

for deserialization check which fields are present, and decide based on that as which type to deserialize

What do you think?

Marcono1234 · 2025-11-27T13:42:02Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+   * <p>This is a base class for {@link Calendar}, {@link Duration}, {@link Instant}, {@link
+   * LocalDate}, {@link LocalTime}, {@link LocalDateTime}, and {@link ZoneOffset}.


Not sure if it is really worth it to list all these classes here; it risks that this becomes outdated in case one of the adapters is changed, and users can get this information also by looking at the subclasses of this adapter class.

Marcono1234 · 2025-11-27T13:45:46Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

-              case HOUR_OF_DAY:
-                hourOfDay = value;
+              default:
+                // Ignore other fields.


Similar to #2948 (comment), this is missing a in.skipValue() call for unknown fields?

Marcono1234 · 2025-11-27T13:47:28Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

+                throw new JsonSyntaxException(
+                    "Failed parsing '" + id + "' as ZoneId; at path " + in.getPreviousPath(), e);
+              }
+            }


Similar to #2948 (comment), this is missing a in.skipValue() call for unknown fields?

Marcono1234 · 2025-11-27T13:48:31Z

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

                break;
              default:
-                // Ignore unknown JSON property
+                // Ignore other fields.


Similar to #2948 (comment), this is missing a in.skipValue() call for unknown fields?

eamonnmcmanus added 6 commits November 24, 2025 16:18

Apply a couple of Error Prone suggestions.

28393e1

Reformat a couple of source files.

9b12b2e

Fix another EP warning.

3a38f77

Fix a particularly annoying EP warning/error.

9b48171

Update the Android API level.

f20ca75

I think it is time to do this, but obviously it should be a separate PR.

eamonnmcmanus requested a review from cpovirk November 25, 2025 01:54

cpovirk reviewed Nov 25, 2025

View reviewed changes

cpovirk mentioned this pull request Nov 25, 2025

Change the minimum supported Android API to 26. #2949

Open

Marcono1234 reviewed Nov 27, 2025

View reviewed changes

		return requireNonNull(localTime, "Missing time field")
		.atDate(requireNonNull(localDate, "Missing date field"));

		Instant.class.getDeclaredField("seconds").setAccessible(true);
		accessible = true;

	+ "\"zone\":{\"id\":\"Z\"}}";
	+ "\"zone\":{"totalSeconds":0}}";

		* <p>This is a base class for {@link Calendar}, {@link Duration}, {@link Instant}, {@link
		* LocalDate}, {@link LocalTime}, {@link LocalDateTime}, and {@link ZoneOffset}.

Add type adapters for java.time classes #2948

Are you sure you want to change the base?

Add type adapters for java.time classes #2948

Conversation

eamonnmcmanus commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add type adapters for `java.time` classes #2948

Add type adapters for `java.time` classes #2948

eamonnmcmanus commented Nov 25, 2025 •

edited

Loading