Update to JSpecify 1.0 and use JSpecify annotations in NullAway code

[msridhar]

• Update our JSpecify dependence to 1.0
• Change NullAway source code to use JSpecify annotations (test inputs were left alone)
• Fix position of type use @Nullable annotations to be next to type and enable the AnnotationPosition Error Prone check
• Update README to encourage use of JSpecify

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.95%. Comparing base (655159f) to head (15929e5).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1000   +/-   ##
=========================================
  Coverage     85.95%   85.95%           
  Complexity     2073     2073           
=========================================
  Files            83       83           
  Lines          6873     6873           
  Branches       1322     1322           
=========================================
  Hits           5908     5908           
  Misses          551      551           
  Partials        414      414           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[msridhar]

@cpovirk could I ask you to look over the changes in README.md? Will tag you with a couple questions also

[yuxincs]

🚀

[msridhar]

@cpovirk are we currently encouraging Android users to switch over from Android support annotations to JSpecify as well? Or will that cause problems for them?

[cpovirk]

Phrasing is difficult :) We have had similar conversations internally.

On the one hand, it would be nice if we could find a way to avoid implying that the Android annotations (which of course are declaration annotations) are "just as good" as the JSpecify ones.

On the other hand, it would be nice to make clear there there are still tradeoffs, by which I mainly mean "Dagger (if you don't use a recent javac)." (On the plus side, maybe Android users can more upgrade their compiler especially easily, since they don't need to worry about setting --release? Part of me now wants for the NullAway docs to demonstrate configuring a Java 22 toolchain for Android, except that it's not really fair to put that on the critical path for NullAway adopters :))

I was hoping to convince myself that we had an existing doc that covered this, but as you saw, I ended up leaving a comment on a JSpecify issue instead.

The best link at this point is probably https://jspecify.dev/docs/whether/#annotation-processors. Let me see if I can think of a way that I'd phrase that once I get back to my desk after a quick break. I'm hoping to propose a different framing for this sentence.

[cpovirk]

On second thought: I might be putting too much emphasis on the ability to annotate type arguments, type-parameter bounds, and array-element types yet, since NullAway support there is still developing. (And of course NullAway already offers a way to treat unannotated types as non-nullable when you want it, so there's less reason for @NullMarked, too.)

I think I'd probably just keep this as you have it for now, and we can revisit once we have docs that speak more directly to the tradeoffs. Maybe we'll also tweak the comment in the code block at line 29 at that point.

---

Hmm, but now that I'm here: Nowadays, people use androidx.annotation.Nullable instead of android.support.annotation.Nullable, right?

[msridhar]

Will switch to refer to androidx, thanks!

[msridhar]

@cpovirk this is encouraged, correct? This will bring in the JSpecify jar onto the annotation processor path for users, but they will have it there anyway I think if they are using a recent version of Error Prone. I think this could only cause a problem if for some reason they already have a dep on an old version of the JSpecify jar.

[cpovirk]

I was wondering about Error Prone, too. All I've found there so far is a test dep, but it could have missed a full dep, especially since I didn't look at all at indirect deps. (So someday Error Prone will get it through Guava or Caffeine or whatever.)

I think that including it should be fine. I think you're right that there could in theory be trouble if some other annotation processor wanted the old package, but that seems very unlikely, and things would generally work out OK even in that case.

[cpovirk]

Error Prone will have the dep in 2.30.0 (or 2.29.3 or whatever the next release after 2.29.2 is):

• Migrate Error Prone to use JSpecify nullness annotations google/error-prone#4500
• google/error-prone@bfe69f8

[cpovirk]

On second thought: I might be putting too much emphasis on the ability to annotate type arguments, type-parameter bounds, and array-element types yet, since NullAway support there is still developing. (And of course NullAway already offers a way to treat unannotated types as non-nullable when you want it, so there's less reason for @NullMarked, too.)

I think I'd probably just keep this as you have it for now, and we can revisit once we have docs that speak more directly to the tradeoffs. Maybe we'll also tweak the comment in the code block at line 29 at that point.

---

Hmm, but now that I'm here: Nowadays, people use androidx.annotation.Nullable instead of android.support.annotation.Nullable, right?

[cpovirk]

I was wondering about Error Prone, too. All I've found there so far is a test dep, but it could have missed a full dep, especially since I didn't look at all at indirect deps. (So someday Error Prone will get it through Guava or Caffeine or whatever.)

I think that including it should be fine. I think you're right that there could in theory be trouble if some other annotation processor wanted the old package, but that seems very unlikely, and things would generally work out OK even in that case.