Why does Resharper suggest a code change and then complain about the change?

Question

I had this code:

return children.SelectMany(c => GetChildControls<TControl>(c)).Concat(children);

...and Resharper suggested I change it to this:

return children.SelectMany(GetChildControls<TControl>).Concat(children);

In context:

internal static IEnumerable<TControl> GetChildControls<TControl>(this Control control) where TControl : Control
{
    var children = control.Controls != null ? control.Controls.OfType<TControl>() : Enumerable.Empty<TControl>();
    return children.SelectMany(GetChildControls<TControl>).Concat(children);
}

The difference, as you probably saw, is that the "improved" version of the line has ommitted the inserted "(c)"

But now it flags that as suspicious, saying, "Possible multiple enumeration of IEnumerable"

Yeah, that's right, there could/should be multiple, but why is it whin[g]ing about that? Should I revert it to its previous state?

My guess is that Resharper has limits on the number of code changes it will suggest at once for each line. The "possible multiple enumeration" issue is present in both versions of the code, but Resharper only displays it when the "improvement" has been made and that message goes away. — BJ Myers, Apr 12 '16 at 23:24

score 2 · Accepted Answer · edited May 23 '17 at 10:28

The problem that ReSharper is picking up for you is that you are enumerating children twice. Specifically, you enumerate children the first time when you call children.SelectMany(GetChildControls<TControl>) and another time when you call .Concat(children) on that. ReSharper warns you about this because each enumeration could result in different results, or each enumeration could be costly. See this question for more details.

One solution would be

internal static IEnumerable<TControl> GetChildControls<TControl>(this Control control) where TControl : Control
{
    var children = control.Controls.OfType<TControl>().ToList();
    return children.SelectMany(GetChildControls<TControl>).Concat(children);
}

which makes sure children will not vary inside your method after you create it (unless, of course, you mutate it yourself) and that it will only be enumerated once.

With your code I get, "The type arguments for method 'System.Linq.Enumerable.SelectMany(System.Collections.Generic.IEnumerable, System.Func>)' cannot be inferred from the usage. Try specifying the type arguments explicitly. " — B. Clay Shannon-B. Crow Raven, Apr 12 '16 at 23:33
Sorry about that! I didn't notice your extension method was `this Control` instead of `this TControl`. The updated answer should work — AGB, Apr 12 '16 at 23:36

Why does Resharper suggest a code change and then complain about the change?

1 Answers1