ccrewrite: properly escaping type names when serializing metadata

Foxtrot/Tests/FoxtrotTests10.csproj

@@ -142,6 +142,14 @@
       <Project>{9D404D0C-0921-4ADD-BC22-A14AAE2CA7E2}</Project>
       <Name>CRASanitizer</Name>
     </ProjectReference>
+    <ProjectReference Include="..\..\System.Compiler\System.CompilerCC.csproj">
+      <Project>{d7e16b38-3893-4eef-847f-a3be807e9546}</Project>
+      <Name>System.CompilerCC</Name>
+    </ProjectReference>
+    <ProjectReference Include="..\Foxtrot\Foxtrot.Extractor.csproj">
+      <Project>{f7364559-90f5-480b-b975-c95a04e0daf6}</Project>
+      <Name>Foxtrot.Extractor</Name>
+    </ProjectReference>
     <ProjectReference Include="AssemblyWithContracts\AssemblyWithContracts.csproj">
       <Project>{8602713E-68E4-4A8F-BB6C-9AEA279E494F}</Project>
       <Name>AssemblyWithContracts</Name>

Foxtrot/Tests/Options.cs

@@ -47,6 +47,12 @@ public string ReferencesFramework
 
         public bool UseExe = true;
 
+        /// <summary>
+        /// If set to true, verification will include reading contracts back from the rewritten
+        /// assembly in addition to running peverify.
+        /// </summary>
+        public bool DeepVerify = false;
+
         private int instance;
 
         private TestContext TestContext;

Foxtrot/Tests/RewriterTests.cs

@@ -114,7 +114,8 @@ public void Roslyn_CompatibilityCheckInReleaseMode()
         }
 
         /// <summary>
-        /// Unit test for #47 - "Could not resolve type reference" for some iterator methods in VS2015
+        /// Unit test for #47 - "Could not resolve type reference" for some iterator methods in VS2015 and
+        /// #186 - ccrewrite produces an incorrect type name in IteratorStateMachineAttribute with some generic types
         /// </summary>
         [DeploymentItem("Foxtrot\\Tests\\TestInputs.xml"), DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\TestInputs.xml", "IteratorWithComplexGeneric", DataAccessMethod.Sequential)]
         [TestMethod]
@@ -125,6 +126,7 @@ public void Roslyn_IteratorBlockWithComplexGeneric()
             // Bug with metadata reader could be reproduced only with a new mscorlib and new System.dll.
             options.BuildFramework = @".NetFramework\v4.6";
             options.ReferencesFramework = @".NetFramework\v4.6";
+            options.DeepVerify = true;
             TestDriver.BuildRewriteRun(options);
         }
 

Foxtrot/Tests/TestDriver.cs

@@ -18,6 +18,7 @@
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using System.Text;
 using System.Collections.Generic;
+using Microsoft.Contracts.Foxtrot;
 
 namespace Tests
 {
@@ -112,6 +113,37 @@ private static int PEVerify(string assemblyFile, Options options)
             return exitCode;
         }
 
+        /// <summary>
+        /// Attempts to extract contracts from the specified assembly file. Used to verify that
+        /// Foxtrot can still extract contracts from a rewritten assembly.
+        /// </summary>
+        private static void ExtractContracts(string assemblyFile, Options options)
+        {
+            // use assembly resolver from Foxtrot.Extractor
+            var resolver = new AssemblyResolver(
+                resolvedPaths: new string[0],
+                libpaths: options.LibPaths,
+                usePDB: false,
+                preserveShortBranches: true,
+                trace: false,
+                postLoad: (r, assemblyNode) => {
+                    ContractNodes contractNodes = null;
+                    Extractor.ExtractContracts(
+                        assemblyNode, null, null, null, null, out contractNodes,
+                        e => Assert.Fail(e.ToString()), false);
+                });
+
+            var assembly = resolver.ProbeForAssembly(
+                assemblyName: Path.GetFileNameWithoutExtension(assemblyFile),
+                referencingModuleDirectory: Path.GetDirectoryName(assemblyFile),
+                exts: new string[] { Path.GetExtension(assemblyFile) });
+
+            // the assembly must be resolved and have no metadata import errors
+            Assert.IsNotNull(assembly);
+            Assert.IsTrue(assembly.MetadataImportErrors == null || assembly.MetadataImportErrors.Count == 0,
+                          "Parsing back the rewritten assembly produced metadata import errors");
+        }
+
         internal static string RewriteAndVerify(string sourceDir, string binary, Options options)
         {
             if (!Path.IsPathRooted(sourceDir)) { sourceDir = options.MakeAbsolute(sourceDir); }
@@ -120,6 +152,9 @@ internal static string RewriteAndVerify(string sourceDir, string binary, Options
             if (target != null)
             {
                 PEVerify(target, options);
+
+                if (options.DeepVerify)
+                    ExtractContracts(target, options);
             }
             return target;
         }

System.Compiler/Writer.cs

@@ -4678,16 +4678,27 @@ string GetSerializedTypeName(TypeNode/*!*/ type, ref bool isAssemblyQualified) {
         sb.Append('&');
         goto done;
       }
+
       if (type.Template == null)
-        sb.Append(type.FullName);
-      else{
-        sb.Append(type.Template.FullName);
+      {
+        AppendEscapedTypeName(sb, type);
+      }
+      else
+      {
+        AppendEscapedTypeName(sb, type.Template);
+
         sb.Append('[');
-        for (int i = 0, n = type.ConsolidatedTemplateArguments == null ? 0 : type.ConsolidatedTemplateArguments.Count; i < n; i++) {
-          //^ assert type.ConsolidatedTemplateArguments != null;
-          bool isAssemQual = true;
-          this.AppendSerializedTypeName(sb, type.ConsolidatedTemplateArguments[i], ref isAssemQual);
-          if (i < n-1) sb.Append(',');
+        if (type.ConsolidatedTemplateArguments != null)
+        {
+          for (int i = 0; i < type.ConsolidatedTemplateArguments.Count; i++)
+          {
+            if (i > 0)
+              sb.Append(',');
+
+            //^ assert type.ConsolidatedTemplateArguments != null;
+            bool isAssemQual = true;
+            this.AppendSerializedTypeName(sb, type.ConsolidatedTemplateArguments[i], ref isAssemQual);
+          }
         }
         sb.Append(']');
       }
@@ -4696,6 +4707,52 @@ string GetSerializedTypeName(TypeNode/*!*/ type, ref bool isAssemblyQualified) {
         this.AppendAssemblyQualifierIfNecessary(sb, type, out isAssemblyQualified);
       return sb.ToString();
     }
+
+    /// <summary>
+    /// Appends the escaped full name of the type to a string builder.
+    /// </summary>
+    /// <param name="sb">String builder to append to.</param>
+    /// <param name="type">Type whose full name to append.</param>
+    void AppendEscapedTypeName(StringBuilder sb, TypeNode type)
+    {
+        // NOTE: we have to carefully deal with types defined within other types to avoid 
+        // escaping the '+' character when it is used to separate inner types from their
+        // declaring type.
+        if (type.DeclaringType != null)
+        {
+            // if this type is defined within another type, append the declaring type name first
+            // followed by the '+' separator
+            AppendEscapedTypeName(sb, type.DeclaringType);
+            sb.Append('+');
+        }
+        else if (type.Namespace != null)
+        {
+            // append the namespace name followed by a dot
+            sb.Append(type.Namespace.Name);
+            sb.Append('.');
+        }
+
+        // deal with the actual type name
+        foreach (char c in type.Name.Name)
+        {
+            if (IsTypeNameReservedChar(c))
+                sb.Append('\\');
+            sb.Append(c);
+        }
+    }
+
+    /// <summary>
+    /// Determines if the provided character has a special meaning in a fully qualified type name
+    /// and therefore has to be escaped if used as part of a type name identifier.
+    /// </summary>
+    /// <param name="ch">Character to test.</param>
+    /// <returns>True if the character has to be escaped or false - otherwise.</returns>
+    static bool IsTypeNameReservedChar(char ch)
+    {
+        return ch == ',' || ch == '[' || ch == ']' || ch == '&' || 
+               ch == '*' || ch == '+' || ch == '\\';
+    }
+
     void AppendAssemblyQualifierIfNecessary(StringBuilder/*!*/ sb, TypeNode type, out bool isAssemQualified) {
       isAssemQualified = false;
       if (type == null) return;