Support annotation of guard methods about nullaway HOT 10 CLOSED

Similarly - supporting jetbrains' @Contract annotations could help here

from nullaway.

Maybe something like @ImpliesNonNull(when = true, param = 0)

from nullaway.

NullAway does not reason across procedure boundaries about nullness checks, so it won't be able to prove this safe. If you frequently use the getIfReady method to check for nullness, you can handle this with a custom library model; see #51

Closing, but please re-open if you have more questions

from nullaway.

Re-opening as we should support this case with an annotation. The user should be able to write something like:

@RetTrueImpliesNonNullParam(0)
static boolean isReady(@Nullable CompletableFuture<?> future) {
  return (future != null) && future.isDone()
      && !future.isCompletedExceptionally()
      && (future.join() != null);
}

The annotation means that if the function returns true, the 0th parameter future cannot be null. We also want @RetFalseImpliesNonNullParam or the like (naming TBD). We could trust the annotations at first but maybe eventually check them.

from nullaway.

Actually, for an Iterator I think it should be params with an array of strings. For example, I'd like to assert that key and value are non-null if hasNext() is true.

public Entry<K, V> next() {
  if (!hasNext()) {
    throw new NoSuchElementException();
  }
  Entry<K, V> entry = new WriteThroughEntry<>(cache, key, value);
  removalKey = key;
  value = null;
  next = null;
  key = null;
  return entry;
}

Though that would best be if suppressed on the call-site, which isn't always possible, so It definitely messy.

from nullaway.

I think the hasNext example is about non-nullness of fields of the receiver object, yes? We could support that too, though it'd take a bit more work.

from nullaway.

Yep, they are different cases of a similar problem. Thanks!

from nullaway.

Ok, so, @msridhar, question: Are we currently shipping any support library for NullAway where we could put this new custom annotation? If not, I think we should, together with the castToNonNull method. As for @ImpliesNonNull, I think it can take either an array of arguments or an array of access paths. The later case would probably come at a performance cost, but we can make sure we only pay it when people use this annotation. We can talk more about this 1:1.

from nullaway.

Thanks you!

from nullaway.

To be clear, we don't yet support cases involving fields like the one @ben-manes outlined here. I'll open a follow-up task on that.

from nullaway.