monogame-patcher: Refactor method ref finding

So far we have searched for methods in two steps: First get all the
types and then find all of the constructors or method definitions.

The match for the actual method definition was done first for the type
and then again for the full definition of the method/constructor.

For the replace-call subcommand this also means that we don't have a
very good way to find the type, because if we wanted to keep doing this
in two steps we would need to parse the type name out of the one given
as the replacement string in replace-call.

So now we recurse through *all* the constructors and methods and do a
full match against the specified replacement string, both for
replace-call and for fix-filestreams.

Compared to the implementation so far we also do this just once instead
of every time we find a call or FileStream constructor.

I also overhauled the way we do testing, because in the end writing
tests using NUnit would introduce a lot of code churn because we need to
make a lot of external calls. The shell is especially useful for exactly
that and our tests are now just a plain shell script.

Signed-off-by: aszlig <aszlig@nix.build>

5 files changed, 113 insertions, 67 deletions

diff --git a/pkgs/games/build-support/monogame-patcher/default.nix b/pkgs/games/build-support/monogame-patcher/default.nix
index 457d124a..bd7b8c9c 100644
--- a/pkgs/games/build-support/monogame-patcher/default.nix
+++ b/pkgs/games/build-support/monogame-patcher/default.nix
@@ -20,26 +20,8 @@ buildDotnetPackage {
       sha256 = "0wf8mzr16d2ni008m60rrk738v8ypk74llk6g8mlyx7rrlchnxaf";
       outputFiles = [ "lib/net45/*" ];
     })
-
-    (fetchNuGet {
-      baseName = "NUnit";
-      version = "3.10.1";
-      sha256 = "159m1wpb9yy2x77x7nl0647jkpzj5j801a2inhdl7hcjys8xrqxi";
-      outputFiles = [ "lib/net45/*" ];
-    })
-
-    (fetchNuGet {
-      baseName = "NUnit.ConsoleRunner";
-      version = "3.8.0";
-      sha256 = "1gspqzfhvpc8yapni7zcr5h2y025swihv78cw07v048l3myf3pzk";
-      outputFiles = [ "tools/*" ];
-    })
   ];
 
-  doCheck = true;
-  checkPhase = ''
-    nunitLibs="$(pkg-config nunit.framework --variable=Libraries)"
-    MONO_PATH="$(dirname "$nunitLibs")" HOME="$PWD" \
-      nunit3-console bin/Release/monogame-patcher.exe
-  '';
+  doInstallCheck = true;
+  installCheckPhase = "$SHELL -e test.sh";
 }
diff --git a/pkgs/games/build-support/monogame-patcher/src/patcher.cs b/pkgs/games/build-support/monogame-patcher/src/patcher.cs
index 794db98e..c9046f0a 100644
--- a/pkgs/games/build-support/monogame-patcher/src/patcher.cs
+++ b/pkgs/games/build-support/monogame-patcher/src/patcher.cs
@@ -27,6 +27,24 @@ class Command {
         this.module = ModuleDefinition.ReadModule(this.infile, rp);
     }
 
+    protected MethodReference find_method_ref(string fullSig) {
+        foreach (var aref in this.module.AssemblyReferences) {
+            var resolved = this.module.AssemblyResolver.Resolve(aref);
+            foreach (var type in resolved.MainModule.Types) {
+                foreach (var ctor in type.GetConstructors()) {
+                    if (ctor.ToString() != fullSig) continue;
+                    return this.module.ImportReference(ctor);
+                }
+                foreach (var meth in type.GetMethods()) {
+                    if (meth.ToString() != fullSig) continue;
+                    return this.module.ImportReference(meth);
+                }
+            }
+        }
+
+        throw new Exception($"Method reference for {fullSig} not found.");
+    }
+
     public void save() {
         if (this.outfile == this.infile)
             this.module.Write();
@@ -36,10 +54,17 @@ class Command {
 }
 
 class FixFileStreams : Command {
+    private MethodReference betterFileStream;
+
     public FixFileStreams(FixFileStreamsCmd options) : base(options) {
         var filtered = this.module.Types
             .Where(p => options.typesToPatch.Contains(p.Name));
 
+        this.betterFileStream = this.find_method_ref(
+            "System.Void System.IO.FileStream::.ctor" +
+            "(System.String,System.IO.FileMode,System.IO.FileAccess)"
+        );
+
         foreach (var toPatch in filtered)
             patch_type(toPatch);
 
@@ -57,24 +82,7 @@ class FixFileStreams : Command {
         foreach (Instruction i in fileStreams.ToList()) {
             var fileAccessRead = il.Create(OpCodes.Ldc_I4_1);
             il.InsertBefore(i, fileAccessRead);
-
-            var ctorType = this.module.AssemblyReferences.Select(
-                x => new {
-                    type = this.module.AssemblyResolver.Resolve(x)
-                        .MainModule.GetType("System.IO.FileStream")
-                }
-            ).Where(x => x.type != null).Select(x => x.type).First();
-
-            string wantedCtor = "System.Void System.IO.FileStream"
-                              + "::.ctor(System.String,"
-                              + "System.IO.FileMode,"
-                              + "System.IO.FileAccess)";
-
-            var newCtor = ctorType.GetConstructors()
-                .Single(x => x.ToString() == wantedCtor);
-
-            var refCtor = this.module.ImportReference(newCtor);
-            il.Replace(i, il.Create(OpCodes.Newobj, refCtor));
+            il.Replace(i, il.Create(OpCodes.Newobj, this.betterFileStream));
         }
     }
 
@@ -86,11 +94,11 @@ class FixFileStreams : Command {
 
 class ReplaceCall : Command {
     private string search;
-    private string replace;
+    private MethodReference replace;
 
     public ReplaceCall(ReplaceCallCmd options) : base(options) {
         this.search = options.replaceCall.ToList()[0];
-        this.replace = options.replaceCall.ToList()[1];
+        this.replace = this.find_method_ref(options.replaceCall.ToList()[1]);
 
         var filtered = this.module.Types
             .Where(p => options.typesToPatch.Contains(p.Name));
@@ -108,20 +116,8 @@ class ReplaceCall : Command {
             .Where(i => i.OpCode == OpCodes.Call)
             .Where(i => i.Operand.ToString() == this.search);
 
-        var ctorType = this.module.AssemblyReferences.Select(
-            x => new {
-                // XXX: Don't hardcode UnityEngine.Application!
-                type = this.module.AssemblyResolver.Resolve(x)
-                    .MainModule.GetType("UnityEngine.Application")
-            }
-        ).Where(x => x.type != null).Select(x => x.type).First();
-
-        var newMethod = ctorType.GetMethods()
-            .Single(x => x.ToString() == this.replace);
-        var refMethod = this.module.ImportReference(newMethod);
-
         foreach (Instruction i in found.ToList())
-            il.Replace(i, il.Create(OpCodes.Call, refMethod));
+            il.Replace(i, il.Create(OpCodes.Call, this.replace));
     }
 
     private void patch_type(TypeDefinition td) {
diff --git a/pkgs/games/build-support/monogame-patcher/src/patcher.csproj b/pkgs/games/build-support/monogame-patcher/src/patcher.csproj
index 994b78e5..03307287 100644
--- a/pkgs/games/build-support/monogame-patcher/src/patcher.csproj
+++ b/pkgs/games/build-support/monogame-patcher/src/patcher.csproj
@@ -7,7 +7,6 @@
     <Compile Include="assembly-info.cs"/>
     <Compile Include="options.cs"/>
     <Compile Include="patcher.cs"/>
-    <Compile Include="tests.cs"/>
   </ItemGroup>
 
   <PropertyGroup>
@@ -16,7 +15,6 @@
 
     <CecilInfo>Version=0.10.0.0$(Blurb)50cebf1cceb9d05e</CecilInfo>
     <CmdLineInfo>Version=2.2.1.0$(Blurb)de6f01bd326f8c32</CmdLineInfo>
-    <NUnitInfo>Version=3.10.1.0$(Blurb)2638cd05610744eb</NUnitInfo>
   </PropertyGroup>
 
   <ItemGroup>
@@ -24,12 +22,14 @@
       <SpecificVersion>False</SpecificVersion>
       <Private>True</Private>
     </Reference>
-    <Reference Include="Mono.Cecil.Rocks"/>
+    <Reference Include="Mono.Cecil.Rocks, $(CecilInfo)">
+      <SpecificVersion>False</SpecificVersion>
+      <Private>True</Private>
+    </Reference>
     <Reference Include="CommandLine, $(CmdLineInfo)">
       <SpecificVersion>False</SpecificVersion>
       <Private>True</Private>
     </Reference>
-    <Reference Include="nunit.framework, $(NUnitInfo)"/>
   </ItemGroup>
 
   <PropertyGroup>
diff --git a/pkgs/games/build-support/monogame-patcher/src/test.sh b/pkgs/games/build-support/monogame-patcher/src/test.sh
new file mode 100644
index 00000000..80260c80
--- /dev/null
+++ b/pkgs/games/build-support/monogame-patcher/src/test.sh
@@ -0,0 +1,78 @@
+set -x
+cd "$(mktemp -d)"
+
+cat > "a.cs" <<EOF
+using System;
+
+public class a {
+    public static string replaceMe(string foo) {
+        Console.WriteLine("foo called: " + foo);
+        return "foo";
+    }
+}
+EOF
+
+cat > "b.cs" <<EOF
+using System;
+using System.IO;
+
+public class b {
+    public static string replacement(string bar) {
+        if (bar == "nope")
+            return "nope";
+        Console.WriteLine("bar called: " + bar);
+        return "bar";
+    }
+
+    public static void wrongFileStreamUse() {
+        var fs = new FileStream("write_test.txt", FileMode.Open);
+        if (fs.CanWrite)
+            Console.WriteLine("can write");
+        else
+            Console.WriteLine("can not write");
+    }
+}
+EOF
+
+cat > "test1.cs" <<EOF
+class test1 {
+    public static void Main() {
+        a.replaceMe("xxx");
+        b.replacement("nope");
+    }
+}
+EOF
+
+cat > "test2.cs" <<EOF
+class test1 {
+    public static void Main() {
+        b.wrongFileStreamUse();
+    }
+}
+EOF
+
+mcs a.cs -target:library -out:a.dll
+mcs b.cs -target:library -out:b.dll
+
+mcs test1.cs -r:a -r:b -out:test1.exe
+mcs test2.cs -r:a -r:b -out:test2.exe
+
+! "$out/bin/monogame-patcher" replace-call -i test1.exe \
+    "System.String a::replaceMe(System.String)" \
+    "System.String b::notfound(System.String)" \
+    test1 2> /dev/null
+
+"$out/bin/monogame-patcher" replace-call -i test1.exe \
+    "System.String a::replaceMe(System.String)" \
+    "System.String b::replacement(System.String)" \
+    test1
+
+test "$(mono test1.exe)" = "bar called: xxx"
+
+echo foo > write_test.txt
+
+test "$(mono test2.exe)" = "can write"
+
+"$out/bin/monogame-patcher" fix-filestreams -i b.dll b
+
+test "$(mono test2.exe)" = "can not write"
diff --git a/pkgs/games/build-support/monogame-patcher/src/tests.cs b/pkgs/games/build-support/monogame-patcher/src/tests.cs
deleted file mode 100644
index 8b9d3ee3..00000000
--- a/pkgs/games/build-support/monogame-patcher/src/tests.cs
+++ /dev/null
@@ -1,10 +0,0 @@
-using NUnit.Framework;
-
-[TestFixture]
-public class PatcherTests {
-    [Test]
-    public void foo() {
-        // Dummy test
-        Assert.IsTrue(true);
-    }
-}