Efcore: Compare to all Queryable/Enumerable methods via MethodInfo, not by name

[roji]

In various place, we resolve methods on Queryable/Enumerable via name, and don't verify the exact number and type of parameters, assuming overloads will never be added. We should do a pass to compare against MethodInfo.

We already have LinqMethodHelpers which should contain all commonly-used MethodInfos - this should be used rather than looking up MethodInfos in various places. However, that class currently doesn't lookup in a bulletproof way, checking number of parameters but not types.

Original conversation: https://github.com/aspnet/EntityFrameworkCore/pull/16249#discussion_r297369667

• [ ] InMemoryLinqOperatorProvider

[roji]

Do after #16339 is merged to avoid conflict.

[roji]

Also move LinqMethodHelpers out of NavigationExpansion and consider making public for use by providers.

[roji]

@smitpatel holding off on this as discussed, since you want to replace LinqMethodHelpers (otherwise let me know and I'll do this).

Regarding big switches like in QueryableMethodTranslatingExpressionVisitor, we could also do something like:

c# case nameof(Queryable.Any) when method == LinqMethodHelpers.QueryableAnyMethodInfo:

Giving the best of both worlds - a nice clean switch and a tight reference comparison with a MethodInfo that's obtained once in a central place (also less verbose than checking parameter count and types).

[smitpatel]

That seems pretty. We can certainly do that. 👍

[roji]

You want me to do that and tighten method lookup in LinqMethodHelpers or are you working on something on your own side?

[smitpatel]

I will complete it.

[smitpatel]

blocked label -> Refactoring QueryableMethodProvider in nav expansion take 2.
Un-assigning myself - once that PR is merged, anyone can go through and make the logic more robust as needed.

[roji]

I can do that once the PR is merged.

[smitpatel]

Thanks @roji

[smitpatel]

Isn't this done already?