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

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

Closed
sid-6581 opened this issue Sep 26, 2024 · 10 comments · Fixed by #1544
Closed

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

sid-6581 opened this issue Sep 26, 2024 · 10 comments · Fixed by #1544

Comments

@sid-6581
Copy link

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
Copy link
Collaborator

cc: @BenjaminBrienen

@BenjaminBrienen
Copy link
Contributor

Opened an issue in the relevant repository:

dotnet/efcore#34767

This issue can be closed, I think. @josefpihrt

@sid-6581
Copy link
Author

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
Copy link
Contributor

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

@sid-6581
Copy link
Author

sid-6581 commented Sep 26, 2024

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
Copy link
Collaborator

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

@BenjaminBrienen
Copy link
Contributor

BenjaminBrienen commented Sep 27, 2024

This issue is already fixed in EFCore. You need to update your dependency: dotnet/efcore#28634

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

It is currently in release candidate status.

@sid-6581
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Contributor

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

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

Successfully merging a pull request may close this issue.

3 participants