CommunityToolkit / CommunityToolkit/dotnet

Incorrect partial method prototypes for generated observable properties

Open
#1,109 0 comments 0 reactions 0 assignees View on GitHub
bug :bug:
Dominant language
C#
Stars
3.8k
Forks
400
PR merge metrics
No merged PRs in 30d

Description

### Describe the bug

When decorating a property of a reference or otherwise nullable type with `[ObservableProperty]` the two-argument partial property-changing and property-changed method prototypes are generated differently than for decorated fields, not considering that the `oldValue` argument _may_ be null.

### Regression

_No response_

### Steps to reproduce

1. Create a new project targeting .NET 9.0 and depending on CommunityToolkit.Mvvm
2. Create a class inheriting from `ObservableObject`
3. Add the following code to the class:
```
[ObservableProperty]
private object _testA;

[ObservableProperty]
public object TestB { get; set; }

partial void OnTestAChanged(object? oldValue, object newValue) { /* do nothing */ }

partial void OnTestBChanged(object? oldValue, object newValue) { /* do nothing */ }
```
4. Note that `OnTestBChanged` has warning CS8826 on it, because the generated partial declaration is `partial void OnTestBChanged(global::System.Object oldValue, global::System.Object newValue);` but there's no such warning for `OnTestAChanged`.

### Expected behavior

The OnChanging(oldValue,newValue) and OnChanged(oldValue,newValue) partial method declarations for properties decorated with ObservablePropertyAttribute will be generated the same as for fields decorated with same.

### Screenshots

_No response_

### IDE and version

VS 2022

### IDE version

17.14.9

### Nuget packages

- [ ] CommunityToolkit.Common
- [ ] CommunityToolkit.Diagnostics
- [ ] CommunityToolkit.HighPerformance
- [x] CommunityToolkit.Mvvm (aka MVVM Toolkit)

### Nuget package version(s)

8.4.0

### Additional context

The `oldValue` argument should always be generated as nullable for reference types as the initial call to the methods may be due to setting the property in the constructor (e.g. initial value may be based on a constructor argument, or the change methods may be used to set up and remove event handlers on the property value).

### Help us help you

No, just wanted to report this

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.