Neither solution is completely bulletproof, and I don't think that's what we are looking for. We need something that provides a reasonable value, but also does not clutter the code with too many annotations.
I would also keep in mind that annotations are used not only for static analysis, but by developers as well. And from this standpoint, I feel that @Nullable is much more important, because 'null' is a special value which is treated differently in different circumstances. Therefore, it's crucial to bring the attention of the API's user to such parameters. On the other hand, non-nullable parameters are inherently safer, so there is less need to annotate them. -Val On Mon, Dec 20, 2021 at 7:42 AM Alexander Polovtcev <alexpolovt...@gmail.com> wrote: > Hi, Ivan. > > > it seems not bulletproof > > I completely agree with you. As I wrote in the original message, this > becomes even worse in case of 3-rd party dependencies, because they may not > be annotated, which can lead to confusions. But looks like this is not a > big deal, because these annotations are not intended to cover 100% of cases > and should not be treated as a guarantee against NPEs. Maybe covering > *some* cases is enough. > > > Perhaps we can insist on not using nulls as much as possible. > > Same here, I agree with you and it correlates with clear API design in > general. However, sometimes nullable parameters and return values are > justified, so how can we formalize that? > > On Sat, Dec 18, 2021 at 10:52 AM Ivan Pavlukhin <vololo...@gmail.com> > wrote: > > > Hi, > > > > While option #2 looks very appealing it seems not bulletproof > > reliable, someone can occasionally miss @Nullable annotation. Option > > #3 seems more practical too me, as missed @NotNull annotations cannot > > do much harm. > > > > Also I am thinking about using nullable parameters in general. Perhaps > > we can insist on not using nulls as much as possible. With such > > requirement in guidelines option #2 becomes practical. > > > > 2021-12-17 14:47 GMT+03:00, Alexander Polovtcev <alexpolovt...@gmail.com > >: > > > Maksim, thank you for the suggestion. > > > > > > I've never used NullAway, but after having a quick look I think it > might > > be > > > an overkill, since it is a plugin for the ErrorProne, which is a > separate > > > tool. I recall some efforts of introducing ErrorProne to Ignite 3 and > > they > > > were not successful. But again, I don't have much experience in that > > > regard. I wonder if IDEA inspections would be enough, since they are > easy > > > to use during development and AFAIK are already run during the TC > builds. > > > > > > Regarding Ignite 2, I don't know if it would be viable to add these > > > annotations to the existing code (in order to have meaningful checks), > > > since the codebase is so large. But nevertheless we can try to adopt > the > > > same approach there as well. > > > > > > On Fri, Dec 17, 2021 at 12:10 PM Maksim Timonin < > timoninma...@apache.org > > > > > > wrote: > > > > > >> Hi! > > >> > > >> There is a pretty popular project NullAway [1] that checks code of a > > >> project in compile-time for possible NPE. AFAIK, it works only with > the > > >> "@Nullable" annotation. I think we can try to add this check to > Ignite2 > > >> and > > >> 3. > > >> > > >> But I wonder, whether smbd already tried to introduce this check? If > > not, > > >> I > > >> can try, WDYT? > > >> > > >> [1] https://github.com/uber/NullAway > > >> > > >> On Fri, Dec 17, 2021 at 9:04 AM ткаленко кирилл <tkalkir...@yandex.ru > > > > >> wrote: > > >> > > >> > I'm for the second option. > > >> > > > >> > > > > > > > > > -- > > > With regards, > > > Aleksandr Polovtcev > > > > > > > > > -- > > > > Best regards, > > Ivan Pavlukhin > > > > > -- > With regards, > Aleksandr Polovtcev >
