Roslyn: Incorrect nullability warning in local function

[JohanLarsson]

void M()
{
    if (Try(out var s))
    {
        int Local()
        {
            //     ↓ CS8602 here with 16.4
            return s.Length;
        }
    }
}

bool Try([NotNullWhen(true)] out string? text)
{
    text = string.Empty;
    return true;
}

[jnm2]

I think this is https://github.com/dotnet/roslyn/issues/38548?

[JohanLarsson]

Looks similar, I would add tests for both.

[jcouv]

This is a problem with analysis of local functions, which doesn't seem particularly related to the attribute.
Here's a repro without attribute:

using System.Diagnostics.CodeAnalysis;

#nullable enable
class C
{
void M(string? s)
{

    if (s is null)
    {
    }
    else
    {
        int Local()
        {
            //     ↓ CS8602 here with 16.4
            return s.Length;
        }
    }
}

bool Try([NotNullWhen(true)] out string? text)
{
    text = string.Empty;
    return true;
}
}

[RikkiGibson]

When a local function is never used, it is analyzed using variable state from the declared type of the variables. I think we should consider instead handling the case where the local function is never used by analyzing it with the state at the point of its declaration. When the code is changed to contain one or more uses of the local function, we can use our existing strategy of Joining the states at the usage points into the top state. That is to say, we would no longer care where the local function is declared when determining variable state within the local function.

[RikkiGibson]

There is at least one other issue with the same root cause as this which I'd like to dupe against this.

[jnm2]

@RikkiGibson I agree with @mikernet's suggestion in Gitter (A, B) that local functions should not warn about nullability of variables in the containing method until they begin to be called. Maybe with the exception of variables that would cause warnings no matter where the local function could be called.