Fix S2743 FP: Should not raise when base type is generic

[zsolt-kolbay-sonarsource]

Description

S2743 raises a warning when non-generic static fields are declared in a generic type. The recommendation is to move these fields to a non-generic base class (or create one if it doesn't exist).
Such a solution is not feasible if the base type is also generic, as the same issue would be raised there.
This FP is also raised in the sonar-dotnet repo.

Repro steps

class BaseClass<T> { }
class DerivedClass<T> : BaseClass<T>
{
    private static readonly string[] field = new[] { "a", "b", "c" }; // FP
    public string[] SomeMethod() => field;
}

Expected behavior

S2743 should not be raised.

Actual behavior

S2743 is raised on the static field.

Known workarounds

None.

Related information

• C#/VB.NET Plugins version: 9.4
• Visual Studio version: 17.6
• MSBuild / dotnet version: .NET 7.0
• Operating System: Windows 10 (10.0.19045)

[Tim-Pohlmann]

@zsolt-kolbay-sonarsource this can be solved by creating an additional base class or am I missing something?

class BaseClass
{
    protected static readonly string[] field = new[] { "a", "b", "c" }; // Compliant
}
class BaseClass<T> : BaseClass { }
class DerivedClass<T> : BaseClass<T>
{
    public string[] SomeMethod() => field;
}

[zsolt-kolbay-sonarsource]

@Tim-Pohlmann
What if the generic base class is not under our control or we just don't wish to modify it?
How would you fix the raised issue in this case? The analyzer raises a warning on those 5 static readonly members.
Where can we move them to eliminate the warnings?

• move them to a different class? Possible, but they are only used by the DoNotUseDateTimeNowBase class, I think they should stay inside.
• move them to a nested class inside DoNotUseDateTimeNowBase? The analyzer doesn't raise warnings, but I don't think that will fix the issue. I'm not sure if the nested class save the readonly members from being created multiple times.
• Any other ideas?

[Tim-Pohlmann]

According to RSPEC, they should go to SonarDiagnosticAnalyzer (the non-generic base class). I agree, however, that this is suboptimal. Maybe the better approach for S2743 is to not raise on read-only fields with an immutable type. The latter might be hard to detect, though.
Another workaround would be to make the fields non-static.