Val, > Therefore, it's crucial to bring the attention of the API's user to such > parameters. (@Nullable)
This sounds wrong for me. If a method parameter is marked as @Nullable, then a user can put anything there without much thinking. Opposite happens with @NotNull parameters, with them a user should think whether his variable can be null or not. As for me using nullable variables should be avoided as much as possible, but not all developers share this point of view. And especially in Ignite codebase it was quite common to use nullable variables, for the first time it was very unusual for me. 2021-12-20 22:06 GMT+03:00, Valentin Kulichenko <valentin.kuliche...@gmail.com>: > 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 >> > -- Best regards, Ivan Pavlukhin
