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.97k stars 4.03k forks source link

GetTypeInfo reports no nullability when deconstructing #59875

Open shuebner opened 2 years ago

shuebner commented 2 years ago

Version Used: SDK 6.0.200 Microsoft.CodeAnalysis.CSharp.Workspaces 3.11

Steps to Reproduce:

This is not a truly minimal example, but the smallest use-case with which I can readily confirm the behavior:

// this is part of a nullability-enabled context (enabled on the project level)
public static class Reproduce
{
    public static void HandleEither(Either either)
    {
        // TypeInfo of either has NullabilityInfo Annotation None and FlowState None (unexpected)
        // SymbolInfo Symbol of either has NullableAnnotation NotNull (inconsistent with TypeInfo)
        (int v1, int v2) = either switch
        {
            Either.Left => (1, 2),
            Either.Right => (3, 4),
        };

        // TypeInfo of either has Annotation Annotated and FlowState NotNull (as expected)
        var v = either switch
        {
            Either.Left => (1, 2),
            Either.Right => (3, 4),
        };
    }
}

// this has nothing to do with the behavior, but only with triggering my diagnostic suppressor on whose tests the behavior occurs
public abstract class Either
{
    Either() { }

    public sealed class Left : Either { }
    public sealed class Right : Either { }
}

Expected Behavior: TypeInfo of either reports nullability as Annotation Annotated and FlowState NotNull in both cases. TypeInfo of either reports nullability consistently with its Symbol.

Actual Behavior: TypeInfo of either reports nullability differently when either is deconstructed. TypeInfo of either reports nullability differently from SymbolInfo.

Przemyslaw-W commented 1 year ago

Related, maybe even manifestation of the same bug: Given the code:

namespace ClassLibrary1ForNuget
{
    public class Class1
    {
        private void MethodName1()
        {
            Class1? instance = null;
            instance.MethodName2();
        }
        private void MethodName2()
        {
        }
    }
}

Assuming I have instance of IdentifierNameSyntax called "name" representing the "instance" local variable: semanticModel.GetSymbolInfo(name).Symbol returns ILocalSymbol with the the type properly annotated with nullable annotation semanticModel.GetTypeInfo(name).Type returns INamedTypeSymbol which represents the ClassLibrary1ForNuget.Class1 type, but it is not annotated with nullable annotation

I believe TypeInfo should properly carry the nullable annotation

badeend commented 4 months ago

I can reproduce the bug. I currently use the following code to work around it:

private static NullableFlowState GetExpressionNullability(SyntaxNodeAnalysisContext context, ExpressionSyntax expression)
{
    var flowState = context.SemanticModel.GetTypeInfo(expression).Nullability.FlowState;
    if (flowState != NullableFlowState.None)
    {
        return flowState;
    }

    // Ideally, `flowState` should have been all we needed. Sadly, Roslyn thinks otherwise... :)
    // See: https://github.com/dotnet/roslyn/issues/59875
    return context.SemanticModel.GetSymbolInfo(expression).Symbol switch
    {
        IParameterSymbol s => AnnotationToFlowState(s.NullableAnnotation),
        ILocalSymbol s => AnnotationToFlowState(s.NullableAnnotation),
        IPropertySymbol s => AnnotationToFlowState(s.NullableAnnotation),
        IFieldSymbol s => AnnotationToFlowState(s.NullableAnnotation),
        _ => NullableFlowState.None,
    };

    static NullableFlowState AnnotationToFlowState(NullableAnnotation annotation) => annotation switch
    {
        NullableAnnotation.NotAnnotated => NullableFlowState.NotNull,
        NullableAnnotation.Annotated => NullableFlowState.MaybeNull,
        _ => NullableFlowState.None,
    };
}