`NotNullIfNotNullAttribute` – inconsistent behavior for multiple ones; `||` operator – call site expects at least one satisfied; method fails on it

[bravequickcleverfibreyarn]

Version Used: Visual Studio Info.txt Example:

static void Test () => Console.WriteLine (NotNullIfNotNull1 (null, "").Length); // No error!

[return: NotNullIfNotNull ("a")]
[return: NotNullIfNotNull ("b")]
static string? NotNullIfNotNull1 ( string? a, string? b )
{
  if ( a is not null || b is not null )
    return null; // Not error!

  return null;
}

Expected Behavior:

null return is NOT allowed.

Actual Behavior:

null return IS allowed.

[CyrusNajmabadi]

@RikkiGibson ?

[RikkiGibson]

This is a known hole in NotNullIfNotNull analysis. The existing NotNullIfNotNull analysis only makes it disallowed to output something which is "maybe null" when its corresponding input is in the "not null" state. At the point in the code where the diagnostic is expected, both of the inputs are in the "maybe null" state.

To fix this, we would probably have to introduce something like "definitely null" flow state in addition to the existing "maybe null" and "not null" flow states. Then it becomes disallowed to output something which is "definitely null" or "maybe null" when its corresponding input is not in the "definitely null" state. However, adding a new flow state is expensive to do from an engineering standpoint compared to the benefit of improving this particular analysis. (Some further design work would probably be needed here since this would result in the function [return: NotNullIfNotNull("x")] string? func(string? x) => x; getting a warning.)

I would say that if we could identify a whole category of high-impact bugs that people are hitting due to the lack of a "definitely null" state, it could be worth investing in adding it.