Allow for specify return value on System.Linq.Enumerable.*OrDefault methods

[Foxtrek64]

Fixes #20064

[ghost]

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #20064

Author:	Foxtrek64
Assignees:	-
Labels:	area-System.Linq
Milestone:	-

[Foxtrek64]

Are the errors here code issues or an issue with changes being synced? I haven't contributed since the dotnet runtime was split across several repositories so I'm not familiar with all that's going on behind the scenes.

Edit: Didn't push ref assemblies. This has been resolved.

[eiriktsarpalis]

Please add unit tests for the new methods as well. Thanks.

[eiriktsarpalis]

Hi @Foxtrek64, have you been able to take a look at PR feedback? Thanks!

[Foxtrek64]

Hi @Foxtrek64, have you been able to take a look at PR feedback? Thanks!

Yes. Sorry, I've had a busy few days. I'll implement these changes tomorrow and have them ready for review.

[eiriktsarpalis]

Please add similar unit tests for the overloads accepting predicates.

approved accidentally, PR still has pending feedback

[eiriktsarpalis]

Other than some minor changes, LGTM.

Thank you for your contribution!

[eiriktsarpalis]

Thanks @Foxtrek64. Note that there are a few failing unit tests related to these changes. Could you take a look?

[Foxtrek64]

I did notice what I think was a logic error on my part and patched it. Hopefully it resolves the test issues, or at least some of them. I tried testing on my ends but I get strange results, like MissingMethodExceptions even after a build (which does fix some problems). I presume the entire environment has to be built, not just the individual projects.

Edit: I've fixed this issue but now I'm running into issues with the Queryable side of things. After hashing this out in the .Net Foundation Discord, we believe this to be an issue with the BCL itself in that the overload I'm attempting to implement does not exist there. I'm not entirely sure doing something like this is even possible given that it would require splitting things into multiple queries on the BCL side.

I can "fix" it by doing something like this:

[DynamicDependency("FirstOrDefault`1", typeof(Enumerable))]
public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, TSource defaultValue)
{
    if (source == null)
        throw Error.ArgumentNull(nameof(source));
    if (predicate == null)
        throw Error.ArgumentNull(nameof(predicate));

    using var result = source.Where(predicate).Take(1).GetEnumerator();

    if (result.MoveNext())
        return result.Current;

    return defaultValue;

    // Skip this stuff
    /*   
    return source.Provider.Execute<TSource>(
        Expression.Call(
            null,
            CachedReflectionInfo.FirstOrDefault_TSource_4(typeof(TSource)),
            source.Expression, Expression.Quote(predicate), Expression.Constant(defaultValue, typeof(TSource))
        ));
    */
}

But this is less than ideal for the obvious reason that it cannot be used in the inner part of a query, like this:

var query = context.SomeTable.Select(t => t.SubTable.Select(st => st.Value).FirstOrDefault(v => v > 0, -1));

@eiriktsarpalis @stephentoub How would you recommend going about this? It may be best to only update System.Linq for IEnumerable for now and make a separate issue for Queryable since it potentially involves updating more than just the Linq libraries to implement. Or I can implement a simplification like in the above sample and we just document that it can't be used in an inner query but rather only to return the final value.

Alternatively, if you two or anyone else has any ideas, I'm all ears. It may be possible to implement this in an appropriate manner, but I'm not entirely certain what that might look like.

[eiriktsarpalis]

@Foxtrek64 I rebased your branch and added fixes for the failing tests.

[Foxtrek64]

@Foxtrek64 I rebased your branch and added fixes for the failing tests.

Seems a mix of me just being blind and missing a couple things and some random number being wrong somewhere that I would have never thought to look at. Glad we got that sorted.

[eiriktsarpalis]

Thanks @Foxtrek64!