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

Anlayzer to warn against multiple evaluations of LINQ deferred execution query in a function #89

Open
bretehlert opened this issue Aug 23, 2019 · 1 comment

Comments

@bretehlert
Copy link

bretehlert commented Aug 23, 2019

We recently ran across this code:

var succeeded = flatResults.Where(r => r.Value.Outcome.IsSuccessful() || r.Value.Outcome.IsDisabledOrInactive()).Select(r => r.Key.Code);

var failed = flatResults.Where(r => !succeeded.Contains(r.Key.Code));
if (failed.Any())
{
	foreach (var failedTask in failed)
	{
		ReportNudgeFailed(new[] { failedTask.Key.Code }, failedTask.Value, 0);
	}
}

if (succeeded.Any())
{
	ReportNudgeSucceeded(succeeded);
}

The result of this code will be that the LINQ expression to calculate succeeded will be executed many times. We also execute the expression to calculate failed multiple times, there is no point in doing an Any() before the enumeration.

Add a rule or set of rules to warn against multiple evaluations of a LINQ query within the same function.

@brian-reichle
Copy link
Contributor

This can get a little tricky to detect, particularly if the field that Any() is called on gets reused for different collections.

foreach (var entity in entities)
{
    var stuff = entity.Stuff;
    stuff = stuff.Where(x => x.IsMagic);

    if (stuff.Any())
    {
        // ...
    }
}

At the very least, this would probably require the flow graph stuff, which would require that we upgrade the version of Roslyn we build against to somewhere around 2.6.0 (i think).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants