Search code examples
c#wpfnullablecommunity-toolkit-mvvm

What is the common way to handle CS8602 in a WPF RelayCommand?


If a nullable property gets used in the Execute part of a RelayCommand, and that property is already checked against null in the CanExecute method, what is the proper way to handle the CS8602: Possible dereference of null warning for the property?

[ObservableProperty, NotifyCanExecuteChangedFor(nameof(FooCommand)]
private FooType? _fooObject;

[RelayCommand(CanExecute = nameof(CanExecuteFooCommand)]
private void Foo()
{
    SomeMethod(FooObject.ID);    // gives CS8602: Possible dereference of null
}

private bool CanExecuteFooCommand()
{
    return FooObject is not null;
}

private void SomeMethod(Guid? fooObject)
{
}

Which of the ways would you consider the correct way?

Another null check of FooObject in Execute?

Use of nullable type ? in SomeMethod(FooObject?.ID)

Use of the null forgiving operator ! in SomeMethod(FooObject!.ID)

Or is there any other way i don't know of?

Could FooObject ever become null in between the CanExecute and Execute part?


Solution

  • The code CS8602 is just a warning from the static analyzer. It does not analyze your code semantically. It does not know that the variable will never be null - at least at this point of your code or execution path. You marked the variable FooObject as nullable and hence the analyzer shows the warnings for a "Possible dereference of null".

    You can use the null-forgiving operator ! to tell the analyzer that you are aware of the risk but know that, although theoretically possible, at this point the value will never be null.

    Note that the null-forgiving operator will only silence the analyzer. It does not add robustness to your code.
    For example, if you refactor your code and for example change the flow, which now allows the variable to be null, then your code will throw. So, there is always the risk that the guarantee of the variable being never null becomes void after some future code changes. I think it should be used sparingly or carefully for this reason as the analyzer nullable checks can be very helpful.

    I guess most of the time using compiler checks to satisfy the analyzer adds the necessary robustness. For example, pattern matching is a nice way. There are many ways to check for null.
    Throwing ArgumentNullException exceptions is also an option that adds robustness as this prevent null parameter values (and also satisfy the analyzer).

    "Could FooObject ever become null in between the CanExecute and Execute part?"

    If you can guarantee that ICommand.CanExecute is always called directly before ICommand.Execute (which is the case for WPF ICommandSource implementations like the Button and the member variables, that your Execute implementation references, are not shared between threads, then you could safely use the null-forgiving operator in this case. Otherwise, I would explicitly perform a compiler check.

    private FooType? _fooObject;
    
    private void Foo()
    {
      // To add robustness, especially in anticipation of future changes, 
      // you probably want to let the compiler check the property 
      // e.g. by using pattern matching
      if (_fooObject is not null)
      {
        SomeMethod(_fooObject.ID);
      }
    }
    
    // Guid is a struct. As such, it can NEVER be NULL!
    // The NULL value would be implicitly converted to default(Guid)
    // However, when you make it nullable you must check the parameter to avoid bugs
    private void SomeMethod(Guid? fooObject)
    {
      if (fooObject == Guid.Empty || fooObject == default)
      {
        return;
      }
    }