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.74k stars 3.99k forks source link

No reliable way to tell if a reference type is nullable from a source generator #49555

Open trampster opened 3 years ago

trampster commented 3 years ago

Version Used: 5.0.100

Roslyn doesn't seem to provide a reliable way of determining if NRT is enabled for a ITypeSymbol

Steps to Reproduce:

  1. Create a project with NRT turned off
  2. Add an assembly level attribute like the following: [assembly: JsonArray(typeof(string))]
  3. In a different file turn on NRT with: #nullable enable and then off again with #nullable restore
  4. In a Source Generator get a ITypeSymbol to the string from the attribute
  5. Notice that ITypeSymbol.NullableAnnotation returns 'NotAnnotated' when it should be 'None'
  6. Remove the #nullable enable and #nullable restore from the other file
  7. Notice ITypeSymbol.NullableAnnotation now is 'None'

Expected Behavior:

Actual Behavior:

The help on NullableAnnotation is kind of vague about what 'None' means:

        //
        // Summary:
        //     The expression has not been analyzed, or the syntax is not an expression (such
        //     as a statement).
        //
        // Remarks:
        //     There are a few different reasons the expression could have not been analyzed:
        //     1) The symbol producing the expression comes from a method that has not been
        //     annotated, such as invoking a C# 7.3 or earlier method, or a method in this compilation
        //     that is in a disabled context. 2) Nullable is completely disabled in this compilation.
        None = 0,

If None cannot be used to determine that NRT is off or on, then Roslyn needs to provide a way to determine this.

I run into this problem writing JsonSrcGen which is a Json Source Generator.

Ideally I would want to detect the the property type is non nullable and then throw an exception if the Json is null for that property. By making the property type non nullable the user is telling me that they don't expect null in the Json and therefore if there is this is an error (so an exception is the correct thing)

333fred commented 3 years ago

Assembly-level attributes don't really belong to the place they're defined, they belong to the assembly. As such, I wouldn't necessarily expect the type symbol that you get from such an attribute to have the same nullability in which it was defined. Do you encounter these issues in a place where this determination actually matters, such as a user-defined property that you're trying to source generate around?

Also, you can determine the exact nullable context of any code position by using SemanticModel.GetNullableContext(int position).

trampster commented 3 years ago

I'm generating a json serializer/deserializer for the array with the type specified by:

[assembly: JsonArray(typeof(string))]

I need to know if I am generating for a non nullable string or a nullable string. So the determination does actually matter here.

sharwell commented 3 years ago

I need to know if I am generating for a non nullable string or a nullable string

There is no such thing as typeof(string?), so I'm not sure what the question here is.

trampster commented 3 years ago

Source Generators open up a whole new world of possibilities.

In this case, I am using the existence of that attribute as an instruction to generate a json serializer and deseralizer for string[] or string?[]

I need some way for the consumer of my source generator to tell me that they want me to generate a json serializer/deseralizer for a specific array type.

I'm open to other methods of doing this, however using assembly level attributes is common amongst other source generators I have seen.

333fred commented 3 years ago

In this case, I am using the existence of that attribute as an instruction to generate a json serializer and deseralizer for string[] or string?[]

Sam's point is that typeof(string?) is not valid C#. So yes, you'll need a different manner of indicating that a type is nullable for your generator to key off of. Without more context, it's hard to make any specific recommendations.

trampster commented 3 years ago

I wasn't aware of the typeof() nullable limitation in c#.

I will have to look for an alternative approach.

I'll have a play with the other property based generators and see if I can reliably determine nullability on those.