SonarSource/sonar-dotnet

Fix S6964 FN: Rule should raise in case of value type property annotated with RequiredAttribute

Closed this issue · 3 comments

Description

S6964 is reporting an error on value type model properties.

Repro steps

Here is a value type model property that is triggering the rule:

public class ChangeEmailCodeViewModel : VerificationCodeViewModelBase
{
  public int CanRetry
  {
      get;
      set;
  }

  // (...)
}

Expected behavior

The rule should not be trigger for value type properties.

Actual behavior

See above.

Known workarounds

None.

Related information

  • SonarAnalyzer.CSharp version 9.25.0.90414
  • Visual Studio 17.9.6
  • .NET 8.0.204
  • Windows 10

Hello @hugoqribeiro,
This rule raises on value type properties of classes if they are used in a controller action, as input, and are not required or nullable.
If, for example, the value type properties are not set while posting, this leads to under posting and might lead to issues due to the value types ending up getting their default values.

From my point of view, this is not a false positive, but I might be missing some context regarding your code base.

I see your point if you think of [Required] as metadata.

In my opinion, [Required] is foremost for validation and there is no way to implement [Required] on value type properties (as there will always be a value)... That's why people don't annotate these properties with [Required].

@hugoqribeiro I checked the rule implementation and you are right. The rule should ask for nullable and required at the same time (with required not being mandatory - but as metadata) - or [JsonRequired] in the case of a web API controller action that accepts a model object as input in JSON form(if this can be detected).

I will add this ticket to our backlog, and we'll handle it soon as we are working now on hardening the new ASP.NET rules.

Thanks a lot for taking the time to report this and the other issues!