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

AllowNullAttribute not working #37024

Closed
JamesNK opened this issue Jul 5, 2019 · 2 comments
Closed

AllowNullAttribute not working #37024

JamesNK opened this issue Jul 5, 2019 · 2 comments
Labels
Area-Compilers Area-Language Design Feature - Nullable Reference Types Nullable Reference Types Resolution-By Design The behavior reported in the issue matches the current design
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Jul 5, 2019

Similar to #36986


I'm converting Newtonsoft.Json to use nullable types and I can't get AllowNullAttribute to work. My remaining warnings are related to generic types.

Note that I have many old targets so I'm using internal nullable attributes (located in NullableAttributes.cs). NotNullAttribute and NotNullWhenAttribute work fine, but I can't get AllowNullAttribute to work. I've tried it in System.Runtime.CompilerServices and System.Diagnostics.CodeAnalysis namespaces with no result.

Version Used: VS2019 + Microsoft.Net.Compilers.Toolset=3.2.0-beta3-final

Steps to Reproduce:

  1. git clone https://github.com/JamesNK/Newtonsoft.Json.git
  2. git checkout jamesnk/nullable-types
  3. Go to JsonConverter<T>.WriteJson method (JsonConverter.cs, line 97)

Expected Behavior:

There should not be a nullable warning because [AllowNull] is on the parameter type.

Actual Behavior:

There is a warning casting object? to T.

image

@jcouv jcouv added this to the 16.3 milestone Jul 8, 2019
@jcouv
Copy link
Member

jcouv commented Jul 11, 2019

The warning seems to be on converting from object? to T (which I assume is unconstrained).
Here's a self-contained repro which hopefully represents your situation: sharplab.
This is currently by-design and the only mitigation is to suppress with !. If T were substituted with non-nullable type string, then the warning would be there. So it should be there for T before substitution.

@jcouv jcouv modified the milestones: 16.3, Compiler.Next Jul 11, 2019
@jcouv jcouv added the Resolution-By Design The behavior reported in the issue matches the current design label Jul 11, 2019
@jcouv jcouv modified the milestones: Compiler.Next, Backlog Jul 18, 2019
@jcouv jcouv closed this as completed Dec 11, 2019
@jcouv
Copy link
Member

jcouv commented Dec 11, 2019

Updated notes above. This is by-design (assigning maybe-null value to a T which may be non-nullable, such as string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Area-Language Design Feature - Nullable Reference Types Nullable Reference Types Resolution-By Design The behavior reported in the issue matches the current design
Projects
None yet
Development

No branches or pull requests

2 participants