FP: S2333 reports on not redundent partial keyword

In the following snippet:

public static partial class TargetDependent // FP, partial is required for .NET 8.0 and up targets
{
    public static bool Matches(string ns) => Pattern.IsMatch(ns);

#if NET8_0_OR_GREATER
    private static readonly Regex Pattern = GetPattern();

    [GeneratedRegex(@"\.v(?<Version>[1-9][0-9]+(_[0-9]+)*)\.", RegexOptions.ExplicitCapture | RegexOptions.CultureInvariant)]
    private static partial Regex GetPattern();
#else
    private static readonly Regex Pattern = new(@"\.v(?<Version>[1-9][0-9]+(_[0-9]+)*)\.", RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture);
#endif
}

Rule S2333 reports that the use of partial is gratuitous. This is, for target frameworks (.NET standard 2.0 in my case, but that is less relavent) that do not have the [GeneratedRegex]included.

To some extend, the rule is not wrong, as for the compiled target framework, there is no partial required. However, both suppressing the rule, and making the partial also target framework dependent with a pragma #if NET8_0_OR_GREATER does not improve the code, and removing the keyword obviously makes that the code does not longer compile.

So, to improve the usability of this rule, it would be really nice if it can do a attempt to alt least on the use of the partial keyword that is under a pragma condition.

Hi,

Confirm for me you’re seeing this in a recent version of SonarQube (Server / Community Build / Cloud / IDE), please?

 
Thx,
Ann

1 Like

Sure. Version 10.0.6 if I say it by hard.

Hi,

I can’t find a product that 10.0.6 is a “recent” version for.

Could you clarify which product you’re seeing this in, and upgrade to the latest?

 
Thx,
Ann

1 Like

Hi,

Thanks for that. I’ve flagged this for the language experts.

 
Ann

Hello @Corniel,

Thank you for reporting this issue!

This is a known limitation of our analyzer, as we rely on Roslyn compilation we only see what’s built for the current target thus we are not able to take into account that for a different target the partial is relevant.

I think you know this limitation as you mentioned it here.

Anyway, I have open a ticket in our backlog to tackle it in the future.

I will also raise this point to the rest of the team as I am seeing this coming up often recently for several rules.

Have a nice day!

I am away, and I mentioned it in the report, at least, I tried to. The question is: is it doable to work around this limitation ?

I’m certain you can detect the #pragma being there, Roslyn indeed will not tell you the meaning of the code that is currently not compiled, but also that is available. But how extensive has the evaluation to be, to make it not raising (too many) FNs?

In that case you mention, I think it is really tricky, in this case it might be doable. Another difference is, that in that other case you could (without too much noise use [System.Obsolete] and get rid of the using statement: here, the real problem (for me), is that both suppressing and making the partial also done with a #pragma extremely ugly. Specially given the small impact of the violation of the rule.