search

dotnet / roslynator

Roslynator is a set of code analysis tools for C#, powered by Roslyn.
https://josefpihrt.github.io/docs/roslynator
Other
3.1k stars 260 forks source link

Incorrect RCS1077 suggestion to change OrderBy to Order in EF Core queries #1541

Closed sid-6581 closed 1 month ago

sid-6581 commented 2 months ago

Product and Version Used: 4.12.6

Steps to Reproduce:

Use OrderBy(x => x) in an EF Core LINQ expression.

Actual Behavior:

Roslynator will suggest changing it to Order(), which isn't currently supported in EF Core as of version 8. Using Order() will cause the code to fail at runtime when it can't be translated to SQL.

Expected Behavior:

OrderBy() used on anything that implements IQueryable should not trigger this suggestion.

josefpihrt commented 2 months ago

cc: @BenjaminBrienen

BenjaminBrienen commented 2 months ago

Opened an issue in the relevant repository:

https://github.com/dotnet/efcore/issues/34767

This issue can be closed, I think. @josefpihrt

sid-6581 commented 2 months ago

Even if they add that in another version of EF Core (and they're notoriously understaffed and slow to do so), RCS1077 will be very annoying to leave enabled in any project that uses any version of EF Core that doesn't support it, which will be the majority of projects for a while. RCS1077 has enough other useful suggestions that I don't want to disable it completely, but it's also very frustrating to pragma disable it in all my queries that use ordering. This is something that should be handled properly in Roslynator.

BenjaminBrienen commented 2 months ago

So Roslynator should special case every project that doesn't properly support IEnumberable methods?

sid-6581 commented 2 months ago

This isn't an IEnumerable method, it's an IQueryable method:

https://learn.microsoft.com/en-us/dotnet/api/System.Linq.Queryable.OrderBy?view=net-8.0

Roslynator shouldn't suggest this for the IQueryable versions.

josefpihrt commented 2 months ago

I agree with @sid-6581 that it shouldn't be suggested for IQueryable.

BenjaminBrienen commented 1 month ago

This issue is already fixed in EFCore. You need to update your dependency: https://github.com/dotnet/efcore/issues/28634

https://github.com/dotnet/core/releases/tag/v9.0.0-rc.1

It is currently in release candidate status.

sid-6581 commented 1 month ago

As I explained above,that's not an acceptable solution. I'm on the latest released version, EF Core 9 won't be out until later this year, and even then, most people won't update immediately.

josefpihrt commented 1 month ago

I think everyone expressed their opinion.

This needs to be fixed. Best would if it would be fixed by you, @BenjaminBrienen, as you implemented it. Let me know if you will be able to fix it in a next couple of days (The fix should be straightforward).

BenjaminBrienen commented 1 month ago

I'm busy with other projects at the moment, sorry.