Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have you considered writing Roslyn analyzers & quick fixes for Moq4? #522

Closed
Litee opened this issue Nov 18, 2017 · 12 comments
Closed

Have you considered writing Roslyn analyzers & quick fixes for Moq4? #522

Litee opened this issue Nov 18, 2017 · 12 comments

Comments

@Litee
Copy link

Litee commented Nov 18, 2017

Hi,

I noticed that in moq/moq repository there was a very basic analyzer functionality. Do you think that Moq4 will benefit from having an official pack of analyzers/quick fixes? I can see that xUnit project did such thing and find it very useful. I have build several Moq helpers myself and could contribute those.

@Litee
Copy link
Author

Litee commented Nov 18, 2017

Also - in VS 2017+ there is a usable auto-completion API, which can be used for nice tricks like suggesting myMock.Object.

@stakx
Copy link
Contributor

stakx commented Nov 19, 2017

Hi @Litee, thanks for raising this topic. I'm also quite impressed by xUnit.NET's analyzers. I can't speak for other devs, but yes, I've thought about this before. Just didn't get around to discussing it with kzu yet, as I've been prioritising bugfixing so far.

Do you think that Moq4 will benefit from having an official pack of analyzers/quick fixes?

This obviously depends on the quality and usefulness of the analyzers & quick fixes that would be included in such a package. What did you have in mind?

@Litee
Copy link
Author

Litee commented Nov 19, 2017

Here is a Resharper plugin I wrote - https://github.com/Litee/AgentZorge. Recently I started porting this functionality into Roslyn (https://github.com/Litee/Agent.Zorge.Moq), but have not battle-tested this port yet.

My plugin is focused on code completions to speed up tests writing. I created only one analyzer to verify callback signatures (problem that happened with me from time to time).

@stakx
Copy link
Contributor

stakx commented Nov 21, 2017

Cheers, I'll check out your analyzer package when I find a free moment.

I created only one analyzer to verify callback signatures

That's the kind of thing I was also thinking of... catching usage errors as early as possible. 👍

Another example would be checks whether you're trying to set up things that can't be mocked / intercepted (private or invisible internal members requiring a [InternalsVisibleTo] assembly-level attribute for DynamicProxy, sealed or static members, etc.).

One approach to identify further opportunities for analyzers might be to look at Moq's code base and see where it throws ArgumentExceptions (and subclasses of it). Those often indicate usage errors; if those could be caught already at compile-time, that might be helpful.

I think having analyzers would be neat, but we'd probably need more than just one or two to start with (to make them worthwhile for users), and to make a commitment to actively maintain them along with the main library.

@Litee
Copy link
Author

Litee commented Nov 21, 2017

Yes, makes sense. We can leave this thread for ourselves and other people to throw in ideas on what can be checked.

@Litee
Copy link
Author

Litee commented Nov 23, 2017

FYI - the fastest way to see potential analyzers is to look at Assert.Throws usages in tests. I can see at least several candidates in MockFixture class alone.

@kzu
Copy link
Member

kzu commented Jan 5, 2018

Totally. Currently, the vNext moq/moq repo uses analyzers + codefixes to generate the "proxy" classes at design-time and avoid all runtime codegen (allows it to run everywhere, generated code is netstandard 2.0 :D). They also detect out of date generated code for quickly updating it.

Down the road, I'm going to create analyzers to flag as errors those invalid usages (things that can't be mocked or interecepted, as mentioned). I once the baseline 5.0 is out, we can start figuring out which ones are the most useful.

Stay tunned! :)

@stakx
Copy link
Contributor

stakx commented Feb 26, 2018

@Litee: Now that we are getting closer to the release of Moq 5, I see one potential (but admittedly minor) usability issue coming up: Your package Moq.Analyzers might have been written with Moq 4 in mind, and given its "official"-sounding name, it might be a good idea to make sure (as soon as you reasonably can) that it also works well against Moq 5.

Otherwise I think it would be good to update the package with either (a) an explicit package dependency on Moq, version somewhere <5, or (b) a hint in the release notes. Just so users know what they're getting and what will work.

Also, given that your package is in the now reserved package namespace Moq.* (see #574 (comment)) and not in the contrib namespace, perhaps it would be good to discuss with @kzu how collaboration / coordination should happen between your project and the main Moq project to ensure that users have a good experience.

These are only suggestions. Any thoughts?

@Litee
Copy link
Author

Litee commented Feb 26, 2018

@stakx, all makes sense, let me think what I can do. Will reply in next couple of days.

@stakx
Copy link
Contributor

stakx commented Feb 26, 2018

@Litee, I forgot to mention, if you want to reach out to kzu, it's probably easiest right now to use the Gitter chat for Moq.

@Litee
Copy link
Author

Litee commented Mar 10, 2018

@stakx, little delay on my side because of a biz trip, but now I am on it.

@stakx
Copy link
Contributor

stakx commented Jul 13, 2018

Closing this dormant issue, but marking it as "unresolved" so it can be easily found again. Please see #642 for details. If you'd like to pick this up again, please post here briefly.

@stakx stakx closed this as completed Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants