Yes, the idea is to simplify things by using only one of @Nullable
and @Nonnull in Polaris code.
However, I lean toward using @Nullable when necessary and assume @Nonnull by
default. Again this is only for Polaris code.
Cheers,
Dmitri.
On Tue, May 27, 2025 at 5:07 PM Eric Maynard <eric.w.mayn...@gmail.com>
wrote:
> If we've checked something is non-null or received a non-null value from a
> library, we can continue annotating it as @Nonnull within Polaris. We can
> also use Optional to get similar functionality to typescript's ? and handle
> missing values that way.
>
> In all other cases, developers must assume any given reference can be null
> and do their own checks. It seems to me that there is no need for @Nullable
> in that case. That is, if we label everything that cannot be null as
> @Nonnull and everything that can be null as @Nullable, then every reference
> should be annotated as one or the other. We can simplify things by just
> removing the @Nullable annotations.
>
> --EM
>
> On Tue, May 27, 2025 at 1:56 PM Dmitri Bourlatchkov <di...@apache.org>
> wrote:
>
> > Your example is quite valid, Mike. In my proposal (several emails ago)
> I'd
> > classify this as "inputs into Polaris code".
> >
> > However, I guess the topic of this thread is slightly different. The
> > question we're trying to reach consensus on, is about what we should
> > annotate as @Nullable in Polaris code.
> >
> > I'd say if a value is received from external code, which is not
> annotated,
> > and returned from a Polaris method without validation, and the returned
> > value can reasonably be null, then the Polaris method should be annotated
> > as @Nullable. However, if Polaris code checks the value to be non-null,
> the
> > return value need not be annotated in Polaris.
> >
> > Does this sound reasonable?
> >
> > Thanks,
> > Dmitri.
> >
> > On Tue, May 27, 2025 at 4:34 PM Michael Collado <collado.m...@gmail.com>
> > wrote:
> >
> > > I generally assume the other way around. Many (most) libraries don't
> > > annotate the return values of their methods, so I assume everything is
> > > nullable unless specifically told otherwise. I would prefer everything
> be
> > > non-nullable unless specifically stated (the ? is the one thing I would
> > > steal from typescript if I could), but given that library code can't be
> > > modified to be explicit when null return values are possible, I don't
> > think
> > > we can make that assumption.
> > >
> > > Mike
> > >
> > > On Thu, May 22, 2025 at 10:57 AM Dmitri Bourlatchkov <di...@apache.org
> >
> > > wrote:
> > >
> > > > My preference is for option 1 below.
> > > >
> > > > On Thu, May 22, 2025 at 1:53 PM Alex Dutra
> > <alex.du...@dremio.com.invalid
> > > >
> > > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > My proposal was centered around compile-time checks and targets
> > mostly
> > > > > developers and contributors. I am not questioning the usefulness of
> > > > > runtime checks when these make sense.
> > > > >
> > > > > Maybe an example is better than a thousand words. Let's imagine a
> > > > > simple getOrDefault() method. Which version do you prefer?
> > > > >
> > > > > 1. Annotate only nullable items:
> > > > > public String getOrDefault(@Nullable String s, String def) {
> return s
> > > > > == null ? def : Objects.requireNonNull(def); }
> > > > >
> > > > > 2. Annotate only non-null items:
> > > > > @Nonnull public String getOrDefault(String s, @Nonnull String def)
> {
> > > > > return s == null ? def : Objects.requireNonNull(def); }
> > > > >
> > > > > 3. Annotate everything:
> > > > > @Nonnull public String getOrDefault(@Nullable String s, @Nonnull
> > > > > String def) { return s == null ? def :
> Objects.requireNonNull(def); }
> > > > >
> > > > > Many places in Polaris are using option 3, which is too verbose and
> > > > > leads to visual fatigue. What I was suggesting to the community is
> to
> > > > > adopt option 1, that reduces the visual clutter and also assumes
> > > > > non-null by default.
> > > > >
> > > > > (You will notice that I added a runtime check to all three
> versions.)
> > > > >
> > > > > Hope that helps to clarify the discussion.
> > > > >
> > > > > Alex
> > > > >
> > > > > On Wed, May 21, 2025 at 8:54 PM Yufei Gu <flyrain...@gmail.com>
> > wrote:
> > > > > >
> > > > > > >
> > > > > > > In my opinion, assuming everything is nullable by default isn't
> > the
> > > > > > > best approach for writing robust code. I believe a bias towards
> > > > > > > non-nullness leads to more reliable systems.
> > > > > >
> > > > > > I agreed with the intention, but am concerned that assuming
> > > everything
> > > > is
> > > > > > non-nullness may discourage null-checking, which is problematic
> as
> > > > > runtime
> > > > > > null-checking isn't a thing.
> > > > > >
> > > > > > Yufei
> > > > > >
> > > > > >
> > > > > > On Wed, May 21, 2025 at 9:52 AM Alex Dutra
> > > > <alex.du...@dremio.com.invalid
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Eric,
> > > > > > >
> > > > > > > You're right that annotations don't change the bytecode at
> > runtime.
> > > > > > > Their real value comes during compilation, as many static
> > analysis
> > > > > > > tools use them to flag potential issues. They can even cause
> > build
> > > > > > > failures depending on how you configure them. My IDE (IntelliJ)
> > > > > > > frequently warns me when I forget to handle a potential NPE; if
> > > > you're
> > > > > > > not seeing similar feedback, it might be worth checking your
> IDE
> > > > > > > settings.
> > > > > > >
> > > > > > > While the annotations are primarily for compile-time checks,
> that
> > > > > > > doesn't mean we can't also incorporate runtime checks. We
> should
> > > aim
> > > > > > > to include these whenever this makes sense, for example by
> using
> > > > > > > Guava's Preconditions. This is especially useful if we can't
> > > > guarantee
> > > > > > > that a method parameter, for instance, will never be null,
> > because
> > > > > > > it's being provided by some external system.
> > > > > > >
> > > > > > > In my opinion, assuming everything is nullable by default isn't
> > the
> > > > > > > best approach for writing robust code. I believe a bias towards
> > > > > > > non-nullness leads to more reliable systems.
> > > > > > >
> > > > > > > I am also a big fan of Optional and think we should strive to
> use
> > > it
> > > > > > > as much as possible. That said, it's not always possible,
> > > especially
> > > > > > > if you are implementing a third-party interface that doesn't
> use
> > > it.
> > > > > > > Using Optional in class fields and method parameters is also
> > > > > > > controversial: Optional was designed primarily as a signal from
> > the
> > > > > > > callee to the caller, to signify: "no result". In other words,
> > its
> > > > > > > main purpose is to clarify method return types. This post on
> > Stack
> > > > > > > Overflow by Brian Goetz is worth reading: [1].
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Alex
> > > > > > >
> > > > > > > [1]:
> > > > > > >
> > > > >
> > > >
> > >
> >
> https://stackoverflow.com/questions/26327957/should-java-8-getters-return-optional-type/26328555#26328555
> > > > > > >
> > > > > > >
> > > > > > > On Wed, May 21, 2025 at 4:37 PM Eric Maynard <
> > > > eric.w.mayn...@gmail.com
> > > > > >
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Thanks for bringing this up as I’ve been confused by this a
> few
> > > > > times.
> > > > > > > >
> > > > > > > > Before Polaris I hadn’t really encountered these annotations
> > and
> > > I
> > > > > was
> > > > > > > > surprised to learn they don’t “do anything” — that is, there
> is
> > > no
> > > > > > > > additional safety you get at runtime when a null value is
> > passed
> > > > > into a
> > > > > > > > parameter marked non-null. Similarly nothing enforces that
> you
> > > > handle
> > > > > > > null
> > > > > > > > values when something is annotated as nullable.
> > > > > > > >
> > > > > > > > For that reason, I tend to assume everything is nullable
> > > regardless
> > > > > of
> > > > > > > > annotation and I would be in favor of standardizing around
> that
> > > > > > > assumption.
> > > > > > > > Iff something is annotated as Non-null a developer should
> feel
> > > safe
> > > > > > > > skipping a check for null, but otherwise they should handle
> > null.
> > > > > > > >
> > > > > > > > I am a big fan of Optional and of trying to reduce the usage
> of
> > > > null
> > > > > as
> > > > > > > > much as possible though.
> > > > > > > >
> > > > > > > > On Wed, May 21, 2025 at 3:02 PM Alex Dutra
> > > > > <alex.du...@dremio.com.invalid
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > A while ago, we had a discussion regarding which nullness
> > > > > annotations
> > > > > > > > > to use and whether we should consider favoring non-null by
> > > > > default. I
> > > > > > > > > would like to revive that discussion.
> > > > > > > > >
> > > > > > > > > We are currently using the `jakarta.annotation` package
> > > > > consistently,
> > > > > > > > > but the defaults are not clear: should we consider
> everything
> > > as
> > > > > > > > > non-null by default and only annotate the nullable things,
> or
> > > the
> > > > > > > > > other way around? Some classes are cluttered with both
> > > > annotations,
> > > > > > > > > which seems unnecessary and confusing.
> > > > > > > > >
> > > > > > > > > I would personally be in favor of considering everything as
> > > > > non-null by
> > > > > > > > > default.
> > > > > > > > >
> > > > > > > > > Please let me know your thoughts.
> > > > > > > > >
> > > > > > > > > Alex
> > > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> >
>
