FirstOrDefault(condition) is flagged as null unchecked even if its checked with .Exists(condition)

Rules and Languages Report False-positive / False-negative...
, ,
1
  • Enterprise Edition
  • Version 9.9.2 (build 77730)

Hi Sonarcommunity, I have what I think to be a false positive flag in the following code where the object operationState.Screens is queried with a condition.

Now I understand that FirstOrDefault query is not checked to be not null, however as this query is inside an if block with operationState.Screens.Exists(SameCondition) it is logically not possible that the query will return null, by going for the default value.

if (operationState != null && operationState.Screens != null)
                    {
                        if (operationState.Screens.Exists(x => x.ViewName == MusteriTipi_ViewName))
                        {
                            operationState.Screens.FirstOrDefault(x => x.ViewName == MusteriTipi_ViewName).ViewModel = model.MusteriTipModel;
                        }
}

‘operationState.Screens.FirstOrDefault(x => x.ViewName == MusteriTipi_ViewName)’ is null on at least one execution path.

2

Hi,

Welcome to the community and thanks for this report!

You seem to have overlooked the topic template. Could you help us out with the language and rule ID?

 
Thx,
Ann

3

Hi Ann,

Thanks for the feedback and sorry for that overlook.

The language is C# with version .NET6 and the Rule is “Null pointers should not be dereferenced” with ruleID "csharpsquid:S2259 ".

Halil

1 Like
5

Hi Halil,

I’ve flagged this for the language experts.

 
Ann

1 Like
6

Hi @HalilGurer, welcome to Sonar community!

Our SymbolicExecution engine does not track the content of collections, which is why we are not aware that you’ve already verified the existence of the object in question.
However, this does highlight a potential issue. In this instance, you are using a FirstOrDefault function where a default case will never be reached. I suggest using LINQ First or Single. This way S2259 should disappear and the code would be cleaner.

Have a good day!

1 Like
8

Hi Cristian,

Thank you for your help. We will implement your suggestion.

Have a good day

9

I would suggest you should refactor it like this:

if (operationState?.Screens?.Find(x => x.ViewName == MusteriTipi_ViewName) is { } screen)
{
    screen.ViewModel = model.MusteriTipModel;
}
1 Like
10

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.