CSHARP-5749: Handle C# 14 mangling of Contains/SequenceEquals to MemoryExtensions #1790
Conversation
|
|
||
| static Expression VisitContainsMethod(MethodCallExpression node, MethodInfo method, ReadOnlyCollection<Expression> arguments) | ||
| { | ||
| if (method.IsOneOf(MemoryExtensionsMethod.ContainsWithReadOnlySpanAndValue, MemoryExtensionsMethod.ContainsWithSpanAndValue)) |
There was a problem hiding this comment.
This one line accurately checks what method we are rewriting.
The many lines of code that it used to take to check this are now encapsulated in the MemoryExtensionsMethod class.
| { | ||
| var itemType = method.GetGenericArguments().Single(); | ||
| var span = arguments[0]; | ||
| var value = arguments[1]; |
There was a problem hiding this comment.
A few intermediate variables go a long way to make the code understandable.
| var value = arguments[1]; | ||
|
|
||
| if (TryUnwrapSpanImplicitCast(span, out var unwrappedSpan) && | ||
| unwrappedSpan.Type.ImplementsIEnumerableOf(itemType)) |
There was a problem hiding this comment.
Alternatively we could check that unwrappedSpan is an array, but technically all that is required for this rewriting to be OK is that it implements IEnumerable<TItem>.
| public static Expression Preprocess(Expression expression) | ||
| { | ||
| expression = ClrCompatExpressionRewriter.Rewrite(expression); | ||
| expression = PartialEvaluator.EvaluatePartially(expression); |
There was a problem hiding this comment.
I get exceptions if I try to call EvaluatePartially first.
| { | ||
| internal static class MethodInfoExtensions | ||
| { | ||
| public static bool Has1GenericArgument(this MethodInfo method, out Type genericArgument) |
There was a problem hiding this comment.
Helper methods that make reading if statements easier by reducing multiple lines of tests to one line with clear intent.
| var collection = Fixture.Collection; | ||
| var names = new[] { "Two", "Three" }; | ||
|
|
||
| var queryable = collection.AsQueryable().Where(Rewrite((C x) => names.Contains(x.Name))); |
There was a problem hiding this comment.
I found the ManglingQueryable approach too indirect.
All that is needed is to rewrite the expression to use MemoryExtensions, and we can accomplish that without any hidden levels of indirection by calling Rewrite.
If Rewrite were not a private method local to this class I would give it a longer name.
| var queryable = collection.AsQueryable().Where(Rewrite((C x) => names.Contains(x.Name))); | ||
|
|
||
| var results = queryable.ToArray(); | ||
| results.Select(x => x.Id).Should().Equal(2, 3); |
There was a problem hiding this comment.
We use FluentAssertions.
Also changed to check the actual results and not just the count.
| // static constructor | ||
| static MemoryExtensionsMethod() | ||
| { | ||
| __containsWithReadOnlySpanAndValue = GetContainsWithReadOnlySpanAndValueMethod(); |
There was a problem hiding this comment.
We can't use our usual approach of extracting the MethodInfo from a LambdaExpression because the compiler gives compilation errors for the required lambda expression.
|
|
||
| namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira; | ||
|
|
||
| #if NET6_0_OR_GREATER || NETCOREAPP3_1_OR_GREATER |
There was a problem hiding this comment.
Two comments:
- Might as well ifdef the whole file away if we're not going to run the tests on a particular TF
- Tests can run on NETCOREAPP3_1 also
rstam
changed the title
2 participants
No description provided.