automatically detect broken code (#453)

5 files changed, 250 insertions, 23 deletions

diff --git a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs
index 7dcf94bf..f0e186a1 100644
--- a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs
+++ b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs
@@ -238,10 +238,13 @@ namespace StardewModdingAPI.Framework.ModLoading
 
                 // check CIL instructions
                 ILProcessor cil = method.Body.GetILProcessor();
-                foreach (Instruction instruction in cil.Body.Instructions.ToArray())
+                var instructions = cil.Body.Instructions;
+                // ReSharper disable once ForCanBeConvertedToForeach -- deliberate access by index so each handler sees replacements from previous handlers
+                for (int offset = 0; offset < instructions.Count; offset++)
                 {
                     foreach (IInstructionHandler handler in handlers)
                     {
+                        Instruction instruction = instructions[offset];
                         InstructionHandleResult result = handler.Handle(module, cil, instruction, this.AssemblyMap, platformChanged);
                         this.ProcessInstructionHandleResult(mod, handler, result, loggedMessages, logPrefix, assumeCompatible, filename);
                         if (result == InstructionHandleResult.Rewritten)
diff --git a/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMissingMemberFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMissingMemberFinder.cs
new file mode 100644
index 00000000..60373c9d
--- /dev/null
+++ b/src/SMAPI/Framework/ModLoading/Finders/ReferenceToMissingMemberFinder.cs
@@ -0,0 +1,87 @@
+using System.Linq;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+
+namespace StardewModdingAPI.Framework.ModLoading.Finders
+{
+    /// <summary>Finds references to a field, property, or method which no longer exists.</summary>
+    /// <remarks>This implementation is purely heuristic. It should never return a false positive, but won't detect all cases.</remarks>
+    internal class ReferenceToMissingMemberFinder : IInstructionHandler
+    {
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>A brief noun phrase indicating what the instruction finder matches.</summary>
+        public string NounPhrase { get; private set; } = "";
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Perform the predefined logic for a method if applicable.</summary>
+        /// <param name="module">The assembly module containing the instruction.</param>
+        /// <param name="method">The method definition containing the instruction.</param>
+        /// <param name="assemblyMap">Metadata for mapping assemblies to the current platform.</param>
+        /// <param name="platformChanged">Whether the mod was compiled on a different platform.</param>
+        public virtual InstructionHandleResult Handle(ModuleDefinition module, MethodDefinition method, PlatformAssemblyMap assemblyMap, bool platformChanged)
+        {
+            return InstructionHandleResult.None;
+        }
+
+        /// <summary>Perform the predefined logic for an instruction if applicable.</summary>
+        /// <param name="module">The assembly module containing the instruction.</param>
+        /// <param name="cil">The CIL processor.</param>
+        /// <param name="instruction">The instruction to handle.</param>
+        /// <param name="assemblyMap">Metadata for mapping assemblies to the current platform.</param>
+        /// <param name="platformChanged">Whether the mod was compiled on a different platform.</param>
+        public virtual InstructionHandleResult Handle(ModuleDefinition module, ILProcessor cil, Instruction instruction, PlatformAssemblyMap assemblyMap, bool platformChanged)
+        {
+            // field reference
+            FieldReference fieldRef = RewriteHelper.AsFieldReference(instruction);
+            if (fieldRef != null)
+            {
+                FieldDefinition target = fieldRef.DeclaringType.Resolve()?.Fields.FirstOrDefault(p => p.Name == fieldRef.Name);
+                if (target == null)
+                {
+                    this.NounPhrase = $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (no such field)";
+                    return InstructionHandleResult.NotCompatible;
+                }
+            }
+
+            // method reference
+            MethodReference methodRef = RewriteHelper.AsMethodReference(instruction);
+            if (methodRef != null && !this.IsUnsupported(methodRef))
+            {
+                MethodDefinition target = methodRef.DeclaringType.Resolve()?.Methods.FirstOrDefault(p => p.Name == methodRef.Name);
+                if (target == null)
+                {
+                    this.NounPhrase = this.IsProperty(methodRef)
+                        ? $"reference to {methodRef.DeclaringType.FullName}.{methodRef.Name.Substring(4)} (no such property)"
+                        : $"reference to {methodRef.DeclaringType.FullName}.{methodRef.Name} (no such method)";
+                    return InstructionHandleResult.NotCompatible;
+                }
+            }
+
+            return InstructionHandleResult.None;
+        }
+
+
+        /*********
+        ** Private methods
+        *********/
+        /// <summary>Get whether a method reference is a special case that's not currently supported (e.g. array methods).</summary>
+        /// <param name="method">The method reference.</param>
+        private bool IsUnsupported(MethodReference method)
+        {
+            return
+                method.DeclaringType.Name.Contains("["); // array methods
+        }
+
+        /// <summary>Get whether a method reference is a property getter or setter.</summary>
+        /// <param name="method">The method reference.</param>
+        private bool IsProperty(MethodReference method)
+        {
+            return method.Name.StartsWith("get_") || method.Name.StartsWith("set_");
+        }
+    }
+}
diff --git a/src/SMAPI/Framework/ModLoading/Rewriters/ReferenceToMemberWithUnexpectedTypeFinder.cs b/src/SMAPI/Framework/ModLoading/Rewriters/ReferenceToMemberWithUnexpectedTypeFinder.cs
new file mode 100644
index 00000000..f2080e40
--- /dev/null
+++ b/src/SMAPI/Framework/ModLoading/Rewriters/ReferenceToMemberWithUnexpectedTypeFinder.cs
@@ -0,0 +1,131 @@
+using System.Linq;
+using System.Text.RegularExpressions;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+
+namespace StardewModdingAPI.Framework.ModLoading.Rewriters
+{
+    /// <summary>Finds references to a field, property, or method which returns a different type than the code expects.</summary>
+    /// <remarks>This implementation is purely heuristic. It should never return a false positive, but won't detect all cases.</remarks>
+    internal class ReferenceToMemberWithUnexpectedTypeFinder : IInstructionHandler
+    {
+        /*********
+        ** Properties
+        *********/
+        /// <summary>A pattern matching type name substrings to strip for display.</summary>
+        private readonly Regex StripTypeNamePattern = new Regex(@"`\d+(?=<)", RegexOptions.Compiled);
+
+
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>A brief noun phrase indicating what the instruction finder matches.</summary>
+        public string NounPhrase { get; private set; } = "";
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Perform the predefined logic for a method if applicable.</summary>
+        /// <param name="module">The assembly module containing the instruction.</param>
+        /// <param name="method">The method definition containing the instruction.</param>
+        /// <param name="assemblyMap">Metadata for mapping assemblies to the current platform.</param>
+        /// <param name="platformChanged">Whether the mod was compiled on a different platform.</param>
+        public virtual InstructionHandleResult Handle(ModuleDefinition module, MethodDefinition method, PlatformAssemblyMap assemblyMap, bool platformChanged)
+        {
+            return InstructionHandleResult.None;
+        }
+
+        /// <summary>Perform the predefined logic for an instruction if applicable.</summary>
+        /// <param name="module">The assembly module containing the instruction.</param>
+        /// <param name="cil">The CIL processor.</param>
+        /// <param name="instruction">The instruction to handle.</param>
+        /// <param name="assemblyMap">Metadata for mapping assemblies to the current platform.</param>
+        /// <param name="platformChanged">Whether the mod was compiled on a different platform.</param>
+        public virtual InstructionHandleResult Handle(ModuleDefinition module, ILProcessor cil, Instruction instruction, PlatformAssemblyMap assemblyMap, bool platformChanged)
+        {
+            // field reference
+            FieldReference fieldRef = RewriteHelper.AsFieldReference(instruction);
+            if (fieldRef != null)
+            {
+                // get target field
+                FieldDefinition targetField = fieldRef.DeclaringType.Resolve()?.Fields.FirstOrDefault(p => p.Name == fieldRef.Name);
+                if (targetField == null)
+                    return InstructionHandleResult.None;
+
+                // validate return type
+                string actualReturnTypeID = this.GetComparableTypeID(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)})";
+                    return InstructionHandleResult.NotCompatible;
+                }
+            }
+
+            // method reference
+            MethodReference methodReference = RewriteHelper.AsMethodReference(instruction);
+            if (methodReference != null)
+            {
+                // get potential targets
+                MethodDefinition[] candidateMethods = methodReference.DeclaringType.Resolve()?.Methods.Where(found => found.Name == methodReference.Name).ToArray();
+                if (candidateMethods == null || !candidateMethods.Any())
+                    return InstructionHandleResult.None;
+
+                // compare return types
+                MethodDefinition methodDef = methodReference.Resolve();
+                if (methodDef == null)
+                {
+                    this.NounPhrase = $"reference to {methodReference.DeclaringType.FullName}.{methodReference.Name} (no such method)";
+                    return InstructionHandleResult.NotCompatible;
+                }
+
+                string expectedReturnType = this.GetComparableTypeID(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)})";
+                    return InstructionHandleResult.NotCompatible;
+                }
+            }
+
+            return InstructionHandleResult.None;
+        }
+
+
+        /*********
+        ** Private methods
+        *********/
+        /// <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, string typeID)
+        {
+            // most common built-in types
+            switch (type.FullName)
+            {
+                case "System.Boolean":
+                    return "bool";
+                case "System.Int32":
+                    return "int";
+                case "System.String":
+                    return "string";
+            }
+
+            // most common unambiguous namespaces
+            foreach (string @namespace in new[] { "Microsoft.Xna.Framework", "Netcode", "System", "System.Collections.Generic" })
+            {
+                if (type.Namespace == @namespace)
+                    return typeID.Substring(@namespace.Length + 1);
+            }
+
+            return typeID;
+        }
+    }
+}
diff --git a/src/SMAPI/Metadata/InstructionMetadata.cs b/src/SMAPI/Metadata/InstructionMetadata.cs
index 5bb461c1..9e2b7967 100644
--- a/src/SMAPI/Metadata/InstructionMetadata.cs
+++ b/src/SMAPI/Metadata/InstructionMetadata.cs
@@ -21,7 +21,27 @@ namespace StardewModdingAPI.Metadata
             return new IInstructionHandler[]
             {
                 /****
-                ** throw exception for incompatible code
+                ** rewrite CIL to fix incompatible code
+                ****/
+                // crossplatform
+                new MethodParentRewriter(typeof(SpriteBatch), typeof(SpriteBatchMethods), onlyIfPlatformChanged: true),
+
+                // Stardew Valley 1.2
+                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.activeClickableMenu)),
+                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.currentMinigame)),
+                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.gameMode)),
+                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.player)),
+                new FieldReplaceRewriter(typeof(Game1), "borderFont", nameof(Game1.smallFont)),
+                new FieldReplaceRewriter(typeof(Game1), "smoothFont", nameof(Game1.smallFont)),
+
+                // SMAPI 1.9
+                new TypeReferenceRewriter("StardewModdingAPI.Inheritance.ItemStackChange", typeof(ItemStackChange)),
+
+                // SMAPI 2.0
+                new VirtualEntryCallRemover(), // Mod.Entry changed from virtual to abstract in SMAPI 2.0, which breaks the few mods which called base.Entry()
+
+                /****
+                ** detect incompatible code
                 ****/
                 // changes in Stardew Valley 1.2 (with no rewriters)
                 new FieldFinder("StardewValley.Item", "set_Name", InstructionHandleResult.NotCompatible),
@@ -66,6 +86,10 @@ namespace StardewModdingAPI.Metadata
                 new PropertyFinder("StardewModdingAPI.Mod", "PerSaveConfigFolder", InstructionHandleResult.NotCompatible),
                 new PropertyFinder("StardewModdingAPI.Mod", "PerSaveConfigPath", InstructionHandleResult.NotCompatible),
 
+                // broken code
+                new ReferenceToMissingMemberFinder(),
+                new ReferenceToMemberWithUnexpectedTypeFinder(),
+
                 /****
                 ** detect code which may impact game stability
                 ****/
@@ -74,27 +98,7 @@ namespace StardewModdingAPI.Metadata
                 new FieldFinder(typeof(SaveGame).FullName, nameof(SaveGame.serializer), InstructionHandleResult.DetectedSaveSerialiser),
                 new FieldFinder(typeof(SaveGame).FullName, nameof(SaveGame.farmerSerializer), InstructionHandleResult.DetectedSaveSerialiser),
                 new FieldFinder(typeof(SaveGame).FullName, nameof(SaveGame.locationSerializer), InstructionHandleResult.DetectedSaveSerialiser),
-                new EventFinder(typeof(SpecialisedEvents).FullName, nameof(SpecialisedEvents.UnvalidatedUpdateTick), InstructionHandleResult.DetectedUnvalidatedUpdateTick), 
-
-                /****
-                ** rewrite CIL to fix incompatible code
-                ****/
-                // crossplatform
-                new MethodParentRewriter(typeof(SpriteBatch), typeof(SpriteBatchMethods), onlyIfPlatformChanged: true),
-
-                // Stardew Valley 1.2
-                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.activeClickableMenu)),
-                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.currentMinigame)),
-                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.gameMode)),
-                new FieldToPropertyRewriter(typeof(Game1), nameof(Game1.player)),
-                new FieldReplaceRewriter(typeof(Game1), "borderFont", nameof(Game1.smallFont)),
-                new FieldReplaceRewriter(typeof(Game1), "smoothFont", nameof(Game1.smallFont)),
-
-                // SMAPI 1.9
-                new TypeReferenceRewriter("StardewModdingAPI.Inheritance.ItemStackChange", typeof(ItemStackChange)),
-
-                // SMAPI 2.0
-                new VirtualEntryCallRemover() // Mod.Entry changed from virtual to abstract in SMAPI 2.0, which breaks the few mods which called base.Entry()
+                new EventFinder(typeof(SpecialisedEvents).FullName, nameof(SpecialisedEvents.UnvalidatedUpdateTick), InstructionHandleResult.DetectedUnvalidatedUpdateTick)
             };
         }
     }
diff --git a/src/SMAPI/StardewModdingAPI.csproj b/src/SMAPI/StardewModdingAPI.csproj
index 8ef3022f..a04208e4 100644
--- a/src/SMAPI/StardewModdingAPI.csproj
+++ b/src/SMAPI/StardewModdingAPI.csproj
@@ -103,6 +103,7 @@
     <Compile Include="Framework\ModLoading\Finders\FieldFinder.cs" />
     <Compile Include="Framework\ModLoading\Finders\MethodFinder.cs" />
     <Compile Include="Framework\ModLoading\Finders\PropertyFinder.cs" />
+    <Compile Include="Framework\ModLoading\Finders\ReferenceToMissingMemberFinder.cs" />
     <Compile Include="Framework\ModLoading\Finders\TypeFinder.cs" />
     <Compile Include="Framework\ModLoading\IInstructionHandler.cs" />
     <Compile Include="Framework\ModLoading\IncompatibleInstructionException.cs" />
@@ -112,6 +113,7 @@
     <Compile Include="Framework\ModLoading\RewriteHelper.cs" />
     <Compile Include="Framework\ModLoading\Rewriters\FieldReplaceRewriter.cs" />
     <Compile Include="Framework\ModLoading\Rewriters\FieldToPropertyRewriter.cs" />
+    <Compile Include="Framework\ModLoading\Rewriters\ReferenceToMemberWithUnexpectedTypeFinder.cs" />
     <Compile Include="Framework\ModLoading\Rewriters\VirtualEntryCallRemover.cs" />
     <Compile Include="Framework\ModLoading\Rewriters\MethodParentRewriter.cs" />
     <Compile Include="Framework\ModLoading\Rewriters\TypeReferenceRewriter.cs" />