Skip to content

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

Closed

Description

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

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

Metadata

Assignees

No one assigned

    Labels

    analyzer 👓A new analyzer being implemented or updatedfeature request 📬A request for new changes to improve functionalitymvvm-toolkit 🧰Issues/PRs for the MVVM Toolkit

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions