Tag Archives: roslyn

Roslyn analyzer with external references

I was creating a Roslyn powered analyzer to detect proper usage of WinForms components that are not added to a form through designer. When you add control through designer it is properly disposed when the form itself is disposed. The designer also adds field components of type System.ComponentModel.IContainer that is disposed in Dispose method:

protected override void Dispose(bool disposing)
{
  if (disposing && (components != null))
  {
    components.Dispose();
  }
  base.Dispose(disposing);
} 

But if you create, for example a timer, manually, you will have to dispose it manually later. Instead of handling the disposal of that timer this way, it is easier to add it to that container where it will be disposed along with all other controls.

The analyzer worked fine when a new instance of VS.NET was triggered and the debugger was actually debugging the code running in that instance. But this process is somewhat slow and you spend some time waiting for the new instance of VS.NET to be open.

When you create new Roslyn analyzer project using the project template, a test project is created as well with ready-to-run code. Because it is much easier to do it TDD way, I created new test based on the test classes already available. But my tests were failing.

I quickly discovered the problem – in the analyzer code, there is some type checking when I do not want to carry on with the analysis when the class being analyzed is not inherited from System.Windows.Forms class:

var formType = context.SemanticModel.Compilation.GetTypeByMetadataName("System.Windows.Forms.Form");

and the reason the test was failing was that formType is null and that’s because there is no metadata available when the test is executed. But it does not fail in the second instance of VS.NET, because the project is fully loaded with all references there.

So I went straight to Roslyn GitHub repository and searched for mscorlib, System.Linq, etc. and found out that the tests there are adding references through properties like this one:

private static MetadataReference s_systemWindowsFormsRef;
public static MetadataReference SystemWindowsFormsRef
{
  get
  {
    if (s_systemWindowsFormsRef == null)
    {
      s_systemWindowsFormsRef = AssemblyMetadata.CreateFromImage(TestResources.NetFX.v4_0_30319.System_Windows_Forms).GetReference(display: "System.Windows.Forms.v4_0_30319.dll");
    }

    return s_systemWindowsFormsRef;
  }
}

And the most important code here is AssemblyMetadata.CreateFromImage(TestResources.NetFX.v4_0_30319.System_Windows_Forms) which creates the assembly metadata object for System.Windows.Forms assembly. After cloning the solution I discovered that the TestResources namespace comes from external assembly Microsoft.CodeAnalysis.Test.Resources.Proprietary that was added as a Nuget package.

This library contains assembly metadata images for selected .NET framework versions and assemblies (System, System.Data, System.Drawing, …). Note that the only version of that library available is prerelease version.

What was left was to change the code generated by project template to propagate the references

VerifyCSharpDiagnostic(test, new[]
{
  AssemblyMetadata.CreateFromImage(TestResources.NetFX.v4_0_30319.System_Windows_Forms)
    .GetReference(display: "System.Windows.Forms.v4_0_30319.dll"),
  AssemblyMetadata.CreateFromImage(TestResources.NetFX.v4_0_30319.System)
    .GetReference(display: "System.v4_0_30319.dll")
}, expected); 

deeper to where the Compilation is created and change the code there:

var compilation = CSharpCompilation.Create(
   "MyName",
   documents.Select(d => d.GetSyntaxTreeAsync().Result),
   references, // there are those references
   new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, optimizationLevel: OptimizationLevel.Debug));
// and pass the analyzer 
var compilationWithAnalyzers = compilation.WithAnalyzers(ImmutableArray.Create(analyzer));

And then my unit tests worked! I recommend you to check the unit testing part of Roslyn project for some guidelines on how to test the analyzers and also how to write reusable test components (and rewrite is something my code needs right now).

Roslyn powered live code analyzer

First there was a problematic WPF binding property, then I had to check all binding properties and then I thought about using FxCop to do that dirty job for me. But unfortunately FxCop is no longer developed and supported. That made me a little bit sad, since I really liked that tool and its power and usefulness.

But then I found this article by Alex Turner Use Roslyn to Write a Live Code Analyzer for Your API and after reading that I no longer mourned for FxCop. The new Roslyn powered while-you-type code analysis and error reporting is incredible!

And I decided to write my own DiagnosticAnalyzer rule. The rule I implemented is the good old CA2000: Dispose objects before losing scope. It is already there among other FxCop rules in the Microsoft.CodeAnalyzers.ManagedCodeAnalysis rule set. But I just wanted to try it and find out how difficult it is to create such rule.

First I installed required VSIX packages, then created project with given template, copied the code from MSDN page and started the investigation on how to check if the object is disposed after all references to the object are out of scope.

First I created a dummy class with both invalid and valid usages of a disposable objects.

This is the code of DummyClass class I used for testing (test project is generated using the VS template and added to the solution).

using System.IO;
using System.Text;

class DummyClass
{
    private Stream _s;

    public DummyClass()
    {
        /* 
         * almost correct usage: 
         * value of field _s must be disposed later 
         * (maybe the rule can suggest to implement IDisposable interface) 
         */
        _s = Create();

        /* 
         * correct usage: 
         * assigning IDisposable inside using block to variables
         */
        using (Stream a = Create(), b = Create()) { }

        /* 
         * correct usage: 
         * assigning IDisposable inside using block to a previously declared variable 
         */
        Stream c;
        using (c = Create()) { }

        /* 
         * incorrect usage: 
         * not using using statement for declaration and initialization of a IDisposable variable 
         */
        var d = Create();

        /*
         * these lines were added just to prove that the rule is ignoring non-IDisposable variables
         */
        var sb = new StringBuilder(); // declaration and initialization of a non-IDisposable variable  
        StringBuilder sb2;
        sb2 = new StringBuilder(); // assigning non-IDisposable to a previously declared variable
    }

    Stream Create()
    {
        return null; // the real value is not important, return type is
    }

    public void Method()
    {
        /* 
         * incorrect usage: 
         * not using using statement for declaration and initialization of a IDisposable variable 
         */
        var stream = new MemoryStream();
    }
}

Note: I have found it very useful to keep the sample code in a separate compilable file and not in string variable in a test method. The advantage is that you know the code is valid and it is easier to locate the reported error (unless you’re debugging your project running sand-boxed Visual Studio).

Roslyn Syntax Visualizer helped me to identify the nodes I have to check, those are

  • VariableDeclaration – for example var a = DisposableObject();, note that there can be more then one variable being assigned
  • SimpleAssignmentExpression – for example a = DisposableObject();

Action has to be registered to trigger the analysis after the semantic analysis of those syntax nodes is completed

public override void Initialize(AnalysisContext context)
{
    context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.VariableDeclaration);
    context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.SimpleAssignmentExpression);
}

Note: I used one callback action for both syntax nodes, but you can register one for each node and make the code cleaner.

And the final result is here – the basic idea is to check the type of RHS node if it implements IDisposable and if it does then check if that is happening inside using block. With one exception when the value is assigned to class field.

using System.Linq;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Dev5.CodeFix.Analyzers
{
    [DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class DisposeObjectsBeforeLosingScopeRule : DiagnosticAnalyzer
    {
        public const string DiagnosticId = "DisposeObjectsBeforeLosingScopeRule";
        internal const string Title = "Dispose objects before losing scope";
        internal const string MessageFormat = "A local object of a IDisposable type is created but the object is not disposed before all references to the object are out of scope.";
        internal const string Category = "Reliability";
        internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Error, isEnabledByDefault: true);

        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
        {
            get { return ImmutableArray.Create(Rule); }
        }

        public override void Initialize(AnalysisContext context)
        {
            context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.VariableDeclaration);
            context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.SimpleAssignmentExpression);
        }

        /// <summary>
        /// Gets type symbol for System.IDisposable interface.
        /// </summary>
        /// <param name="compilation"></param>
        /// <returns></returns>
        public static INamedTypeSymbol IDisposable(Compilation compilation)
        {
            return compilation.GetTypeByMetadataName("System.IDisposable");
        }

        /// <summary>
        /// Returns boolean value indicating if the <paramref name="typeInfo"/> implements System.IDisposable interface.
        /// </summary>
        /// <param name="typeInfo">TypeInfo to check</param>
        /// <param name="compilation"></param>
        /// <returns></returns>
        private static bool IsDisposable(TypeInfo typeInfo, Compilation compilation)
        {
            if(typeInfo.Type == null)
            {
                return false;
            }
            return !typeInfo.Type.IsValueType && typeInfo.Type.AllInterfaces.Any(i => i.Equals(IDisposable(compilation)));
        }

        private void AnalyzeNode(SyntaxNodeAnalysisContext context)
        {
            var semanticModel = context.SemanticModel;
            var compilation = context.SemanticModel.Compilation;
            // are we inside using block? i.e. is the Parent of current node UsingStatement
            var insideUsingStatement = context.Node.Parent is UsingStatementSyntax;

            var declaration = context.Node as VariableDeclarationSyntax;
            // variable declaration node
            if (declaration != null)
            {
                // more than one variable can be declared
                foreach (var declarator in declaration.Variables)
                {
                    var variable = declarator.Identifier;
                    var variableSymbol = semanticModel.GetDeclaredSymbol(declarator);
                    var eq = declarator.Initializer as EqualsValueClauseSyntax;
                    var varTypeInfo = semanticModel.GetTypeInfo(eq?.Value);
                    // non-disposable variable is declared or currently inside using block
                    if (!IsDisposable(varTypeInfo, compilation) || insideUsingStatement)
                    {
                        continue;
                    }

                    // report this
                    context.ReportDiagnostic(Diagnostic.Create(Rule, declarator.GetLocation()));
                }
                return;
            }

            var assignment = context.Node as AssignmentExpressionSyntax;
            if (assignment != null)
            {
                // does the type of the RHS node implement IDisposable?
                var typeInfo = semanticModel.GetTypeInfo(assignment.Right);                
                if (!IsDisposable(typeInfo, compilation))
                {
                    return;
                }

                var identifier = assignment.Left as IdentifierNameSyntax;
                var kind = semanticModel.GetSymbolInfo(identifier).Symbol;
                // assigning field value or currently inside using block
                if (kind?.Kind == SymbolKind.Field || insideUsingStatement)
                {
                    return;
                }

                // report this
                context.ReportDiagnostic(Diagnostic.Create(Rule, assignment.GetLocation()));
                return;
            }
        }
    }
}
DisposeObjectsBeforeLosingScopeRule.csview rawview file on GitHub

During the development of the analysis rule I found that the syntax visualizer is really really helpful (but if you have it installed it is good to Pin it for the sample code only, VS was quite sluggish when I switched tabs to the rule file). Google is also very helpful (as usual) since I was struggling to get the type and symbol infos.

But overall I am super excited about this functionality, writing the rules is not that difficult, the live analysis is pretty impressive and the possibilities are infinite!

The code is available on GitHub.