Overload Substitute.ForPartsOf to use Expression<Func<T>>

[DunetsNM]

Partial substitute could be created in a type-safe manner:

ForPartsOf<T>(Expression<Func<T>> constructorExpression)

Constructor parsed to determine constructor method and arguments.

Before: Substitute.ForPartsOf<MyClass>(ctorArg1, ctorArg2);

After: Substitute.ForPartsOf(() => new MyClass(ctorArg1, ctorArg2));

Benefits: static type checking if .ctor signature changed.

[dtchepak]

Hi @DunetsNM, Just to clarify:

var ctorArg1 = 42;
var ctorArg2 = "example";
// Current:
var sub = Substitute.ForPartsOf<Sample>(ctorArg1, ctorArg2);
// Proposed:
var sub = Substitute.ForPartsOf<Sample>(() => new Sample(ctorArg1, ctorArg2));

I assume the same thing should be implemented for Substitute.For<T>(ctorArgs) as well?

[DunetsNM]

Hi, Yes potentially could be useful for Substitute.For<T> too, although I've used it only for interface substitution so far so never really thought about it

[dtchepak]

Sounds good to me. Is this something you have time to look at implementing?

[DunetsNM]

Yep I can have a look in my spare time

[dtchepak]

Great! The project has fairly recently been converted to .netstandard so if there is any trouble loading/building then please let us know. (We still haven't ironed out all the rough edges.)

[alexandrnikitin]

There's a similar PR for received calls https://github.com/nsubstitute/NSubstitute/pull/72/files You might find it useful 😉

[zvirja]

I was thinking about this issue a bit and concluded that I don't see much value from it. Usually, you want to create a partial proxy for the abstract class, rather than for a regular one. And the problem is that according to the best practices, all the abstract type constructors should be marked as protected. Meaning, it will be impossible to write an expression like Substitute.ForPartsOf(() => new MyAbstractType("answer is", 42)) - it will simply not compile.

It means that usage of this feature will be limited to the non-abstract types with the public constructors only. And I'm not sure that it's a primary scenario our clients have.

Another thing I'm worried about is that we might foster people to make constructors public/internal for the abstract types, so that it's easier to test them. And that is definitely not what we want to do 😟

Given that I'd suggest to not implement the feature, unless you know that a lot of clients making proxies for non-abstract types. We might want to implement the compile time verification via analyzer in future if we wish, but not in the suggested way.

@dtchepak @alexandrnikitin Your thoughts? 😉

[dtchepak]

@zvirja Good points.

We definitely can't replace the existing call, this would be an alternate overload for use if (and only if) the class does have an appropriate constructor. It is definitely handy to have refactoring and compiler support for these. You are probably right that it won't be too many cases, but I not sure if it is going to encourage bad practices. 🤔

I raised NSubAnalysers#1 for discussion of the Roslyn option for this.

[zvirja]

but I not sure if it is going to encourage bad practices. 🤔

Well, for me this point isn't that surprising 😊 Currently most of the software I'm writing actively uses DI and the main goal is to make the code testable. Testability is a part of my design principles and very often I rewrite the code if that's hard to test it. See e.g. Test-induced design damage article about that.

In this case developers might find that if constructor is public, they could use this nice feature and therefore will make the ctor public whenever possible, violating the best practices.

---

I'd solve that purely via analyzers, as feature looks quite rare. Or, at least, de-scope that from v4 and implement it later if it's required by more guys.

[dtchepak]

@zvirja Fair enough, I've removed from v4.0 and will keep this as as a feature-request for now, pending the outcome of https://github.com/nsubstitute/NSubstitute.Analyzers/issues/1.

[DunetsNM]

Usually, you want to create a partial proxy for the abstract class, rather than for a regular one

Not necessarily, it's been a while and I can't remember what my use case was but it definitely was a non-abstract non-sealed class with virtual methods

[BADF00D]

Maybe I can provide an example. I've encountered a situation where I (with my limited knowledge of NSubstitude) can't find a solution using the current Api. The suggestion would prove useful here.

I have a sut, that writes to file in a Debian file system. In order to test it, I use System.IO.Abstractions. In my tests, I want to make use of MockFileSystem, that is provided by System.IO.Abstractions, since it has a lot of handy functions to check the current state of the file system. For a bug fix, I not only have to test the final state of the file system, but also what has been written to individual files in between. For this purpose I want to put a mock over the file property that I can monitor. In FakeItEasy it would look like this:

var mockFileSystem = new MockFileSystem(new Dictionary<string, MockFileData>
{
    ["/etc/hostname"] = new("hostnamecontent"),
    ["/data/etc"] = new MockDirectoryData(),
    ["/etc/hosts"] = new("hostsContent"),
});
var fakeMockFileSystem = A.Fake<IFileSystem>(o => o.Wrapping(mockFileSystem));
var fakeFile = A.Fake<IFile>(o => o.Wrapping(mockFileSystem.File));
A.CallTo(() => fakeMockFileSystem.File)
    .Returns(fakeFile);

In NSubstitude I have failed so far because I cannot put a mock over an existing instance like in FakeItEasys (var fakeFile = A.Fake<IFile>(o => o.Wrapping(mockFileSystem.File))).