NET-1909 S4158 FP: Collection filled in a local function

Description

A S4158 violation is incorrectly reported when elements to a collection are added in a local function.

Repro steps

public static class Program
{
    public static void Main()
    {
        var list = new List<String>();

        void AddElement()
        {
            list.Add("Item1");
        }

        AddElement();

        for (int i = 0; i < list.Count; i++)
        {
            var element = list[i]; // SonarLint reports violation of rule S4158 here.
            Console.Out.WriteLine(element);
        }
    }
}

Expected behavior

Since the collection is clearly not empty in the code sample, SonarLint should not report a violation of rule S4158.

Actual behavior

SonarLint reports a violation of rule S4158.

Known workarounds

None.

Related information

C#/VB.NET Plugins version: SonarLint for Visual Studio 2019, Version 4.35.0.32570
Visual Studio version: Visual Studio 2019, Version 16.9.0
MSBuild / dotnet version: MSBuild 16.9.0.11203 / .NET Framework 4.6.2
SonarScanner for .NET version (if used)
Operating System: Windows 10 x64

@rent-a-developer

Hi @rent-a-developer ,

Thank you for reporting this, I can confirm it as False Positive, as our engine doesn't support local functions for this rule (yet).

S4158: Add FP repro for #4478

S4158: Add FP repro for #4478 (#4489)

Hi,
Not sure if I should post it here, or create a new issue, as my false positive is not related to local functions. However, it looks quite similar.
The following code (a little bit simplified from my original code) also reports a false positive S4158, although the collections are definitely not empty:

Environment:

Visual Studio Enterprise 17.3.0
SonarAnalyzer.CSharp 8.46.0.54807
.NET Framework 4.8

Windows 10 x64

  public void FalsePositive()
  {
  	var data = new[] { 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, -1, -1, -1 };
  	var rising = new List<List<int>>();
  	var falling = new List<List<int>>();
  	List<int> currentBin = null;
  	var currentDirection = 0;
  	for (var i = 0; i < data.Length; i++)
  	{
  		if (data[i] == 0)
  		{
  			if (currentBin != null)
  			{
  				if (currentDirection == 1)
  				{
  					rising.Add(currentBin);
  				}
  				else
  				{
  					falling.Add(currentBin);
  				}
  				currentBin = null;
  			}

  			continue;
  		}

  		currentBin ??= new List<int>();
  		currentBin.Add(i);
  		currentDirection = data[i] > 0 ? 1 : -1;
  	}

  	for (var i = 0; i < rising.Count; i++)
  	{
  		// SonarAnalyzer reports S4158 here:
  		Console.WriteLine($"{rising[i].Count}, {falling[i].Count}");
  	}
  }

@Keke71

Hi @Keke71,

Thank you for reporting it, this is a good place to gather this evidence. This rule will get migrated to the new engine under MMF-2402 (non-public link). After that, we should be able to add support for local functions.

@pavel-mikula-sonarsource

Hi @pavel-mikula-sonarsource,

Thanks for your quick answer. However, please note that there are no local functions involved in my example. Maybe it is related though and will be fixed together with the local functions problem.

Best regards

@Keke71

@Keke71 indeed, I'm sorry, I've missread the "not" in "not related".

I've taken a closer look on the flow, it's quite complicated in terms of tracking values. There's a chance it will be fixed by the migration I mentioned (MMF-2402).

Please feel free to submit a new issue, as yours is related to tracking state in for loops rather than local function.

I created a new issue here

We cannot fix the original issue yet, as the new SE doesn't support local functions yet.

At the same time, the rule is migrated to the new SE, so the actual local function support is the only thing needed to fix this.

@pavel-mikula-sonarsource

@pavel-mikula-sonarsource , according to MMF-2228, we should support local functions.

Is this still relevant, or was it fixed?

Yes, this is still relevant.
MMF-2228 analyzes local function in a stand-alone mode.
This issue requires us to do a light version of cross-procedure analysis because the outer method has to analyze the inner local function on the way. It's the size of an MMF and should be tackled before our actual cross-procedure capabilities - the solution for this will be unrolling the content of the local function.

Internal ticket NET-1909

Navigation Menu

Repository navigation

NET-1909 S4158 FP: Collection filled in a local function #4478

Description

Repro steps

Expected behavior

Actual behavior

Known workarounds

Related information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

NET-1909 S4158 FP: Collection filled in a local function #4478

Description

Description

Repro steps

Expected behavior

Actual behavior

Known workarounds

Related information

Activity

pavel-mikula-sonarsource commented on May 28, 2021

Keke71 commented on Oct 5, 2022

pavel-mikula-sonarsource commented on Oct 6, 2022

Keke71 commented on Oct 7, 2022

pavel-mikula-sonarsource commented on Oct 7, 2022

Keke71 commented on Oct 7, 2022

pavel-mikula-sonarsource commented on Jun 14, 2023

andrei-epure-sonarsource commented on Apr 8, 2024

pavel-mikula-sonarsource commented on Apr 9, 2024

pavel-mikula-sonarsource commented on Jun 6, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions