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

MaybeNullAttribute not working #36986

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

MaybeNullAttribute not working #36986

JamesNK opened this issue Jul 4, 2019 · 5 comments

Comments

@JamesNK
Copy link
Member

JamesNK commented Jul 4, 2019

I'm converting Newtonsoft.Json to use nullable types and I can't get MaybeNullAttribute 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 MaybeNullAttribute 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 JsonConvert.DeserializeObject method (JsonConvert.cs, line 787)

Expected Behavior:

There should not be a nullable warning because [return: MaybeNull] is on the return type.

Actual Behavior:

There is a warning casting object? to T.

image

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jul 5, 2019

I think this example should not warn:

#nullable enable
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics.CodeAnalysis
{
    class MaybeNullAttribute : System.Attribute { }
}

public class C {
    [return: MaybeNull]
    public T M<T>(object? o) {
        return (T)o; // gives warning CS8601, but probably shouldn't
    }
}

/cc @jcouv

@JamesNK
Copy link
Member Author

JamesNK commented Jul 5, 2019

This also shouldn't warn:

public class C {
    [return: MaybeNull]
    public T M<T>() {
        return default;
    }
}

@jcouv jcouv modified the milestones: 16.3.P2, 16.3 Jul 8, 2019
@jcouv
Copy link
Member

jcouv commented Jul 8, 2019

Thanks for reporting this.
This is currently by-design. The attributes ([AllowNull] and such) currently only affect callers. They do not affect implementations or OHI yet. This is tracked by #36039

@jcouv jcouv closed this as completed Jul 8, 2019
@JamesNK
Copy link
Member Author

JamesNK commented Jul 8, 2019

What should I do in this situation? Suppress CS8601?

@jcouv
Copy link
Member

jcouv commented Jul 8, 2019

Use the suppression operator. For example: return default!;.

mavasani added a commit to mavasani/roslyn that referenced this issue Dec 11, 2019
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

3 participants