Upgrade Sonarqube 9.9 to 10.2 - Now see more bugs when reviewing the c# code

Clean Code Report False-positive / False-negative...
,
1

Since the update, I now see more bugs when reviewing the c# code.
Is there any documentation that explains why this change?

Example: S2259 # Null pointers should not be dereferenced

In that rule, this code did not jump before, something changed.
if (array.Any())
{
TreeItem itemMain = array.FirstOrDefault();
itemMain.Selected = OneHelper.SetSelectedFolder(itemMain);
}

itemMain this variable is the one marked by that rule

Thank you
Dario Flores

2

Hi Dario,

With each new release, some rules get smarter. It seems that there have been several iterations on S2259 since the 9.9 release, so it’s not surprising to me that more issues are found.

Do you disagree with some of them? (I’ve never done C#, so it’s not obvious to me whether your code snippet is a FP or not, altho I’m leaning that way…?)

 
Ann

3

Hello Ann.

First of all, thank you for responding.

What we notice in this example is what happened, which would never be null since we are checking the quantity:
if (array.Any())
{
}

I think that now the bug appears due to a question that asks

array.First();

instead

array.FirstOrDefault();

There is the issue.

Thank you again
Dario Flores

4

Hi Dario,

Thanks for the clarification. I’ve moved this to the False Postive category and will flag this for the language experts.

 
Ann

1 Like
7

Hello @DarioFlores,

Thank you for reporting this false positive.

I confirm that the issue was raised incorrectly, we don’t take into account .Any().
I created a ticket to track this issue.
Know that it will be part of the improvement of S4158 in this ticket, especially about:

Learn from well-known (extension-) methods like list.Any()

Checking if the array is not empty with .Any() using .FirstOrDefault() is not necessary since you already know that .FirstOrDefault() will never return null (or default), as you pointed out.
Because of the .FirstOrDefault(), we assume that itemMain can be null; thus, we raise S2259.

Then it is relevant to update your snippet to this (assuming array is of type Array):

if (array.Any())
{
    TreeItem itemMain = array[0]; // Using .First() would raise S6608
    itemMain.Selected = SetSelectedFolder(itemMain);
}

Have a nice weekend,

8

Hi Sebastian.

First of all thanks for the response.

Beyond the proposed case and how we should solve it, the unwanted effect in production was not good when updating the version.

Originally we had fewer bugs and after the update they increased a lot, and we believe that beyond First or FirstOrDefault that code will never fail, the issue is semantic.

Greetings
Dario Flores