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 S2259 FP: is null check on generic type #6241

Closed
snakefoot opened this issue Oct 24, 2022 · 2 comments · Fixed by #6923
Closed

Fix S2259 FP: is null check on generic type #6241

snakefoot opened this issue Oct 24, 2022 · 2 comments · Fixed by #6923
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

@snakefoot
Copy link

snakefoot commented Oct 24, 2022

Description

S2259 - is null on at least one execution path

Repro steps

        public static ReadOnlySingleBucketDictionary<TKey, IList<TValue>> BucketSort<TValue, TKey>(this IList<TValue> inputs, KeySelector<TValue, TKey> keySelector, IEqualityComparer<TKey> keyComparer)
        {
            Dictionary<TKey, IList<TValue>> buckets = null;
            TKey singleBucketKey = default(TKey);
            for (int i = 0; i < inputs.Count; i++)
            {
                TKey keyValue = keySelector(inputs[i]);

                if (i == 0)
                {
                    singleBucketKey = keyValue;
                }
                else if (buckets is null)
                {
                    if (!keyComparer.Equals(singleBucketKey, keyValue))
                    {
                        // Multiple buckets needed, allocate full dictionary
                        buckets = CreateBucketDictionaryWithValue(inputs, keyComparer, i, singleBucketKey, keyValue);
                    }
                }
                else
                {
                    if (!buckets.TryGetValue(keyValue, out var eventsInBucket)) // BUG S2259 - 'buckets' is null on at least one execution path
                    {
                        eventsInBucket = new List<TValue>();
                        buckets.Add(keyValue, eventsInBucket);
                    }
                    eventsInBucket.Add(inputs[i]);
                }
            }

            if (buckets is null)
                return new ReadOnlySingleBucketDictionary<TKey, IList<TValue>>(new KeyValuePair<TKey, IList<TValue>>(singleBucketKey, inputs), keyComparer);
            else
                return new ReadOnlySingleBucketDictionary<TKey, IList<TValue>>(buckets, keyComparer);
        }

Expected behavior

Not reported as bug

Actual behavior

Reported as bug

Known workarounds

Rewrite code

Related information

https://sonarcloud.io/project/issues?resolved=false&types=BUG&branch=dev&id=nlog2&open=AYP2NU0SZinl2h1ztXSa

@pavel-mikula-sonarsource
Copy link
Contributor

Hi @snakefoot,

Thank you for reporting this case. I confirm it as False Positive. It looks pretty tricky - see the minimal reproducer in the PR above.

@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title S2259 - False positive after is null check Fix S2259 FP: 'is null' check on generic type Oct 25, 2022
@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix S2259 FP: 'is null' check on generic type Fix S2259 FP: is null check on generic type Oct 25, 2022
@pavel-mikula-sonarsource
Copy link
Contributor

Normal: IsPattern with LocalReference and ConstantPattern. Where ConstantPattern has Literal value null.

Generic: IsPattern with LocalReference and ConstantPattern. Where ConstantPattern has a Conversion value, with Literal null value.

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
Development

Successfully merging a pull request may close this issue.

3 participants