I have some deeper concerns with the null checks. The fact that many
libraries we use (including guava) don't always annotate their methods
forces a lot of workarounds. As a very simple example, the return value
from Preconditions.checkNotNull clearly can never be null, yet the
nullability checks don't know this. This and other similar cases require
constantly adding extra unnecessary null checks in the code just to make
the checker happy. There have been other cases where I haven't been able to
figure out a way to make the checker happy (often these seem to involve
using lambdas), and after 10-15 minutes of investigation have given up and
disabled the check.
Now you might say that it's worth the extra pain and ugliness of writing
"useless" code to ensure that we have null-safe code. However I think this
ignores a sociological aspect of software development. I have a
higher tolerance than many for this sort of pain, and I'm willing to spend
some time figuring out how to rewrite my code such that it makes the
checker happy (even though often it forced me to write much more awkward
code). However even I have often found myself giving up and just disabling
the check. Many others will have less tolerance than me, and will simply
disable the checks. At that point we'll have a codebase littered with
@SuppressWarnings("nullness"), which doesn't really get us where we want to
be. I've seen similar struggles in other codebases, and generally having a
static checker with too many false positives often ends up being worse than
having no checker.
On Mon, Mar 15, 2021 at 10:37 AM Ismaël Mejía <ieme...@gmail.com> wrote:
> +1
>
> Even if I like the strictness for Null checking, I also think that
> this is adding too much extra time for builds (that I noticed locally
> when enabled) and also I agree with Jan that the annotations are
> really an undesired side effect. For reference when you try to auto
> complete some method signatures on IntelliJ on downstream projects
> with C-A-v it generates some extra Checkers annotations like @NonNull
> and others even if the user isn't using them which is not desirable.
>
>
>
> On Mon, Mar 15, 2021 at 6:04 PM Kyle Weaver <kcwea...@google.com> wrote:
> >>
> >> Big +1 for moving this to separate CI job. I really don't like what
> annotations are currently added to the code we ship. Tools like Idea add
> these annotations to code they generate when overriding classes and that's
> very annoying. Users should not be exposed to internal tools like
> nullability checking.
> >
> >
> > I was only planning on moving this to a separate CI job. The job would
> still be release blocking, so the same annotations would still be required.
> >
> > I'm not sure which annotations you are concerned about. There are two
> annotations involved with nullness checking, @SuppressWarnings and
> @Nullable. @SuppressWarnings has retention policy SOURCE, so it shouldn't
> be exposed to users at all. @Nullable is not just for internal tooling, it
> also provides useful information about our APIs to users. The user should
> not have to guess whether a method argument etc. can be null or not, and
> for better or worse, these annotations are the standard way of expressing
> that in Java.
>
