↓Skip to main content

Under the hood

·5 mins
developer story .net csharp roslyn il
Table of Contents

It’s always good to have static code analysis in your build process. I guess no one these days argues with this statement. This usually forces developer to make conscious decisions on code-level performance, reliability, security, design etc. Few times CA warnings saved me from producing a quite nasty bugs. Sometimes however FxCop yields some really strange stuff.
This post will describe the one I stuck with some time ago. But more interestingly it shows that sometimes .Net developer must look deep under the hood of high-level language abstraction to solve certain issues.

The story #

I wrote a simple unit test class for Web API controller:

public class NewsWebApiControllerTests
{
    ...
    // some mocks and testing methods goes here
    ...
    private readonly IEnumerable<NewsListItem> _expectedNews = new List<NewsListItem>
    {
        ...
        // really long list initialization...
        ...
    }

    // ...and other similar fields declarations for testing expectations
    ...
}

As you can see - nothing special. It was running locally pretty fine so I made a commit, push and… after few minutes saw this error in Team City log:

[11:21:22][Abb.One.InsidePlus\Abb.One.InsidePlus.UnitTests\Abb.One.InsidePlus.UnitTests.csproj] RunCodeAnalysis (12s)
[11:21:22][RunCodeAnalysis] CodeAnalysis (12s)
[11:21:22][CodeAnalysis] Running Code Analysis...
[11:21:22][CodeAnalysis] C:\Program Files (x86)\Microsoft Visual Studio 12.0\Team Tools\Static Analysis Tools\FxCop\FxCopCmd.exe
...
[11:21:35][CodeAnalysis] d:\BuildAgent\Work\TrunkInside\Abb.One.InsidePlus\Abb.One.InsidePlus.UnitTests\Web\Controllers\NewsWebApiControllerTests.cs(147, 0): warning CA1809: Microsoft.Performance : 'NewsWebApiControllerTests.NewsWebApiControllerTests()' has 80 local variables, some of which may have been generated by the compiler. Refactor 'NewsWebApiControllerTests.NewsWebApiControllerTests()' so that it uses fewer than 64 local variables.
[11:21:35][CodeAnalysis] Code Analysis Complete -- 0 error(s), 1 warning(s)

Our build process recipe in TC treats all such warnings as errors so the build failed.

I double-checked the build output on local configuration (I have the same FxCop rules applied in my VS2015 build) but found no CA1809 warning there. There was nothing useful on MSDN documentation for this case. The issue was even more mysterious because the test class doesn’t actually have any constructor. I quickly concluded that it’s rather about the code “generated by the compiler” as suggested in the message above.

Quick fix #

Since I didn’t have any constructor method to apply the suppression attribute (or do it in GlobalSupressions.cs file) I created one:

[SuppressMessage("Microsoft.Performance", "CA1809:AvoidExcessiveLocals")]
public NewsWebApiControllerTests() { }

This really did the trick - no CA warning was reported from that time in this class.

Going deeper #

What was still bothering me was the question - why this warning appeared on the build server and on my local machine it did not?
After a little digging in build logs from both environments I found the first difference. The version on build machine was 12.0 while my local builds were made on 14.0. My guess was that Roslyn compiler (14.0) was producing more compact IL code than the previous versions.

The last thing to check was the actual IL output from both versions. It’s quite easy with ILSpy The result was not a big surprise in face of above findings. This is the part of constructor IL generated by the classic compiler (12.0):

/* TC: Abb.One.InsidePlus.UnitTests.dll */

.method /*060000E6*/ public hidebysig specialname rtspecialname
        instance void  .ctor() cil managed
{
  .custom /*0C00023C:0A00000F*/ instance void [mscorlib/*23000001*/]System.Diagnostics.CodeAnalysis.SuppressMessageAttribute/*0100009C*/::.ctor(string,
                                                                                                                                                string) /* 0A00000F */ = ( 01 00 15 4D 69 63 72 6F 73 6F 66 74 2E 50 65 72   // ...Microsoft.Per
                                                                                                                                                                           66 6F 72 6D 61 6E 63 65 1B 43 41 31 38 30 39 3A   // formance.CA1809:
                                                                                                                                                                           41 76 6F 69 64 45 78 63 65 73 73 69 76 65 4C 6F   // AvoidExcessiveLo
                                                                                                                                                                           63 61 6C 73 00 00 )                               // cals..
  // Code size       4438 (0x1156)
  .maxstack  6
  .locals /*11000048*/ init (class [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItem/*0100003F*/ V_0,
           class [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000129*/ V_1,
           class [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000129*/ V_2,

		   /* more local variables declarations */

		   class [mscorlib/*23000001*/]System.Collections.Generic.List`1/*010000CC*/<class [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.NewsLink/*0100012B*/> V_78,
          class [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.NewsLink/*0100012B*/ V_79)
  IL_0000:  ldarg.0
  IL_0001:  newobj     instance void [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItem/*0100003F*/::.ctor() /* 0A0001E0 */
  IL_0006:  stloc.0
  IL_0007:  ldloc.0
  IL_0008:  newobj     instance void [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000129*/::.ctor() /* 0A0001E1 */
  IL_000d:  stloc.1
  IL_000e:  ldloc.1
  IL_000f:  ldstr      "http://www.example.com" /* 70004469 */
  IL_0014:  callvirt   instance void [Abb.One.InsidePlus.Lib/*23000002*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000129*/::set_Url(string) /* 0A0001E2 */

  /* ...around 1.5k lines of IL code goes here */

  IL_1154:  nop
  IL_1155:  ret
} // end of method NewsWebApiControllerTests::.ctor

and this the one is from Roslyn (14.0):

/* Local: Abb.One.InsidePlus.UnitTests.dll */

.method /*06000099*/ public hidebysig specialname rtspecialname
        instance void  .ctor() cil managed
{
  .custom /*0C0001A7:0A000005*/ instance void [mscorlib/*23000001*/]System.Diagnostics.CodeAnalysis.SuppressMessageAttribute/*01000006*/::.ctor(string,
                                                                                                                                                string) /* 0A000005 */ = ( 01 00 15 4D 69 63 72 6F 73 6F 66 74 2E 50 65 72   // ...Microsoft.Per
                                                                                                                                                                           66 6F 72 6D 61 6E 63 65 1B 43 41 31 38 30 39 3A   // formance.CA1809:
                                                                                                                                                                           41 76 6F 69 64 45 78 63 65 73 73 69 76 65 4C 6F   // AvoidExcessiveLo
                                                                                                                                                                           63 61 6C 73 00 00 )                               // cals..
  // Code size       3803 (0xedb)
  .maxstack  10
  IL_0000:  ldarg.0
  IL_0001:  newobj     instance void [Abb.One.InsidePlus.Lib/*23000005*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItem/*01000074*/::.ctor() /* 0A00015D */
  IL_0006:  dup
  IL_0007:  newobj     instance void [Abb.One.InsidePlus.Lib/*23000005*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000128*/::.ctor() /* 0A00015E */
  IL_000c:  dup
  IL_000d:  ldstr      "http://www.example.com" /* 7000116C */
  IL_0012:  callvirt   instance void [Abb.One.InsidePlus.Lib/*23000005*/]Abb.One.InsidePlus.Lib.Models.ViewModels.NewsListItemLink/*01000128*/::set_Url(string) /* 0A00015F */

  ...
  /* around 1.2k lines of IL code */

  IL_0ec7:  callvirt   instance void class [mscorlib/*23000001*/]System.Collections.Generic.List`1/*01000046*/<class [Abb.One.InsidePlus.Lib/*23000005*/]Abb.One.InsidePlus.Lib.Models.News/*01000075*/>/*1B000059*/::Add(!0) /* 0A000184 */
  IL_0ecc:  nop
  IL_0ecd:  stfld      class [mscorlib/*23000001*/]System.Collections.Generic.IEnumerable`1/*0100002E*/<class [Abb.One.InsidePlus.Lib/*23000005*/]Abb.One.InsidePlus.Lib.Models.News/*01000075*/> Abb.One.InsidePlus.UnitTests.Web.Controllers.NewsWebApiControllerTests/*02000015*/::_NewsForGermany /* 0400003B */
  IL_0ed2:  ldarg.0
  IL_0ed3:  call       instance void [mscorlib/*23000001*/]System.Object/*01000014*/::.ctor() /* 0A00001D */
  IL_0ed8:  nop
  IL_0ed9:  nop
  IL_0eda:  ret
} // end of method NewsWebApiControllerTests::.ctor

It’s clearly visible on above IL listings that the code generated by Roslyn doesn’t use local variables on class initialization (readonly fields) but operates more intensively on stack. I guess this is for sake of memory usage optimizations.

The proper permanent fix for such errors was obviously upgrading the compiler version (together with MSBuild tooling) on the build environment.

Wrapping up #

It was not obvious at the first look that CA warning could have something to do with compiler version. As you can see sometimes you must just go deeper into the rabbit hole to understand what is really happening with your code. And this is not only about coding. I think about the entire process from writing it to running on production. This might not be straightforward but I personally find a joy in solving the lower-level issues from time to time. I hope that it was interesting for you as well or at least somehow inspiring ;-)

Cheers! Marek

↑