roslyn icon indicating copy to clipboard operation
roslyn copied to clipboard

MemberNotNullWhenAttribute doesn't work on properties from base class?

Open SoftCircuits opened this issue 4 years ago • 8 comments

Version Used

Visual Studio 2019 Version 16.11.3

Steps to Reproduce

My class has a Read() method that guarantees the CurrentLine property is not null when the method returns true.

[MemberNotNullWhen(true, "CurrentLine")]
public bool Read()
{
    // ...
}

This works fine. But now I have a class that derives from the class above. And the derived class has a Read2() method that also guarantees the CurrentLine property is not null when the method returns true.

[MemberNotNullWhen(true, "CurrentLine")]
public bool Read2()
{
    return base.Read();
}

Expected Behavior

The MemberNotNullWhen() attribute indicates that CurrentLine is not null when the method returns true.

Actual Behavior

The MemberNotNullWhen() attribute gives a warning.

Warning CS8776 Member 'CurrentLine' cannot be used in this attribute.

It appears this attribute doesn't work with members from the base class. But here the logic is perfectly valid. Without being able to use the attribute this way, the calling code will get an warning if it uses CurrentLine without checking for null in cases where it can never be null.

Also, there is absolutely no information available for this warning.

SoftCircuits avatar Sep 19 '21 17:09 SoftCircuits

This is currently by-design (we have an explicit test MemberNotNull_Inheritance_SpecifiedOnDerived for this scenario). But I don't remember what was the rationale. I'll dig this up. Also, we could improve the diagnostic message.

jcouv avatar Sep 21 '21 08:09 jcouv

Found the notes on MemberNotNull design: https://github.com/dotnet/roslyn/pull/41675 I'll keep this issue open to add documentation for MemberNotNull and to improve the diagnostic.

jcouv avatar Sep 21 '21 08:09 jcouv

Thanks. For clarification, do your notes indicate you'll just work on improving the warning, or is there a chance this could also be changed? (Not sure I understood the relevant notes in #41675.)

SoftCircuits avatar Sep 22 '21 16:09 SoftCircuits

We're not planning to change this at the moment. But if more users find this a problem, they should comment below and I'll bring this up for LDM discussion.

jcouv avatar Sep 23 '21 06:09 jcouv

I've reached this trying to implement following case

class Base { public string? BaseProperty { get; set; } }

class Derived : Base
{
    // CS8776: Member cannot be used in this attribute.
    [MemberNotNullWhen(true, nameof(BaseProperty))]
    public bool Condition => true;
}

class UseCase
{
    void Do()
    {
        var derived = new Derived();
        if (derived.Condition)
        {
            // CS8600: Converting null literal...
            string value = derived.BaseProperty;
        }
    }
}

From my point of view I see no reason to restrict derived class to do such promises (unless we cast to base), so maybe it worth some discuss

etatuev avatar Sep 26 '23 11:09 etatuev

I'm encountering this in Avalonia. I've created a custom TextBox control (public class NumericTextBox : TextBox). They've declared TextBox.Text as string? for whatever reason. I have a method that definitively sets Text to a non-null string, so I'd like to annotate it as [MemberNotNull(nameof(Text))], but I get a CS8776 warning:

[MemberNotNull(nameof(Text))] // WARNING: CS8776
private void SetTextFromValue()
{
    Text = ...;
}

The LDM notes make no mention of why this restriction exists, besides the original proposal having that restriction. And that original proposal doesn't exist in @dotnet/csharplang as far as I can tell. IMHO, it makes no sense why this restriction would even be there. Why would a derived type not be able to say it definitively sets a value as non-null? Sure, Avalonia could update something in the base type that accidentally causes SetTextFromValue to not ensure Text != null, but that can also happen with code I wrote myself.

colejohnson66 avatar Mar 11 '25 15:03 colejohnson66

I've also encountered this when using .net core identity; I wanted to have the Username (from IdentityUser, so no possibility for me to change anything) to be a combination of a user-supplied FirstName and LastName:


public class ApplicationUser : IdentityUser {
    private string _firstName = string.Empty;
    private string _lastName = string.Empty;

    public required string FirstName {
        get => _firstName;
        set {
            _firstName = value;
            SetUsername();
        }
    }

    public required string LastName {
        get => _lastName;
        set {
            _lastName = value;
            SetUsername();
        }
    }

    [MemberNotNull(nameof(UserName))]
    private void SetUsername() => UserName = $"{_firstName.Normalize().ToLowerInvariant()}.{_lastName.Normalize().ToLowerInvariant()}";
}

And this gives CS8776. Not sure about the rationale either; from a layman's point of view it seems semantically correct to be able to tell the compiler that a member in a base class has been set by a member in a derived class?

jonorogers avatar Apr 09 '25 01:04 jonorogers

I am hitting this exact issue when attempting inheritence on the Result pattern.

I have an IResult base as such, for an itemless result

public interface IResult
{
    public bool Success { get; }

    [MemberNotNullWhen(false, nameof(Success))]
    public Exception? FailureException { get; }
}

and a derived IResult<TItem> as such, for an item-y result

public interface IResult<TItem> : IResult
{
    TItem? Item { get; }
}

However adding [MemberNotNullWhen(true, nameof(Success))] to the Item member presents the described message. I really want to be able to not have to null check Item when I've already checked for Success to keep TreatWarningsAsErrors at bay for a potential "null reference exception" when by design in our system Item is never null when Success is true and the type is IResult<TItem>

JSparshottO2 avatar Dec 03 '25 11:12 JSparshottO2