NetHttpTransport.trustCertificates ignores result of SslUtils.initSslContext

[Capstan]

https://github.com/googleapis/google-http-java-client/blob/main/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java#L317

In the monorepo, this fails to compile.

google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java:317: error: [CheckReturnValue] The result of `initSslContext(...)` must be used
      SslUtils.initSslContext(
                             ^
  If you really don't want to use the result, then assign it to a variable: `var unused = ...`.
  
  If callers of `initSslContext(...)` shouldn't be required to use its result, then annotate it with `@CanIgnoreReturnValue`.

This is yet another case where if error prone was used and enforced in the external repo, we would not have import issues.

Does this code simply rely on the side effects on the passed in sslContext? OTOH, why return an SSLContext that was passed in as a variable? Chaining? Does initSslContext guarantee that the returned context is always the passed in context?

[suztomo]

The Javadoc doesn't explain the return value. The implementation does return the instance sslContext passed as argument.

google-http-java-client/google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Line 130 in 17bbfc1

public static SSLContext initSslContext(

Annotating the initSslContext with @CanIgnoreReturnValue seems a good option to me. Would you be willing to create a pull request? Is initSslContext is the only place you got the error?

Note that this library must work with Java 7 as per README.md. -> error_prone_annotations artifact version 2.15.0 is compiled targeting Java 7 (pom.xml). Good.

Related: http://yaqs/1089314756940005376

[cpovirk]

This is yet another case where if error prone was used and enforced in the external repo, we would not have import issues.

I am all in favor of running Error Prone in more places :) But FYI, this particular check runs in a stricter mode in the monorepo than it does externally. So you wouldn't see this error externally even if you were running Error Prone there :\

[cpovirk]

(Also: You could consider disabling this check (or any other check) for your directory tree in the monorepo. Of course, the hope is that such checks are normally, well, useful. But I can see how the CheckReturnValue check in particular could lead to difficulties as we make it more strict in the monorepo. Still, I'm cautiously optimistic that it will be OK for you to continue to run there.)