NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

Allow multiple castToNonNull methods to be configured and castToNonNull methods with multiple arguments

Open lazaroclapp opened this issue 3 years ago • 1 comments

A castToNonNull as defined in the docs is simply an identity function annotated as @Nullable -> @NonNull, with a NullAway suppression, used to cast values we know must be non-null at runtime, but which NullAway can't determine statically to be so.

We currently provide the option -XepOpt:NullAway:CastToNonNullMethod=[...] to register, by name (fully qualified class name, plus method simple name) a particular single canonical castToNonNull method for a given codebase. This method is assumed to take exactly one argument: the value being cast. This is used for multiple reasons:

  1. To detect and report an error when a statically known non-null value is passed as the argument to castToNonNull, to reduce the number of redundant casts and help make surface those casts that are actually needed.
  2. To suggests castToNonNull calls as fixes, including during -XepPatchChecks:NullAway runs (depending on configuration)
  3. Some code during initialization handles castToNonNull methods as a special case to detect safe field reads/uninitialized field escapes before initialization.

Unfortunately, it's not unreasonable to want to have:

  • Multiple castToNonNull methods in use within a codebase (specially given the next point)
  • castToNonNull methods taking extra arguments beyond the value being cast. In particular: a logging or reporting class to which to write in the case of seeing a null value passed to the cast at runtime.
  • castToNonNull methods with the exact same simple name but different method signatures (a special case of the general combination of the two points above)

Supporting such methods requires potentially changing the :CastToNonNullMethod= option, but, also, de-tangling the use cases above:

  • (1) and (3) probably need to happen "for every castToNonNull method" and are fine with extra arguments, so long as it is known which argument is the value being cast (either always the first, or the castToNonNull method must be specified as full signature plus argument number, like our other library models)
  • (2) requires an uniquely distinguished castToNonNull method taking no arguments but the value being cast (or, equivalently, one for which all other arguments can be injected as statically known expressions, but then we can always just make a single argument wrapper and use that)

An option here is to rely on -XepOpt:NullAway:CastToNonNullMethod=[...] exclusively for (2), and to have a separate mechanism to mark "additional" cast methods (@CastToNonNullMethod? Library Models?).

This is low priority, to be honest, but recording this as an issue because this is the second time I've had to rediscover this limitation 😅

lazaroclapp avatar May 04 '22 04:05 lazaroclapp

@lazaroclapp was this fixed by #614?

msridhar avatar Sep 02 '23 16:09 msridhar