Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
trampster opened this issue Nov 22, 2020 · 6 comments

Comments

@trampster
Copy link

trampster commented Nov 22, 2020

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:

  • ITypeSymbol.NullableAnnotation should return 'None' if NRT is off for that section of code

Actual Behavior:

  • ITypeSymbol.NullableAnnotation returns 'NotAnnotated'

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.

  • If I don't' annotate my code the user will get warnings.
  • If I annotate my generated Json code to make everything nullable and the user makes there property non nullable then my generated code will produce a warning about possible null assignment.
  • If I suppress this warning then the users property could end up being set to null which may be surprising to the user.
  • I can't assume non nullable because then I won't support null in the json ever.

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)

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 22, 2020
@trampster trampster changed the title No way to reliably tell if a reference type is nullable from a source generator No way to reliable tell if a reference type is nullable from a source generator Nov 22, 2020
@trampster trampster changed the title No way to reliable tell if a reference type is nullable from a source generator No reliable way to tell if a reference type is nullable from a source generator Nov 22, 2020
@jaredpar jaredpar added Bug New Feature - Source Generators Source Generators and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 24, 2020
@jaredpar jaredpar added this to the 16.9 milestone Nov 24, 2020
@333fred
Copy link
Member

333fred commented Jan 25, 2021

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

trampster commented Jan 26, 2021

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
Copy link
Member

333fred commented Jan 26, 2021

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
Copy link
Author

trampster commented Jan 26, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants