Proper support for type use annotation locations + develop a deprecation path for incorrect locations

[lazaroclapp]

For context, see #706 and particularly this summary by @msridhar (slightly edited from here):

Let me try to summarize the overall situation here so we can think about potential solutions and their tradeoffs. #702 addressed an issue where writing List<@Nullable String> list made list itself @Nullable, due to the type use annotation on the type argument (identified in https://github.com/uber/NullAway/issues/674#issuecomment-1363205545). The change there was to only consider a type-use @Nullable annotation to be relevant a field / parameter / return type if the annotation was not in an array, inner type, wildcard, or type argument position.

Unfortunately, this change was bad for a couple of reasons:

1. As shown in #705, sometimes type use annotations are in an inner type position even when it doesn't syntactically look like it (see example in https://github.com/uber/NullAway/issues/705#issuecomment-1368082909).
2. Syntactically, the "correct" location for a type use annotation is not what NullAway users (who mostly use declaration annotations) are used to. Relevant discussion is in the JSpecify user guide. The change in #702 unintentionally broke cases where users used a type use annotation (like the Checker Framework or JSpecify @Nullable) and wrote @Nullable Object[] foo2 = null;, as it changed NullAway to only look for the annotations in the syntactically-correct place (Object @Nullable [] foo2).

For inner types, we have a fix, with caveats, in #706. It fixes point 1 above and gets rid of the surprising change in checking behavior for inner types from point 2. It does lead to a bit of weirdness, as shown in here:

https://github.com/uber/NullAway/blob/1b1a5b1008612606a36062a99e7f5e21eb704680/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java#L1113-L1117

In particular, writing A.@Nullable B.C foo2 makes foo2 @Nullable. But this seems like a corner case that shouldn't affect users much right now.

Dealing with arrays is a bit more complicated, since users may want to write annotations to express the nullability of array contents. If we adopt a solution like the one used in this PR for inner types, writing either @Nullable Object[] foo or Object @Nullable[] foo would make foo @Nullable. But, this would mean that if foo were intended to be a @NonNull reference to an array containing @Nullable Objects, a user could not write the correct annotation for this case (@Nullable Object[] foo) without adverse impacts from NullAway (since foo itself would then be treated as @Nullable). Such NullAway users would be forced to either not write the correct annotations for the nullability of array contents for now or to deal with references becoming unintentionally @Nullable (the latter seems worse). A similar issue is seen in #674 for varargs arrays. While this downside is unfortunate, it does match the behavior of NullAway before #702 landed.

We could also just stick with requiring users to write type use nullability annotations for arrays in the right spot. But that kind of breaking change seems weird to introduce by accident, and I'm not sure of other unintended consequences. That seems like a change we should introduce deliberately and possibly behind a flag at first to let users adopt gradually.

So, I think our best option is to handle inner type and array type use annotations in the same way for now, as inner types are currently being handled in this PR. I'm not sure about a combination of these, e.g., A.B.@Nullable C [][] foo = null;. This annotation does not make foo @Nullable under proper usage of type use annotations nor in a backward compatibility scenario. So I guess we can just treat foo as @NonNull for this case?

The stop gap fix for the issues above implemented in #706 does the following:

• Interpret an annotation on any inner or outer type of the field/argument declaration as annotating the field/argument itself (equivalently: as an annotation on the innermost inner type in proper type use annotation semantics)
• Interpret an annotation on any sequence of array locations (including @Nullable Foo [][], Foo @Nullable [][], and Foo [] @Nullable []) as marking the array itself as being nullable.
• We don't support type use annotations on generics or wildcards
• We don't support type use annotations in locations that involve both inner classes and arrays (so, for A.B.C [] foo the only two valid places for the annotation are @Nullable A.B.C [] foo and A.B.C @Nullable [] foo)

Still, this means we fail to correctly follow standard semantics for type use annotations, which will only get more confusing once we want to introduce JSpecify-like support for annotating the nullability of array elements.

We should, relatively soon, introduce a breaking change intentionally that prevents adding @Nullable in the wrong position for arrays and inner types. To do this, we probably need to refactor some of our location parsing logic in NullabilityUtil to provide more information about what is being annotated/queried. We also want a reasonable UX for developers upgrading past this breaking change and/or switching between declaration and type use annotations at any point after the change (i.e. If possible, we should avoid a situation where changing the imports for @Nullable to a different annotation silently causes nullability to switch from applying to the array reference itself to applying to the elements! Though I am not whether we can do something here...)

At any rate, the version introducing this breaking change should bump the "minor" release number, not the "patch" number (not that NullAway follows semver, per se)

[cpovirk]

(When you do revisit this, you might find cushon's recent https://github.com/google/error-prone/commit/5123cd56ba7fdf8be7b81c0812af19c0256856d4 to be helpful in handling annotations on inner types.)

[msridhar]

Related: #1007 #674 #853

My rough plan here is as follows. In an upcoming NullAway release, if you are using type-use annotations like JSpecify's, by default NullAway will only recognize them in the correct locations, even outside JSpecify mode. This is because with the release of JSpecify 1.0 we want to encourage correct placement of these annotations. There will be a flag LegacyAnnotationLocations (or something like that) to get NullAway's previous behavior, but we will remove that flag eventually.

For declaration annotations outside of JSpecify mode, we will try our best to preserve NullAway's extant behavior. Inside JSpecify mode we can do the same, but maybe also warn about their use? Not sure.

If a single annotation is both declaration and type-use, we'll treat it as declaration. JSpecify and Checker Framework @Nullable annotations are just type-use.

I'm not yet sure about multiple annotations on a single element, possibly mixing declaration and type use, that possibly contradict each other. I think mixing contradictory JSpecify annotations is against the JSpecify spec and we can warn on that in JSpecify mode. Seems like maybe we should try to detect and warn on all these cases and see what happens, since it's almost surely some kind of mistake. But, maybe not the highest priority.