dotnet / dotnet/roslyn

Blazor Analyzer/code fix: Add EditorRequiredAttribute to Blazor parameter when using null forgiving operator

Open
#85,606 4 comments 0 reactions 0 assignees View on GitHub
Area-Razor
Dominant language
C#
Stars
20.7k
Forks
4.3k
PR merge metrics
PR metrics pending

Description

## Background and Motivation

When using Blazor, component parameters are declared like this:

```csharp
[Parameter]
public string MyParameter { get; set; }
```

However, this produces a null warning, so typically we put `= default!` or `= null!` to suppress the null warning **if the parameter is intended to be a required parameter**.

```csharp
[Parameter]
public string MyParameter { get; set; } = default!;
```

But the only way to guarantee that the parameter is actually being set is the `EditorRequiredAttribute`.

```csharp
[Parameter]
[EditorRequired]
public string MyParameter { get; set; } = default!;
```

Allowing programmers to override the null warning for Blazor parameters without also adding the `EditorRequiredAttribute` can lead to NullReferenceExceptions when people forget to set the parameter value when using the component.

## Proposed Analyzer

### Analyzer Behavior and Message

The analyzer will trigger for a property when all of these are true:

- In a nullable context
- Has the Blazor `ParameterAttribute` applied
- The type does not have the nullable indicator
- The property is initialized with ` = default!` or ` = null!` to override the null warning

Proposed message: "Component Parameter '{0}' is initialized as non-nullable but does not have EditorRequiredAttribute to ensure that it is set"

There can be 2 separate code fix options:

- Make parameter nullable (removes the `= default!` and adds the nullable indicator `?` to the type
- Add `EditorRequiredAttribute`

### Category

- [ ] Design
- [ ] Documentation
- [ ] Globalization
- [ ] Interoperability
- [ ] Maintainability
- [ ] Naming
- [ ] Performance
- [ ] Reliability
- [ ] Security
- [ ] Style
- [x] Usage

### Severity Level

- [ ] Error
- [x] Warning
- [ ] Info
- [ ] Hidden

## Usage Scenarios

Bad:

```csharp
[Parameter]
public string MyParameter { get; set; } = default!;
```

Bad:

```csharp
[Parameter]
public string MyParameter { get; set; } = null!;
```

Bad:

```csharp
[Parameter]
public required string MyParameter { get; set; }
```

"Add EditorRequiredAttribute" Code fix result (good):

```csharp
[EditorRequired]
[Parameter]
public string MyParameter { get; set; } = default!;
```

```csharp
[Parameter, EditorRequired]
public string MyParameter { get; set; } = default!;
```

## Risks

For the analyzer, none. I can't imagine why anybody would want to override the null check of a Blazor parameter without requiring it to be set when the component is used.

The code fix may not work as intended for all people. Some other analyzers (ex: StyleCop) enforce preferences about whether or not to keep both attributes on one line, or split them into two lines. However, this seems minor, since they can do another code fix with the other analyzer for the attribute preferences. I suggest the code fix adds the attribute on a separate line by default, since it's better in git diffs.

Overall, this proposed solution reduces risks, since it reduces the risk of runtime NullReferenceExceptions in Blazor apps.

Contributor guide

Open the contributing guide

Research direction

No implementation files, tests, or entry points are named. Start by translating the proposed trigger conditions and two code-fix outcomes into the Roslyn analyzer and code-fix test structure; done means warnings appear only for the listed non-nullable Blazor parameters initialized with default! or null!, and both fixes produce the shown forms.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
devtools
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.