search

google / gson

A Java serialization/deserialization library to convert Java Objects into JSON and back
Apache License 2.0
23.13k stars 4.27k forks source link

[2.11.0] error_prone_annotations dependency #2676

Closed ctabin closed 1 month ago

ctabin commented 1 month ago

Hello,

We just saw the latest version 2.11.0 on Maven and updated to it. However, after a mvn dependency:tree this version now requires error_prone_annotations as dependency:

[INFO] +- com.google.code.gson:gson:jar:2.11.0:compile
[INFO] |  \- com.google.errorprone:error_prone_annotations:jar:2.27.0:compile

Is that really wanted or is it a build issue ? :thinking: ErrorProne is mainly used during compilation time and it seems strange to us that it would be used during runtime.

Thanks for the job !

Marcono1234 commented 1 month ago

Yes this is intended, and is mentioned now in the release notes (which were published after you created this issue):

Added dependency on com.google.errorprone:error_prone_annotations This dependency can be considered optional, but if missing it might lead to compiler warnings. Therefore Gson does not declare it as optional.

See https://github.com/google/gson/pull/2346 and https://github.com/google/gson/pull/2320#issuecomment-1455233938. These Error Prone annotations are not only useful for Gson, but also for other projects using Gson which have configured Error Prone as well (and some annotations might also be recognized by IDEs). For example @InlineMe can help to migrate from deprecated Gson methods.

Is this causing any problems for you, or were you just curious if this was done intentionally? You can try to add an exclusion for error_prone_annotations in your project configuration, but you might run into "Cannot find annotation method" warnings then when compiling against some of Gson's API. See the linked pull requests above.

ctabin commented 1 month ago

Hi @Marcono1234,

Thanks for the reply. It was mainly a question, nothing to be related to a bug, but the dependency seemed suspect at first glance.

So, thanks for the clarification :thumbsup:

chadlwilson commented 1 month ago

It seems to me that this is a little like https://github.com/google/guava/pull/6606 where an attempt was made (is being made?) to correct the way these dependencies work for Gradle projects to avoid pulling in largely unnecessary runtime dependencies - and at least workaround Maven's inability to express the nature of the dependency correctly.

eamonnmcmanus commented 1 month ago

Re https://github.com/google/guava/pull/6606, @cpovirk from our team is following that and might want to comment here. My inclination is not to lose too much sleep over this "unneeded" dependency, since as @Marcono1234 notes they are potentially useful to clients that also use Error Prone, and also the jar is only 19K.

cpovirk commented 1 month ago

Now that you mention it, if we want Error Prone users to benefit, we should probably add @CheckReturnValue to our packages :) Until we do, the only benefit (at the moment) would be from a couple @InlineMe annotations.

(Others have already covered my main thoughts on the dependency here and in https://github.com/google/gson/pull/2346#issuecomment-1475301875.)