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: The analyzer incorrectly concludes the HashSet is guaranteed to be empty #8041

Closed
JelkeH opened this issue Sep 21, 2023 · 1 comment · Fixed by #8066
Closed
Assignees
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Area: VB.NET VB.NET rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Milestone

Comments

@JelkeH
Copy link

JelkeH commented Sep 21, 2023

Description

HashSet.Remove causes S4158 on an empty HashSet. It should not.

Repro steps

I tried to remove parts of this repro, but I could not make it cleaner without loosing the FP. _ = toDoLater.Remove(item); issues S4158

    public interface IFoo<T>
    {
        public bool someProperty { get; }

        public IEnumerable<T> Successors { get; }
    }

    public static void Func<T>(this ICollection<T> toDo)
        where T : class, IFoo<T>
    {
        Queue<T> toDoNow = new();
        HashSet<T> toDoLater = new();

        foreach (T item in toDo)
        {
            if (item.someProperty)
            {
                toDoNow.Enqueue(item);
            }
            else
            {
                _ = toDoLater.Add(item);
            }
        }

        while (toDoNow.Count > 0)
        {
            T current = toDoNow.Dequeue();

            // Handle current

            foreach (T item in current.Successors)
            {
                _ = toDoLater.Remove(item);
                toDoNow.Enqueue(item);
            }
        }
    }

Expected behavior

S4158 is not issued as the HashSet is not guaranteed to be empty

Actual behavior

S4158 is issued.

Known workarounds

Suppress the issued warning.

Related information

SonarAnalyzer version 9.7.0.75501

@Tim-Pohlmann
Copy link
Contributor

Hi @JelkeH, thanks for the report. I confirm this as an FP and created a PR to fix it. You can follow the progress here:
#8066

@Tim-Pohlmann Tim-Pohlmann added Area: CFG/SE CFG and SE related issues. Area: VB.NET VB.NET rules 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 Sep 25, 2023
@Tim-Pohlmann Tim-Pohlmann added this to the 9.11 milestone Sep 25, 2023
@costin-zaharia-sonarsource costin-zaharia-sonarsource modified the milestones: 9.11, 9.12 Sep 27, 2023
@Tim-Pohlmann Tim-Pohlmann modified the milestones: 9.12, 9.13 Oct 9, 2023
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. Area: VB.NET VB.NET rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Projects
None yet
4 participants