Class name `$Gson$Preconditions` breaks module system when shading

[overheadhunter]

Gson version

2.12.1

Java / Android version

JDK 21

Used tools

• Maven 3.9.9
• maven-shade-plugin 3.6.0

Description

In a downstream project we're trying to shade gson.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-shade-plugin</artifactId>
    <version>3.6.0</version>
    <configuration>
        <shadedArtifactAttached>false</shadedArtifactAttached>
        <artifactSet>
            <includes>
                <include>com.google.code.gson:gson</include>
            </includes>
        </artifactSet>
        <relocations>
            <relocation>
                <pattern>com.google.gson</pattern>
                <shadedPattern>com.nimbusds.jose.shaded.gson</shadedPattern>
            </relocation>
        </relocations>
        <filters>
            <filter>
                <artifact>com.google.code.gson:gson</artifact>
                <excludes>
                    <exclude>**/module-info.class</exclude>
                </excludes>
            </filter>
        </filters>
    </configuration>
    <executions>
        <execution>
            <phase>package</phase>
            <goals>
                <goal>shade</goal>
            </goals>
        </execution>
    </executions>
</plugin>

The class in question is correctly moved to com.nimbusds.jose.shaded.gson.internal.$Gson$Preconditions. However classes importing this are now broken and incorrectly do this:

import com.nimbusds.jose.shaded.gson.internal..Gson.Preconditions;

This leads to all kinds of follow-up bugs when trying to load these classes from the module path. Loading from class path is fine for some reason (I suspect the com.nimbusds.jose.shaded.gson.internal.. can't be matched by a corresponding exports statement in the module-info)

Expected behavior

relocated classes have a proper name

Actual behavior

the naming scheme breaks the shade plugin.

Reproduction steps

1. Build https://bitbucket.org/connect2id/nimbus-jose-jwt/src/10.1/ (mvn package -DskipTests)
2. Unpack resulting jar
3. Inspect com.nimbusds.jose.shaded.gson.GsonBuilder

Exception stack trace

N/A

Clarification / Justification

I'm posting this in support of #1744. I am aware that you could argue this is an issue with the shade plugin. However, since other users reported this as a JLS violation I believe it is up to GSON to fix it.

According to 4efb133 the whole point of this naming scheme is to affect behaviour of some "IDE's auto import functionality". All modern IDEs have been respecting the module-info.java for several years now, so this rationale is obsolete, since com.nimbusds.jose.shaded.gson.internal is not exported.

[eamonnmcmanus]

You're right, I am going to argue that this is an issue with the shade plugin. :-)

Seriously, though, these class names are bizarre. When $Gson$Preconditions was called just Preconditions you could imagine that it might show up in autocomplete alongside more standard Preconditions classes. If it's called GsonPreconditions then even if something is not using modules, it should be clear to users that it's not what they want.

[overheadhunter]

If it's called GsonPreconditions then even if something is not using modules, it should be clear to users that it's not what they want.

I agree, then what's holding you back from merging #1745?

[eamonnmcmanus]

If it's called GsonPreconditions then even if something is not using modules, it should be clear to users that it's not what they want.

I agree, then what's holding you back from merging #1745?

I expect that bringing a 5-year-old PR up to date is more work than just doing the renaming from scratch. Which I am proposing in #2838.

FYI, we can probably chalk this up to cultural differences, but your question came across as rather rude.

[overheadhunter]

FYI, we can probably chalk this up to cultural differences, but your question came across as rather rude.

I try to understand why. Could you explain, so I can avoid this next time?

My intention was to understand why it has not been dealt with. If there is a compelling reason not to change the name and if there is a chance for an updated PR to be merged or if I should rather seek a different solution.

[Marcono1234]

My intention was to understand why it has not been dealt with. If there is a compelling reason not to change the name and if there is a chance for an updated PR to be merged or if I should rather seek a different solution.

I assume it was mostly due to inactivity of the Gson project back then, and probably because I did not follow up on the PR afterwards. And now the PR probably also had large conflicts because we had added automatic code formatting in the meantime.

---

As side note: Even the $ did not prevent external users from (accidentally?) importing the class, see #2180 (comment).