Creating an Analyzer For Sealing Classes
I was talking with a co-worker today when the topic of code analyzers came up. He stated that one analyzer he would really like to see is one that warns you if you are not sealing classes that do not explicitly define abstract or virtual methods. This is somewhat of a religious debate among developers. If you don't believe me, just read the comments on Eric Lippert's post on the subject. I am not going to debate the validity of the claims on either side, rather I am going to focus on how one would implement an analyzer to see if a class should be sealed if it does not explicitly define functionality to be extended.
To start, I will register an analyzer to look at all class declarations.
context.RegisterSyntaxNodeAction((syntaxNodeContext) =>
{
} , SyntaxKind.ClassDeclaration);
Next, I will check if the class is static using the Modifiers
property. If it is, I won't bother analyzing it:
var node = syntaxNodeContext.Node as ClassDeclarationSyntax;
// We don't care about sealing static classes
if (node.Modifiers.Where(x => x.IsKind(SyntaxKind.StaticKeyword)).Any())
return;
We can also rule out any classes that are already sealed by checking the Modifiers
for the SealedKeyword
:
// The class is already sealed, no reason to analyze it
if (node.Modifiers.Where(x => x.IsKind(SyntaxKind.SealedKeyword)).Any())
return;
At this point, we have a non-static, non-sealed class, so we need to check the points that can be extended via inheritance. Based on the MSDN docs for abstract and virtual, we know that we need to check methods, properties, events, and indexers. We can get those from the Members
of the class:
var methods = node.Members.Where(x => x.IsKind(SyntaxKind.MethodDeclaration));
var props = node.Members.Where(x => x.IsKind(SyntaxKind.PropertyDeclaration));
var events = node.Members.Where(x => x.IsKind(SyntaxKind.EventDeclaration));
var indexers = node.Members.Where(x => x.IsKind(SyntaxKind.IndexerDeclaration));
Next, we just have to loop over those declarations to determine if any of them are abstract or virtual. If any of them are, then we know we don't need to raise a diagnostic:
foreach (var m in methods)
{
var modifiers = (m as MethodDeclarationSyntax)?.Modifiers.Where(x=>x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var p in props)
{
var modifiers = (p as PropertyDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var e in events)
{
var modifiers = (e as EventDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var i in indexers)
{
var modifiers = (i as IndexerDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
Finally, if we have gone through all of the possible inheritance points and still have not hit an explicit declaration of intent for inheritance, we can raise our diagnostic:
// We got here, so there are no abstract or virtual methods/properties/events/indexers
syntaxNodeContext.ReportDiagnostic(Diagnostic.Create(Rule, node.GetLocation()));
So, our full diagnostic is:
public override void Initialize(AnalysisContext context)
{
context.RegisterSyntaxNodeAction((syntaxNodeContext) =>
{
var node = syntaxNodeContext.Node as ClassDeclarationSyntax;
// We don't care about sealing static classes
if (node.Modifiers.Where(x => x.IsKind(SyntaxKind.StaticKeyword)).Any())
return;
// The class is already sealed, no reason to analyze it
if (node.Modifiers.Where(x => x.IsKind(SyntaxKind.SealedKeyword)).Any())
return;
var methods = node.Members.Where(x => x.IsKind(SyntaxKind.MethodDeclaration));
var props = node.Members.Where(x => x.IsKind(SyntaxKind.PropertyDeclaration));
var events = node.Members.Where(x => x.IsKind(SyntaxKind.EventDeclaration));
var indexers = node.Members.Where(x => x.IsKind(SyntaxKind.IndexerDeclaration));
foreach (var m in methods)
{
var modifiers = (m as MethodDeclarationSyntax)?.Modifiers.Where(x=>x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var p in props)
{
var modifiers = (p as PropertyDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var e in events)
{
var modifiers = (e as EventDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
foreach (var i in indexers)
{
var modifiers = (i as IndexerDeclarationSyntax)?.Modifiers.Where(x => x.IsKind(SyntaxKind.AbstractKeyword) || x.IsKind(SyntaxKind.VirtualKeyword));
if (modifiers != null && modifiers.Any())
return;
}
// We got here, so there are no abstract or virtual methods/properties/events/indexers
syntaxNodeContext.ReportDiagnostic(Diagnostic.Create(Rule, node.GetLocation()));
} , SyntaxKind.ClassDeclaration);
}
I have also posted the full diagnostic in my analyzer samples project on GitHub.
Despite what side of the argument you are for in this debate, you can see how to create an analyzer that would allow catch classes that are not intended for inheritance and ensure they are not inheritable. It is up to you whether you would include this analyzer in your list of enabled analyzers.