dotnet / dotnet/roslyn

Blazor Analyzer/code fix: Inject properties should be declared non-nullable and with null forgiving operator

Open
#85,605 3 comments 0 reactions 1 assignee Claimed by @chsienki View on GitHub
Area-Razor
Dominant language
C#
Stars
20.7k
Forks
4.3k
PR merge metrics
PR metrics pending

Description

## Background and Motivation

This is typically how we declare `Inject` services with Blazor (private or public is a separate question, but I'll assume public for all of my examples):

```csharp
[Inject]
public IMyService MyService { get; set; } = default!;
```

Any other variation of this makes no sense.

Someone might try to do something like this as a first attempt (you see this in a lot of tutorials where people are not taking into account the nullability checking):

```csharp
[Inject]
public IMyService MyService { get; set; }
```

Then it gives them a null warning, so they may not understand how to correctly declare it to avoid the warning. Someone new may do something like this:

```csharp
[Inject]
public IMyService? MyService { get; set; }
```

Which compiles with no warnings, but then they must check if it's null anywhere they use it.

The methods to register services, such as `AddSingleton` do not accept null values, so having it be nullable would never make sense.

## Proposed Analyzer

### Analyzer Behavior and Message

My suggestion is to make an analyzer to ensure all properties with `InjectAttribute` (when in a nullable context):
* are declared as non-nullable
* have the null-forgiving operator (` = default!` or ` = null!`)

These kinds of context-specific analyzers could be helpful for people who struggle with adopting best practices when it comes to nullability.

Proposed message: "Inject property '{0}' should be declared as non-nullable and have the null forgiveness operator"

There can be a code fix for correcting this as well.

### Category

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

### Severity Level

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

## Usage Scenarios

Bad:

```csharp
[Inject]
public IMyService? MyService { get; set; }
```

Bad (there will also be the regular null warning in this case, but having this warning as well would be helpful for giving people better information on the correct fix):

```csharp
[Inject]
public IMyService MyService { get; set; }
```

Good:

```csharp
[Inject]
public IMyService MyService { get; set; } = default!;
```

## Risks

For code cleanup purposes, I don't see any reason why the `Inject` properties should be declared any differently.

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.