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

Do not use OfType<T>() with impossible types #33770

Closed
Tracked by #57797
terrajobst opened this issue Mar 19, 2020 · 5 comments
Closed
Tracked by #57797

Do not use OfType<T>() with impossible types #33770

terrajobst opened this issue Mar 19, 2020 · 5 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Linq code-analyzer Marks an issue that suggests a Roslyn analyzer in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@terrajobst
Copy link
Member

terrajobst commented Mar 19, 2020

Warn if use of OfType<T>() would provably result in an empty sequence, because we can see that the input type in the sequence will never be the specified type.

Category: Reliability
Suggested severity: Warning

UPDATE FROM roslyn-analyzer issue :

The following will always throw an InvalidCastException at runtime:

new int[] { 1 }.Cast<string>().ToList();
new float[] { 1f }.Cast<double>().ToList();
new int[] { 1 }.Cast<long>().ToList();

Perhaps worse, this will return an enumerable that fully enumerates the source and then returns no items silently:

enumerableOfApple.OfType<Orange>()

Both of these situations are likely to be a mistake.

Ther is a PR out for review

@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq code-analyzer Marks an issue that suggests a Roslyn analyzer labels Mar 19, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 19, 2020
@dotnet dotnet deleted a comment from Dotnet-GitSync-Bot Mar 19, 2020
@jeffhandley jeffhandley added this to the Future milestone Mar 20, 2020
@jeffhandley
Copy link
Member

Estimates:

  • Analyzer: Small
  • Fixer: Not Applicable

@terrajobst terrajobst removed the untriaged New issue has not been triaged by the area owner label Mar 21, 2020
@drewnoakes
Copy link
Member

drewnoakes commented Jun 4, 2020

Possibly at the same time, though via a different diagnostic ID: calling OfType<T>() on an IEnumerable<T> where the T is identical or assignable is redundant.

Enumerable.Range(1, 100).OfType<int>();

// @jnyrup points out that this is actually a non-null check,
// so isn't completely safe to remove

// myString.Split(",").OfType<object>();

@fowl2
Copy link

fowl2 commented Jun 4, 2020

I did a little prototype for fun over at dotnet/roslyn-analyzers#3608

@jnyrup
Copy link
Contributor

jnyrup commented Jun 5, 2020

@drewnoakes Note that for reference types this can be used as a less clear way of writingWhere(e => e != null).

@bartonjs
Copy link
Member

bartonjs commented Nov 10, 2020

Video

Approved as proposed

Category: Reliability
Suggested severity: Warning

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 10, 2020
@buyaa-n buyaa-n modified the milestones: 6.0.0, Future, 7.0.0 Aug 13, 2021
@carlossanlop carlossanlop added the in-pr There is an active PR which will close this issue when it is merged label Jul 6, 2022
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 9, 2022
@buyaa-n buyaa-n modified the milestones: Future, 8.0.0 Nov 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Linq code-analyzer Marks an issue that suggests a Roslyn analyzer in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

No branches or pull requests