SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
797 stars 229 forks source link

Fix S3459 FP: support classes marked with [AutoConstructor] attribute #9674

Open nycdotnet opened 1 month ago

nycdotnet commented 1 month ago

Description

We use the NuGet package Autoconstructor in many of our solutions. https://www.nuget.org/packages/AutoConstructor With this package, if a class is marked partial and has the [AutoConstructor] attribute, this NuGet package will generate a constructor that accepts an argument for and will assign a value to any readonly non-static fields (with various customizations possible).

In Visual Studio, the Sonar NuGet analyzer is able to interpret the generated constructor, but not in CI where we are wiring in sonarcloud.io. This means we will get S3459 wherever there are fields that are only assigned by the automatically generated constructor. (We don't check in the generated code).

Repro steps

namespace MyNameSpace;

[AutoConstructor]
public partial class SonarFalsePositive
{
    private TimeProvider TimeProvider { get; }

    public DateTimeOffset DoSomething()
    {
        return TimeProvider.GetUtcNow();
    }
}

Expected behavior

No S3459 just like in Visual Studio.

Actual behavior

S3459 is raised in SonarCloud.io: Remove unassigned auto-property 'TimeProvider', or set its value.

Known workarounds

We could disable S3459 in classes that use Autoconstructor or generally. I wonder if this rule could be disabled maybe for partial classes if it's expected that your CI product can't use the generated code? Possibly only for partial classes that also have the Autoconstructor attribute? I see you already have an override for classes with [Serializable].

Related information

CristianAmbrosini commented 1 month ago

Hi @nycdotnet,

while it is a bit niche, it does appear to be a false positive. As you pointed out, we are already opting out if specific attributes are found in the class. Let's see if I can add a reproducer and fix it; it should be a low-hanging fruit.

CristianAmbrosini commented 1 month ago

I tried adding a reproducer but it's not an easy task. The AutoConstructorAttribute is auto-generated and this adds some complexity to our test infrastructure. I'll leave this open for the future, we might add a syntax-level check and avoid raising on classes marked with attributes that are called "AutoConstructor".

nycdotnet commented 1 month ago

OK thanks.