S4158 FP when modifying collection via alias

Rules and Languages Report False-positive / False-negative...
1
  • What language is this for? C#
  • Which rule? S4158 “Empty collections should not be accessed or iterated”
  • Why do you believe it’s a false-positive/false-negative? The collection is not always empty.
  • Versions: VS 17.13.4, SonarQube for VS 8.20.0.13314, connected mode with SonarQube Server v2025.1.1

Here is a reduced example of this FP that we found in our production code base:

public static void Repro(string[] args)
{
	var set = new HashSet<string>();

	foreach (var arg in args)
	{
		var target = set;

		if (target.Contains(arg)) // S4158
		{
			throw new ArgumentException("duplicate");
		}

		target.Add(arg);
	}
}

set is added to through the alias target, so it is not guaranteed to be empty.

This example is simple to work around of course just by removing the alias; in our production code we have multiple collections and one of them is assigned to target based on some condition within the loop.

4

Hello @a10r and thank you for reporting this.

This particular example that you provided us with, is a true positive - since the set is always empty. For each arg it’s being reassigned to the target, without it being updated somehow.

I assume this is not the case in your code. Could you provide me with a reproducer closer to your actual code so I understand why our symbolic execution engine considers it empty?

Thanks a lot!

5

As far as I can tell, this is not the case. This reproducer results in a FP:

public static class S4158
{
	public static HashSet<string> Repro(string[] args)
	{
		var set = new HashSet<string>();

		foreach (var arg in args)
		{
			var target = set;

			if (target.Contains(arg)) // S4158
			{
				throw new ArgumentException("duplicate");
			}

			target.Add(arg);
		}

		return set;
	}
}

public class S4158Test
{
	[Test]
	public void Not_empty()
	{
		var x = S4158.Repro(["a", "b", "c"]);
		x.Should().HaveCount(3);
	}
}

This test succeeds.

1 Like
6

Yes - it adds the elements to the set via reference!
I confirm this is a false positive.

Thanks Corniel for the heads up.

@a10r I’ll add a ticket to our backlog to fix this in a future sprint.
Unfortunately the links are internal so I cannot share something with you to follow.

2 Likes