Merge pull request #559 from ebehar/develop

Expand validation to respect CIL placeholders

src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs

@@ -1,6 +1,5 @@
 using System.Collections.Generic;
 using System.Linq;
-using System.Text.RegularExpressions;
 using Mono.Cecil;
 using Mono.Cecil.Cil;
 
@@ -16,9 +15,6 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
         /// <summary>The assembly names to which to heuristically detect broken references.</summary>
         private readonly HashSet<string> ValidateReferencesToAssemblies;
 
-        /// <summary>A pattern matching type name substrings to strip for display.</summary>
-        private readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled);
-
 
         /*********
         ** Accessors
@@ -65,11 +61,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
                     return InstructionHandleResult.None;
 
                 // validate return type
-                string actualReturnTypeID = this.GetComparableTypeID(targetField.FieldType);
+                if (!RewriteHelper.LooksLikeSameType(fieldRef.FieldType, targetField.FieldType))
-                string expectedReturnTypeID = this.GetComparableTypeID(fieldRef.FieldType);
-                if (actualReturnTypeID != expectedReturnTypeID)
                 {
-                    this.NounPhrase = $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (field returns {this.GetFriendlyTypeName(targetField.FieldType, actualReturnTypeID)}, not {this.GetFriendlyTypeName(fieldRef.FieldType, expectedReturnTypeID)})";
+                    this.NounPhrase = $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (field returns {this.GetFriendlyTypeName(targetField.FieldType)}, not {this.GetFriendlyTypeName(fieldRef.FieldType)})";
                     return InstructionHandleResult.NotCompatible;
                 }
             }
@@ -91,10 +85,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
                     return InstructionHandleResult.NotCompatible;
                 }
 
-                string expectedReturnType = this.GetComparableTypeID(methodDef.ReturnType);
+                if (candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(method.ReturnType, methodDef.ReturnType)))
-                if (candidateMethods.All(method => this.GetComparableTypeID(method.ReturnType) != expectedReturnType))
                 {
-                    this.NounPhrase = $"reference to {methodDef.DeclaringType.FullName}.{methodDef.Name} (no such method returns {this.GetFriendlyTypeName(methodDef.ReturnType, expectedReturnType)})";
+                    this.NounPhrase = $"reference to {methodDef.DeclaringType.FullName}.{methodDef.Name} (no such method returns {this.GetFriendlyTypeName(methodDef.ReturnType)})";
                     return InstructionHandleResult.NotCompatible;
                 }
             }
@@ -113,17 +106,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
             return type != null && this.ValidateReferencesToAssemblies.Contains(type.Scope.Name);
         }
 
-        /// <summary>Get a unique string representation of a type.</summary>
-        /// <param name="type">The type reference.</param>
-        private string GetComparableTypeID(TypeReference type)
-        {
-            return this.StripTypeNamePattern.Replace(type.FullName, "");
-        }
-
         /// <summary>Get a shorter type name for display.</summary>
         /// <param name="type">The type reference.</param>
-        /// <param name="typeID">The comparable type ID from <see cref="GetComparableTypeID"/>.</param>
+        private string GetFriendlyTypeName(TypeReference type)
-        private string GetFriendlyTypeName(TypeReference type, string typeID)
         {
             // most common built-in types
             switch (type.FullName)
@@ -140,10 +125,10 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders
             foreach (string @namespace in new[] { "Microsoft.Xna.Framework", "Netcode", "System", "System.Collections.Generic" })
             {
                 if (type.Namespace == @namespace)
-                    return typeID.Substring(@namespace.Length + 1);
+                    return type.Name;
             }
 
-            return typeID;
+            return type.FullName;
         }
     }
 }

src/SMAPI/Framework/ModLoading/RewriteHelper.cs

@@ -9,6 +9,13 @@ namespace StardewModdingAPI.Framework.ModLoading
     /// <summary>Provides helper methods for field rewriters.</summary>
     internal static class RewriteHelper
     {
+        /*********
+        ** Properties
+        *********/
+        /// <summary>The comparer which heuristically compares type definitions.</summary>
+        private static readonly TypeReferenceComparer TypeDefinitionComparer = new TypeReferenceComparer();
+
+
         /*********
         ** Public methods
         *********/
@@ -59,6 +66,15 @@ namespace StardewModdingAPI.Framework.ModLoading
             return true;
         }
 
+        /// <summary>Determine whether two type IDs look like the same type, accounting for placeholder values such as !0.</summary>
+        /// <param name="typeA">The type ID to compare.</param>
+        /// <param name="typeB">The other type ID to compare.</param>
+        /// <returns>true if the type IDs look like the same type, false if not.</returns>
+        public static bool LooksLikeSameType(TypeReference typeA, TypeReference typeB)
+        {
+            return RewriteHelper.TypeDefinitionComparer.Equals(typeA, typeB);
+        }
+
         /// <summary>Get whether a method definition matches the signature expected by a method reference.</summary>
         /// <param name="definition">The method definition.</param>
         /// <param name="reference">The method reference.</param>

src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs

@@ -0,0 +1,201 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Mono.Cecil;
+
+namespace StardewModdingAPI.Framework.ModLoading
+{
+    /// <summary>Performs heuristic equality checks for <see cref="TypeReference"/> instances.</summary>
+    /// <remarks>
+    /// This implementation compares <see cref="TypeReference"/> instances to see if they likely
+    /// refer to the same type. While the implementation is obvious for types like <c>System.Bool</c>,
+    /// this class mainly exists to handle cases like <c>System.Collections.Generic.Dictionary`2&lt;!0,Netcode.NetRoot`1&lt;!1&gt;&gt;</c>
+    /// and <c>System.Collections.Generic.Dictionary`2&lt;TKey,Netcode.NetRoot`1&lt;TValue&gt;&gt;</c>
+    /// which are compatible, but not directly comparable. It does this by splitting each type name
+    /// into its component token types, and performing placeholder substitution (e.g. <c>!0</c> to
+    /// <c>TKey</c> in the above example). If all components are equal after substitution, and the
+    /// tokens can all be mapped to the same generic type, the types are considered equal.
+    /// </remarks>
+    internal class TypeReferenceComparer : IEqualityComparer<TypeReference>
+    {
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Get whether the specified objects are equal.</summary>
+        /// <param name="a">The first object to compare.</param>
+        /// <param name="b">The second object to compare.</param>
+        public bool Equals(TypeReference a, TypeReference b)
+        {
+            if (a == null || b == null)
+                return a == b;
+
+            return
+                a == b
+                || a.FullName == b.FullName
+                || this.HeuristicallyEquals(a, b);
+        }
+
+        /// <summary>Get a hash code for the specified object.</summary>
+        /// <param name="obj">The object for which a hash code is to be returned.</param>
+        /// <exception cref="T:System.ArgumentNullException">The object type is a reference type and <paramref name="obj" /> is null.</exception>
+        public int GetHashCode(TypeReference obj)
+        {
+            return obj.GetHashCode();
+        }
+
+
+        /*********
+        ** Private methods
+        *********/
+        /// <summary>Get whether two types are heuristically equal based on generic type token substitution.</summary>
+        /// <param name="typeA">The first type to compare.</param>
+        /// <param name="typeB">The second type to compare.</param>
+        private bool HeuristicallyEquals(TypeReference typeA, TypeReference typeB)
+        {
+            bool HeuristicallyEquals(string typeNameA, string typeNameB, IDictionary<string, string> tokenMap)
+            {
+                // analyse type names
+                bool hasTokensA = typeNameA.Contains("!");
+                bool hasTokensB = typeNameB.Contains("!");
+                bool isTokenA = hasTokensA && typeNameA[0] == '!';
+                bool isTokenB = hasTokensB && typeNameB[0] == '!';
+
+                // validate
+                if (!hasTokensA && !hasTokensB)
+                    return typeNameA == typeNameB; // no substitution needed
+                if (hasTokensA && hasTokensB)
+                    throw new InvalidOperationException("Can't compare two type names when both contain generic type tokens.");
+
+                // perform substitution if applicable
+                if (isTokenA)
+                    typeNameA = this.MapPlaceholder(placeholder: typeNameA, type: typeNameB, map: tokenMap);
+                if (isTokenB)
+                    typeNameB = this.MapPlaceholder(placeholder: typeNameB, type: typeNameA, map: tokenMap);
+
+                // compare inner tokens
+                string[] symbolsA = this.GetTypeSymbols(typeNameA).ToArray();
+                string[] symbolsB = this.GetTypeSymbols(typeNameB).ToArray();
+                if (symbolsA.Length != symbolsB.Length)
+                    return false;
+
+                for (int i = 0; i < symbolsA.Length; i++)
+                {
+                    if (!HeuristicallyEquals(symbolsA[i], symbolsB[i], tokenMap))
+                        return false;
+                }
+
+                return true;
+            }
+
+            return HeuristicallyEquals(typeA.FullName, typeB.FullName, new Dictionary<string, string>());
+        }
+
+        /// <summary>Map a generic type placeholder (like <c>!0</c>) to its actual type.</summary>
+        /// <param name="placeholder">The token placeholder.</param>
+        /// <param name="type">The actual type.</param>
+        /// <param name="map">The map of token to map substitutions.</param>
+        /// <returns>Returns the previously-mapped type if applicable, else the <paramref name="type"/>.</returns>
+        private string MapPlaceholder(string placeholder, string type, IDictionary<string, string> map)
+        {
+            if (map.TryGetValue(placeholder, out string result))
+                return result;
+
+            map[placeholder] = type;
+            return type;
+        }
+
+        /// <summary>Get the top-level type symbols in a type name (e.g. <code>List</code> and <code>NetRef&lt;T&gt;</code> in <code>List&lt;NetRef&lt;T&gt;&gt;</code>)</summary>
+        /// <param name="typeName">The full type name.</param>
+        private IEnumerable<string> GetTypeSymbols(string typeName)
+        {
+            int openGenerics = 0;
+
+            Queue<char> queue = new Queue<char>(typeName);
+            string symbol = "";
+            while (queue.Any())
+            {
+                char ch = queue.Dequeue();
+                switch (ch)
+                {
+                    // skip `1 generic type identifiers
+                    case '`':
+                        while (int.TryParse(queue.Peek().ToString(), out int _))
+                            queue.Dequeue();
+                        break;
+
+                    // start generic args
+                    case '<':
+                        switch (openGenerics)
+                        {
+                            // start new generic symbol
+                            case 0:
+                                yield return symbol;
+                                symbol = "";
+                                openGenerics++;
+                                break;
+
+                            // continue accumulating nested type symbol
+                            default:
+                                symbol += ch;
+                                openGenerics++;
+                                break;
+                        }
+                        break;
+
+                    // generic args delimiter
+                    case ',':
+                        switch (openGenerics)
+                        {
+                            // invalid
+                            case 0:
+                                throw new InvalidOperationException($"Encountered unexpected comma in type name: {typeName}.");
+
+                            // start next generic symbol
+                            case 1:
+                                yield return symbol;
+                                symbol = "";
+                                break;
+
+                            // continue accumulating nested type symbol
+                            default:
+                                symbol += ch;
+                                break;
+                        }
+                        break;
+
+
+                    // end generic args
+                    case '>':
+                        switch (openGenerics)
+                        {
+                            // invalid
+                            case 0:
+                                throw new InvalidOperationException($"Encountered unexpected closing generic in type name: {typeName}.");
+
+                            // end generic symbol
+                            case 1:
+                                yield return symbol;
+                                symbol = "";
+                                openGenerics--;
+                                break;
+
+                            // continue accumulating nested type symbol
+                            default:
+                                symbol += ch;
+                                openGenerics--;
+                                break;
+                        }
+                        break;
+
+                    // continue symbol
+                    default:
+                        symbol += ch;
+                        break;
+                }
+            }
+
+            if (symbol != "")
+                yield return symbol;
+        }
+    }
+}

src/SMAPI/Metadata/InstructionMetadata.cs

@@ -17,7 +17,7 @@ namespace StardewModdingAPI.Metadata
         *********/
         /// <summary>The assembly names to which to heuristically detect broken references.</summary>
         /// <remarks>The current implementation only works correctly with assemblies that should always be present.</remarks>
-        private readonly string[] ValidateReferencesToAssemblies = { "StardewModdingAPI", "Stardew Valley", "StardewValley" };
+        private readonly string[] ValidateReferencesToAssemblies = { "StardewModdingAPI", "Stardew Valley", "StardewValley", "Netcode" };
 
 
         /*********

src/SMAPI/StardewModdingAPI.csproj

@@ -110,6 +110,7 @@
     <Compile Include="Framework\ContentManagers\IContentManager.cs" />
     <Compile Include="Framework\ContentManagers\ModContentManager.cs" />
     <Compile Include="Framework\Models\ModFolderExport.cs" />
+    <Compile Include="Framework\ModLoading\TypeReferenceComparer.cs" />
     <Compile Include="Framework\Patching\GamePatcher.cs" />
     <Compile Include="Framework\Patching\IHarmonyPatch.cs" />
     <Compile Include="Framework\Serialisation\ColorConverter.cs" />