Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S4158 FP: rule raised where there is no guarantees of emptiness #7582

Closed
rjgotten opened this issue Jul 12, 2023 · 2 comments · Fixed by #8066
Closed

Fix S4158 FP: rule raised where there is no guarantees of emptiness #7582

rjgotten opened this issue Jul 12, 2023 · 2 comments · Fixed by #8066
Assignees
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Milestone

Comments

@rjgotten
Copy link

Description

S4158 regarding iteration of known empty collections may be raised in situations where there is no guarantee that the collection is empty.

Repro steps

private static void Demo(IEnumerable<int> input)
{
  var list = new List<int>();
  var flag = false;

  foreach(var item in input)
  {
    if (item > 10)
    {
      flag = true;
      continue;
    }

    list.Add(item);
  }

  if (flag)
  {
    foreach(var item in list)
    {
      // do something with items that were <= 10
      // only when items > 10 were found.
    }
  }
}

Expected behavior

S4158 is not raised, because there is no guarantee that list will be empty when flag is true.

Actual behavior

S4158 is raised.

Known workarounds

Suppress the rule.

Related information

  • C#/VB.NET Plugins version : C# 9.4.0.72892 (SonarLint 7.0.0.74072)
  • Visual Studio version: 17.6.4
  • MSBuild / dotnet version: dotnet 7.0.304
  • Operating System: Windows 11
@Tim-Pohlmann Tim-Pohlmann added Area: CFG/SE CFG and SE related issues. Area: C# C# rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules. labels Jul 13, 2023
@Tim-Pohlmann
Copy link
Contributor

Thanks @rjgotten, for reporting this. I confirm it as an FP.

@pavel-mikula-sonarsource
Copy link
Contributor

pavel-mikula-sonarsource commented Jul 18, 2023

It is due to the fact that we don't visit the instruction more than twice.
1st run learns flag=true and keeps list=empty
2nd run keeps flag=false and learns list=NotEmpty
So if(flag) is true only when list=empty and that's why we raise.

Finetuning these limits will be done later.

There's a possible high performance price for these changes that we need to evaluate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Projects
None yet
4 participants