Add explicit support for trimming and AOT (#7)

Tyrrrz · May 22, 2024 · 911bec8 · 911bec8
1 parent 86c0848
commit 911bec8
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 69 deletions.
diff --git a/Cogwheel.Tests/Cogwheel.Tests.csproj b/Cogwheel.Tests/Cogwheel.Tests.csproj
@@ -9,13 +9,13 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="coverlet.collector" Version="6.0.0" PrivateAssets="all" />
-    <PackageReference Include="CSharpier.MsBuild" Version="0.26.5" PrivateAssets="all" />
+    <PackageReference Include="coverlet.collector" Version="6.0.2" PrivateAssets="all" />
+    <PackageReference Include="CSharpier.MsBuild" Version="0.28.2" PrivateAssets="all" />
     <PackageReference Include="FluentAssertions" Version="6.12.0" />
     <PackageReference Include="GitHubActionsTestLogger" Version="2.3.3" PrivateAssets="all" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
-    <PackageReference Include="xunit" Version="2.6.3" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.5" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
+    <PackageReference Include="xunit" Version="2.8.0" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.0" PrivateAssets="all" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/Cogwheel.Tests/Fakes/FakeSettingsWithSourceGeneration.cs b/Cogwheel.Tests/Fakes/FakeSettingsWithSourceGeneration.cs
@@ -1,13 +1,9 @@
-using System.Text.Json;
-using System.Text.Json.Serialization;
+using System.Text.Json.Serialization;
 
 namespace Cogwheel.Tests.Fakes;
 
 internal partial class FakeSettingsWithSourceGeneration(string filePath)
-    : SettingsBase(
-        filePath,
-        new JsonSerializerOptions { TypeInfoResolver = SerializerContext.Default }
-    )
+    : SettingsBase(filePath, SerializerContext.Default)
 {
     public int IntProperty { get; set; }
 

diff --git a/Cogwheel/Cogwheel.csproj b/Cogwheel/Cogwheel.csproj
@@ -3,6 +3,8 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;net8.0</TargetFrameworks>
     <IsPackable>true</IsPackable>
+    <IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable>
+    <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible>
   </PropertyGroup>
 
   <PropertyGroup>
@@ -21,10 +23,10 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="CSharpier.MsBuild" Version="0.26.5" PrivateAssets="all" />
+    <PackageReference Include="CSharpier.MsBuild" Version="0.28.2" PrivateAssets="all" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="all" />
-    <PackageReference Include="PolyShim" Version="1.8.0" PrivateAssets="all" />
-    <PackageReference Include="System.Text.Json" Version="8.0.0" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
+    <PackageReference Include="PolyShim" Version="1.11.0" PrivateAssets="all" />
+    <PackageReference Include="System.Text.Json" Version="8.0.3" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
   </ItemGroup>
 
 </Project>
diff --git a/Cogwheel/SettingsBase.cs b/Cogwheel/SettingsBase.cs
@@ -1,10 +1,10 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
-using System.Reflection;
 using System.Text.Json;
-using System.Text.Json.Serialization;
+using System.Text.Json.Serialization.Metadata;
 
 namespace Cogwheel;
 
@@ -14,44 +14,60 @@ namespace Cogwheel;
 public abstract class SettingsBase
 {
     private readonly string _filePath;
-    private readonly JsonSerializerOptions _jsonOptions;
 
-    private readonly IReadOnlyList<PropertyInfo> _properties;
-    private readonly IReadOnlyDictionary<PropertyInfo, object?> _defaults;
+    private readonly JsonTypeInfo _rootTypeInfo;
+    private readonly IReadOnlyDictionary<JsonPropertyInfo, object?> _rootPropertyDefaults;
 
     /// <summary>
     /// Initializes an instance of <see cref="SettingsBase" />.
     /// </summary>
+    /// <remarks>
+    /// If you are relying on compile-time serialization, the <paramref name="jsonOptions" /> instance
+    /// must have a valid <see cref="JsonSerializerOptions.TypeInfoResolver"/> set.
+    /// </remarks>
     protected SettingsBase(string filePath, JsonSerializerOptions jsonOptions)
     {
         _filePath = filePath;
-        _jsonOptions = jsonOptions;
-
-        _properties = GetType()
-            .GetProperties(BindingFlags.Public | BindingFlags.Instance)
-            .Where(p => p.DeclaringType != typeof(SettingsBase))
-            .Where(p => p is { CanRead: true, CanWrite: true })
-            .Where(p => p.GetCustomAttribute<JsonIgnoreAttribute>() is null)
-            .ToArray();
-
-        // Default values for properties are initialized before the constructor is called,
-        // so we can safely retrieve them here.
-        _defaults = _properties.ToDictionary(p => p, p => p.GetValue(this));
+
+        _rootTypeInfo = jsonOptions.GetTypeInfo(GetType());
+        _rootPropertyDefaults = _rootTypeInfo.Properties.ToDictionary(
+            p => p,
+            p => p.Get?.Invoke(this)
+        );
     }
 
     /// <summary>
     /// Initializes an instance of <see cref="SettingsBase" />.
     /// </summary>
+    protected SettingsBase(string filePath, IJsonTypeInfoResolver jsonTypeInfoResolver)
+        : this(
+            filePath,
+            new JsonSerializerOptions
+            {
+                WriteIndented = true,
+                TypeInfoResolver = jsonTypeInfoResolver
+            }
+        ) { }
+
+    /// <summary>
+    /// Initializes an instance of <see cref="SettingsBase" />.
+    /// </summary>
+    [RequiresUnreferencedCode(
+        "This constructor initializes the settings manager with reflection-based serialization, which is incompatible with assembly trimming."
+    )]
+    [RequiresDynamicCode(
+        "This constructor initializes the settings manager with reflection-based serialization, which is incompatible with ahead-of-time compilation."
+    )]
     protected SettingsBase(string filePath)
-        : this(filePath, new JsonSerializerOptions { WriteIndented = true }) { }
+        : this(filePath, new DefaultJsonTypeInfoResolver()) { }
 
     /// <summary>
     /// Resets the settings to their default values.
     /// </summary>
     public virtual void Reset()
     {
-        foreach (var property in _properties)
-            property.SetValue(this, _defaults[property]);
+        foreach (var property in _rootTypeInfo.Properties)
+            property.Set?.Invoke(this, _rootPropertyDefaults[property]);
     }
 
     /// <summary>
@@ -60,7 +76,7 @@ public virtual void Reset()
     public virtual void Save()
     {
         // Write to memory first, so that we don't end up producing a corrupted file in case of an error
-        var data = JsonSerializer.SerializeToUtf8Bytes(this, GetType(), _jsonOptions);
+        var data = JsonSerializer.SerializeToUtf8Bytes(this, _rootTypeInfo);
 
         var dirPath = Path.GetDirectoryName(_filePath);
         if (!string.IsNullOrWhiteSpace(dirPath))
@@ -87,40 +103,30 @@ public virtual bool Load()
                 }
             );
 
-            // This mess is required because System.Text.Json cannot populate an existing object
+            // This mess is required because System.Text.Json cannot populate an existing object.
+            // We also can't deserialize into a new object and then copy the properties over,
+            // because the target type may not have a parameterless or otherwise accessible constructor.
+            // https://github.com/dotnet/runtime/issues/92877
             foreach (var jsonProperty in document.RootElement.EnumerateObject())
             {
-                var property = _properties.FirstOrDefault(
-                    p =>
-                        string.Equals(
-                            // Use custom name if set
-                            p.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name ?? p.Name,
-                            jsonProperty.Name,
-                            StringComparison.Ordinal
-                        )
+                var property = _rootTypeInfo.Properties.FirstOrDefault(p =>
+                    string.Equals(p.Name, jsonProperty.Name, StringComparison.Ordinal)
                 );
 
                 if (property is null)
                     continue;
 
-                var jsonOptions = new JsonSerializerOptions(_jsonOptions);
-
-                // Use custom converter if set
-                if (
-                    property.GetCustomAttribute<JsonConverterAttribute>()?.ConverterType
-                        is { } converterType
-                    && Activator.CreateInstance(converterType) is JsonConverter converter
-                )
-                {
-                    jsonOptions.Converters.Add(converter);
-                }
+                // HACK: Use custom converter specified on the property.
+                // This will also apply the converter to any other nested properties of the same type,
+                // but unfortunately there's no way to avoid that for now.
+                var propertyOptions = new JsonSerializerOptions(property.Options);
+                if (property.CustomConverter is not null)
+                    propertyOptions.Converters.Add(property.CustomConverter);
 
-                property.SetValue(
+                property.Set?.Invoke(
                     this,
-                    JsonSerializer.Deserialize(
-                        jsonProperty.Value.GetRawText(),
-                        property.PropertyType,
-                        jsonOptions
+                    jsonProperty.Value.Deserialize(
+                        propertyOptions.GetTypeInfo(property.PropertyType)
                     )
                 );
             }

diff --git a/Readme.md b/Readme.md
@@ -44,15 +44,11 @@ To define your own application settings, create a class that inherits from `Sett
 ```csharp
 using Cogwheel;
 
-public class MySettings : SettingsBase
+public class MySettings : SettingsBase("path/to/settings.json")
 {
     public string StringSetting { get; set; } = "foo";
 
     public int IntSetting { get; set; } = 42;
-
-    public MySettings() : base("path/to/settings.json")
-    {
-    }
 }
 ```
 
@@ -90,16 +86,30 @@ You can use various attributes defined in that namespace to customize the serial
 using Cogwheel;
 using System.Text.Json.Serialization;
 
-public class MySettings : SettingsBase
+public class MySettings : SettingsBase("path/to/settings.json")
 {
     [JsonPropertyName("string_setting")]
     public string StringSetting { get; set; } = "foo";
 
     [JsonIgnore]
     public int IntSetting { get; set; } = 42;
+}
+```
 
-    public MySettings() : base("path/to/settings.json")
-    {
-    }
+If you want to use compile-time serialization as opposed to reflection-based, you need to provide a valid `IJsonTypeInfoResolver` instance, either directly or as part of `JsonSerializerOptions`:
+
+```csharp
+using Cogwheel;
+
+public class MySettings : SettingsBase(
+    "path/to/settings.json",
+    MyJsonSerializationContext.Default
+    // Or:
+    // new JsonSerializationOptions { TypeInfoResolver = MyJsonSerializationContext.Default }
+)
+{
+    public string StringSetting { get; set; } = "foo";
+
+    public int IntSetting { get; set; } = 42;
 }
 ```