Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vorburger opened this issue Aug 19, 2024 · 7 comments
Open

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

vorburger opened this issue Aug 19, 2024 · 7 comments
Labels
build Build, CI, etc. (w.o. #testing) enhancement New feature or request help wanted Extra attention is needed question Further information is requested testing Tests, Test Frameworks, CI, CD

Comments

@vorburger
Copy link
Member

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. lastnpe/eclipse-null-eea-augments#165 and maybe @sebthom in lastnpe/eclipse-null-eea-augments#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 jspecify/jspecify#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?

@vorburger vorburger added enhancement New feature or request help wanted Extra attention is needed question Further information is requested build Build, CI, etc. (w.o. #testing) testing Tests, Test Frameworks, CI, CD labels Aug 19, 2024
@cpovirk
Copy link
Contributor

cpovirk commented Aug 19, 2024

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
Copy link
Member Author

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

@vorburger
Copy link
Member Author

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
Copy link
Contributor

cpovirk commented Aug 20, 2024

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
Copy link
Member Author

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 uber/NullAway#1024...

@cpovirk
Copy link
Contributor

cpovirk commented Aug 21, 2024

JSpecify JDK nullness information

Ah, yes, jspecify/jspecify#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., jspecify/jspecify#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
Copy link

wmdietl commented Aug 27, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build, CI, etc. (w.o. #testing) enhancement New feature or request help wanted Extra attention is needed question Further information is requested testing Tests, Test Frameworks, CI, CD
Projects
None yet
Development

No branches or pull requests

3 participants