Build: Fix minor error-prone warnings

[ajantha-bhat]

I have observed that the build [./gradlew clean build -x test] has some warnings.
So it is an effort to keep the build green.

Before:

After:

[ajantha-bhat]

cc: @nastra, @Fokko

[nastra]

LGTM, thanks for fixing @ajantha-bhat

[nastra]

nit: should we extract this to a private static final Splitter DOT_SPLITTER?

[ajantha-bhat]

As it is not used in multiple places (in the same file), I thought no need for an extra variable.

[XN137]

should we raise the severity of the StringSplitter check to ERROR like here ?
this prevents similar problems from being introduced again

[ajantha-bhat]

should we raise the severity of the StringSplitter check to ERROR like here ?
this prevents similar problems from being introduced again

We could add this. But I am not sure on what basis we add here because there are 400 plus bug patterns in errorProne. Even if we prevent this error, there can be some other error-prone warnings that can still prevent the build from looking green.

[XN137]

if these warnings are worth fixing, shouldnt they also be worth preventing?
automatic prevention is usually cheaper than manually observing and fixing future warnings.

this also has the added benefit that we are sure this PR addressed all these types of violations, not just the ones you happened to see.

i dont see how the existence of other errorprone checks, that are not being enforced, matters in this discussion.
ideally all of the checks we agree are worth fixing should be enforced... the problem is identifying those and addressing all their current violations.

[Fokko]

Great work @ajantha-bhat. I also did quite some of these fixes because they clutter up your output when building locally and in the CI.

I agree with @XN137 that we should find some way to get rid of the warnings by either turning them into errors or if they aren't important, just suppressing them.

[XN137]

or if they aren't important, just suppressing them.

its probably what you mean, but instead of suppressing them (=adding annotations), if we dont agree with a check, it should be turned OFF instead, so it does not add to the compile time and does not add future warnings

[ajantha-bhat]

@Fokko, @XN137, @nastra: Thanks for the review.
I have enforced StringSplitter cases to throw the error as discussed above and the builds are passing.

[Fokko]

@XN137 I agree with you, but there are some tricky cases where you want to use keep the check in place, but just suppress them at certain places. I think this should be checked on a per-rule basis.