Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Thanks everybody for the opinions. Looks like we all agree on option 2, but nevertheless I'll organize a formal vote after the New Year holidays. On Wed, Dec 22, 2021 at 9:18 AM Ivan Pavlukhin wrote: > Val, > > > Basically, what I'm suggesting is to accept non-nullable as default > > Sounds good for me. Then it should be clearly stated in code > guidelines and regular static analysis for this should be organized > from the beginning. > > 2021-12-21 20:35 GMT+03:00, Valentin Kulichenko < > valentin.kuliche...@gmail.com>: > > Ivan, > > > > I agree with you. Basically, what I'm suggesting is to accept > non-nullable > > as default, but if a parameter must be nullable for whatever reason, mark > > it with the @Nullable annotation. > > > > Regarding "a user can put anything there without much thinking": I > > understand what you mean, but I believe the annotation is not the > problem. > > It's nullability itself that causes the ambiguity - null value is always > a > > special case that means different things for different values. @Nullable > at > > least sends a signal that nulls need to be considered. If a user doesn't > > want to think anyway, there is not much we can do :) (Except trying to > make > > the variable non-nullable, of course) > > > > -Val > > > > On Mon, Dec 20, 2021 at 11:40 PM Ivan Pavlukhin > > wrote: > > > >> 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 > >> > > >> > 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 > > >> >> 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 > >> >> > >> >> >: > >> >> > > 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
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Val, > Basically, what I'm suggesting is to accept non-nullable as default Sounds good for me. Then it should be clearly stated in code guidelines and regular static analysis for this should be organized from the beginning. 2021-12-21 20:35 GMT+03:00, Valentin Kulichenko : > Ivan, > > I agree with you. Basically, what I'm suggesting is to accept non-nullable > as default, but if a parameter must be nullable for whatever reason, mark > it with the @Nullable annotation. > > Regarding "a user can put anything there without much thinking": I > understand what you mean, but I believe the annotation is not the problem. > It's nullability itself that causes the ambiguity - null value is always a > special case that means different things for different values. @Nullable at > least sends a signal that nulls need to be considered. If a user doesn't > want to think anyway, there is not much we can do :) (Except trying to make > the variable non-nullable, of course) > > -Val > > On Mon, Dec 20, 2021 at 11:40 PM Ivan Pavlukhin > wrote: > >> 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 >> > >> > 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 >> >> 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 >> >> > > >> >: >> >> > > 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
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Ivan, I agree with you. Basically, what I'm suggesting is to accept non-nullable as default, but if a parameter must be nullable for whatever reason, mark it with the @Nullable annotation. Regarding "a user can put anything there without much thinking": I understand what you mean, but I believe the annotation is not the problem. It's nullability itself that causes the ambiguity - null value is always a special case that means different things for different values. @Nullable at least sends a signal that nulls need to be considered. If a user doesn't want to think anyway, there is not much we can do :) (Except trying to make the variable non-nullable, of course) -Val On Mon, Dec 20, 2021 at 11:40 PM Ivan Pavlukhin wrote: > 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 > > > > 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 > >> 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 > >> > >> >: > >> > > 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. > >> > >> > >> >
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 : > 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 > > 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 >> 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 >> > > >: >> > > 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 ткаленко кирилл >> > >> > > >> > >> wrote: >> > >> >> > >> > I'm for the second option. >> > >> > >> > >> >> > > >> > > >> > > -- >> > > With regards, >> > > Aleksandr Polovtcev >> > > >> > >> > >> > -- >> > >> > Best regards, >> > Ivan Pavlukhin >> > >> >> >> -- >> With regards, >> Aleksandr Polovtcev >> > -- Best regards, Ivan Pavlukhin
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 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 > 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 >: > > > 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 ткаленко кирилл > > > >> wrote: > > >> > > >> > I'm for the second option. > > >> > > > >> > > > > > > > > > -- > > > With regards, > > > Aleksandr Polovtcev > > > > > > > > > -- > > > > Best regards, > > Ivan Pavlukhin > > > > > -- > With regards, > Aleksandr Polovtcev >
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 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 : > > 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 > > > 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 ткаленко кирилл > >> wrote: > >> > >> > I'm for the second option. > >> > > >> > > > > > > -- > > With regards, > > Aleksandr Polovtcev > > > > > -- > > Best regards, > Ivan Pavlukhin > -- With regards, Aleksandr Polovtcev
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 : > 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 > 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 ткаленко кирилл >> wrote: >> >> > I'm for the second option. >> > >> > > > -- > With regards, > Aleksandr Polovtcev > -- Best regards, Ivan Pavlukhin
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 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 ткаленко кирилл > wrote: > > > I'm for the second option. > > > -- With regards, Aleksandr Polovtcev
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
+1 for option No. 2. On Fri, 17 Dec 2021 at 12:10, Maksim Timonin 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 ткаленко кирилл > wrote: > > > I'm for the second option. > > >
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
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 ткаленко кирилл wrote: > I'm for the second option. >
Re:[DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
I'm for the second option.
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Same here. The second option seems the most reasonable. -Val On Thu, Dec 16, 2021 at 8:07 AM Alexei Scherbakov < alexey.scherbak...@gmail.com> wrote: > +1 for 2 > > чт, 16 дек. 2021 г. в 18:50, Pavel Tupitsyn : > > > Option 2 seems the most sensible. > > It translates to/from other languages and should be already familiar to > > some developers. > > > > For example, with nullable checks enabled, C# treats everything as not > > null, unless specified otherwise with "?". > > Same for other languages where Option/Maybe type is present. Nothing is > > null by default. > > > > On Thu, Dec 16, 2021 at 6:14 PM Alexander Polovtcev < > > alexpolovt...@gmail.com> > > wrote: > > > > > Dear Igniters! > > > > > > I would like to propose a discussion about defining a policy regarding > > > where and how to use @Nullable/@NotNull annotations. These annotations > > are > > > used in conjunction with a static analysis engine (e.g. built in IDEA) > > and > > > are useful for avoiding null dereferencing and specifying method > > contracts. > > > > > > I can see the following possible options: > > > > > > 1. *Use both @Nullable and @NotNull annotations everywhere* (i.e. > method > > > parameters and return types, class fields). Pros: the most robust and > > > expressive variant; easy to agree on and specify. Cons: very verbose; > may > > > lead to code cluttering; > > > 2. *Use only @Nullable *for specifying method parameters that accept > null > > > or class fields that can be null, treating @NotNull as an implicit > > default. > > > Pros: correlates with the default IDEA settings (with all corresponding > > > inspections enabled); not as verbose as option 1, since nullable > > parameters > > > are quite rare. Cons: less sound and complete, especially when working > > with > > > third-party libraries that are not annotated, since we cannot apply the > > > implicit @NotNull there; > > > 3. *Use only @NotNull *and treat @Nullable as an implicit default. > Pros: > > > less verbose than option 1, better correlates with Java language > > semantics > > > (since all Java references are nullable). Cons: more verbose than > option > > 2; > > > may be impossible to properly set up the analysis engine or may require > > > switching to a different annotation provider that supports jsr-305 > > > @ParametersAreNullableByDefault; > > > 4. *Do not use @Nullable nor @NotNull*. The most radical option in case > > we > > > will not be able to agree on any of the above three. No annotations - > no > > > need for the discussion. > > > > > > What do you think? Are there any other options out there? I would like > to > > > collect as many options as possible and organize a vote some time > later. > > > > > > -- > > > With regards, > > > Aleksandr Polovtcev > > > > > > > > -- > > Best regards, > Alexei Scherbakov >
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
+1 for 2 чт, 16 дек. 2021 г. в 18:50, Pavel Tupitsyn : > Option 2 seems the most sensible. > It translates to/from other languages and should be already familiar to > some developers. > > For example, with nullable checks enabled, C# treats everything as not > null, unless specified otherwise with "?". > Same for other languages where Option/Maybe type is present. Nothing is > null by default. > > On Thu, Dec 16, 2021 at 6:14 PM Alexander Polovtcev < > alexpolovt...@gmail.com> > wrote: > > > Dear Igniters! > > > > I would like to propose a discussion about defining a policy regarding > > where and how to use @Nullable/@NotNull annotations. These annotations > are > > used in conjunction with a static analysis engine (e.g. built in IDEA) > and > > are useful for avoiding null dereferencing and specifying method > contracts. > > > > I can see the following possible options: > > > > 1. *Use both @Nullable and @NotNull annotations everywhere* (i.e. method > > parameters and return types, class fields). Pros: the most robust and > > expressive variant; easy to agree on and specify. Cons: very verbose; may > > lead to code cluttering; > > 2. *Use only @Nullable *for specifying method parameters that accept null > > or class fields that can be null, treating @NotNull as an implicit > default. > > Pros: correlates with the default IDEA settings (with all corresponding > > inspections enabled); not as verbose as option 1, since nullable > parameters > > are quite rare. Cons: less sound and complete, especially when working > with > > third-party libraries that are not annotated, since we cannot apply the > > implicit @NotNull there; > > 3. *Use only @NotNull *and treat @Nullable as an implicit default. Pros: > > less verbose than option 1, better correlates with Java language > semantics > > (since all Java references are nullable). Cons: more verbose than option > 2; > > may be impossible to properly set up the analysis engine or may require > > switching to a different annotation provider that supports jsr-305 > > @ParametersAreNullableByDefault; > > 4. *Do not use @Nullable nor @NotNull*. The most radical option in case > we > > will not be able to agree on any of the above three. No annotations - no > > need for the discussion. > > > > What do you think? Are there any other options out there? I would like to > > collect as many options as possible and organize a vote some time later. > > > > -- > > With regards, > > Aleksandr Polovtcev > > > -- Best regards, Alexei Scherbakov
Re: [DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Option 2 seems the most sensible. It translates to/from other languages and should be already familiar to some developers. For example, with nullable checks enabled, C# treats everything as not null, unless specified otherwise with "?". Same for other languages where Option/Maybe type is present. Nothing is null by default. On Thu, Dec 16, 2021 at 6:14 PM Alexander Polovtcev wrote: > Dear Igniters! > > I would like to propose a discussion about defining a policy regarding > where and how to use @Nullable/@NotNull annotations. These annotations are > used in conjunction with a static analysis engine (e.g. built in IDEA) and > are useful for avoiding null dereferencing and specifying method contracts. > > I can see the following possible options: > > 1. *Use both @Nullable and @NotNull annotations everywhere* (i.e. method > parameters and return types, class fields). Pros: the most robust and > expressive variant; easy to agree on and specify. Cons: very verbose; may > lead to code cluttering; > 2. *Use only @Nullable *for specifying method parameters that accept null > or class fields that can be null, treating @NotNull as an implicit default. > Pros: correlates with the default IDEA settings (with all corresponding > inspections enabled); not as verbose as option 1, since nullable parameters > are quite rare. Cons: less sound and complete, especially when working with > third-party libraries that are not annotated, since we cannot apply the > implicit @NotNull there; > 3. *Use only @NotNull *and treat @Nullable as an implicit default. Pros: > less verbose than option 1, better correlates with Java language semantics > (since all Java references are nullable). Cons: more verbose than option 2; > may be impossible to properly set up the analysis engine or may require > switching to a different annotation provider that supports jsr-305 > @ParametersAreNullableByDefault; > 4. *Do not use @Nullable nor @NotNull*. The most radical option in case we > will not be able to agree on any of the above three. No annotations - no > need for the discussion. > > What do you think? Are there any other options out there? I would like to > collect as many options as possible and organize a vote some time later. > > -- > With regards, > Aleksandr Polovtcev >
[DISCUSSION] @Nullable/@NotNull annotation usage in Ignite 3
Dear Igniters! I would like to propose a discussion about defining a policy regarding where and how to use @Nullable/@NotNull annotations. These annotations are used in conjunction with a static analysis engine (e.g. built in IDEA) and are useful for avoiding null dereferencing and specifying method contracts. I can see the following possible options: 1. *Use both @Nullable and @NotNull annotations everywhere* (i.e. method parameters and return types, class fields). Pros: the most robust and expressive variant; easy to agree on and specify. Cons: very verbose; may lead to code cluttering; 2. *Use only @Nullable *for specifying method parameters that accept null or class fields that can be null, treating @NotNull as an implicit default. Pros: correlates with the default IDEA settings (with all corresponding inspections enabled); not as verbose as option 1, since nullable parameters are quite rare. Cons: less sound and complete, especially when working with third-party libraries that are not annotated, since we cannot apply the implicit @NotNull there; 3. *Use only @NotNull *and treat @Nullable as an implicit default. Pros: less verbose than option 1, better correlates with Java language semantics (since all Java references are nullable). Cons: more verbose than option 2; may be impossible to properly set up the analysis engine or may require switching to a different annotation provider that supports jsr-305 @ParametersAreNullableByDefault; 4. *Do not use @Nullable nor @NotNull*. The most radical option in case we will not be able to agree on any of the above three. No annotations - no need for the discussion. What do you think? Are there any other options out there? I would like to collect as many options as possible and organize a vote some time later. -- With regards, Aleksandr Polovtcev
