Skip to content

Reduce code duplication#1

Merged
MukjepScarlet merged 4 commits into
MukjepScarlet:gson_defaultfrom
Marcono1234:gson-default
Nov 8, 2025
Merged

Reduce code duplication#1
MukjepScarlet merged 4 commits into
MukjepScarlet:gson_defaultfrom
Marcono1234:gson-default

Conversation

@Marcono1234

@Marcono1234 Marcono1234 commented Nov 7, 2025

Copy link
Copy Markdown

Resolves some of the issues with google#2864, mainly:

  • reduces code duplication; GsonBuilderHelper is not creating the list of factories anymore
  • fixes the test issues

See also my review comments below for additional explanations for the changes.
Feel free to adjust the changes here in case you don't want to directly integrate them as is into your PR.

Generally I really like your approach though!

stringToConstant = new HashMap<>();
constantToName = new EnumMap<>(classOfT);
// Don't use `EnumMap` here; that can break when using ProGuard or R8
constantToName = new HashMap<>();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have reverted this to a HashMap because the EnumMap was causing issues for the test-shrinker integration tests; the JDK was unable to create the EnumMap for the enum

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What issue does EnumMap have? I'm not sure because we have the classOfT here

@Marcono1234 Marcono1234 Nov 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run mvn clean verify --projects test-shrinker --also-make, it will fail with the following error when using EnumMap:

...
Caused by: java.lang.RuntimeException: Test failed: Write: Enum
        at com.example.ai.a(Unknown Source)
        at com.example.Main.runTests(Unknown Source)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        ... 39 more
Caused by: java.lang.NullPointerException: Cannot read the array length because "this.keyUniverse" is null
        at java.base/java.util.EnumMap.<init>(EnumMap.java:139)
        at com.a.a.b.a.i.<init>(Unknown Source)
        at com.a.a.b.a.i.<init>(Unknown Source)
...

I think the issue is that ProGuard and R8 shrink the enum class so much that the JDK cannot obtain the enum constants. This is also why EnumTypeAdapter uses class#getDeclaredFields() and isEnumConstant() instead of simply Class#getEnumConstants().

Comment thread gson/src/main/java/com/google/gson/GsonBuilder.java
Comment thread gson/src/main/java/com/google/gson/GsonBuilder.java Outdated
Comment thread gson/src/main/java/com/google/gson/GsonBuilderHelper.java Outdated
Comment thread gson/src/main/java/com/google/gson/BuilderHelper.java
Comment thread gson/src/main/java/com/google/gson/GsonBuilderHelper.java Outdated
Comment thread gson/src/main/java/com/google/gson/LongSerializationPolicy.java
@MukjepScarlet MukjepScarlet merged commit 82e935a into MukjepScarlet:gson_default Nov 8, 2025
10 of 11 checks passed
@Marcono1234 Marcono1234 deleted the gson-default branch November 8, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants