CommunityToolkit / CommunityToolkit/dotnet

RelayCommand<T>.CanExecute(null) returns false for value types without a predicate

Open Beginner friendly
#1,206 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

`RelayCommand.CanExecute(object?)` unconditionally returns `false` when `T` is a non-nullable value type (e.g. `int`, enums) and the `parameter` is `null` — even when no `canExecute` predicate was provided to the constructor.

This violates the documented contract: *"The default return value for the CanExecute method is `true`."* A command constructed without a predicate is meant to always be executable, regardless of the parameter value — including `null`.

In WPF applications on .NET Framework, the framework may call `CanExecute(null)` during control initialization before the `CommandParameter` binding has produced a value. With the unconditional `return false`, commands without predicates (e.g. `RelayCommand(DoSomething)`) are permanently disabled.

**Current code** (`src/CommunityToolkit.Mvvm/Input/RelayCommand{T}.cs`):

```csharp
if (parameter is null && default(T) is not null)
{
return false; // unconditional — even when no predicate exists
}
```

### Regression

_No response_

### Steps to reproduce

```csharp
// 1. Create a RelayCommand without a predicate
var command = new RelayCommand(i => Console.WriteLine(i));

// 2. Call CanExecute with null (as WPF does during initialization)
bool result = ((System.Windows.Input.ICommand)command).CanExecute(null);

// 3. Observe the result
Console.WriteLine(result); // Actual: False, Expected: True
```

### Expected behavior

`CanExecute(null)` should return `true` when no `canExecute` predicate was provided, because:
1. The class documentation states the default return value is `true`
2. Without a predicate, the command has no condition to evaluate — it should always be executable
3. The `null` guard's purpose is to prevent a `null` from being incorrectly mapped to `default(T)` (e.g., 0 as SelectedIndex), but when there's no predicate, there's nothing to incorrectly map to

For commands *with* a predicate, returning `false` is correct — we cannot guess the user's intent. This proposal keeps that behavior unchanged.

### Screenshots

_No response_

### IDE and version

Other

### IDE version

Insiders [12020.428]

### Nuget packages

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

### Nuget package version(s)

latest stable

### Additional context

- Related discussion: [WindowsCommunityToolkit #3619](https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/3619). That issue correctly identified that mapping `null` to `default(T)` for commands *with* predicates is unsafe. However, the resulting unconditional `return false` is overly broad — it also disables commands without predicates, which have no such ambiguity.
- The proposed fix is a one-line change: `return false;` → `return this.canExecute is null;`
- `TryGetCommandArgument`、`ThrowArgumentExceptionForInvalidCommandArgument`、`Execute(object?)` remain unchanged.

### Help us help you

Yes, I'd like to be assigned to work on this item

Contributor guide

Open the contributing guide

Research direction

Start with src/CommunityToolkit.Mvvm/Input/RelayCommand{T}.cs and inspect the null-parameter guard and the optional canExecute predicate. Verify the repro through ICommand.CanExecute(null), then confirm that predicate-free value-type commands return true while commands with predicates retain the documented false behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
developer-experience
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.