Proposal: Change Arg.Is(Expression<Predicate<T>>) to Arg.Is(Predicate<T>)

[ThomasBleijendaal]

Using expression trees instead of simple lambda's in Arg.Is is less than ideal. For example, Arg.Is<object[]?>(x => x?.Length == 0) is not allowed because expression trees cannot contain null propagating operators. Next to that, the error NSubstitute gives contains the expression tree representation of lambda, instead of the code written. This makes parsing the error harder as the example above becomes x => ((x != null) AndAlso (ArrayLength(x) == 0)).

This issue can be fixed by using [System.Runtime.CompilerServices.CallerArgumentExpression("predicate")] string predicateExpression = "". This attribute captures the exact expression inside the predicate parameter of Arg.Is:

public static ref T Is<T>(Predicate<T> predicate, [System.Runtime.CompilerServices.CallerArgumentExpression("predicate")] string predicateExpression = "")
{
    return ref ArgumentMatcher.Enqueue<T>(new PredicateArgumentMatcher<T>(predicate, predicateExpression));
}

That PredicateArgumentMatcher is a clone of ExpressionArgumentMatcher and looks like:

public class PredicateArgumentMatcher<T>(Predicate<T?> predicate, string predicateExpression) : IArgumentMatcher
{
    private readonly string _predicateDescription = predicateExpression;
    private readonly Predicate<T?> _predicate = predicate;

    public bool IsSatisfiedBy(object? argument) => _predicate((T?)argument!);

    public override string ToString() => _predicateDescription;
}

This results NSubstitute returning errors like:

NSubstitute.Exceptions.ReceivedCallsException : Expected to receive a call matching:
	StoreLuggage(x => x?.Length == 0)
Actually received no matching calls.
Received 1 non-matching call (non-matching arguments indicated with '*' characters):
	StoreLuggage(*<null>*)

Furthermore, calling methods is now possible from those Arg.Is expressions. It might not be the best idea to actually do, but at least all C# code is allowed, and it's no longer constraint by the expression tree limitations.

There a few challenges with this approach and that's why I've opened this issue (I might also have overlooked some details).

1: This approach requires Arg.Is to be wrapped in pragmas targetting NET6 and greater, because this feature is not available in older .NETs:

#if NET6_0_OR_GREATER
    /// <summary>
    /// Match argument that satisfies <paramref name="predicate"/>.
    /// If the <paramref name="predicate"/> throws an exception for an argument it will be treated as non-matching.
    /// </summary>
    public static ref T Is<T>(Predicate<T> predicate, [System.Runtime.CompilerServices.CallerArgumentExpression("predicate")] string predicateExpression = "")
    {
        return ref ArgumentMatcher.Enqueue<T>(new PredicateArgumentMatcher<T>(predicate, predicateExpression));
    }
#else
    /// <summary>
    /// Match argument that satisfies <paramref name="predicate"/>.
    /// If the <paramref name="predicate"/> throws an exception for an argument it will be treated as non-matching.
    /// </summary>
    public static ref T Is<T>(Expression<Predicate<T>> predicate)
    {
        return ref ArgumentMatcher.Enqueue<T>(new ExpressionArgumentMatcher<T>(predicate));
    }
#endif

2: predicateExpression is writable by the caller. SonarLinter, for example, gives a warning indicating that you should not do that, but it's not really problematic. It might also be considered a feature, so this depends on whether we like it.

3: When I was running my experiments the test CompatAndArgShouldBeInSync failed. I don't know if we can actually keep those in sync. Stuff to use in pre-C#7 compilers might not be amused by this change. So I rather discuss this before looking into that.

Let me know what you think! The change itself is rather small, but I can imagine that you might be hesitant to change something like this.

[dtchepak]

Thanks @ThomasBleijendaal . I'm spiking out a way to support more flexible arg matching at the moment. I'll will try to ensure it meets this use case. I'll ping you when i'm done to take a look if that's ok?