SonarSource/sonar-dotnet

Fix S6964 FP: Should not raise for reference properties in nullable context

Closed this issue · 3 comments

Description

The rule S6964 should not be active for model properties in a nullable aware context (where nullable reference types are enabled). This is because of:

By default, MVC will treat a non-nullable reference type parameters and properties as-if [Required] has been applied, resulting in validation errors when no value was bound.

Source: https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.mvcoptions.suppressimplicitrequiredattributefornonnullablereferencetypes

Even if a project sets this property to true, the rule should not be reported in my opinion. Because then it was an explicit decision made by the developer (e.g. because the project uses a different validation framework like FluentValidation).

Repro steps

  1. Create a new ASP.NET Core Web API project (nullable reference types are enabled by default in current .NET versions)
  2. Add a model with not-nullable reference type properties (e.g. string) and use it in a POST controller method.
  3. Call the POST method without providing the property. See that ASP.NET Core returns an validation error, even though you didn't apply the Required attribute manually.

Expected behavior

Rule should not be reported.

Actual behavior

Rule is reported.

Known workarounds

I had to create a new quality profile in SonarCloud to disable the rule completely.

Related information

  • C#/VB.NET Plugins version: 9.25.0.90414 (used by the SonarCloudPrepare@1 task)
  • Visual Studio version: not relevant
  • MSBuild / dotnet version: 8.0.300
  • SonarScanner for .NET version: SonarScanner for MSBuild 5.15 (used by the SonarCloudAnalyze@1 task)
  • Operating System: windows-latest build agent on Azure DevOps

Hello @cremor and thanks for reporting this.

The rule should be raised only in value-type properties, not on reference-type properties.
Could you please provide me with a code snippet where you mark where the issue is raised?

Thanks a lot!

Here is an example:

public sealed record class TodoItemCreateRequest
{
    public required string Title { get; init; }

    public string? Description { get; init; }
}

grafik

Note that this is a double false positive, the other one is #9285

Hey, @cremor - it should not raise on a string, indeed - regardless of the nullable context.
EDIT: It should not raise for reference types in nullable context.
Thanks!

I've added this issue to our backlog for this sprint.