search

dotnet / codeformatter

Tool that uses Roslyn to automatically rewrite the source to follow our coding styles
MIT License
1.24k stars 244 forks source link

Allow codeformatter to use analyzers from external DLLs #217

Closed danquirk closed 8 years ago

danquirk commented 8 years ago

No corresponding issue for this as far as I know but I can create one if you'd like. I've talked to @srivatsn about this a bit in generalities.

The basic idea is to pass codeformatter a DLL which contains some analyzers and it will run those analyzer(s) against the given target(s). The targets can be project path or a file listing project path and the analyzers can be a path to the DLL or a file listing paths to multiple DLLs. A new analyze command is added to do only analysis of the target without applying any corresponding code fixes from the analyzers.

I had to upgrade to newer versions of Roslyn and System.Reflection.Metadata to support the latest analyzers from Roslyn and 3rd parties. I had tried to add new analyzers just using MEF rather than IAnalyzerFileReference (see this branch) which seemed like it should be cleaner but didn't seem to work for certain analyzers. There's some unfortunate reflection into Roslyn for internal types that may not be the fastest/best solution long term.

TODO:

dnfclas commented 8 years ago

Hi @danquirk, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

dnfclas commented 8 years ago

@danquirk, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.
Thanks, DNFBOT;

basoundr commented 8 years ago

LGTM pending the testcase

basoundr commented 8 years ago

@danquirk Do you plan on addressing the TODO in this PR or a follow-up PR? If you are planning on doing a follow-up PR then we can merge this change in

danquirk commented 8 years ago

@basoundr unless you have an immediate need for it in its current form I think I'd prefer to just do the fixes in this PR and then have it all merged at once in a better state. I'll try to get to these other review comments shortly.

basoundr commented 8 years ago

Our team is having a FxCop day(All our team members work on just the analyzers-related things on this day) this Thursday. We were hoping that, if you could have this change merged in and have CodeFormatter run on repos like CoreFx & others, which are amazing testbeds for analyzers, then we might hit some bugs in the Analyzers and have those bugs addressed during our FxCop day. Is this something that would fit into your schedule?

danquirk commented 8 years ago

Yeah, that seems doable.

basoundr commented 8 years ago

Awesome! I will merge this PR in and I will create an issue with the TODO mentioned in the PR

basoundr commented 8 years ago

@danquirk I have created the issue #225 for you to track the TODOs. I am not able to assign the issue to you, so please assign it yourself.

Also it would be great if you could send any bugs that you find to @srivatsn, @mavasani, @heejaechang or @basoundr(me). Thanks :)