enola-dev / enola

Enola πŸ•΅πŸΎβ€β™€οΈ Holmes was an SRE.
https://docs.enola.dev/
Apache License 2.0
16 stars 5 forks source link

Add `null` safety check to Enola.dev build process #845

Open vorburger opened 1 month ago

vorburger commented 1 month ago

Many πŸŒ” ago I kicked off http://www.lastnpe.org.

I have not had the free cycles anymore recently to maintain that, but there are apparently still some folks interested in it; note e.g. @agentgt re. https://github.com/lastnpe/eclipse-null-eea-augments/issues/165 and maybe @sebthom in https://github.com/lastnpe/eclipse-null-eea-augments/issues/166.

My current passion project is this repo, see https://docs.enola.dev. I actually do use https://jspecify.dev annotations here and there in this repo already, and occasionally IntelliJ IDEA will point something out (although it apparently has some limitations).

The "real proper thing to do" here, in an ideal world, with unlimited time in the day, would of course be for the build process of this project (not just IDE UI) to do nullness analysis, and fail CI/CD in case of bugs, etc. Let that be the goal of this issue - perhaps one day.

If I had this here, perhaps I would be more interested (again) in contributing to external annotations for JDK, and perhaps other libraries, such as https://github.com/jspecify/jspecify/issues/25... 🀞🏽

Which of these is easiest to integrate into the (currently) Bazel based build process of this project?

@cushon @cpovirk @netdpb do you perhaps have any guidance among the available options for which Nullness checker to adopt now, for a "greenfield" project such as this one, with a Bazel-based build, and existing org.jspecify usage?

cpovirk commented 1 month ago

Just to restate this for anyone who stumbles across this thread later: As a general rule, there's no slam-dunk choice for which nullness checker to use at this point.

For the particular case of a greenfield project with ~zero(?) Java dependencies and a small (n β‰ˆ 1 :)) number of developers who would need to be educated, I would probably try EISOP if you care about generics and NullAway if you don't.

The most likely source of complications for EISOP would be the "Bazel" part. The EISOP docs link to a Stack Overflow question with no "answer" per se, just "Here's how you'd set up NullAway." I would expect the general approach to transfer to EISOP, but our internal setup at Google is (as you've probably seen) different (mostly (but not exclusively) in ways that make it worse :)), so I haven't actually tried a "normal" Bazel setup. If you're lucky, all you need is to add some deps.

So you might start with NullAway to get at least something set up (since the instructions there are more likely to Just Work), iterate until the nullness errors are gone, and then decide whether to pursue using the stricter EISOP.

(To enable either checker automatically more widely than a single target, you may be able to take advantage of java_package_configuration or exported_plugins.)

vorburger commented 1 month ago

https://github.com/uber/NullAway/issues/119 may be a useful starting point...

vorburger commented 1 month ago

uber/NullAway#119 may be a useful starting point...

https://github.com/uber/NullAway/wiki/Configuration#bazel actually documents it.

with ~zero(?) Java dependencies

@cpovirk this project, like most, depends on a number of external libraries.

If my memory from my past passion in this space serves me well, the API boundaries with such libraries is where the real "fun" 😊 typically is? That's what at the time had driven me to create LastNPE.org...

If I'm digging this correctly, https://github.com/uber/NullAway/wiki/Configuration#library-models is how they do that? But ... this is not pretty, compared with EEAs. 😒

cpovirk commented 1 month ago

Oops, I had been looking in MODULE.bazel for the deps. I am not up to speed on such things, if that wasn't already clear :)

For library models, NullAway has been investigating consuming the JSpecify JDK nullness information. But they don't yet....

vorburger commented 1 month ago

For library models, NullAway has been investigating consuming the JSpecify JDK nullness information.

@cpovirk what is "JSpecify JDK nullness information"? Any links to share? Just on the (unlikely) off chance that I would like to climb back out from under the rock πŸͺ¨ I've been living under since creating LastNPE.org 8 years (OMG!) ago... ⏳

But they don't yet....

FYI https://github.com/uber/NullAway/issues/1024...

cpovirk commented 1 month ago

JSpecify JDK nullness information

Ah, yes, https://github.com/jspecify/jspecify/issues/25 talks around this but probably doesn't have a good summary. (And I'm not rereading it to see if there is one :))

We have https://github.com/jspecify/jdk, which is based on the JDK annotations used by the Checker Framework. It's not anything that we've tried to drive to broad consensus (neither the format nor the choice of nullness for the APIs), even though we occasionally discuss specific classes as examples in broader discussions (e.g., https://github.com/jspecify/jspecify/issues/78). Google builds attach those annotations to the JDK and Android classes to support Kotlin and other nullness checking. (And here was NullAway's expression of interest.)

the API boundaries with such libraries is where the real "fun" 😊 typically is?

Yes, very often :) We'd like to have more of a story for overlays / external annotations / library models in general, but I don't expect notable progress on that anytime soon. We of course hope that many libraries will be able to adopt annotations directly, but that would still leave the JDK on top of stragglers.

wmdietl commented 1 month ago

This PR illustrates how to use Bazel with EISOP. This is my first use of Bazel and I assembled this from tutorials. All feedback welcome!