Update patcher to use concrete type instead of an interface for collections, disposal of any assembly reading

Author: scorteanucosmin

src/AssemblyResolver.cs

@@ -1,6 +1,6 @@
 extern alias References;
-using System.IO;
 using Oxide.Core;
+using Oxide.CSharp.Common;
 using References::Mono.Cecil;
 
 namespace Oxide.CSharp;
@@ -14,35 +14,35 @@ public AssemblyResolver()
     {
         _resolver = new DefaultAssemblyResolver();
         _resolver.AddSearchDirectory(Interface.Oxide.ExtensionDirectory);
-        _mscorlib = AssemblyDefinition.ReadAssembly(Path.Combine(Interface.Oxide.ExtensionDirectory, "mscorlib.dll"));
+        _mscorlib = AssemblyDefinition.ReadAssembly(Constants.MscorlibPath);
     }
 
     public AssemblyDefinition Resolve(AssemblyNameReference assemblyNameReference)
     {
-        if (assemblyNameReference.Name == "System.Private.CoreLib")
+        if (assemblyNameReference.Name != "System.Private.CoreLib")
         {
-            Interface.Oxide.RootLogger.WriteDebug(Core.Logging.LogType.Warning,
-                new Logging.LogEvent(50, "Resolve"), "Resolver",
-                "Redirecting reference to System.Private.CoreLib to mscorlib");
-
-            return _mscorlib;
+            return _resolver.Resolve(assemblyNameReference);
         }
 
-        return _resolver.Resolve(assemblyNameReference);
+        Interface.Oxide.RootLogger.WriteDebug(Core.Logging.LogType.Warning,
+            new Logging.LogEvent(50, "Resolve"), "Resolver",
+            "Redirecting reference to System.Private.CoreLib to mscorlib");
+
+        return _mscorlib;
     }
 
     public AssemblyDefinition Resolve(AssemblyNameReference assemblyNameReference, ReaderParameters readerParameters)
     {
-        if (assemblyNameReference.Name == "System.Private.CoreLib")
+        if (assemblyNameReference.Name != "System.Private.CoreLib")
         {
-            Interface.Oxide.RootLogger.WriteDebug(Core.Logging.LogType.Warning,
-                new Logging.LogEvent(50, "Resolve"), "Resolver",
-                "Redirecting reference to System.Private.CoreLib to mscorlib");
-
-            return _mscorlib;
+            return _resolver.Resolve(assemblyNameReference, readerParameters);
         }
 
-        return _resolver.Resolve(assemblyNameReference, readerParameters);
+        Interface.Oxide.RootLogger.WriteDebug(Core.Logging.LogType.Warning,
+            new Logging.LogEvent(50, "Resolve"), "Resolver",
+            "Redirecting reference to System.Private.CoreLib to mscorlib");
+
+        return _mscorlib;
     }
 
     public void Dispose()

src/CSharpPluginLoader.cs

@@ -26,11 +26,12 @@ public class CSharpPluginLoader : PluginLoader
         public static CompilablePlugin GetCompilablePlugin(string directory, string name)
         {
             string className = Regex.Replace(name, "_", "");
-            if (!plugins.TryGetValue(className, out CompilablePlugin plugin))
+            if (!plugins.TryGetValue(className, out CompilablePlugin? plugin))
             {
                 plugin = new CompilablePlugin(extension, Instance, directory, name);
                 plugins[className] = plugin;
             }
+
             return plugin;
         }
 
@@ -285,10 +286,12 @@ private void PluginLoadingCompleted(CompilablePlugin plugin)
             foreach (string loadingName in LoadingPlugins.ToArray())
             {
                 CompilablePlugin loadingPlugin = GetCompilablePlugin(plugin.Directory, loadingName);
-                if (loadingPlugin.IsLoading && loadingPlugin.Requires.Contains(plugin.Name))
+                if (!loadingPlugin.IsLoading || !loadingPlugin.Requires.Contains(plugin.Name))
                 {
-                    Load(loadingPlugin);
+                    continue;
                 }
+
+                Load(loadingPlugin);
             }
         }
 

src/Common/Constants.cs

@@ -41,5 +41,6 @@ internal static class Constants
 
         internal static readonly string IncludePath = Path.Combine(Interface.Oxide.PluginDirectory, "include");
         internal static readonly string CSharpPath = Path.Combine(Interface.Oxide.ExtensionDirectory, "Oxide.CSharp.dll");
+        internal static readonly string MscorlibPath = Path.Combine(Interface.Oxide.ExtensionDirectory, "mscorlib.dll");
     }
 }

src/CompiledAssembly.cs

@@ -111,7 +111,7 @@ private void ValidateAssembly(Action<byte[], byte[]> callback)
                         SymbolReaderProvider = new PortablePdbReaderProvider()
                     };
 
-                    AssemblyDefinition baseAssembly = AssemblyDefinition.ReadAssembly(Constants.CSharpPath,
+                    using AssemblyDefinition baseAssembly = AssemblyDefinition.ReadAssembly(Constants.CSharpPath,
                         new ReaderParameters
                         {
                             AssemblyResolver = assemblyResolver,
@@ -122,7 +122,8 @@ private void ValidateAssembly(Action<byte[], byte[]> callback)
                     using MemoryStream symbolStream = new(Symbols);
                     readerParameters.SymbolStream = symbolStream;
 
-                    AssemblyDefinition assemblyDefinition = AssemblyDefinition.ReadAssembly(assemblyStream, readerParameters);
+                    using AssemblyDefinition assemblyDefinition =
+                        AssemblyDefinition.ReadAssembly(assemblyStream, readerParameters);
 
                     int foundPlugins = 0;
                     int totalPlugins = CompilablePlugins.Count(p => p.CompilerErrors.Count == 0);

src/CompilerStream/CompilerFile.cs

@@ -21,7 +21,7 @@ public class CompilerFile
         internal static readonly Dictionary<string, CompilerFile> FileCache =
             new Dictionary<string, CompilerFile>(StringComparer.InvariantCultureIgnoreCase);
 
-        public static CompilerFile CachedReadFile(string directory, string fileName, byte[] data = null)
+        public static CompilerFile? CachedReadFile(string directory, string fileName, byte[]? data = null)
         {
             string fullPath = Path.Combine(directory, fileName);
 
@@ -34,6 +34,7 @@ public static CompilerFile CachedReadFile(string directory, string fileName, byt
                     {
                         file.Data = data;
                     }
+
                     file.LastRead = DateTime.Now;
                     return file;
                 }

src/Patching/Patcher.cs

@@ -24,19 +24,21 @@ private static void GetPatches(Assembly module, ref Dictionary<Type, List<PatchV
             try
             {
                 Type[] types = module.GetTypes();
-
-                for (int i = 0; i < types.Length; i++)
+                int typeCount = types.Length;
+                for (int i = 0; i < typeCount; i++)
                 {
                     Type type = types[i];
 
-                    if (!type.IsAbstract && PatchType.IsAssignableFrom(type))
+                    if (type.IsAbstract || !PatchType.IsAssignableFrom(type))
                     {
-                        List<PatchValidationAttribute> validators = GetValidationRules(type.GetCustomAttributes(PatchValidationType, true)
-                            .Concat(type.Assembly.GetCustomAttributes(PatchValidationType, true)).ToArray());
-
-                        patchTypes.Add(type, validators);
-                        Interface.Oxide.RootLogger.WriteDebug(LogType.Info, Logging.LogEvent.Patch, "Patcher", $"Found {validators.Count} total validators for patch {type.Name}");
+                        continue;
                     }
+
+                    List<PatchValidationAttribute> validators = GetValidationRules(type.GetCustomAttributes(PatchValidationType, true)
+                        .Concat(type.Assembly.GetCustomAttributes(PatchValidationType, true)).ToArray());
+
+                    patchTypes.Add(type, validators);
+                    Interface.Oxide.RootLogger.WriteDebug(LogType.Info, Logging.LogEvent.Patch, "Patcher", $"Found {validators.Count} total validators for patch {type.Name}");
                 }
             }
             catch (Exception e)
@@ -47,7 +49,8 @@ private static void GetPatches(Assembly module, ref Dictionary<Type, List<PatchV
 
         private static void GetPatches(Assembly[] modules, ref Dictionary<Type, List<PatchValidationAttribute>> patchTypes)
         {
-            for (int i = 0; i < modules.Length; i++)
+            int moduleCount = modules.Length;
+            for (int i = 0; i < moduleCount; i++)
             {
                 GetPatches(modules[i], ref patchTypes);
             }
@@ -63,23 +66,27 @@ public static bool Run(AssemblyDefinition module)
             }
 
             PatchContext context = new PatchContext(module);
-            foreach (var kv in Patches)
+            foreach (KeyValuePair<Type, List<PatchValidationAttribute>> kv in Patches)
             {
 
                 Type patchType = kv.Key;
                 List<PatchValidationAttribute> validators = kv.Value;
                 context.PatchValidators = validators;
                 bool failed = false;
-                for (int n = 0; n < validators.Count; n++)
+
+                int validatorCount = validators.Count;
+                for (int i = 0; i < validatorCount; i++)
                 {
-                    PatchValidationAttribute valid = validators[n];
-                    bool pass = valid.Validate(module);
+                    PatchValidationAttribute patchValidationAttribute = validators[i];
+                    bool pass = patchValidationAttribute.Validate(module);
                     // Interface.Oxide.RootLogger.WriteDebug(LogType.Info, Logging.LogEvent.Patch, "Patcher", $"Validation {valid.GetType().Name}: {(pass ? "passed" : "failed")}");
-                    if (!pass)
+                    if (pass)
                     {
-                        failed = true;
-                        break;
+                        continue;
                     }
+
+                    failed = true;
+                    break;
                 }
 
                 if (failed)
@@ -96,10 +103,10 @@ public static bool Run(AssemblyDefinition module)
                     Interface.Oxide.RootLogger.WriteDebug(LogType.Info, Logging.LogEvent.Patch, "Patcher",
                         $"{patchType.Name} has applied {context.ContextPatches} patches to {module.Name?.Name ?? module.FullName}");
                 }
-                catch (Exception e)
+                catch (Exception exception)
                 {
                     Interface.Oxide.RootLogger.WriteDebug(LogType.Error, Logging.LogEvent.Patch, "Patcher",
-                        $"{patchType.Name} has applied {context.ContextPatches} patches to {module.Name?.Name ?? module.FullName} but threw a error", e);
+                        $"{patchType.Name} has applied {context.ContextPatches} patches to {module.Name?.Name ?? module.FullName} but threw a error", exception);
                 }
             }
 
@@ -110,25 +117,20 @@ public static byte[] Run(byte[] data, out bool patched)
         {
             try
             {
-                using (MemoryStream inStream = new MemoryStream(data))
-                {
-                    AssemblyDefinition assembly = AssemblyDefinition.ReadAssembly(inStream);
+                using MemoryStream inStream = new(data);
+                using AssemblyDefinition assembly = AssemblyDefinition.ReadAssembly(inStream);
 
-                    if (Run(assembly))
-                    {
-                        using (MemoryStream outStream = new MemoryStream())
-                        {
-                            assembly.Write(outStream);
-                            patched = true;
-                            return outStream.ToArray();
-                        }
-                    }
+                if (Run(assembly))
+                {
+                    using MemoryStream outStream = new();
+                    assembly.Write(outStream);
+                    patched = true;
+                    return outStream.ToArray();
                 }
-
             }
-            catch (Exception e)
+            catch (Exception exception)
             {
-                Interface.Oxide.RootLogger.WriteDebug(LogType.Error, Logging.LogEvent.Patch, "Patcher", $"Failed to patch", e);
+                Interface.Oxide.RootLogger.WriteDebug(LogType.Error, Logging.LogEvent.Patch, "Patcher", $"Failed to patch", exception);
             }
 
             patched = false;
@@ -137,15 +139,17 @@ public static byte[] Run(byte[] data, out bool patched)
 
         public static List<PatchValidationAttribute> GetValidationRules(object[] attributes)
         {
-            List<PatchValidationAttribute> validators = new List<PatchValidationAttribute>();
+            List<PatchValidationAttribute> validators = new();
 
-            for (int i = 0; i < attributes.Length; i++)
+            int attributeCount = attributes.Length;
+            for (int i = 0; i < attributeCount; i++)
             {
-                Attribute a = attributes[i] as Attribute;
-                if (a is PatchValidationAttribute valid)
+                if (attributes[i] is not PatchValidationAttribute patchValidationAttribute)
                 {
-                    validators.Add(valid);
+                    continue;
                 }
+
+                validators.Add(patchValidationAttribute);
             }
 
             return validators;

src/Patching/Publicizer.cs

@@ -30,19 +30,19 @@ protected override bool OnMemberDefinition(IMemberDefinition member)
 
         protected override bool OnTypeDefinition(TypeDefinition type)
         {
-            if (type.IsNested && !type.IsNestedPublic)
+            if (type is { IsNested: true, IsNestedPublic: false })
             {
                 type.IsNestedPublic = true;
                 return true;
             }
 
-            if (!type.IsPublic)
+            if (type.IsPublic)
             {
-                type.IsPublic = true;
-                return true;
+                return false;
             }
 
-            return false;
+            type.IsPublic = true;
+            return true;
         }
 
         protected override bool OnFieldDefinition(FieldDefinition field)
@@ -58,9 +58,8 @@ protected override bool OnFieldDefinition(FieldDefinition field)
 
         protected override bool OnPropertyDefinition(PropertyDefinition property)
         {
-            bool get = property.GetMethod != null ? OnMethodDefinition(property.GetMethod) : false;
-            bool set = property.SetMethod != null ? OnMethodDefinition(property.SetMethod) : false;
-
+            bool get = property.GetMethod != null && OnMethodDefinition(property.GetMethod);
+            bool set = property.SetMethod != null && OnMethodDefinition(property.SetMethod);
             return get || set;
         }
 
@@ -78,26 +77,27 @@ protected override bool OnMethodDefinition(MethodDefinition method)
         protected override void OnPatchFinished(PatchContext context)
         {
             string writePath = EnvironmentHelper.GetVariable("PublicizerOutput");
+            if (string.IsNullOrEmpty(writePath))
+            {
+                return;
+            }
 
-            if (!string.IsNullOrEmpty(writePath))
+            string name = context.Assembly.Name.Name;
+            if (!Directory.Exists(writePath))
             {
-                string name = context.Assembly.Name.Name;
-                if (!Directory.Exists(writePath))
-                {
-                    Log($"Failed to write {name} because PublicizeOutput {writePath} doesn't exist", Core.Logging.LogType.Error);
-                    return;
-                }
+                Log($"Failed to write {name} because PublicizeOutput {writePath} doesn't exist", Core.Logging.LogType.Error);
+                return;
+            }
 
-                try
-                {
-                    name = Path.Combine(writePath, name + ".dll");
-                    context.Assembly.Write(name);
-                    Log($"Wrote publicized assembly to {writePath}");
-                }
-                catch (Exception e)
-                {
-                    Log($"Failed to write publicized assembly to {writePath}", Core.Logging.LogType.Error, e);
-                }
+            try
+            {
+                name = Path.Combine(writePath, $"{name}.dll");
+                context.Assembly.Write(name);
+                Log($"Wrote publicized assembly to {writePath}");
+            }
+            catch (Exception exception)
+            {
+                Log($"Failed to write publicized assembly to {writePath}", Core.Logging.LogType.Error, exception);
             }
         }
     }