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
786 stars 226 forks source link

S3626: Jump statements should not be redundant -> Competes with Resharper SeparateLocalFunctionsWithJumpStatement #8536

Closed naefp closed 9 months ago

naefp commented 9 months ago

Description

This is not really a false positive, but just an idea to align with Resharper inspections.

S3626 competes with the Resharper inspection SeparateLocalFunctionsWithJumpStatement "Add explicit return before local function".

Sonar might consider accepting redundant jump statements before local functions as well.

Repro steps

Create a local function within any method or loop. Resharper will advise to add an explicit return and place all local functions after that. Sonar will then advise to remove the redundant jump statement.

private void Some()
{
    DoSomething();
    return;

    void DoSomething()
    {
        // Something
    }
}

Expected behavior

Not competing with Resharper.

Actual behavior

Resharper and Sonar telling me exactly the opposite to do on that specific code snippet.

Known workarounds

Disable either Sonar or Resharper rule.

Related information

pavel-mikula-sonarsource commented 9 months ago

Hi,

As I see it, our rule is more broad than the local-function specific from ReSharper that solves one corner case.

The superfluous return; might cause more confusion IMO, as it goes against the natural syntax of the language. For me, it raises a question about why it's there, when it doesn't do anything.

So I don't foresee us creating an exception here, and then explaining to all users why the exception exists in this case.