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
798 stars 229 forks source link

Fix S2583 FP: Unreachable code falsely detected with List count #9678

Closed nrankin18 closed 1 month ago

nrankin18 commented 1 month ago

Description

Rule S2583 is raised when List is considered to be populated.

Repro steps

List<string> foo = [];
List<string> bar = GetBar(); // Returns any number of strings in a List
foo.AddRange(bar);

if (foo.Count > 0) { // S2583 is raised
  return;
}

Expected behavior

S2583 should not be raised since it's possible for foo to contain any number of elements, including 0.

Actual behavior

S2583 is raised.

Known workarounds

Inhibit S2583

Related information

CristianAmbrosini commented 1 month ago

Hi @nrankin18! You are using an old version of the analyzer, v.8.4.0.10426. The FP you are experiencing does not reproduce in the latest version of the plugin. Therefore, updating to the most recent version should resolve your issue 👍

nrankin18 commented 3 weeks ago

@CristianAmbrosini I have updated to 8.6.0.10679 and am still experiencing this false positive.

List<string> foo = [];
foo.AddRange([]);

if (foo.Count > 0) { // S2589 is raised
    return;
}
CristianAmbrosini commented 2 weeks ago

Hi @nrankin18, it looks like you're not using the latest version. I recommend updating to 9.32.0.97167.

nrankin18 commented 2 weeks ago

Hi @CristianAmbrosini, thanks for the reply. I double-checked and the SonarLint for Visual Studio 2022 extension is showing 8.6.0.10679 as the most recent version (last updated on 10/23).

CristianAmbrosini commented 2 weeks ago

I didn't realize you were referring to the SonarLint version rather than the plugin version. I have double-checked, and the fix is included in plugin version 10. However, it has not yet been integrated into SonarLint or SonarQube. I will need to ask you to be patient for a while, have a good day!

nrankin18 commented 2 weeks ago

Sorry for the confusion! I'm glad to hear it's in the pipeline. Thanks for the clarification.