SonarSource/sonar-dotnet

Fix S6964 FP: Add more attributes to the exclusions

Closed this issue · 3 comments

Description

Rule S6964 raises on members that are defined with the [JSonProperty(Require = Always)] attribute.
Since a missing value will cause a deserialization error, the rule should not raise.
Please also consider [JSonProperty(Require = DisallowNull)]

Repro steps

Please provide a minimal code snippet that compiles which reproduces the problem. Alternatively, you can share a link to the SonarCloud public project where the issue appears. If the issue requires some specific project configuration to reproduce, you can create a minimal reproducible project and share its git repository or attach it as a ZIP file.

Example:

/// <summary>This class defines a GetPartnerSubscriptionRequest model.</summary>
[JsonObject(NamingStrategyType = typeof(CamelCaseNamingStrategy))]
public class GetPartnerSubscriptionRequest
{
    /// <summary>Gets or sets the ClientSiteId.</summary>
    [ValidateNever] // Prevents sonar error/warning S6964
    [JsonProperty(Required = Required.Always)]
    public Guid ClientSiteId { get; set; }

    /// <summary>Gets or sets the PartnerId.</summary>
    [ValidateNever] // Prevents sonar error/warning S6964
    [JsonProperty(Required = Required.Always)]
    public Guid PartnerId { get; set; }
}

I've added an exception for JSonProperty(Require = Always) and SonProperty(Require = AllowNull) and also for Range (since it's a minor change).

With this PR I'm excluding properties annotated with:

  • JsonProperty(Required = Required.Always)
  • JsonProperty(Required = Required.AllowNull)
  • Newtonsoft.Json.JsonIgnore
  • Newtonsoft.Json.JsonRequired
  • System.Text.Json.Serialization.JsonRequired
  • System.Text.Json.Serialization.JsonIgnore
  • Range

It Also has an issue when you have multiple next to required
image

Hello @knopa, thanks for reporting this. This is a different issue. However, this behavior will be fixed with the new release coming in the next few days.
One more, the rule will raise if your value type property is annotated with Required. Required has no effect on value type properties as they always have a value (default one).