[GH-1] - Checking constructor arguments passed via Substitute.For/Substitute.ForPartsOf

[tpodolak]

Substitute.For/Substitute.ForPartsOf constructor usage analyzers

This one comes together with two code fix providers. First one removes arguments from Substitute.For when used with interface. So it converts Substitute.For<IFoo>(1, 2, 3) to Substitute.For<IFoo>(). The second one replaces usages of Substitute.ForPartsOf to Substitute.For when ForPartsOf is used with interface or delegate.
When it comes to analyzers apart from the one which corresponds to beforementioned fix-providers, I've implemented:

• analyzers for checking if constructors are accessible
• analyzers for checking if parameters match any constructor
• analyzers for checking if parameters count matches any constructor
• analyzers for checking for proper usage of multi-generic For/ForPartsOf e.g Substitute.For<Class1,Class2> is invalid whereas Substitute.For<Class1, Interfacd1> is ok

There were a couple of unexpected behaviors I wasn't aware of before, such as

public class Foo(decimal x)
{
}

var x = new Foo(1); // works fine
var @for = Substitute.For<Foo>(1); // crashes in runtime during proxy generation

so if you are aware of any other corner-cases let me know. I am a bit afraid of VisualBasic implementation as I don't know that language that well (and I had to analyze the syntax model not only semantic model), it seems to work fine with the examples converted from c# but I might have missed something.

PS
I am not that good at naming things/messages so if you think given diagnostic message is not descriptive enough let me know

PPS
For some reasons github show much more files in diff then it is supposed to. Either I did a wrong merge or it is a Github issue

[coveralls]

Pull Request Test Coverage Report for Build a94vxlc6i7y4fhg2

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-1.1%) to 97.273%

---

Totals
Change from base Build 2llh38knwbdcuwj7:	-1.1%
Covered Lines:	428
Relevant Lines:	440

---

💛 - Coveralls

[dtchepak]

This looks amazing @tpodolak! I should be able to have a more thorough look at this over the weekend.

PS: I've updated the docs to strongly advise the installation of NSubstitute.Analyzers along with the core NSubstitute package, and also included a dedicated doc page for the Analyzers project. This will go live with the next NSub release. I'll try to keep the dedicated doc page up to date with Analyzer features, but if you notice I miss any please raise an issue and I'll sort it out.

[dtchepak]

Running through a large test project found false-positives if there is a generic T : class constraint. Minimal repro:

public class Sample {
        public T Foo<T>() where T : class {
            return Substitute.For<T>();
        }
}

Very edge-case, but can come up when writing some test infrastructure for a project.

[dtchepak]

How do I activate the quick fixes? Jumping to the compile error location doesn't seem to give me the option:

Also these compiler errors should have NS codes like the other errors?

Ideally I think the errors should contain info on how to fix the problems. For example:

Arguments passed to Substitute.For<Foo> do not match the constructor arguments for Foo. Check the Foo constructors and make sure you have passed the required arguments and argument types.

Can only substitute for parts of classes, not interfaces or delegates. Use Substitute.For<IFoo> instead of Substitute.ForPartsOf<IFoo> here.

Can not provide constructor arguments when substituting for an interface. Use Substitute.For<IFoo>() instead.

Ideally with Foo and IFoo replaced with the actual types causing a problem. 😉

(sorry for brain-dumping all this info; I was just scribbling notes while playing around with this)

[tpodolak]

@dtchepak

How do I activate the quick fixes? Jumping to the compile error location doesn't seem to give me the option:

Quick fixes are visible once the warning squiggle is shown in editor. This is managed by VS and it works fine for my instance of VS2017 (didnt test it on other instances). I had similar behavior once after upgrading the analyzer, I had to restart VS and reinstall the analyzer itself, so that VS picks it correctly.

Also these compiler errors should have NS codes like the other errors?

when ti comes to compiler errors, VS adds them automatically based on what is returned by analyzer, in my case it works as expected as well

Ideally I think the errors should contain info on how to fix the problems. For example:

I will add more descriptive messaging ;]

Running through a large test project found false-positives if there is a generic T : class constraint.

Will take a lookt at this one

[alexandrnikitin]

Great job!! @tpodolak Thank you for doing this!

LGTM! (Although I superficially reviewed it)

[tpodolak]

@dtchepak

Running through a large test project found false-positives if there is a generic T : class constraint. Minimal repro:

public class Sample {
public T Foo() where T : class {
return Substitute.For();
}
}
Very edge-case, but can come up when writing some test infrastructure for a project.

This one should be fixed now

[dtchepak]

@tpodolak Ah, I forgot the first rule of computing: try turning it off and on again 🤦‍♂️ . Works fine now. 😄Can confirm the T : class thing works now too. 👍

Did you want to merge this now? We can push description changes to another change (feel free to assign to me). Happy for you to merge this in whatever way is easiest for you.

[tpodolak]

I would wait with merge as I am now working on message changes. Will push them today or tomorrow morning for the review, then we can merge. No need to create separate PR for that.

[dtchepak]

@tpodolak Great. Feel free to push things out to other issues though if it is something others can work on, or even if it's just something you don't feel like working on at the moment. You're carrying the whole workload for this at the moment so don't want you getting burned out chasing up my little suggestions. 😂

[tpodolak]

@dtchepak messages should be fixed now, please take a look if they are good enough

[dtchepak]

@tpodolak Looks great 👍. I fixed a spacing issue in #14. If you're happy with that please feel free to merge the whole lot in. 😄