Add ability to provide a 'do' action on Arg.Is

[asos-AdamCox]

This change allows NSubstitute to support Capture.In functionality

[asos-AdamCox]

One question around this stuff - should this be an overload of Arg.Do (as it currently is in the PR) or Arg.Is or maybe some other method being as it does both things? (IsDo?)

[asos-AdamCox]

I realised yesterday that Arg.Do can be used both when setting up the mock and also when verifying and the changes I'd made only worked during setup - I've now added a test for the usage during verification and implemented the changes required for the test. Is anyone available to review this PR?

[asos-AdamCox]

I'm really very sorry to keep chasing about this - is there some step I'm missing to get this PR moving forward?

[dtchepak]

Hi @asos-AdamCox ,

Thanks a lot for this MR, and apologies for the delay in looking at it.

should this be an overload of Arg.Do (as it currently is in the PR) or Arg.Is or maybe some other method being as it does both things? (IsDo?)

Yeah I'm not sure about this either. Arg.Is(match, doThing) doesn't read that nicely imo.

What about something like this?

public class Match<T>
{
    private Expression<Predicate<T>> matches;
    private Action<T> action;

    internal Match(Expression<Predicate<T>> matches, Action<T> action)
    {
        this.matches = matches;
        this.action = action;
    }

    public Match<T> AndDo(Action<T> action) =>
        new Match<T>(matches, x => { this.action(x); action(x); });

    public static implicit operator T(Match<T> match) =>
        ArgumentMatcher.Enqueue<T>(
            new ExpressionArgumentMatcher<T>(match.matches),
            x => match.action((T)x)
        );

}

public static class Match
{

    public static Match<T> When<T>(Expression<Predicate<T>> matches) =>
        new Match<T>(matches, x => { });
}

Then the test becomes:

            var stringArgs = new List<string>();
            _sub.Bar(
                Match.When<string>(x => x.StartsWith("h"))
                .AndDo(x => stringArgs.Add(x)),
                Arg.Any<int>(),
                _someObject
            );

WDYT?

⚠️ Haven't tested received, reports of non-matches, by-ref, or other use cases that could cause issues 😅

[asos-AdamCox]

Hi @dtchepak, thanks for getting back to me - I like the Match syntax my only reservation is that by introducing Match it'll mean setups and verifications then may have a combination of Arg and Match and Arg.Is<T>(Expression<Predicate<T>> predicate) is exactly the same as Match.When<T>(Expression<Predicate<T>> predicate) which feels a little odd to me and feels like it might be confusing to consumers. Having said that I'm not familiar enough with this library to propose any more alternatives so I'm happy to go with Match if that's your preference.

For now I've added Match to the PR and left unit tests in place for both approaches to allow you to compare - please let me know your preference. I've also renamed the Is method to IsAndDo in case that's any better?

with regards to:

Haven't tested received, reports of non-matches, by-ref, or other use cases that could cause issues

I will try to spend some time of the next few days investigating where there's missing unit tests - if you know off-hand please let me know.