dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.99k stars 4.03k forks source link

Nullable reference types: string.IsNullOrEmpty / IsNullOrWhitespace not recognized #9977

Closed bkoelman closed 6 years ago

bkoelman commented 8 years ago

I have been playing with nullable reference types, which is already working remarkably great! To try it out, I have patched up my analyzer to convert Resharper's (Item)NotNull/CanBeNullAttribute annotations to nullable reference types. Then ran that on an actual annotated codebase.

During that process I discovered some oddities, which I'll create issues for here.

Version Used: Built from source, branch NullableReferenceTypes (commit https://github.com/dotnet/roslyn/commit/8553cdac75c2b0901e8ae401aaa23022f0689935)

Steps to Reproduce: The next block of code raises warning 8202: Possible dereference of a null reference:

public class Test
{
    public static void M(string? text)
    {
        if (string.IsNullOrWhiteSpace(text))
        {
            return;
        }
        string trimmed = text.Trim(); // CS8202
    }
}

Expected Behavior: No CS8202 warning, because string.IsNullOrWhiteSpace (and string.IsNullOrEmpty) returns true if text is null, which makes the text.Trim() line unreachable.

I believe these two methods are so commonly used that it's worth special-casing them.

Actual Behavior: CS8202 warning on the text after the if statement.

@AlekseyTs

jcouv commented 6 years ago

Confirmed this was fixed.

A very small set of APIs are recognized at the moment (Debug.Assert, IsNullOrEmpty, ...). We'll likely have a more general solution. I'll go ahead and close this issue. Thanks

qidydl commented 3 years ago

Has there been a regression with this lately? I'm seeing this same problem in Visual Studio 2019 16.9.0 / .NET SDK 5.0.200:

image

Edit: replaced example with one that uses a local variable, so there's no property de-reference that might be confusing things.