SonarSource/sonar-dotnet

Fix S6964 FP: Issue is raised for properties in record constructor even if they are annotated as required

Opened this issue · 6 comments

Description

Rule ID: S6964

Repro steps

using System.ComponentModel.DataAnnotations;
public record MyRecord([Required] string MyString);

Expected behavior

The rule should not activate.

Actual behavior

The rule activates.

Known workarounds

Related information

  • C# 10
  • TargetFramework: net6.0
  • Visual Studio version: 17.9.6
  • MSBuild / dotnet version: 8.0.204
  • Operating System: Windows

Hello @elnigno and thank you for reporting this.
My understanding is that the record is a model property. Could you please provide a reproducer with more context?
Thanks a lot!

Hi @mary-georgiou-sonarsource,
Yes, that record is used in a controller action, binding to the model with the FromBody attribute.

[HttpPut]
[Route("{productId}")]
public async Task<IActionResult> DoSomething(
    int productId,
    [FromBody] MyRecord requestBody)
{
    ...
}

Notice this is not exact code that's triggering the rule, because I can't disclose it, but I what I am providing captures the essence of it. Thanks for looking into this!

Thanks @elnigno.

Your model is the record itself?

Yes.

Hello @elnigno!
I did some tests with the following example :

[ApiController]
[Route("[controller]")]
public class AController : ControllerBase
{
    [HttpPost("new")]
    public string PostValue([FromBody] RecordModel model) =>$"MyString:{model.MyString}, MyInt: {model.MyInt}";
}

public record RecordModel(string MyString, int MyInt);

The rule correctly raises an issue for MyInt and not for MyString since the first is a value type.
However, I see that for records, our rule does not take into consideration the attributes, and this needs to be fixed.

If you agree I'll add this issue to my backlog but with a different title "Raises on record properties even if they are annotated as required".

PS: This rule has been updated in the latest analyzer version and it does not take into account the RequiredAttribute as it has no effect on value types. See here for more info.

@mary-georgiou-sonarsource sounds good, thanks for looking into it!