DotNet alert for multiple evaluation of an IEnumerable?

[Ted_Spence]

Hi team!

We recently had an issue where a piece of our code was evaluating the same IEnumerable multiple times. This is generally harmless if the IEnumerable is an array, but problematic if the IEnumerable is an entity framework repository or query set.

Is there a Sonar Cloud alert or monitor that could detect whether a single IEnumerable is evaluated more than once?

[Colin]

Hey there.

I’ve moved your topic to the section on reporting false-postives.

Can you please include the details requested in this post?

How to Report a False-positive / False-negative

Hey SonarSource Community! False-positives happen, as do false-negatives, and we’re eager to fix them. We are thrilled when our users report problems, so we can make our products better. What is a false-positive (FP)? A false-positive is when an issue is raised unexpectedly on code that should not trigger an issue, or where the suggested action doesn’t make any sense for the code. What is a false-negative (FN)? A false-negative is when an issue should be raised on a piece of code, but isn’t…

[Ted_Spence]

Hi Colin ! Thanks for responding. I don’t believe I’m reporting a false positive. In this case, my team did not get a sonar cloud alert.

Instead, what happened is we investigated an issue reported with our website, and we discovered it was due to a single query being evaluated multiple times. I researched the problem and found that some people had used static code analysis tools like resharper or stylecop to detect the problem, and since my company has standardized on SonarCloud, I was curious to know if we can do this same thing in SonarCloud.

The title of my post perhaps should have been “Does SonarCloud have an alert for DotNet code that evaluates a Queryable multiple times?”

[Colin]

Ah thanks.

Can you give a brief example of code where you would expect the issue to be raised, then? At that point I can pass it over to the right team.

[Ted_Spence]

Yes, here’s roughly what the code that caused the problem looks like:

var myQueryResult = Context.MyWidgets.Where(q => q.FieldOne == alert.FieldOne && !q.Processed && q.UserId == alert.UserId);

// This triggers the query once, and it is fast, because there is an index intended for this use
foreach (var items in myQueryResult)
{
    ... do something with alerts ...
}

// The person who wrote this code believed that myQueryResult was an array, and could be re-sorted
// in memory.  Because it's still an IQueryable, it triggers a second database query but because of 
// the "order by" clause it is nonperformant.
if (someCondition) {
    // I would hope that sonar cloud could label this with a code smell: "IQueryable executed multiple times"
    var lastItem = myQueryResult.OrderByDescending(a => a.CreatedDate).FirstOrDefault();
    if (item) {
        ... do something with this single item ...
    }
}

EDIT: The fix we applied was this:

var myQueryResult = Context.MyWidgets.Where(q => q.FieldOne == alert.FieldOne && !q.Processed && q.UserId == alert.UserId)
  .ToList(); // Converting this to a list means it is only executed once

[zsolt.kolbay]

Hi Ted. There’s a Microsoft Analyzer that does exactly what you need: CA1851. Have you tried it on your project? It seems like a fairly new rule (introduced with .NET 7.0).

[Ted_Spence]

Thanks! I’ll ask the team whether we have this rule enabled. Is this something I can turn on in SonarCloud, or do I need to seek out an alternative linter for this?

[zsolt.kolbay]

Yes, SonarCloud can use Roslyn rules. And if you want to try it locally, then you can enable it in Visual Studio by setting the Analysis level to Latest All in the .NET analyzers section in the project settings.