[Analyzer Proposal]: Replace `.Where(x => x is T)` for `.OfType<T>()`

[alexandrehtrb]

Background and motivation

Hello,

The usage .Where(x => x is T) is fairly common when sequences have objects of different classes. The .OfType<T>() is a better option, but it is not widely known. An analyzer could help with this.

API Proposal

object[] myStuff = new[] { 1, 2, "abc", "def"  };
string[] myStrings = myStuff.Where(x => x is string).Cast<string>().ToArray();
// information level: replace Where type filtering with OfType

API Usage

Described above

Alternative Designs

Risks

[stephentoub]

I think this is reasonable as a style rule, but there's already IDE0120 from https://github.com/dotnet/roslyn/pull/50318 and doc'd at https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0120 that's all about simplifying Where-based operations.. I think it'd be better for this to just be added to that analyzer.

Also worth pointing out that, perf-wise, the Where can actually be more efficient, purely as an implementation detail.

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

[jhudsoncedaron]

I currently have quite a bit of code that would be very unhappy with this.

We have a number of projected collection interfaces where you can't get the actual implementation type; these interfaces implement self-pooling of the enumerable via IEnumerable<T>: IDispose; but .OfType() won't call Dispose().

So I would have to be able to disable this rule independently of the others.

In theory the problem could be avoided by copying and pasting .OfType's implementation to make a new .OfType<T, TSource>; however I've been burned too many times to do that one myself.

[vladd]

@jhudsoncedaron

but .OfType() won't call Dispose().

But .Where(x is T).Cast<T>() shouldn't be calling it either, so there must be no difference.

[jhudsoncedaron]

@vladd : In our codebase, .Where(x is T) is almost never followed by .Cast<T>(); if the analyzer only picked up .Where(x is T).Cast<T>() I would have no problem with it.

[alexandrehtrb]

Should we make the analyzer for .Where(x => x is T) or .Where(x => x is T).Cast<T>()?

My opinion is in favour of the first (just where), because the most common scenario is people filtering for a type and then retrieving those objects.

If using OfType is less desirable than Where, for reasons such as performance, cited above, the code suggestion can be disabled manually.

[stephentoub]

Moving to roslyn per https://github.com/dotnet/roslyn/issues/74968

[dotnet-issue-labeler[bot]]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[dotnet-issue-labeler[bot]]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.