On Tue, Apr 25, 2023 at 12:26 PM Kenneth Knowles <k...@apache.org> wrote:
> This is great work. Thanks for writing about it. > > I hold a stronger opinion than "the danger of nullness and the validity of > the warnings depend on the code context": nullness is a solved problem, and > type systems solved it, decades ago in some languages and more recently in > Java. Nullness errors are a choice, and we should not choose to have them. > Treat them as you would treat any other type error. > > And I hold even strong opinions than that! Almost all the time, unclear > nullness/typing has an underlying cause of lack of clarity in design and > thinking. This is why fixing nullness almost always improves the code, but > also why it can be difficult to do after the fact, because it can require > fixing a bad or broken design. But sometimes it is easy, and you should do > it with gusto in either way :-) > > And more! If checkerframework cannot determine the correctness of > something, probably neither can you. You may think you can, but you are > probably wrong. And even if you are correct today, the next person coming > along will not have your internal mental state and will easily break it. > > As an example of both of the above: it can be very hard to fix API designs > like DoFn (or JUnit classes, etc) of the start/doSomething/end variety that > rely on a caller to invoke methods in a particular order and have > significant inter-method stateful dependencies that are not at all > guaranteed by their design. > Maybe warrants a new topic, but has there been any talk about what a newer, more principled Beam 3.0 user facing API might look like and if not, is it something you've given some thought to? > Such APIs are extremely fragile and the source of a huge number of bugs > because it is very easy and common to invoke methods in an order or > multiplicity that was not designed for. Not at all coincidentally, it is > quite hard and annoying to add the needed "Requires" and "Ensures" > annotations to get them to type check, and also very hard and annoying to > satisfy those requirements when calling the methods. This is all a feature > - the design was bad, and the difficulty setting up and satisfying the > invariants helps you realize it. > > /lecture :-) > > Kenn > > On Tue, Apr 25, 2023 at 9:56 AM Damon Douglas <damondoug...@apache.org> > wrote: > >> Hello Everyone, >> >> *For those experienced with Beam*: >> >> PR/26413 [1] begins the long journey of addressing [2] which is a subtask >> of [3]. 千里之行，始於足下 :-) >> >> *For those beginning on the learning path*: >> >> The pull request referenced in the email is not directly related to Beam >> but involves a Java annotation called SuppressWarnings. Per its namesake, >> it suppresses warnings according to the values you set in the annotation. >> In [2] and [3]'s context the particular setting is "nullness" that >> suppresses any warnings related to potentially null references in the code. >> >> In the Beam repository, we utilize The Checker Framework [4] to help >> maintain a clean codebase. When such annotation applies, it will >> essentially mask these warnings and let the code pass without errors. When >> removing the SuppressWarnings with "nullness" annotation, the checks fail. >> >> Why is this a big deal? The danger of nullness and the validity of the >> warnings depend on the code context. However, as of this writing there are >> many Java classes in our repository with the SuppressWarnings annotation. >> What is the likelihood they are masking something serious? >> >> Here in our discussion would be a great place to be practical and relay >> what can be done when receiving a null-related error from the checker >> framework. However, I want to wait after I do more of these and find >> validation through peer review. Then, I will follow-up with the dev list. >> >> Meanwhile, you can take a look at my attempt in the PR [1]. >> >> Best, >> >> Damon >> >> *References*: >> 1. https://github.com/apache/beam/pull/26413 >> 2. https://github.com/apache/beam/issues/20498 >> 3. https://github.com/apache/beam/issues/20497 >> 4. https://checkerframework.org/ >> >> >> *~Vincent*
