[ObservableProperty] with OnPropertyChanged partial(s) which update additional properties mask nullability analysis - CS8618

[hawkerm]

Describe the bug

Seems like extended scenario/case from #645? Where one property modified in the constructor will update one or more other properties through the On'Property'Changed (or similar partial method callbacks).

Not entirely sure but feel like this should work (in theory). Let me share by example:

public partial class MyObject
{
    [ObservableProperty]
    ////[NotifyPropertyChangedFor(nameof(Coordinates))] // Realizing now this is not essential (I mean it is to my code's scenario/function, but not to the repro here.)
    private Vector3 _position;

    // Compiler complains about this property not being set, but it always will with this code (by OnPositionChanged partial method).
    public Vector2 Coordinates { get; private set; }

    // CS8618 on line below: Non-nullable property 'Coordinates' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
    public MyObject(Vector3 position)
    {
        Position = position;
        ////OnPositionChanged(null, Position); // If I add this and the extra MemberNotNull attribute explicitly below, then it's fine - but redundant.
    }

    ////[MemberNotNull(nameof(Coordinates))] // Tried this, didn't make a difference, unless I explicitly called it in the constructor (again)
    partial void OnPositionChanged(Vector3? oldValue, Vector3 newValue)
    {
        Coordinates = new(Position.X, Position.Y);
    }
}

So, in my constructor, I set Position (This seems to be what #645 fixed), but in this case, that will also call my OnPositionChanged method, which then sets Coordinates. So, it explicitly won't be null in any case here, but that's not seen by the compiler...

I tried decorating my OnPositionChanged method with [MemberNotNull(nameof(Coordinates))], but that didn't help... unless I also just recalled the OnPositionChanged method in the constructor (see commented code above in the example). But then that's still calling the OnPositionChanged method twice...

Regression

No response

Steps to reproduce

1. Setup a new C# project with 8.3 version of the MVVM Toolkit
2. Copy in the above code block into a new file
3. See CS8618 compiler warning

Expected behavior

No compiler warning, ideally without additional changes.
Acceptable if I have to annotate the OnChanged partial method myself for this scenario, I guess? I'm still wrapping my mind around how these annotations work a bit... 😋

Screenshots

No response

IDE and version

VS 2022

IDE version

17.11.1

Nuget packages

• CommunityToolkit.Common
• CommunityToolkit.Diagnostics
• CommunityToolkit.HighPerformance
• CommunityToolkit.Mvvm (aka MVVM Toolkit)

Nuget package version(s)

8.3.0

Additional context

Is this analysis that the MVVM source generator has to do extra for this type of scenario or is there something else in .NET that can help with this? I don't think partial properties changes anything here with this scenario in the future either?

Ah, also just saw #846, so not sure if there was some regression from #645 as well?

Help us help you

Yes, but only if others can assist

[Sergio0694]

This is not a bug, it's expected behavior. You need [MemberNotNull(nameof(Coordinates))] on the property setter of the generated property to fix this. This will be supported just fine with partial properties (#555), but it's not currently supported for annotated fields. I could add support for it I guess, just not entirely sure it's worth the extra complexity 🤔

The nice thing is that it would be able to reuse all the same code to forward attributes at least.
Basically the correct version of your code would be the following:

public partial class MyObject
{
    [ObservableProperty]
    [set: MemberNotNull(nameof(B))]
    private string _a;

    public MyObject()
    {
        A = "";
    }

    public string B { get; private set; }

    [MemberNotNull(nameof(B))]
    partial void OnPositionChanged(string? oldValue, string newValue)
    {
        B = "";
    }
}