From 2b2860637d36b17d51ce279afaa4d81cefef289d Mon Sep 17 00:00:00 2001 From: Evan Behar Date: Fri, 6 Jul 2018 23:08:09 -0700 Subject: [PATCH 01/12] Linux-compatible scope resolution in validator --- .../ReferenceToMemberWithUnexpectedTypeFinder.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index ecad649a..6364cec8 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs @@ -110,7 +110,15 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders /// The type reference. private bool ShouldValidate(TypeReference type) { - return type != null && this.ValidateReferencesToAssemblies.Contains(type.Scope.Name); + // Extract scope name from type string representation for compatibility + // Under Linux, type.Scope.Name sometimes reports incorrectly + string scopeName = type.ToString(); + if (scopeName[0] != '$') + return false; + + scopeName = scopeName.Substring(0, scopeName.IndexOf(".", System.StringComparison.CurrentCulture)); + + return type != null && this.ValidateReferencesToAssemblies.Contains(scopeName); } /// Get a unique string representation of a type. From 829e24b23e23ed44392c07d266107bf4a2f36998 Mon Sep 17 00:00:00 2001 From: "E. Behar" Date: Fri, 6 Jul 2018 23:21:13 -0700 Subject: [PATCH 02/12] Fix type==null case --- .../Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index 6364cec8..bd5c97d6 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs @@ -110,6 +110,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders /// The type reference. private bool ShouldValidate(TypeReference type) { + if (type == null) + return false; + // Extract scope name from type string representation for compatibility // Under Linux, type.Scope.Name sometimes reports incorrectly string scopeName = type.ToString(); @@ -118,7 +121,7 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders scopeName = scopeName.Substring(0, scopeName.IndexOf(".", System.StringComparison.CurrentCulture)); - return type != null && this.ValidateReferencesToAssemblies.Contains(scopeName); + return this.ValidateReferencesToAssemblies.Contains(scopeName); } /// Get a unique string representation of a type. From 88f754e5b134f43ed6c7a833834aaeb92e44a62e Mon Sep 17 00:00:00 2001 From: Evan Behar Date: Sat, 7 Jul 2018 23:45:02 -0700 Subject: [PATCH 03/12] Expand validation to respect CIL placeholders --- ...ferenceToMemberWithUnexpectedTypeFinder.cs | 7 +- .../Framework/ModLoading/RewriteHelper.cs | 163 ++++++++++++++++++ 2 files changed, 167 insertions(+), 3 deletions(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index bd5c97d6..79db6921 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs @@ -67,7 +67,8 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders // validate return type string actualReturnTypeID = this.GetComparableTypeID(targetField.FieldType); string expectedReturnTypeID = this.GetComparableTypeID(fieldRef.FieldType); - if (actualReturnTypeID != expectedReturnTypeID) + + if (!RewriteHelper.LooksLikeSameType(expectedReturnTypeID, actualReturnTypeID)) { this.NounPhrase = $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (field returns {this.GetFriendlyTypeName(targetField.FieldType, actualReturnTypeID)}, not {this.GetFriendlyTypeName(fieldRef.FieldType, expectedReturnTypeID)})"; return InstructionHandleResult.NotCompatible; @@ -110,8 +111,8 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders /// The type reference. private bool ShouldValidate(TypeReference type) { - if (type == null) - return false; + if (type != null) + return true; // Extract scope name from type string representation for compatibility // Under Linux, type.Scope.Name sometimes reports incorrectly diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index 56a60a72..9eb8b3a5 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; using Mono.Cecil; @@ -90,5 +91,167 @@ namespace StardewModdingAPI.Framework.ModLoading .GetMethods(BindingFlags.Instance | BindingFlags.DeclaredOnly | BindingFlags.Public) .Any(method => RewriteHelper.HasMatchingSignature(method, reference)); } + + /// Determine whether this type ID has a placeholder such as !0. + /// The type to check. + /// true if the type ID contains a placeholder, false if not. + public static bool HasPlaceholder(string typeID) + { + return typeID.Contains("!0"); + } + + /// returns whether this type ID is a placeholder, i.e., it begins with "!". + /// The symbol to validate. + /// true if the symbol is a placeholder, false if not + public static bool IsPlaceholder(string symbol) + { + return symbol.StartsWith("!"); + } + + /// Determine whether two type IDs look like the same type, accounting for placeholder values such as !0. + /// The type ID to compare. + /// The other type ID to compare. + /// true if the type IDs look like the same type, false if not. + public static bool LooksLikeSameType(string typeA, string typeB) + { + string placeholderType, actualType = ""; + + if (RewriteHelper.HasPlaceholder(typeA)) + { + placeholderType = typeA; + actualType = typeB; + } else if (RewriteHelper.HasPlaceholder(typeB)) + { + placeholderType = typeB; + actualType = typeA; + } else + { + return typeA == typeB; + } + + return RewriteHelper.PlaceholderTypeValidates(placeholderType, actualType); + } + + protected class SymbolLocation + { + public string symbol; + public int depth; + + public SymbolLocation(string symbol, int depth) + { + this.symbol = symbol; + this.depth = depth; + } + } + + private static List symbolBoundaries = new List{'<', '>', ','}; + + /// Traverses and parses out symbols from a type which does not contain placeholder values. + /// The type to traverse. + /// A List in which to store the parsed symbols. + private static void TraverseActualType(string type, List typeSymbols) + { + int depth = 0; + string symbol = ""; + + foreach (char c in type) + { + if (RewriteHelper.symbolBoundaries.Contains(c)) + { + typeSymbols.Add(new SymbolLocation(symbol, depth)); + symbol = ""; + switch (c) { + case '<': + depth++; + break; + case '>': + depth--; + break; + default: + break; + } + } + else + symbol += c; + } + } + + /// Determines whether two symbols in a type ID match, accounting for placeholders such as !0. + /// A symbol in a typename which contains placeholders. + /// A symbol in a typename which does not contain placeholders. + /// A dictionary containing a mapping of placeholders to concrete types. + /// true if the symbols match, false if not. + private static bool SymbolsMatch(SymbolLocation symbolA, SymbolLocation symbolB, Dictionary placeholderMap) + { + System.Console.Write($"comparing {symbolA.symbol} at depth {symbolA.depth} to {symbolB.symbol} at {symbolB.depth}"); + if (symbolA.depth != symbolB.depth) + return false; + + if (!RewriteHelper.IsPlaceholder(symbolA.symbol)) { + return symbolA.symbol == symbolB.symbol; + } + + if (placeholderMap.ContainsKey(symbolA.symbol)) + { + return placeholderMap[symbolA.symbol] == symbolB.symbol; + } + + placeholderMap[symbolA.symbol] = symbolB.symbol; + + return true; + } + + /// Determines whether a type which has placeholders correctly resolves to the concrete type provided. + /// A type containing placeholders such as !0. + /// The list of symbols extracted from the concrete type. + /// true if the type resolves correctly, false if not. + private static bool PlaceholderTypeResolvesToActualType(string type, List typeSymbols) + { + Dictionary placeholderMap = new Dictionary(); + + int depth = 0, symbolCount = 0; + string symbol = ""; + + foreach (char c in type) + { + if (symbolBoundaries.Contains(c)) + { + bool match = RewriteHelper.SymbolsMatch(new SymbolLocation(symbol, depth), typeSymbols[symbolCount], placeholderMap); + System.Console.Write($"match: {match}"); + if (typeSymbols.Count <= symbolCount || + !match) + return false; + + symbolCount++; + symbol = ""; + switch (c) + { + case '<': + depth++; + break; + case '>': + depth--; + break; + default: + break; + } + } + else + symbol += c; + } + + return true; + } + + /// Determines whether a type with placeholders in it matches a type without placeholders. + /// The type with placeholders in it. + /// The type without placeholders. + /// true if the placeholder type can resolve to the actual type, false if not. + private static bool PlaceholderTypeValidates(string placeholderType, string actualType) { + List typeSymbols = new List(); + + RewriteHelper.TraverseActualType(actualType, typeSymbols); + return PlaceholderTypeResolvesToActualType(placeholderType, typeSymbols); + } } } From a30794894bd9bd3e152c882286f0f3600ea41400 Mon Sep 17 00:00:00 2001 From: Evan Behar Date: Sat, 7 Jul 2018 23:46:28 -0700 Subject: [PATCH 04/12] Revert ShouldValidate --- .../ReferenceToMemberWithUnexpectedTypeFinder.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index 79db6921..88ba36ee 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs @@ -111,18 +111,7 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders /// The type reference. private bool ShouldValidate(TypeReference type) { - if (type != null) - return true; - - // Extract scope name from type string representation for compatibility - // Under Linux, type.Scope.Name sometimes reports incorrectly - string scopeName = type.ToString(); - if (scopeName[0] != '$') - return false; - - scopeName = scopeName.Substring(0, scopeName.IndexOf(".", System.StringComparison.CurrentCulture)); - - return this.ValidateReferencesToAssemblies.Contains(scopeName); + return type != null && this.ValidateReferencesToAssemblies.Contains(type.Scope.Name); } /// Get a unique string representation of a type. From 1a3810d722c89749dadbf2bc260ff7a886e08368 Mon Sep 17 00:00:00 2001 From: "E. Behar" Date: Sat, 7 Jul 2018 16:50:01 -0700 Subject: [PATCH 05/12] Remove extraneous debug output --- src/SMAPI/Framework/ModLoading/RewriteHelper.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index 9eb8b3a5..fcbb7063 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -183,7 +183,6 @@ namespace StardewModdingAPI.Framework.ModLoading /// true if the symbols match, false if not. private static bool SymbolsMatch(SymbolLocation symbolA, SymbolLocation symbolB, Dictionary placeholderMap) { - System.Console.Write($"comparing {symbolA.symbol} at depth {symbolA.depth} to {symbolB.symbol} at {symbolB.depth}"); if (symbolA.depth != symbolB.depth) return false; From 1dfcbc61736de1d3631cf0571b945e0c4dfc50f5 Mon Sep 17 00:00:00 2001 From: "E. Behar" Date: Sat, 7 Jul 2018 16:51:03 -0700 Subject: [PATCH 06/12] Remove another extraneous debug output. =_= --- src/SMAPI/Framework/ModLoading/RewriteHelper.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index fcbb7063..74498a3e 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -216,7 +216,6 @@ namespace StardewModdingAPI.Framework.ModLoading if (symbolBoundaries.Contains(c)) { bool match = RewriteHelper.SymbolsMatch(new SymbolLocation(symbol, depth), typeSymbols[symbolCount], placeholderMap); - System.Console.Write($"match: {match}"); if (typeSymbols.Count <= symbolCount || !match) return false; From f6254e17ead217cf7b2488a9b2d5b8d0bc9b23d8 Mon Sep 17 00:00:00 2001 From: "E. Behar" Date: Sun, 8 Jul 2018 10:22:23 -0700 Subject: [PATCH 07/12] Fix missing assignment. --- src/SMAPI/Framework/ModLoading/RewriteHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index 74498a3e..1600069d 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -114,7 +114,7 @@ namespace StardewModdingAPI.Framework.ModLoading /// true if the type IDs look like the same type, false if not. public static bool LooksLikeSameType(string typeA, string typeB) { - string placeholderType, actualType = ""; + string placeholderType = "", actualType = ""; if (RewriteHelper.HasPlaceholder(typeA)) { From 40fbafdb73d0501f5239d3b857b6cb3bf2929bab Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 8 Jul 2018 13:26:07 -0400 Subject: [PATCH 08/12] fix new logic not applied to method return types --- .../Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index 88ba36ee..47c8b33c 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs @@ -93,7 +93,7 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders } string expectedReturnType = this.GetComparableTypeID(methodDef.ReturnType); - if (candidateMethods.All(method => this.GetComparableTypeID(method.ReturnType) != expectedReturnType)) + if (candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(this.GetComparableTypeID(method.ReturnType), expectedReturnType))) { this.NounPhrase = $"reference to {methodDef.DeclaringType.FullName}.{methodDef.Name} (no such method returns {this.GetFriendlyTypeName(methodDef.ReturnType, expectedReturnType)})"; return InstructionHandleResult.NotCompatible; From befeafd31d7a3351cb138c210b26f126716d05f0 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 8 Jul 2018 13:56:46 -0400 Subject: [PATCH 09/12] encapsulate GetComparableTypeID --- ...ferenceToMemberWithUnexpectedTypeFinder.cs | 30 +++---------- .../Framework/ModLoading/RewriteHelper.cs | 43 ++++++++++++++----- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs index 47c8b33c..cf5a3175 100644 --- a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMemberWithUnexpectedTypeFinder.cs +++ b/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 /// The assembly names to which to heuristically detect broken references. private readonly HashSet ValidateReferencesToAssemblies; - /// A pattern matching type name substrings to strip for display. - private readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled); - /********* ** Accessors @@ -65,12 +61,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders return InstructionHandleResult.None; // validate return type - string actualReturnTypeID = this.GetComparableTypeID(targetField.FieldType); - string expectedReturnTypeID = this.GetComparableTypeID(fieldRef.FieldType); - - if (!RewriteHelper.LooksLikeSameType(expectedReturnTypeID, actualReturnTypeID)) + if (!RewriteHelper.LooksLikeSameType(fieldRef.FieldType, targetField.FieldType)) { - 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; } } @@ -92,10 +85,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders return InstructionHandleResult.NotCompatible; } - string expectedReturnType = this.GetComparableTypeID(methodDef.ReturnType); - if (candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(this.GetComparableTypeID(method.ReturnType), expectedReturnType))) + if (candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(method.ReturnType, methodDef.ReturnType))) { - 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; } } @@ -114,17 +106,9 @@ namespace StardewModdingAPI.Framework.ModLoading.Finders return type != null && this.ValidateReferencesToAssemblies.Contains(type.Scope.Name); } - /// Get a unique string representation of a type. - /// The type reference. - private string GetComparableTypeID(TypeReference type) - { - return this.StripTypeNamePattern.Replace(type.FullName, ""); - } - /// Get a shorter type name for display. /// The type reference. - /// The comparable type ID from . - private string GetFriendlyTypeName(TypeReference type, string typeID) + private string GetFriendlyTypeName(TypeReference type) { // most common built-in types switch (type.FullName) @@ -141,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; } } } diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index 1600069d..f8684cde 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Text.RegularExpressions; using Mono.Cecil; using Mono.Cecil.Cil; @@ -10,6 +11,13 @@ namespace StardewModdingAPI.Framework.ModLoading /// Provides helper methods for field rewriters. internal static class RewriteHelper { + /********* + ** Properties + *********/ + /// A pattern matching type name substrings to strip for display. + private static readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled); + + /********* ** Public methods *********/ @@ -109,29 +117,39 @@ namespace StardewModdingAPI.Framework.ModLoading } /// Determine whether two type IDs look like the same type, accounting for placeholder values such as !0. - /// The type ID to compare. - /// The other type ID to compare. + /// The type ID to compare. + /// The other type ID to compare. /// true if the type IDs look like the same type, false if not. - public static bool LooksLikeSameType(string typeA, string typeB) + public static bool LooksLikeSameType(TypeReference a, TypeReference b) { + string typeA = RewriteHelper.GetComparableTypeID(a); + string typeB = RewriteHelper.GetComparableTypeID(b); + string placeholderType = "", actualType = ""; if (RewriteHelper.HasPlaceholder(typeA)) { placeholderType = typeA; actualType = typeB; - } else if (RewriteHelper.HasPlaceholder(typeB)) + } + else if (RewriteHelper.HasPlaceholder(typeB)) { placeholderType = typeB; actualType = typeA; - } else - { - return typeA == typeB; } + else + return typeA == typeB; return RewriteHelper.PlaceholderTypeValidates(placeholderType, actualType); } + /// Get a unique string representation of a type. + /// The type reference. + private static string GetComparableTypeID(TypeReference type) + { + return RewriteHelper.StripTypeNamePattern.Replace(type.FullName, ""); + } + protected class SymbolLocation { public string symbol; @@ -144,7 +162,7 @@ namespace StardewModdingAPI.Framework.ModLoading } } - private static List symbolBoundaries = new List{'<', '>', ','}; + private static List symbolBoundaries = new List { '<', '>', ',' }; /// Traverses and parses out symbols from a type which does not contain placeholder values. /// The type to traverse. @@ -160,7 +178,8 @@ namespace StardewModdingAPI.Framework.ModLoading { typeSymbols.Add(new SymbolLocation(symbol, depth)); symbol = ""; - switch (c) { + switch (c) + { case '<': depth++; break; @@ -186,7 +205,8 @@ namespace StardewModdingAPI.Framework.ModLoading if (symbolA.depth != symbolB.depth) return false; - if (!RewriteHelper.IsPlaceholder(symbolA.symbol)) { + if (!RewriteHelper.IsPlaceholder(symbolA.symbol)) + { return symbolA.symbol == symbolB.symbol; } @@ -245,7 +265,8 @@ namespace StardewModdingAPI.Framework.ModLoading /// The type with placeholders in it. /// The type without placeholders. /// true if the placeholder type can resolve to the actual type, false if not. - private static bool PlaceholderTypeValidates(string placeholderType, string actualType) { + private static bool PlaceholderTypeValidates(string placeholderType, string actualType) + { List typeSymbols = new List(); RewriteHelper.TraverseActualType(actualType, typeSymbols); From 0079110870e4944e734be507ede91e7b0b655df6 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 8 Jul 2018 13:58:37 -0400 Subject: [PATCH 10/12] encapsulate type reference comparison --- .../Framework/ModLoading/RewriteHelper.cs | 188 +--------------- .../ModLoading/TypeReferenceComparer.cs | 209 ++++++++++++++++++ src/SMAPI/StardewModdingAPI.csproj | 1 + 3 files changed, 221 insertions(+), 177 deletions(-) create mode 100644 src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs diff --git a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs index f8684cde..2f79809c 100644 --- a/src/SMAPI/Framework/ModLoading/RewriteHelper.cs +++ b/src/SMAPI/Framework/ModLoading/RewriteHelper.cs @@ -1,8 +1,6 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Reflection; -using System.Text.RegularExpressions; using Mono.Cecil; using Mono.Cecil.Cil; @@ -14,8 +12,8 @@ namespace StardewModdingAPI.Framework.ModLoading /********* ** Properties *********/ - /// A pattern matching type name substrings to strip for display. - private static readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled); + /// The comparer which heuristically compares type definitions. + private static readonly TypeReferenceComparer TypeDefinitionComparer = new TypeReferenceComparer(); /********* @@ -68,6 +66,15 @@ namespace StardewModdingAPI.Framework.ModLoading return true; } + /// Determine whether two type IDs look like the same type, accounting for placeholder values such as !0. + /// The type ID to compare. + /// The other type ID to compare. + /// true if the type IDs look like the same type, false if not. + public static bool LooksLikeSameType(TypeReference typeA, TypeReference typeB) + { + return RewriteHelper.TypeDefinitionComparer.Equals(typeA, typeB); + } + /// Get whether a method definition matches the signature expected by a method reference. /// The method definition. /// The method reference. @@ -99,178 +106,5 @@ namespace StardewModdingAPI.Framework.ModLoading .GetMethods(BindingFlags.Instance | BindingFlags.DeclaredOnly | BindingFlags.Public) .Any(method => RewriteHelper.HasMatchingSignature(method, reference)); } - - /// Determine whether this type ID has a placeholder such as !0. - /// The type to check. - /// true if the type ID contains a placeholder, false if not. - public static bool HasPlaceholder(string typeID) - { - return typeID.Contains("!0"); - } - - /// returns whether this type ID is a placeholder, i.e., it begins with "!". - /// The symbol to validate. - /// true if the symbol is a placeholder, false if not - public static bool IsPlaceholder(string symbol) - { - return symbol.StartsWith("!"); - } - - /// Determine whether two type IDs look like the same type, accounting for placeholder values such as !0. - /// The type ID to compare. - /// The other type ID to compare. - /// true if the type IDs look like the same type, false if not. - public static bool LooksLikeSameType(TypeReference a, TypeReference b) - { - string typeA = RewriteHelper.GetComparableTypeID(a); - string typeB = RewriteHelper.GetComparableTypeID(b); - - string placeholderType = "", actualType = ""; - - if (RewriteHelper.HasPlaceholder(typeA)) - { - placeholderType = typeA; - actualType = typeB; - } - else if (RewriteHelper.HasPlaceholder(typeB)) - { - placeholderType = typeB; - actualType = typeA; - } - else - return typeA == typeB; - - return RewriteHelper.PlaceholderTypeValidates(placeholderType, actualType); - } - - /// Get a unique string representation of a type. - /// The type reference. - private static string GetComparableTypeID(TypeReference type) - { - return RewriteHelper.StripTypeNamePattern.Replace(type.FullName, ""); - } - - protected class SymbolLocation - { - public string symbol; - public int depth; - - public SymbolLocation(string symbol, int depth) - { - this.symbol = symbol; - this.depth = depth; - } - } - - private static List symbolBoundaries = new List { '<', '>', ',' }; - - /// Traverses and parses out symbols from a type which does not contain placeholder values. - /// The type to traverse. - /// A List in which to store the parsed symbols. - private static void TraverseActualType(string type, List typeSymbols) - { - int depth = 0; - string symbol = ""; - - foreach (char c in type) - { - if (RewriteHelper.symbolBoundaries.Contains(c)) - { - typeSymbols.Add(new SymbolLocation(symbol, depth)); - symbol = ""; - switch (c) - { - case '<': - depth++; - break; - case '>': - depth--; - break; - default: - break; - } - } - else - symbol += c; - } - } - - /// Determines whether two symbols in a type ID match, accounting for placeholders such as !0. - /// A symbol in a typename which contains placeholders. - /// A symbol in a typename which does not contain placeholders. - /// A dictionary containing a mapping of placeholders to concrete types. - /// true if the symbols match, false if not. - private static bool SymbolsMatch(SymbolLocation symbolA, SymbolLocation symbolB, Dictionary placeholderMap) - { - if (symbolA.depth != symbolB.depth) - return false; - - if (!RewriteHelper.IsPlaceholder(symbolA.symbol)) - { - return symbolA.symbol == symbolB.symbol; - } - - if (placeholderMap.ContainsKey(symbolA.symbol)) - { - return placeholderMap[symbolA.symbol] == symbolB.symbol; - } - - placeholderMap[symbolA.symbol] = symbolB.symbol; - - return true; - } - - /// Determines whether a type which has placeholders correctly resolves to the concrete type provided. - /// A type containing placeholders such as !0. - /// The list of symbols extracted from the concrete type. - /// true if the type resolves correctly, false if not. - private static bool PlaceholderTypeResolvesToActualType(string type, List typeSymbols) - { - Dictionary placeholderMap = new Dictionary(); - - int depth = 0, symbolCount = 0; - string symbol = ""; - - foreach (char c in type) - { - if (symbolBoundaries.Contains(c)) - { - bool match = RewriteHelper.SymbolsMatch(new SymbolLocation(symbol, depth), typeSymbols[symbolCount], placeholderMap); - if (typeSymbols.Count <= symbolCount || - !match) - return false; - - symbolCount++; - symbol = ""; - switch (c) - { - case '<': - depth++; - break; - case '>': - depth--; - break; - default: - break; - } - } - else - symbol += c; - } - - return true; - } - - /// Determines whether a type with placeholders in it matches a type without placeholders. - /// The type with placeholders in it. - /// The type without placeholders. - /// true if the placeholder type can resolve to the actual type, false if not. - private static bool PlaceholderTypeValidates(string placeholderType, string actualType) - { - List typeSymbols = new List(); - - RewriteHelper.TraverseActualType(actualType, typeSymbols); - return PlaceholderTypeResolvesToActualType(placeholderType, typeSymbols); - } } } diff --git a/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs b/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs new file mode 100644 index 00000000..8d128b37 --- /dev/null +++ b/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs @@ -0,0 +1,209 @@ +using System.Collections.Generic; +using System.Text.RegularExpressions; +using Mono.Cecil; + +namespace StardewModdingAPI.Framework.ModLoading +{ + /// Performs heuristic equality checks for instances. + internal class TypeReferenceComparer : IEqualityComparer + { + /********* + ** Properties + *********/ + /// A pattern matching type name substrings to strip for display. + private readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled); + + private List symbolBoundaries = new List { '<', '>', ',' }; + + + /********* + ** Public methods + *********/ + /// Get whether the specified objects are equal. + /// The first object to compare. + /// The second object to compare. + public bool Equals(TypeReference a, TypeReference b) + { + string typeA = this.GetComparableTypeID(a); + string typeB = this.GetComparableTypeID(b); + + string placeholderType = "", actualType = ""; + + if (this.HasPlaceholder(typeA)) + { + placeholderType = typeA; + actualType = typeB; + } + else if (this.HasPlaceholder(typeB)) + { + placeholderType = typeB; + actualType = typeA; + } + else + return typeA == typeB; + + return this.PlaceholderTypeValidates(placeholderType, actualType); + } + + /// Get a hash code for the specified object. + /// The object for which a hash code is to be returned. + /// The object type is a reference type and is null. + public int GetHashCode(TypeReference obj) + { + return obj.GetHashCode(); + } + + + /********* + ** Private methods + *********/ + /// Get a unique string representation of a type. + /// The type reference. + private string GetComparableTypeID(TypeReference type) + { + return this.StripTypeNamePattern.Replace(type.FullName, ""); + } + + /// Determine whether this type ID has a placeholder such as !0. + /// The type to check. + /// true if the type ID contains a placeholder, false if not. + private bool HasPlaceholder(string typeID) + { + return typeID.Contains("!0"); + } + + /// returns whether this type ID is a placeholder, i.e., it begins with "!". + /// The symbol to validate. + /// true if the symbol is a placeholder, false if not + private bool IsPlaceholder(string symbol) + { + return symbol.StartsWith("!"); + } + + /// Traverses and parses out symbols from a type which does not contain placeholder values. + /// The type to traverse. + /// A List in which to store the parsed symbols. + private void TraverseActualType(string type, List typeSymbols) + { + int depth = 0; + string symbol = ""; + + foreach (char c in type) + { + if (this.symbolBoundaries.Contains(c)) + { + typeSymbols.Add(new SymbolLocation(symbol, depth)); + symbol = ""; + switch (c) + { + case '<': + depth++; + break; + case '>': + depth--; + break; + default: + break; + } + } + else + symbol += c; + } + } + + /// Determines whether two symbols in a type ID match, accounting for placeholders such as !0. + /// A symbol in a typename which contains placeholders. + /// A symbol in a typename which does not contain placeholders. + /// A dictionary containing a mapping of placeholders to concrete types. + /// true if the symbols match, false if not. + private bool SymbolsMatch(SymbolLocation symbolA, SymbolLocation symbolB, Dictionary placeholderMap) + { + if (symbolA.depth != symbolB.depth) + return false; + + if (!this.IsPlaceholder(symbolA.symbol)) + { + return symbolA.symbol == symbolB.symbol; + } + + if (placeholderMap.ContainsKey(symbolA.symbol)) + { + return placeholderMap[symbolA.symbol] == symbolB.symbol; + } + + placeholderMap[symbolA.symbol] = symbolB.symbol; + + return true; + } + + /// Determines whether a type which has placeholders correctly resolves to the concrete type provided. + /// A type containing placeholders such as !0. + /// The list of symbols extracted from the concrete type. + /// true if the type resolves correctly, false if not. + private bool PlaceholderTypeResolvesToActualType(string type, List typeSymbols) + { + Dictionary placeholderMap = new Dictionary(); + + int depth = 0, symbolCount = 0; + string symbol = ""; + + foreach (char c in type) + { + if (this.symbolBoundaries.Contains(c)) + { + bool match = this.SymbolsMatch(new SymbolLocation(symbol, depth), typeSymbols[symbolCount], placeholderMap); + if (typeSymbols.Count <= symbolCount || + !match) + return false; + + symbolCount++; + symbol = ""; + switch (c) + { + case '<': + depth++; + break; + case '>': + depth--; + break; + default: + break; + } + } + else + symbol += c; + } + + return true; + } + + /// Determines whether a type with placeholders in it matches a type without placeholders. + /// The type with placeholders in it. + /// The type without placeholders. + /// true if the placeholder type can resolve to the actual type, false if not. + private bool PlaceholderTypeValidates(string placeholderType, string actualType) + { + List typeSymbols = new List(); + + this.TraverseActualType(actualType, typeSymbols); + return PlaceholderTypeResolvesToActualType(placeholderType, typeSymbols); + } + + + + /********* + ** Inner classes + *********/ + protected class SymbolLocation + { + public string symbol; + public int depth; + + public SymbolLocation(string symbol, int depth) + { + this.symbol = symbol; + this.depth = depth; + } + } + } +} diff --git a/src/SMAPI/StardewModdingAPI.csproj b/src/SMAPI/StardewModdingAPI.csproj index c13f5e30..57c2c9e8 100644 --- a/src/SMAPI/StardewModdingAPI.csproj +++ b/src/SMAPI/StardewModdingAPI.csproj @@ -110,6 +110,7 @@ + From 530b120014c0ae7fc2994b21fc388ea36ddb4ce8 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 8 Jul 2018 15:48:32 -0400 Subject: [PATCH 11/12] rewrite TypeReference comparison to handle more edge cases, exit earlier if possible, and encapsulate a bit more --- .../ModLoading/TypeReferenceComparer.cs | 308 +++++++++--------- 1 file changed, 150 insertions(+), 158 deletions(-) diff --git a/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs b/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs index 8d128b37..f7497789 100644 --- a/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs +++ b/src/SMAPI/Framework/ModLoading/TypeReferenceComparer.cs @@ -1,21 +1,23 @@ +using System; using System.Collections.Generic; -using System.Text.RegularExpressions; +using System.Linq; using Mono.Cecil; namespace StardewModdingAPI.Framework.ModLoading { /// Performs heuristic equality checks for instances. + /// + /// This implementation compares instances to see if they likely + /// refer to the same type. While the implementation is obvious for types like System.Bool, + /// this class mainly exists to handle cases like System.Collections.Generic.Dictionary`2<!0,Netcode.NetRoot`1<!1>> + /// and System.Collections.Generic.Dictionary`2<TKey,Netcode.NetRoot`1<TValue>> + /// 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. !0 to + /// TKey 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. + /// internal class TypeReferenceComparer : IEqualityComparer { - /********* - ** Properties - *********/ - /// A pattern matching type name substrings to strip for display. - private readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled); - - private List symbolBoundaries = new List { '<', '>', ',' }; - - /********* ** Public methods *********/ @@ -24,25 +26,13 @@ namespace StardewModdingAPI.Framework.ModLoading /// The second object to compare. public bool Equals(TypeReference a, TypeReference b) { - string typeA = this.GetComparableTypeID(a); - string typeB = this.GetComparableTypeID(b); + if (a == null || b == null) + return a == b; - string placeholderType = "", actualType = ""; - - if (this.HasPlaceholder(typeA)) - { - placeholderType = typeA; - actualType = typeB; - } - else if (this.HasPlaceholder(typeB)) - { - placeholderType = typeB; - actualType = typeA; - } - else - return typeA == typeB; - - return this.PlaceholderTypeValidates(placeholderType, actualType); + return + a == b + || a.FullName == b.FullName + || this.HeuristicallyEquals(a, b); } /// Get a hash code for the specified object. @@ -57,153 +47,155 @@ namespace StardewModdingAPI.Framework.ModLoading /********* ** Private methods *********/ - /// Get a unique string representation of a type. - /// The type reference. - private string GetComparableTypeID(TypeReference type) + /// Get whether two types are heuristically equal based on generic type token substitution. + /// The first type to compare. + /// The second type to compare. + private bool HeuristicallyEquals(TypeReference typeA, TypeReference typeB) { - return this.StripTypeNamePattern.Replace(type.FullName, ""); - } - - /// Determine whether this type ID has a placeholder such as !0. - /// The type to check. - /// true if the type ID contains a placeholder, false if not. - private bool HasPlaceholder(string typeID) - { - return typeID.Contains("!0"); - } - - /// returns whether this type ID is a placeholder, i.e., it begins with "!". - /// The symbol to validate. - /// true if the symbol is a placeholder, false if not - private bool IsPlaceholder(string symbol) - { - return symbol.StartsWith("!"); - } - - /// Traverses and parses out symbols from a type which does not contain placeholder values. - /// The type to traverse. - /// A List in which to store the parsed symbols. - private void TraverseActualType(string type, List typeSymbols) - { - int depth = 0; - string symbol = ""; - - foreach (char c in type) + bool HeuristicallyEquals(string typeNameA, string typeNameB, IDictionary tokenMap) { - if (this.symbolBoundaries.Contains(c)) + // 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++) { - typeSymbols.Add(new SymbolLocation(symbol, depth)); - symbol = ""; - switch (c) - { - case '<': - depth++; - break; - case '>': - depth--; - break; - default: - break; - } - } - else - symbol += c; - } - } - - /// Determines whether two symbols in a type ID match, accounting for placeholders such as !0. - /// A symbol in a typename which contains placeholders. - /// A symbol in a typename which does not contain placeholders. - /// A dictionary containing a mapping of placeholders to concrete types. - /// true if the symbols match, false if not. - private bool SymbolsMatch(SymbolLocation symbolA, SymbolLocation symbolB, Dictionary placeholderMap) - { - if (symbolA.depth != symbolB.depth) - return false; - - if (!this.IsPlaceholder(symbolA.symbol)) - { - return symbolA.symbol == symbolB.symbol; - } - - if (placeholderMap.ContainsKey(symbolA.symbol)) - { - return placeholderMap[symbolA.symbol] == symbolB.symbol; - } - - placeholderMap[symbolA.symbol] = symbolB.symbol; - - return true; - } - - /// Determines whether a type which has placeholders correctly resolves to the concrete type provided. - /// A type containing placeholders such as !0. - /// The list of symbols extracted from the concrete type. - /// true if the type resolves correctly, false if not. - private bool PlaceholderTypeResolvesToActualType(string type, List typeSymbols) - { - Dictionary placeholderMap = new Dictionary(); - - int depth = 0, symbolCount = 0; - string symbol = ""; - - foreach (char c in type) - { - if (this.symbolBoundaries.Contains(c)) - { - bool match = this.SymbolsMatch(new SymbolLocation(symbol, depth), typeSymbols[symbolCount], placeholderMap); - if (typeSymbols.Count <= symbolCount || - !match) + if (!HeuristicallyEquals(symbolsA[i], symbolsB[i], tokenMap)) return false; - - symbolCount++; - symbol = ""; - switch (c) - { - case '<': - depth++; - break; - case '>': - depth--; - break; - default: - break; - } } - else - symbol += c; + + return true; } - return true; + return HeuristicallyEquals(typeA.FullName, typeB.FullName, new Dictionary()); } - /// Determines whether a type with placeholders in it matches a type without placeholders. - /// The type with placeholders in it. - /// The type without placeholders. - /// true if the placeholder type can resolve to the actual type, false if not. - private bool PlaceholderTypeValidates(string placeholderType, string actualType) + /// Map a generic type placeholder (like !0) to its actual type. + /// The token placeholder. + /// The actual type. + /// The map of token to map substitutions. + /// Returns the previously-mapped type if applicable, else the . + private string MapPlaceholder(string placeholder, string type, IDictionary map) { - List typeSymbols = new List(); + if (map.TryGetValue(placeholder, out string result)) + return result; - this.TraverseActualType(actualType, typeSymbols); - return PlaceholderTypeResolvesToActualType(placeholderType, typeSymbols); + map[placeholder] = type; + return type; } - - - /********* - ** Inner classes - *********/ - protected class SymbolLocation + /// Get the top-level type symbols in a type name (e.g. List and NetRef<T> in List<NetRef<T>>) + /// The full type name. + private IEnumerable GetTypeSymbols(string typeName) { - public string symbol; - public int depth; + int openGenerics = 0; - public SymbolLocation(string symbol, int depth) + Queue queue = new Queue(typeName); + string symbol = ""; + while (queue.Any()) { - this.symbol = symbol; - this.depth = depth; + 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; } } } From 8839b6822a475c693a0b4a88c6de861028df3caf Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 8 Jul 2018 15:56:08 -0400 Subject: [PATCH 12/12] also detect broken Netcode references --- src/SMAPI/Metadata/InstructionMetadata.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SMAPI/Metadata/InstructionMetadata.cs b/src/SMAPI/Metadata/InstructionMetadata.cs index aa3e743c..2f0c1b15 100644 --- a/src/SMAPI/Metadata/InstructionMetadata.cs +++ b/src/SMAPI/Metadata/InstructionMetadata.cs @@ -17,7 +17,7 @@ namespace StardewModdingAPI.Metadata *********/ /// The assembly names to which to heuristically detect broken references. /// The current implementation only works correctly with assemblies that should always be present. - private readonly string[] ValidateReferencesToAssemblies = { "StardewModdingAPI", "Stardew Valley", "StardewValley" }; + private readonly string[] ValidateReferencesToAssemblies = { "StardewModdingAPI", "Stardew Valley", "StardewValley", "Netcode" }; /*********