Over the last few months I have been having a wonderful time developing Roslyn analyzers and code fixes. You can find all the Wintellect.Analyzers code at Wintellect’s GitHub page. If you would like to include these analyzers in your own Visual Studio 2015 CTP 6 project, install the NuGet package by executing the following in Visual Studio’s package manager console window:
- Install-Package Wintellect.Analyzers Prerelease
Now that I’m getting more fluent in analyzers and code fixes I wanted to share some of the things I’ve learned to save you some time when you start doing your own in the future. First I’ll go over some of the new analyzers I’ve written. Next I’ll turn to the question of using analyzers and code fixes as NuGet packages vs. a Visual Studio VSIX. Finally, I’ll jump into some of the development aspects of analyzer and code fix development where I’ve had issues. Obviously, in that section, I’m assuming you know something about Roslyn and analyzer development. If this is all new to you Alex Turner’s two part series in MSDN Magazine gives you a solid understanding quickly: Part 1, Part 2.
Since I first blogged about Wintellect.Analyzers I’ve added another eight rules to the mix. If you have ever been to one of my classes or presentations, or even read my old book, you know I am quite fond of Code Analysis (AKA FxCop). Even though it was completely undocumented, I wrote numerous rules for Code Analysis and in Wintellect.Analyzers, I’ve been porting them to Roslyn. The first were rules that looked to see if you filled out the company attribute, copyright attribute, description attribute, and title attribute. Those rules were very simple but when you look at the implementation, you can see how to combine multiple analysis into a single analyzer.
The next analyzer I added was one which looks at any SuppressMessage attributes you have and ensures you have filled out the Justification property. This is an important analyzer because you never want anyone suppressing any error or warning without stating why they are doing that suppression. When I wrote this rule for Code Analysis it took a lot of work because of how Code Analysis is implemented. With Roslyn, the implementation was trivial. It’s nice when an architecture lets you do easy things easily.
The next analyzer I added is one of my favorites: exception documentation missing. You should never wonder what exceptions are directly thrown out of an API because the <exception> XML documentation comment tag is there to document them. The only problem is that it’s not required, which is a wrong in my opinion. Now you have an analyzer which enforces this rule so all directly thrown exceptions are documented. Note that a direct exception is one where you specifically use the throw keyword.
The most recent analyzer and code fixer I wrote is use DebuggerDisplay attribute. The idea is that if you have a publically visible class you should have a DebuggerDisplayAttribute as the debugger will use that to show the key information about that instance in all expression evaluators (Data Tips, Watch/Local/Autos/Immediate windows). Developing the analyzer was relatively easy, but I wanted to include a code fix with this to automatically insert the DebuggerDisplay attribute. If the class was derived from IEnumerable, I wanted the display string to be “Count={Count()}”otherwise, I wanted to automatically include the first two properties. If there were less than two properties, I wanted to fill in the display string with up to two fields. Additionally, I wanted to include a TODO comment to remind developers to update the strings as appropriate. Finally, I had to ensure that System.Diagnostic was imported as well. With that much going on in the code fixer, and since there’s not a lot of documentation yet, it took me quite a while to find all the gotchas and edge cases to ensure I was not going to screw up anything with the changes I was making. Even though the implementation doesn’t look that big, there’s a lot of work found through trial and error as this is where I truly learned the Roslyn API. I’m happy how it came out.
With my new analyzer and code fixer discussion out of the way, I want to turn to the question of how I think you should use anyone’s analysis packages in the real world. Analyzers can be distributed in two ways, a NuGet package or a Visual Studio VSIX. I’ve discussed the two approaches with numerous folks and it’s become abundantly clear to me the only way to consume analyzers and code fixes are through NuGet packages. This way the analyzers and code fixes are always part of the project so everyone on the team, including the build server, gets them. Do keep in mind that I am talking about consumers of analyzers here. When developing analyzers it’s convenient to install the VSIX for testing purposes so you can see how your code runs across many existing projects without needing to touch those projects. But for everyone else, it’s NuGet all the way. Please understand that I’m only talking about analyzers and code fixes here. If you want to develop and share a refactoring, those belong in VSIX files.
For those of you thinking about developing analyzers, I’d encourage you to go for it. The whole Roslyn API feels very mature and there’s a lot of support from the Roslyn Syntax Analyzer and the extension templates. Given that the Roslyn effort has been going on for five years, it’s great to feel like you’re working with a version three on the day you start. There is a learning curve because you are talking about parsers and compilers, which you haven’t seen since college, but after a while it makes a lot of sense.
After developing a few analyzers I was feeling the API made it very easy to do things like look for a specific method call. For example, I wrote the call assert methods with message parameter analyzer to flag calls to the single parameter Debug.Assert because you should be using the multiple parameter version where the assertion is passes as the string in the second parameter. When I first wrote the rule I ask Roslyn to notify me on invocation expressions (i.e., calls to methods) where I would look to see if the string of the call was “Debug.Assert”. After doing some more analyzers getting lulled into the easy calls to .ToString(), I realized that I was falling into a trap. How did I know that the string “Debug.Assert” in the invocation was actually the call to System.Diagnostic.Debug.Assert out of SYSTEM.DLL?
Because it’s so easy to look at strings with the Roslyn API I think a lot of people writing analyzers will take the easy way out and we are going to have a lot of very brittle analyzers that have false negatives or positives. Because there’s no built in support to say is this method, class, or etc. a specific type, I’ve been developing extension methods that wrap symbol types and allow you to check against the reflection type. Under the hood I’m getting all the assembly information from Roslyn to do the checking. Additionally, I’ve been doing extension methods to get the exact method from a Roslyn type based on parameter types in much the same way. You can look at that work here. If there’s demand I’m happy to pull this work, and the symbol node extensions in the same folder, out into their own project so others can benefit. Whatever you do don’t fall into the easy string comparison trap writing analyzers!
When you create an analyzer project the Roslyn templates give you a spiffy test project as well. In all it’s a very good start but the test projects aren’t perfect. Kathleen Dollard has already filed bugs for some of these issues and has discussed them with the team. Right now the templates are not open sourced, but the team has committed to making that happen. Once we have access to the template source, we can start making those tests better.
One of the things that has tripped me up multiple times is that the tests are geared around passing in C# strings of the code to analyze and for code fixes what it’s supposed to look like after your fix has been applied. My mistake has been that I have a bug in the code, which is in a string remember, and I don’t realize it, thus I’m off on a wild debugging chase thinking my analyzer or code fix is wrong when the test fails. After the templates are open sourced, I want to fix the tests to take files so you can keep your tests in a project so you can use Visual Studio to verify the code is correct and have the tests pick up the files off the disk. For now whenever you are changing a test string, make sure to have a second instance of Visual Studio open with a throw away project so you can check the code before you past it back into a string. That will save you a lot of headaches.
One criticism I do have of the Roslyn API is when developing code fixes. When you’re replacing nodes in the syntax tree, the code is taken literally and with ZERO formatting applies. Let me save you a ton of time: whenever you are adding or replacing nodes always add .WithAdditionalAnnotations(Formatter.Annotation) to the end of the calls like ReplaceNode, AddUsings, or any other call that changes the syntax tree. Don’t forget to add the “using Microsoft.CodeAnalysis.Formatting;” because that’s required to get access to the API that saves your sanity.
In my opinion I would think the default should be to pick up the formatting of the workspace associated with the code. Something tells me that if we ask the Roslyn team there’s a whole PhD thesis worth of reasons why they ignore formatting. However, all you need to remember is that WithAdditionalAnnotations(Formatter.Annotations) is the answer to many of your code fix problems.
The last development tip I want to pass on is that your analyzers run everywhere, which seems obvious, but that includes generated or non-user code, like that from RESX designer files. There’s nothing in the Roslyn API that tells if the code you’re being run on has a GeneratedCode attribute or DebuggerNonUserCode attribute. So unless you want to report tons or problems the user can’t fix, you need to check. I’ve taken a first crack at looking for the attributes in the symbol extension project I mentioned earlier. For my purposes I’ve been fine just looking at the attributes but I need to also start looking at filenames like Roslyn does in their internal code.
As I keep developing analyzers and code fixes, I’ll keep blogging about the trials and tribulations. I’ve been having a hell of a good time writing them so I know I’ll keep going. I hope these tips helped you out and I’d love to hear your feedback on Wintellect.Analyzers. What else would you like to see? I’m happy to become your personal analyzer and code fix developer!