I've been quiet on this blog for a little while. Sure some of that is life getting in the way, but when I've had my personal laptop most of that time has been spent working on a new Slack application. What that's meant is that I've been using my own Slack NuGet package intensively and there are a few things about it that are starting to niggle me a little.
That's okay, it's open source, I can make changes. But these changes are...larger than usual. There are some fundamentals I want to change. And as happens with larger codebases, there are rules you want to adhere to moving forward.
One such rule is that within Slack there are "Message Blocks". Now these blocks are just regular classes at the moment - but each block has parameters that have to be passed in for it to be valid within a Slack message. So as part of my little revamp I want to make sure that each class has an empty constructor (so it's valid for JSON deserialization) and a "helper" constructor with the required parameters.
Simple - write a few constructors, and I'm done.
Me knowing it isn't good enough
I'm the package author, I just came up with this rule in my head, I know this rule exists, all is right with the world.
Except the package is open source, and I want people to contribute. Having been on the other side of the conversation it can be a little frustrating when you've worked really hard on a pull request, you submit, then you're told about the unwritten rule that it should be structured differently. I don't want that for the people that open this up.
Sure I could update the docs (I really do need to look at a contributing.md file at some point - I really do) but the issue I have with that is that often that's still only looked at once a submission is ready, and that's a little too late.
What I want is for contributors to be told about this while they're coding it, not breaking the build - but definitely as a warning. I need some Roslyn Analyzers!
Adding the analyzer
I'm putting this section in as much for myself as anyone as this was hard to hunt down. When you do Solution->Add New Project->Roslyn Code Analyzer
you get a bunch of new projects added to your solution, because it assumes you're adding code fixes, a NuGet package as well as a VSIX for a Visual Studio extension. That's serious overkill for this, I just want something I'm referencing locally to add some warnings. So I remove all the new projects with the exception for the actual Analyzer project.
And then I need the main project to use the analyzers, which means tweaking its csproj file to add the following project reference
<ProjectReference Include="..\Slack.NetStandard.Analyzers\Slack.NetStandard.Analyzers.csproj"
PrivateAssets="all"
ReferenceOutputAssembly="false"
OutputItemType="Analyzer" />
So now anything I raise as a concern through the analyzer will appear when building or working on the main project. Fantastic.
The Rule
So now I have the Roslyn Analyzer project set up, time to actually get it to enforce this rule of mine.
In the analyzer class I alter the Initialize
method register an action
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSymbolAction(CheckBlockConstructors, SymbolKind.NamedType);
}
So CheckBlockConstructors
is going to be called each time there's a named type. So first thing for me to do is make sure that method is shortcut if it's called for something that's not a message block class
private static void CheckBlockConstructors(SymbolAnalysisContext context)
{
var namedTypeSymbol = (INamedTypeSymbol)context.Symbol;
if (namedTypeSymbol.TypeKind != TypeKind.Class ||
!namedTypeSymbol.AllInterfaces.Contains(context.Compilation.GetTypeByMetadataName("Slack.NetStandard.Messages.Blocks.IMessageBlock")))
{
return;
}
...
And now I know I'm dealing with an IMessageBlock
, I need to ensure that there are at least two constructors - one of which must be empty for serialization
var constructors = namedTypeSymbol.Constructors;
if (constructors.Length >= 2 && constructors.Any(c => c.Parameters.IsEmpty))
{
return;
}
And if I'm still in the method, that means my rule has been broken, and I should warn whoever is editing the code
var incorrectBlockConstructors = Diagnostic.Create(Rule, namedTypeSymbol.Locations[0], namedTypeSymbol.Name);
context.ReportDiagnostic(incorrectBlockConstructors);
Good Result
So there we are. Not difficult to be maintained as part of the project, someone can read the code to see what we're checking and why. And then when someone (most likely myself) makes a change in the future and I forget to ensure my own rules are being followed - I get this in Visual Studio
This also has the advantage of making sure the rule in my head makes sense, because if I can't cleanly write an analyzer for it - then maybe it's something that needs a little more thought.