Skip to content

Implemented #634: Arg.AnyType can be used in place of a generic type parameter#715

Merged
dtchepak merged 6 commits intonsubstitute:mainfrom
icalvo:anytype
Aug 27, 2023
Merged

Implemented #634: Arg.AnyType can be used in place of a generic type parameter#715
dtchepak merged 6 commits intonsubstitute:mainfrom
icalvo:anytype

Conversation

@icalvo
Copy link
Copy Markdown
Contributor

@icalvo icalvo commented May 22, 2023

The following is a complete NUnit test with all the implemented features. The only compromise I took is Arg.DoForAny(action), instead of Arg.Do<Arg.AnyType>(action), but I couldn't make it work.

[TestFixture]
public class GenericArguments
{
    public interface ISomethingWithGenerics
    {
        void Log<TState>(int level, TState state);
    }

    [Test]
    public void Return_result_for_any_argument()
    {
        string argDoResult = null;
        int? whenDoResult = null;
        bool whenDoCalled = false;

        ISomethingWithGenerics something = Substitute.For<ISomethingWithGenerics>();
        something.Log(Arg.Any<int>(), Arg.DoForAny(a => argDoResult = a.ToString()));
        something
            .When(substitute => substitute.Log(Arg.Any<int>(), Arg.Any<Arg.AnyType>()))
            .Do(info =>
            {
                whenDoResult = info.ArgAt<int>(1);
                whenDoCalled = true;
            });

        something.Log(7, 3409);

        something.Received().Log(Arg.Any<int>(), Arg.Any<Arg.AnyType>());
        something.Received().Log(7, 3409);
        Assert.That(whenDoCalled, Is.True);
        Assert.That(argDoResult, Is.EqualTo("3409"));
        Assert.That(whenDoResult, Is.EqualTo(3409));
    }
}

@icalvo icalvo mentioned this pull request May 23, 2023
Closed
@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Jun 3, 2023

This is really cool, thanks!
I'll have a play around and see if I can sort something out with Arg.Do<AnyType>.

@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Jun 4, 2023

I'm not convinced this alternative is a good idea, but one possible option is to store the specific value in AnyType:

public class AnyType {
    internal AnyType(object value) => Value = value;
    public object Value { get; private set; }
}
// ... 
public static ref T Do<T>(Action<T> useArgument)
{
    if (typeof(T) == typeof(AnyType))
    {
        return ref ArgumentMatcher.Enqueue<T>(
            new AnyArgumentMatcher(typeof(T)),
            x => {
                // Need to cast AnyType to object, then to T to be compatible with Action<T>
                object arg = new AnyType(x!);
                useArgument((T) arg);
            }
        );
    }

    return ref ArgumentMatcher.Enqueue<T>(new AnyArgumentMatcher(typeof(T)), x => useArgument((T) x!));
}

This let's us use the underlying value in the callback, but the .Value is a bit clunky to use.

[Test]
public void When_AnyType_Do() {
    var something = Substitute.For<ISomethingWithGenerics>();
    var output = new List<object>();
    // can we remove the `.Value`?
    something.Log(1, Arg.Do<Arg.AnyType>(x => output.Add(x.Value)));

    something.Log(1, 2);
    something.Log(1, "abc");

    Assert.AreEqual(new object[] { 2, "abc"}, output.ToArray());
}

I'm worried requiring .Value is non-obvious to callers, as it is natural to not see Arg.AnyType as an actual type, but instead more like as generic T. It might be better to stick with the existing DoForAny and exception from this MR.

@icalvo do you have any thoughts on this? (@alexandrnikitin , @zvirja , would be good to get your views on this too if you get a chance.)

@icalvo
Copy link
Copy Markdown
Contributor Author

icalvo commented Jun 5, 2023

@dtchepak I found a solution using an overload that seems to solve all the problems, please take a look!

@icalvo icalvo marked this pull request as ready for review June 13, 2023 09:32
@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Aug 13, 2023

Hi @icalvo ,

Sorry for how long it has taken for me to get to this. 😓

I think I have a fix for the AppVeyor build. Please cherry-pick from #724 and review that commit to make sure it looks ok.

dtchepak
dtchepak reviewed Aug 13, 2023
View reviewed changes
@icalvo
Copy link
Copy Markdown
Contributor Author

icalvo commented Aug 13, 2023

Hi @icalvo ,

Sorry for how long it has taken for me to get to this. 😓

I think I have a fix for the AppVeyor build. Please cherry-pick from #724 and review that commit to make sure it looks ok.

I'll do in a couple weeks as soon as I'm back from holiday.

@icalvo
Copy link
Copy Markdown
Contributor Author

icalvo commented Aug 25, 2023

Hi @icalvo ,

Sorry for how long it has taken for me to get to this. 😓

I think I have a fix for the AppVeyor build. Please cherry-pick from #724 and review that commit to make sure it looks ok.

It works and looks fine to me!

@dtchepak dtchepak merged commit 41bba78 into nsubstitute:main Aug 27, 2023
@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Aug 27, 2023

Thanks so much for this @icalvo ! Sorry for bothering you on your holiday. Hope you had a nice break 😊

@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Aug 27, 2023

@tpodolak Are any analyser changes required for this?

@icalvo icalvo deleted the anytype branch August 27, 2023 10:19
@icalvo icalvo mentioned this pull request Aug 29, 2023
Merged
@akousmata
Copy link
Copy Markdown

akousmata commented Aug 31, 2023

When is this available? I pulled v 5.0.0 via nuget and this change wasn't present.

@ivanfranco502
Copy link
Copy Markdown

ivanfranco502 commented Aug 31, 2023

When is this available? I pulled v 5.0.0 via nuget and this change wasn't present.

v5.0.0 is from February. There is another fix related to AnyType done by icalvo, so probably both need to be merged before releasing it.

@tpodolak
Copy link
Copy Markdown
Member

tpodolak commented Sep 6, 2023 •
edited
Loading

@tpodolak Are any analyser changes required for this?

@dtchepak I am not 100% sure, but we might start getting false positives from call info analyzers in cases like
https://github.com/nsubstitute/NSubstitute/pull/715/files#diff-3e581ff709baca8685f9657dd9c87a951216e44daf2eccf15986552881bd2499R29
but I would need to check that. Do we have any prerelease package ready so I can test couple of scenarios?

@dtchepak
Copy link
Copy Markdown
Member

dtchepak commented Sep 8, 2023

Do we have any prerelease package ready so I can test couple of scenarios?

The latest CI build should have this change:
https://ci.appveyor.com/project/NSubstitute/nsubstitute/build/artifacts

@tpodolak
Copy link
Copy Markdown
Member

tpodolak commented Sep 8, 2023

image
CallInfo analyzers will produce false positives. Added task to disable analysis when Arg.AnyType used nsubstitute/NSubstitute.Analyzers#212

@tpodolak tpodolak mentioned this pull request Sep 8, 2023
Closed
@dependabot dependabot bot mentioned this pull request Aug 18, 2025
Open
@dependabot dependabot bot mentioned this pull request Sep 22, 2025
Closed
@dependabot dependabot bot mentioned this pull request Oct 7, 2025
Merged
This was referenced Oct 23, 2025
Merged
Merged
Closed
@dependabot dependabot bot mentioned this pull request Nov 3, 2025
Merged
@dependabot dependabot bot mentioned this pull request Dec 1, 2025
Closed
@dependabot dependabot bot mentioned this pull request Dec 8, 2025
Closed
@dependabot dependabot bot mentioned this pull request Dec 26, 2025
Merged
@dependabot dependabot bot mentioned this pull request Jan 20, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dtchepak dtchepak dtchepak approved these changes

+1 more reviewer

@ivanfranco502 ivanfranco502 ivanfranco502 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@icalvo @dtchepak @akousmata @ivanfranco502 @tpodolak