From 7fc40ff4c78e1139674f7c3f37a2969522a4c5a6 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 23 Mar 2023 19:47:50 +0000 Subject: [PATCH 01/14] Implement the STJ.DisableDefaultReflection feature switch. --- .../src/ILLink/ILLink.Substitutions.xml | 7 ++ .../src/System.Text.Json.csproj | 4 + .../Text/Json/AppContextSwitchHelper.cs | 8 +- .../Serialization/JsonSerializer.Helpers.cs | 4 +- .../JsonSerializerOptions.Converters.cs | 2 +- .../Serialization/JsonSerializerOptions.cs | 76 +++++++------ .../Serialization/OptionsTests.cs | 102 ++++++++++++++++++ 7 files changed, 165 insertions(+), 38 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml diff --git a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml new file mode 100644 index 0000000000000..151305cdeb9a7 --- /dev/null +++ b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 03734cf251116..261abe0130e79 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -18,6 +18,10 @@ The System.Text.Json library is built-in as part of the shared framework in .NET $(NoWarn);nullable + + + + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs index 9c028f0216517..d45d61a0ed36b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs @@ -5,9 +5,13 @@ namespace System.Text.Json { internal static class AppContextSwitchHelper { - public static bool IsSourceGenReflectionFallbackEnabled => s_isSourceGenReflectionFallbackEnabled; + public static bool IsDefaultReflectionDisabled { get; } = + AppContext.TryGetSwitch( + switchName: "System.Text.Json.Serialization.DisableDefaultReflection", + isEnabled: out bool value) + ? value : false; - private static readonly bool s_isSourceGenReflectionFallbackEnabled = + public static bool IsSourceGenReflectionFallbackEnabled { get; } = AppContext.TryGetSwitch( switchName: "System.Text.Json.Serialization.EnableSourceGenReflectionFallback", isEnabled: out bool value) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index b2815ce96c18e..a3ccc872a9091 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -21,9 +21,9 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type inp options ??= JsonSerializerOptions.Default; - if (!options.IsInitializedForReflectionSerializer) + if (!options.IsConfiguredForJsonSerializer) { - options.InitializeForReflectionSerializer(); + options.ConfigureForJsonSerializer(); } // In order to improve performance of polymorphic root-level object serialization, diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index fe2f658579673..a2a707d5136d5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -45,7 +45,7 @@ public JsonConverter GetConverter(Type typeToConvert) ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert)); } - if (_typeInfoResolver is null) + if (!AppContextSwitchHelper.IsDefaultReflectionDisabled && _typeInfoResolver is null) { // Backward compatibility -- root & query the default reflection converters // but do not populate the TypeInfoResolver setting. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 801436338bea9..4365e632cfc51 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -130,19 +130,6 @@ public JsonSerializerOptions(JsonSerializerOptions options) TrackOptionsInstance(this); } - /// Tracks the options instance to enable all instances to be enumerated. - private static void TrackOptionsInstance(JsonSerializerOptions options) => TrackedOptionsInstances.All.Add(options, null); - - internal static class TrackedOptionsInstances - { - /// Tracks all live JsonSerializerOptions instances. - /// Instances are added to the table in their constructor. - public static ConditionalWeakTable All { get; } = - // TODO https://github.com/dotnet/runtime/issues/51159: - // Look into linking this away / disabling it when hot reload isn't in use. - new ConditionalWeakTable(); - } - /// /// Constructs a new instance with a predefined set of options determined by the specified . /// @@ -161,6 +148,19 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this() } } + /// Tracks the options instance to enable all instances to be enumerated. + private static void TrackOptionsInstance(JsonSerializerOptions options) => TrackedOptionsInstances.All.Add(options, null); + + internal static class TrackedOptionsInstances + { + /// Tracks all live JsonSerializerOptions instances. + /// Instances are added to the table in their constructor. + public static ConditionalWeakTable All { get; } = + // TODO https://github.com/dotnet/runtime/issues/51159: + // Look into linking this away / disabling it when hot reload isn't in use. + new ConditionalWeakTable(); + } + /// /// Binds current instance with a new instance of the specified type. /// @@ -699,35 +699,38 @@ public void MakeReadOnly() } /// - /// Initializes the converters for the reflection-based serializer. + /// Configures the instance for use by the JsonSerializer APIs. /// [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] - internal void InitializeForReflectionSerializer() + internal void ConfigureForJsonSerializer() { - // Even if a resolver has already been specified, we need to root - // the default resolver to gain access to the default converters. - DefaultJsonTypeInfoResolver defaultResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance(); - - switch (_typeInfoResolver) + if (!AppContextSwitchHelper.IsDefaultReflectionDisabled) { - case null: - // Use the default reflection-based resolver if no resolver has been specified. - _typeInfoResolver = defaultResolver; - break; + // Even if a resolver has already been specified, we need to root + // the default resolver to gain access to the default converters. + DefaultJsonTypeInfoResolver defaultResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance(); - case JsonSerializerContext ctx when AppContextSwitchHelper.IsSourceGenReflectionFallbackEnabled: - // .NET 6 compatibility mode: enable fallback to reflection metadata for JsonSerializerContext - _effectiveJsonTypeInfoResolver = JsonTypeInfoResolver.Combine(ctx, defaultResolver); - break; + switch (_typeInfoResolver) + { + case null: + // Use the default reflection-based resolver if no resolver has been specified. + _typeInfoResolver = defaultResolver; + break; + + case JsonSerializerContext ctx when AppContextSwitchHelper.IsSourceGenReflectionFallbackEnabled: + // .NET 6 compatibility mode: enable fallback to reflection metadata for JsonSerializerContext + _effectiveJsonTypeInfoResolver = JsonTypeInfoResolver.Combine(ctx, defaultResolver); + break; + } } MakeReadOnly(); - _isInitializedForReflectionSerializer = true; + _isConfiguredForJsonSerializer = true; } - internal bool IsInitializedForReflectionSerializer => _isInitializedForReflectionSerializer; - private volatile bool _isInitializedForReflectionSerializer; + internal bool IsConfiguredForJsonSerializer => _isConfiguredForJsonSerializer; + private volatile bool _isConfiguredForJsonSerializer; // Only populated in .NET 6 compatibility mode encoding reflection fallback in source gen private IJsonTypeInfoResolver? _effectiveJsonTypeInfoResolver; @@ -852,8 +855,15 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance() { var options = new JsonSerializerOptions { - TypeInfoResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance(), - _isReadOnly = true + // Because're marking the default instance as read-only, + // we need to specify a resolver instance for the case where + // reflection is disabled by default: use one that returns null for all types. + + TypeInfoResolver = AppContextSwitchHelper.IsDefaultReflectionDisabled + ? JsonTypeInfoResolver.Combine() + : DefaultJsonTypeInfoResolver.RootDefaultInstance(), + + _isReadOnly = true, }; return Interlocked.CompareExchange(ref s_defaultOptions, options, null) ?? options; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 410ef93459438..0e1048c217be1 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -482,6 +482,83 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() } [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_DisableDefaultReflectionSwitch_DefaultOptionsDoesNotSupportReflection() + { + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Text.Json.Serialization.DisableDefaultReflection"] = true + } + }; + + RemoteExecutor.Invoke(static () => + { + var options = JsonSerializerOptions.Default; + Assert.True(options.IsReadOnly); + + Assert.NotNull(options.TypeInfoResolver); + Assert.True(options.TypeInfoResolver is not DefaultJsonTypeInfoResolver); + IList resolverList = Assert.IsAssignableFrom>(options.TypeInfoResolver); + + Assert.Empty(resolverList); + Assert.Empty(options.TypeInfoResolverChain); + + Assert.Throws(() => options.GetTypeInfo(typeof(string))); + Assert.Throws(() => options.GetConverter(typeof(string))); + + Assert.Throws(() => JsonSerializer.Serialize("string")); + Assert.Throws(() => JsonSerializer.Serialize("string", options)); + + Assert.Throws(() => JsonSerializer.Deserialize("\"string\"")); + Assert.Throws(() => JsonSerializer.Deserialize("\"string\"", options)); + + }, options).Dispose(); + } + + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSupportReflection() + { + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Text.Json.Serialization.DisableDefaultReflection"] = true + } + }; + + RemoteExecutor.Invoke(static () => + { + var options = new JsonSerializerOptions(); + Assert.False(options.IsReadOnly); + + Assert.Null(options.TypeInfoResolver); + Assert.Empty(options.TypeInfoResolverChain); + + Assert.Throws(() => options.GetTypeInfo(typeof(string))); + Assert.Throws(() => options.GetConverter(typeof(string))); + + Assert.Throws(() => JsonSerializer.Serialize("string", options)); + Assert.Throws(() => JsonSerializer.Deserialize("\"string\"", options)); + + Assert.False(options.IsReadOnly); // failed operations should not lock the instance + + // Can still use reflection via explicit configuration + options.TypeInfoResolver = new DefaultJsonTypeInfoResolver(); + Assert.Equal(new[] { options.TypeInfoResolver }, options.TypeInfoResolverChain); + + Assert.NotNull(options.GetTypeInfo(typeof(string))); + Assert.NotNull(options.GetConverter(typeof(string))); + + string json = JsonSerializer.Serialize("string", options); + string value = JsonSerializer.Deserialize(json, options); + Assert.Equal("string", value); + + }, options).Dispose(); + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(false)] [InlineData(true)] @@ -550,6 +627,31 @@ public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBa }, options).Dispose(); } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisableDefaultReflection() + { + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Text.Json.Serialization.DisableDefaultReflection"] = true, + ["System.Text.Json.Serialization.EnableSourceGenReflectionFallback"] = true + } + }; + + RemoteExecutor.Invoke(static () => + { + JsonContext context = JsonContext.Default; + var unsupportedValue = new MyClass(); + + Assert.Null(context.GetTypeInfo(typeof(MyClass))); + Assert.Throws(() => context.Options.GetConverter(typeof(MyClass))); + Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, context.Options)); + + }, options).Dispose(); + } + [Fact] public static void Options_JsonSerializerContext_Combine_FallbackToReflection() { From 6d2af10d31dd57921fb2de11f0908f3efb4c0633 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 23 Mar 2023 23:24:19 +0000 Subject: [PATCH 02/14] Reinstate accidentally stripped attribute --- .../tests/System.Text.Json.Tests/Serialization/OptionsTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 0e1048c217be1..c31265efccfbe 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -559,6 +559,7 @@ public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSuppo }, options).Dispose(); } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(false)] [InlineData(true)] From ae5af278f47577950d83cb2674a5ef1b09649db0 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 27 Mar 2023 15:33:34 +0100 Subject: [PATCH 03/14] Address feedback. --- .../src/ILLink/ILLink.Substitutions.xml | 6 +++- .../Text/Json/AppContextSwitchHelper.cs | 6 ++-- .../JsonSerializerOptions.Converters.cs | 2 +- .../Serialization/JsonSerializerOptions.cs | 10 +++--- .../Serialization/OptionsTests.cs | 36 +++++++++++++++---- 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml index 151305cdeb9a7..f48fc8eff9132 100644 --- a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml +++ b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml @@ -1,7 +1,11 @@ - + + + \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs index d45d61a0ed36b..6099468cb6df2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs @@ -5,11 +5,11 @@ namespace System.Text.Json { internal static class AppContextSwitchHelper { - public static bool IsDefaultReflectionDisabled { get; } = + public static bool UseReflectionDefault { get; } = AppContext.TryGetSwitch( - switchName: "System.Text.Json.Serialization.DisableDefaultReflection", + switchName: "System.Text.Json.Serialization.UseReflectionDefault", isEnabled: out bool value) - ? value : false; + ? value : true; public static bool IsSourceGenReflectionFallbackEnabled { get; } = AppContext.TryGetSwitch( diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index a2a707d5136d5..f7ddbb39a5432 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -45,7 +45,7 @@ public JsonConverter GetConverter(Type typeToConvert) ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert)); } - if (!AppContextSwitchHelper.IsDefaultReflectionDisabled && _typeInfoResolver is null) + if (AppContextSwitchHelper.UseReflectionDefault && _typeInfoResolver is null) { // Backward compatibility -- root & query the default reflection converters // but do not populate the TypeInfoResolver setting. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 4365e632cfc51..992f90c3ea122 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -705,7 +705,7 @@ public void MakeReadOnly() [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] internal void ConfigureForJsonSerializer() { - if (!AppContextSwitchHelper.IsDefaultReflectionDisabled) + if (AppContextSwitchHelper.UseReflectionDefault) { // Even if a resolver has already been specified, we need to root // the default resolver to gain access to the default converters. @@ -855,13 +855,13 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance() { var options = new JsonSerializerOptions { - // Because're marking the default instance as read-only, + // Because we're marking the default instance as read-only, // we need to specify a resolver instance for the case where // reflection is disabled by default: use one that returns null for all types. - TypeInfoResolver = AppContextSwitchHelper.IsDefaultReflectionDisabled - ? JsonTypeInfoResolver.Combine() - : DefaultJsonTypeInfoResolver.RootDefaultInstance(), + TypeInfoResolver = AppContextSwitchHelper.UseReflectionDefault + ? DefaultJsonTypeInfoResolver.RootDefaultInstance() + : new JsonTypeInfoResolverChain(), _isReadOnly = true, }; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index c31265efccfbe..3712a230221ce 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -483,13 +483,13 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_DisableDefaultReflectionSwitch_DefaultOptionsDoesNotSupportReflection() + public static void Options_DisablingUseReflectionDefaultSwitch_DefaultOptionsDoesNotSupportReflection() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.DisableDefaultReflection"] = true + ["System.Text.Json.Serialization.UseReflectionDefault"] = false } }; @@ -519,13 +519,13 @@ public static void Options_DisableDefaultReflectionSwitch_DefaultOptionsDoesNotS [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSupportReflection() + public static void Options_DisablingUseReflectionDefaultSwitch_NewOptionsDoesNotSupportReflection() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.DisableDefaultReflection"] = true + ["System.Text.Json.Serialization.UseReflectionDefault"] = false } }; @@ -559,6 +559,30 @@ public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSuppo }, options).Dispose(); } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_DisablingUseReflectionDefaultSwitch_CanUseSourceGen() + { + var options = new RemoteInvokeOptions + { + RuntimeConfigurationOptions = + { + ["System.Text.Json.Serialization.UseReflectionDefault"] = false + } + }; + + RemoteExecutor.Invoke(static () => + { + var options = new JsonSerializerOptions(); + options.TypeInfoResolverChain.Add(JsonContext.Default); + + string json = JsonSerializer.Serialize(new WeatherForecastWithPOCOs(), options); + WeatherForecastWithPOCOs result = JsonSerializer.Deserialize(json, options); + Assert.NotNull(result); + + }, options).Dispose(); + } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(false)] @@ -630,13 +654,13 @@ public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBa [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisableDefaultReflection() + public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisablingUseReflectionDefault() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.DisableDefaultReflection"] = true, + ["System.Text.Json.Serialization.UseReflectionDefault"] = false, ["System.Text.Json.Serialization.EnableSourceGenReflectionFallback"] = true } }; From 6095616bc7f821ddd221251e718258947620f2ee Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 27 Mar 2023 16:57:58 +0100 Subject: [PATCH 04/14] Address feedback. --- .../System.Text.Json/src/ILLink/ILLink.Substitutions.xml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml index f48fc8eff9132..71a7c627e8ab8 100644 --- a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml +++ b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml @@ -1,11 +1,8 @@ - - - + \ No newline at end of file From 67535b3048c1258d06f93e95e927806ec31cf796 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 28 Mar 2023 13:22:51 +0100 Subject: [PATCH 05/14] Add a trimming test for STJ --- .../System.Text.Json/System.Text.Json.sln | 7 +++ .../System.Text.Json.TrimmingTests.csproj | 13 +++++ .../UseReflectionDefaultFalse.cs | 48 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj create mode 100644 src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs diff --git a/src/libraries/System.Text.Json/System.Text.Json.sln b/src/libraries/System.Text.Json/System.Text.Json.sln index 46935b79af4c2..90f3e24b11e0a 100644 --- a/src/libraries/System.Text.Json/System.Text.Json.sln +++ b/src/libraries/System.Text.Json/System.Text.Json.sln @@ -67,6 +67,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{E49881A9-09F EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "gen", "gen", "{F254F143-4704-4432-9995-20D87FA8BF8D}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Text.Json.TrimmingTests", "tests\System.Text.Json.TrimmingTests\System.Text.Json.TrimmingTests.csproj", "{779C14CD-7580-4117-8615-5A8253C3FE00}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -193,6 +195,10 @@ Global {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Debug|Any CPU.Build.0 = Debug|Any CPU {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Release|Any CPU.ActiveCfg = Release|Any CPU {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Release|Any CPU.Build.0 = Release|Any CPU + {779C14CD-7580-4117-8615-5A8253C3FE00}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {779C14CD-7580-4117-8615-5A8253C3FE00}.Debug|Any CPU.Build.0 = Debug|Any CPU + {779C14CD-7580-4117-8615-5A8253C3FE00}.Release|Any CPU.ActiveCfg = Release|Any CPU + {779C14CD-7580-4117-8615-5A8253C3FE00}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -228,6 +234,7 @@ Global {256A4653-4287-44B3-BDEF-67FC1522ED2F} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} {F6A18EB5-A8CC-4A39-9E85-5FA226019C3D} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} + {779C14CD-7580-4117-8615-5A8253C3FE00} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {5868B757-D821-41FC-952E-2113A0519506} diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj new file mode 100644 index 0000000000000..324d2992eb9f9 --- /dev/null +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj @@ -0,0 +1,13 @@ + + + + $(NetCoreAppCurrent) + + + + + System.Text.Json.Serialization.UseReflectionDefault + + + + diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs new file mode 100644 index 0000000000000..979ec325bdc7a --- /dev/null +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs @@ -0,0 +1,48 @@ +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; + +public static class Program +{ + public static int Main() + { + string valueToSerialize = "stringValue"; + + if (JsonSerializerOptions.Default.TypeInfoResolver is not IList { Count: 0 }) + { + return -1; + } + + try + { + JsonSerializer.Serialize(valueToSerialize); + return -2; + } + catch (NotSupportedException) + { + } + + var options = new JsonSerializerOptions(); + try + { + JsonSerializer.Serialize(valueToSerialize, options); + return -3; + } + catch (InvalidOperationException) + { + } + + Type reflectionResolver = GetJsonType("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver"); + if (reflectionResolver != null) + { + return -5; + } + + return 100; + } + + // The intention of this method is to ensure the trimmer doesn't preserve the Type. + private static Type GetJsonType(string name) => + typeof(JsonSerializer).Assembly.GetType(name, throwOnError: false); +} From c7dac06a8b69ec5c0407b6f38b240782969068de Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 29 Mar 2023 11:05:46 +0100 Subject: [PATCH 06/14] Move trimming test to existing trimming tests folder. --- src/libraries/System.Text.Json/System.Text.Json.sln | 7 ------- .../System.Text.Json.TrimmingTests.proj | 3 +++ .../TrimmingTests}/UseReflectionDefaultFalse.cs | 0 .../System.Text.Json.TrimmingTests.csproj | 13 ------------- 4 files changed, 3 insertions(+), 20 deletions(-) rename src/libraries/System.Text.Json/tests/{System.Text.Json.TrimmingTests => System.Text.Json.Tests/TrimmingTests}/UseReflectionDefaultFalse.cs (100%) delete mode 100644 src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj diff --git a/src/libraries/System.Text.Json/System.Text.Json.sln b/src/libraries/System.Text.Json/System.Text.Json.sln index 90f3e24b11e0a..46935b79af4c2 100644 --- a/src/libraries/System.Text.Json/System.Text.Json.sln +++ b/src/libraries/System.Text.Json/System.Text.Json.sln @@ -67,8 +67,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{E49881A9-09F EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "gen", "gen", "{F254F143-4704-4432-9995-20D87FA8BF8D}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Text.Json.TrimmingTests", "tests\System.Text.Json.TrimmingTests\System.Text.Json.TrimmingTests.csproj", "{779C14CD-7580-4117-8615-5A8253C3FE00}" -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -195,10 +193,6 @@ Global {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Debug|Any CPU.Build.0 = Debug|Any CPU {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Release|Any CPU.ActiveCfg = Release|Any CPU {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC}.Release|Any CPU.Build.0 = Release|Any CPU - {779C14CD-7580-4117-8615-5A8253C3FE00}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {779C14CD-7580-4117-8615-5A8253C3FE00}.Debug|Any CPU.Build.0 = Debug|Any CPU - {779C14CD-7580-4117-8615-5A8253C3FE00}.Release|Any CPU.ActiveCfg = Release|Any CPU - {779C14CD-7580-4117-8615-5A8253C3FE00}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -234,7 +228,6 @@ Global {256A4653-4287-44B3-BDEF-67FC1522ED2F} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} {F6A18EB5-A8CC-4A39-9E85-5FA226019C3D} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} {A0178BAA-A1AF-4C69-8E4A-A700A2723DDC} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} - {779C14CD-7580-4117-8615-5A8253C3FE00} = {E07C6980-EB71-4D19-A80A-7BEB80B635B1} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {5868B757-D821-41FC-952E-2113A0519506} diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj index 2f4a4e4a33687..d6d8ac87b6f27 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj @@ -60,6 +60,9 @@ + + System.Text.Json.Serialization.UseReflectionDefault + diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs similarity index 100% rename from src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs rename to src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj deleted file mode 100644 index 324d2992eb9f9..0000000000000 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj +++ /dev/null @@ -1,13 +0,0 @@ - - - - $(NetCoreAppCurrent) - - - - - System.Text.Json.Serialization.UseReflectionDefault - - - - From dbf6d3611e9e3de9e66377e2fd8f6903e7430565 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 29 Mar 2023 20:42:46 +0100 Subject: [PATCH 07/14] Add source gen serialization test case to Trimming test. --- .../Serialization/JsonSerializerContext.cs | 4 +- .../Serialization/JsonSerializerOptions.cs | 27 +---- .../DefaultJsonTypeInfoResolver.Helpers.cs | 8 +- .../Metadata/DefaultJsonTypeInfoResolver.cs | 7 +- .../Serialization/Metadata/JsonTypeInfo.cs | 7 +- .../Metadata/JsonTypeInfoResolver.cs | 20 ++++ .../Metadata/JsonTypeInfoResolverChain.cs | 15 ++- .../UseReflectionDefaultFalse.cs | 106 +++++++++++++++++- 8 files changed, 150 insertions(+), 44 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index 582417737f6db..07532b8c50ace 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -9,7 +9,7 @@ namespace System.Text.Json.Serialization /// /// Provides metadata about a set of types that is relevant to JSON serialization. /// - public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver + public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver, IBuiltInJsonTypeInfoResolver { private JsonSerializerOptions? _options; @@ -49,7 +49,7 @@ internal void AssociateWithOptions(JsonSerializerOptions options) /// Indicates whether pre-generated serialization logic for types in the context /// is compatible with the run time specified . /// - internal bool IsCompatibleWithGeneratedOptions(JsonSerializerOptions options) + bool IBuiltInJsonTypeInfoResolver.IsCompatibleWithOptions(JsonSerializerOptions options) { Debug.Assert(options != null); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 992f90c3ea122..50f65883b5226 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -638,32 +638,7 @@ internal bool CanUseFastPathSerializationLogic { Debug.Assert(IsReadOnly); Debug.Assert(TypeInfoResolver != null); - return _canUseFastPathSerializationLogic ??= CanUseFastPath(TypeInfoResolver); - - bool CanUseFastPath(IJsonTypeInfoResolver resolver) - { - switch (resolver) - { - case DefaultJsonTypeInfoResolver defaultResolver: - return defaultResolver.GetType() == typeof(DefaultJsonTypeInfoResolver) && - defaultResolver.Modifiers.Count == 0; - case JsonSerializerContext ctx: - return ctx.IsCompatibleWithGeneratedOptions(this); - case JsonTypeInfoResolverChain resolverChain: - foreach (IJsonTypeInfoResolver component in resolverChain) - { - if (!CanUseFastPath(component)) - { - return false; - } - } - - return true; - - default: - return false; - } - } + return _canUseFastPathSerializationLogic ??= TypeInfoResolver.IsCompatibleWithOptions(this); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs index 2d746e0b9c3d1..8c014763f0473 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs @@ -399,13 +399,13 @@ internal static void DeterminePropertyAccessors(JsonPropertyInfo jsonPrope MethodInfo? getMethod = propertyInfo.GetMethod; if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) { - jsonPropertyInfo.Get = DefaultJsonTypeInfoResolver.MemberAccessor.CreatePropertyGetter(propertyInfo); + jsonPropertyInfo.Get = MemberAccessor.CreatePropertyGetter(propertyInfo); } MethodInfo? setMethod = propertyInfo.SetMethod; if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) { - jsonPropertyInfo.Set = DefaultJsonTypeInfoResolver.MemberAccessor.CreatePropertySetter(propertyInfo); + jsonPropertyInfo.Set = MemberAccessor.CreatePropertySetter(propertyInfo); } break; @@ -413,11 +413,11 @@ internal static void DeterminePropertyAccessors(JsonPropertyInfo jsonPrope case FieldInfo fieldInfo: Debug.Assert(fieldInfo.IsPublic); - jsonPropertyInfo.Get = DefaultJsonTypeInfoResolver.MemberAccessor.CreateFieldGetter(fieldInfo); + jsonPropertyInfo.Get = MemberAccessor.CreateFieldGetter(fieldInfo); if (!fieldInfo.IsInitOnly) { - jsonPropertyInfo.Set = DefaultJsonTypeInfoResolver.MemberAccessor.CreateFieldSetter(fieldInfo); + jsonPropertyInfo.Set = MemberAccessor.CreateFieldSetter(fieldInfo); } break; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs index 5d00959a79a9a..1a16f8de807fb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs @@ -13,7 +13,7 @@ namespace System.Text.Json.Serialization.Metadata /// /// The contract resolver used by . /// - public partial class DefaultJsonTypeInfoResolver : IJsonTypeInfoResolver + public partial class DefaultJsonTypeInfoResolver : IJsonTypeInfoResolver, IBuiltInJsonTypeInfoResolver { private bool _mutable; @@ -122,6 +122,11 @@ protected override void OnCollectionModifying() } } + bool IBuiltInJsonTypeInfoResolver.IsCompatibleWithOptions(JsonSerializerOptions _) + // Metadata generated by the default resolver is compatible by definition, + // provided that no user extensions have been made on the class. + => _modifiers is null or { Count: 0 } && GetType() == typeof(DefaultJsonTypeInfoResolver); + internal static bool IsDefaultInstanceRooted => s_defaultInstance is not null; private static DefaultJsonTypeInfoResolver? s_defaultInstance; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 8ab34bdb8875f..93c464be17737 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -695,12 +695,7 @@ bool IsCurrentNodeCompatible() return false; } - return OriginatingResolver switch - { - JsonSerializerContext ctx => ctx.IsCompatibleWithGeneratedOptions(Options), - DefaultJsonTypeInfoResolver => true, // generates default contracts by definition - _ => false - }; + return OriginatingResolver.IsCompatibleWithOptions(Options); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs index f7c6e125ce212..d1640c6a7d16a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs @@ -38,5 +38,25 @@ public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] reso return resolverChain.Count == 1 ? resolverChain[0] : resolverChain; } + + /// + /// Indicates whether the metadata generated by the current resolver + /// are compatible with the run time specified . + /// + internal static bool IsCompatibleWithOptions(this IJsonTypeInfoResolver? resolver, JsonSerializerOptions options) + => (resolver as IBuiltInJsonTypeInfoResolver)?.IsCompatibleWithOptions(options) == true; + } + + /// + /// Implemented by the built-in converters to avoid rooting + /// unused resolver dependencies in the context of the trimmer. + /// + internal interface IBuiltInJsonTypeInfoResolver + { + /// + /// Indicates whether the metadata generated by the current resolver + /// are compatible with the run time specified . + /// + bool IsCompatibleWithOptions(JsonSerializerOptions options); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolverChain.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolverChain.cs index 26f229210fbcd..e2a56646ff74c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolverChain.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolverChain.cs @@ -6,7 +6,7 @@ namespace System.Text.Json.Serialization.Metadata { [DebuggerDisplay("{DebuggerDisplay,nq}")] - internal class JsonTypeInfoResolverChain : ConfigurationList, IJsonTypeInfoResolver + internal class JsonTypeInfoResolverChain : ConfigurationList, IJsonTypeInfoResolver, IBuiltInJsonTypeInfoResolver { public JsonTypeInfoResolverChain() : base(null) { } public override bool IsReadOnly => true; @@ -44,6 +44,19 @@ internal void AddFlattened(IJsonTypeInfoResolver? resolver) } } + bool IBuiltInJsonTypeInfoResolver.IsCompatibleWithOptions(JsonSerializerOptions options) + { + foreach (IJsonTypeInfoResolver component in _list) + { + if (!component.IsCompatibleWithOptions(options)) + { + return false; + } + } + + return true; + } + internal string DebuggerDisplay { get diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs index 979ec325bdc7a..be2827a642ba2 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs @@ -1,19 +1,29 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; using System.Collections.Generic; using System.Text.Json; +using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; +#nullable enable + public static class Program { + // Validates that expected the components are trimmed when + // the UseReflectionDefault feature switch is turned on. public static int Main() { - string valueToSerialize = "stringValue"; + MyPoco valueToSerialize = new MyPoco { Value = 42 }; + // The default resolver should not surface DefaultJsonTypeInfoResolver. if (JsonSerializerOptions.Default.TypeInfoResolver is not IList { Count: 0 }) { return -1; } + // Serializing with options unset should throw NotSupportedException. try { JsonSerializer.Serialize(valueToSerialize); @@ -23,6 +33,7 @@ public static int Main() { } + // Serializing with default options unset should throw InvalidOperationException. var options = new JsonSerializerOptions(); try { @@ -33,7 +44,15 @@ public static int Main() { } - Type reflectionResolver = GetJsonType("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver"); + // Serializing with a custom resolver should work as expected. + options.TypeInfoResolver = new MyJsonResolver(); + if (JsonSerializer.Serialize(valueToSerialize, options) != "{\"Value\":42}") + { + return -4; + } + + // The Default resolver should have been trimmed from the application. + Type? reflectionResolver = GetJsonType("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver"); if (reflectionResolver != null) { return -5; @@ -43,6 +62,85 @@ public static int Main() } // The intention of this method is to ensure the trimmer doesn't preserve the Type. - private static Type GetJsonType(string name) => + private static Type? GetJsonType(string name) => typeof(JsonSerializer).Assembly.GetType(name, throwOnError: false); } + +public class MyPoco +{ + public int Value { get; set; } +} + +public class MyJsonResolver : JsonSerializerContext, IJsonTypeInfoResolver +{ + public MyJsonResolver() : base(null) { } + protected override JsonSerializerOptions? GeneratedSerializerOptions => null; + public override JsonTypeInfo? GetTypeInfo(Type type) => GetTypeInfo(type, Options); + + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) + { + if (type == typeof(int)) + { + return Create_Int32(options); + } + + if (type == typeof(MyPoco)) + { + return Create_MyPoco(options); + } + + return null; + } + + private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo Create_MyPoco(global::System.Text.Json.JsonSerializerOptions options) + { + global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? jsonTypeInfo = null; + global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues() + { + ObjectCreator = static () => new global::MyPoco(), + ObjectWithParameterizedConstructorCreator = null, + PropertyMetadataInitializer = _ => MyPocoPropInit(options), + ConstructorParameterMetadataInitializer = null, + NumberHandling = default, + }; + + jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo(options, objectInfo); + jsonTypeInfo.OriginatingResolver = this; + return jsonTypeInfo; + } + + private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] MyPocoPropInit(global::System.Text.Json.JsonSerializerOptions options) + { + global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[1]; + + global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues() + { + IsProperty = true, + IsPublic = true, + IsVirtual = false, + DeclaringType = typeof(global::MyPoco), + Converter = null, + Getter = static (obj) => ((global::MyPoco)obj).Value, + Setter = static (obj, value) => ((global::MyPoco)obj).Value = value!, + IgnoreCondition = null, + HasJsonInclude = false, + IsExtensionData = false, + NumberHandling = default, + PropertyName = "Value", + JsonPropertyName = null + }; + + global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo propertyInfo0 = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo(options, info0); + properties[0] = propertyInfo0; + + return properties; + } + + private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo Create_Int32(global::System.Text.Json.JsonSerializerOptions options) + { + global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? jsonTypeInfo = null; + jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo(options, global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.Int32Converter); + jsonTypeInfo.OriginatingResolver = this; + return jsonTypeInfo; + } +} From 145207cde57645e0d512345dfed87a4bb71e14ab Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 29 Mar 2023 20:46:00 +0100 Subject: [PATCH 08/14] Fix style. --- .../Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs index d1640c6a7d16a..d890b9be94205 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs @@ -44,7 +44,7 @@ public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] reso /// are compatible with the run time specified . /// internal static bool IsCompatibleWithOptions(this IJsonTypeInfoResolver? resolver, JsonSerializerOptions options) - => (resolver as IBuiltInJsonTypeInfoResolver)?.IsCompatibleWithOptions(options) == true; + => resolver is IBuiltInJsonTypeInfoResolver bir && bir.IsCompatibleWithOptions(options); } /// From 1d9f86b729af65c37168ac691aa843fb4647260a Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 3 Apr 2023 15:44:24 +0100 Subject: [PATCH 09/14] Expose the feature switch as a property on JsonSerializer -- rename feature switch to match namespace. --- .../System.Text.Json/ref/System.Text.Json.cs | 1 + .../src/ILLink/ILLink.Substitutions.xml | 6 ++-- .../Text/Json/AppContextSwitchHelper.cs | 6 ---- .../Serialization/JsonSerializer.Helpers.cs | 15 ++++++++++ .../JsonSerializerOptions.Converters.cs | 2 +- .../Serialization/JsonSerializerOptions.cs | 4 +-- .../Serialization/OptionsTests.cs | 30 ++++++++++++++----- .../System.Text.Json.Tests.csproj | 3 ++ ...s => IsReflectionEnabledByDefaultFalse.cs} | 2 +- .../System.Text.Json.TrimmingTests.proj | 4 +-- 10 files changed, 50 insertions(+), 23 deletions(-) rename src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/{UseReflectionDefaultFalse.cs => IsReflectionEnabledByDefaultFalse.cs} (98%) diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 12426171d3f1e..3002b59dcff3b 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -280,6 +280,7 @@ public static partial class JsonSerializer [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")] public static TValue? Deserialize(ref System.Text.Json.Utf8JsonReader reader, System.Text.Json.JsonSerializerOptions? options = null) { throw null; } public static TValue? Deserialize(ref System.Text.Json.Utf8JsonReader reader, System.Text.Json.Serialization.Metadata.JsonTypeInfo jsonTypeInfo) { throw null; } + public static bool IsReflectionEnabledByDefault { get { throw null; } } public static void Serialize(System.IO.Stream utf8Json, object? value, System.Text.Json.Serialization.Metadata.JsonTypeInfo jsonTypeInfo) { } [System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications.")] [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")] diff --git a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml index 71a7c627e8ab8..026bbf9f13602 100644 --- a/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml +++ b/src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml @@ -1,8 +1,8 @@ - - + + \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs index 6099468cb6df2..3e1291c676d92 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs @@ -5,12 +5,6 @@ namespace System.Text.Json { internal static class AppContextSwitchHelper { - public static bool UseReflectionDefault { get; } = - AppContext.TryGetSwitch( - switchName: "System.Text.Json.Serialization.UseReflectionDefault", - isEnabled: out bool value) - ? value : true; - public static bool IsSourceGenReflectionFallbackEnabled { get; } = AppContext.TryGetSwitch( switchName: "System.Text.Json.Serialization.EnableSourceGenReflectionFallback", diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index a3ccc872a9091..3b4bda03d8e8a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -13,6 +13,21 @@ public static partial class JsonSerializer internal const string SerializationUnreferencedCodeMessage = "JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved."; internal const string SerializationRequiresDynamicCodeMessage = "JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications."; + /// + /// Indicates whether unconfigured instances + /// should be set to use the reflection-based . + /// + /// + /// The value of the property is backed by the "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault" + /// feature switch and defaults to if unset. For trimmed applications, disabling the feature switch + /// at link time will result in the property being substituted with a constant value. + /// + public static bool IsReflectionEnabledByDefault { get; } = + AppContext.TryGetSwitch( + switchName: "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault", + isEnabled: out bool value) + ? value : true; + [RequiresUnreferencedCode(SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(SerializationRequiresDynamicCodeMessage)] private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type inputType, bool fallBackToNearestAncestorType = false) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index f7ddbb39a5432..b6c501903a602 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -45,7 +45,7 @@ public JsonConverter GetConverter(Type typeToConvert) ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert)); } - if (AppContextSwitchHelper.UseReflectionDefault && _typeInfoResolver is null) + if (JsonSerializer.IsReflectionEnabledByDefault && _typeInfoResolver is null) { // Backward compatibility -- root & query the default reflection converters // but do not populate the TypeInfoResolver setting. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 50f65883b5226..5359499e0cbea 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -680,7 +680,7 @@ public void MakeReadOnly() [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] internal void ConfigureForJsonSerializer() { - if (AppContextSwitchHelper.UseReflectionDefault) + if (JsonSerializer.IsReflectionEnabledByDefault) { // Even if a resolver has already been specified, we need to root // the default resolver to gain access to the default converters. @@ -834,7 +834,7 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance() // we need to specify a resolver instance for the case where // reflection is disabled by default: use one that returns null for all types. - TypeInfoResolver = AppContextSwitchHelper.UseReflectionDefault + TypeInfoResolver = JsonSerializer.IsReflectionEnabledByDefault ? DefaultJsonTypeInfoResolver.RootDefaultInstance() : new JsonTypeInfoResolverChain(), diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 3712a230221ce..d290fd8d259bb 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -481,20 +481,28 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, options)); } + [Fact] + public static void JsonSerializer_IsReflectionEnabledByDefault_DefaultsToTrue() + { + Assert.True(JsonSerializer.IsReflectionEnabledByDefault); + } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_DisablingUseReflectionDefaultSwitch_DefaultOptionsDoesNotSupportReflection() + public static void Options_DisablingIsReflectionEnabledByDefaultSwitch_DefaultOptionsDoesNotSupportReflection() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.UseReflectionDefault"] = false + ["System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault"] = false } }; RemoteExecutor.Invoke(static () => { + Assert.False(JsonSerializer.IsReflectionEnabledByDefault); + var options = JsonSerializerOptions.Default; Assert.True(options.IsReadOnly); @@ -519,18 +527,20 @@ public static void Options_DisablingUseReflectionDefaultSwitch_DefaultOptionsDoe [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_DisablingUseReflectionDefaultSwitch_NewOptionsDoesNotSupportReflection() + public static void Options_DisablingIsReflectionEnabledByDefaultSwitch_NewOptionsDoesNotSupportReflection() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.UseReflectionDefault"] = false + ["System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault"] = false } }; RemoteExecutor.Invoke(static () => { + Assert.False(JsonSerializer.IsReflectionEnabledByDefault); + var options = new JsonSerializerOptions(); Assert.False(options.IsReadOnly); @@ -561,18 +571,20 @@ public static void Options_DisablingUseReflectionDefaultSwitch_NewOptionsDoesNot [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_DisablingUseReflectionDefaultSwitch_CanUseSourceGen() + public static void Options_DisablingIsReflectionEnabledByDefaultSwitch_CanUseSourceGen() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.UseReflectionDefault"] = false + ["System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault"] = false } }; RemoteExecutor.Invoke(static () => { + Assert.False(JsonSerializer.IsReflectionEnabledByDefault); + var options = new JsonSerializerOptions(); options.TypeInfoResolverChain.Add(JsonContext.Default); @@ -654,19 +666,21 @@ public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBa [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisablingUseReflectionDefault() + public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisablingIsReflectionEnabledByDefault() { var options = new RemoteInvokeOptions { RuntimeConfigurationOptions = { - ["System.Text.Json.Serialization.UseReflectionDefault"] = false, + ["System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault"] = false, ["System.Text.Json.Serialization.EnableSourceGenReflectionFallback"] = true } }; RemoteExecutor.Invoke(static () => { + Assert.False(JsonSerializer.IsReflectionEnabledByDefault); + JsonContext context = JsonContext.Default; var unsupportedValue = new MyClass(); diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj index 565f6a8ab3daf..25d2d13344463 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj @@ -264,6 +264,9 @@ + + + diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs similarity index 98% rename from src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs rename to src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs index be2827a642ba2..8d4c97720cc6c 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/UseReflectionDefaultFalse.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs @@ -12,7 +12,7 @@ public static class Program { // Validates that expected the components are trimmed when - // the UseReflectionDefault feature switch is turned on. + // the IsReflectionEnabledByDefault feature switch is turned on. public static int Main() { MyPoco valueToSerialize = new MyPoco { Value = 42 }; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj index d6d8ac87b6f27..db59eac29e719 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/System.Text.Json.TrimmingTests.proj @@ -60,8 +60,8 @@ - - System.Text.Json.Serialization.UseReflectionDefault + + System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault From 7dedca9576ecfd37adba6aa027713e7379f35b09 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 5 Apr 2023 15:35:15 +0100 Subject: [PATCH 10/14] Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs Co-authored-by: Eric Erhardt --- .../System/Text/Json/Serialization/JsonSerializer.Helpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index 3b4bda03d8e8a..30243f07b33e5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -19,7 +19,7 @@ public static partial class JsonSerializer /// /// /// The value of the property is backed by the "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault" - /// feature switch and defaults to if unset. For trimmed applications, disabling the feature switch + /// setting and defaults to if unset. For trimmed applications, disabling the feature switch /// at link time will result in the property being substituted with a constant value. /// public static bool IsReflectionEnabledByDefault { get; } = From 4615116bb808c0a8441c935fa3f195a6d96bf1c6 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 5 Apr 2023 15:35:38 +0100 Subject: [PATCH 11/14] Update src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs Co-authored-by: Eric Erhardt --- .../TrimmingTests/IsReflectionEnabledByDefaultFalse.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs index 8d4c97720cc6c..e0db8f39b080d 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs @@ -12,7 +12,7 @@ public static class Program { // Validates that expected the components are trimmed when - // the IsReflectionEnabledByDefault feature switch is turned on. + // the IsReflectionEnabledByDefault feature switch is turned off. public static int Main() { MyPoco valueToSerialize = new MyPoco { Value = 42 }; From c9db1f3f89b391698650585a6caec83243826001 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 5 Apr 2023 15:39:48 +0100 Subject: [PATCH 12/14] Address feedback. --- .../tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj index 25d2d13344463..565f6a8ab3daf 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj @@ -264,9 +264,6 @@ - - - From e643ec20ac595d78b8b269acd503eb4610999b20 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 5 Apr 2023 16:00:48 +0100 Subject: [PATCH 13/14] Address feedback. --- .../System/Text/Json/Serialization/JsonSerializer.Helpers.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index 30243f07b33e5..3f202888853d5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -19,8 +19,7 @@ public static partial class JsonSerializer /// /// /// The value of the property is backed by the "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault" - /// setting and defaults to if unset. For trimmed applications, disabling the feature switch - /// at link time will result in the property being substituted with a constant value. + /// setting and defaults to if unset. /// public static bool IsReflectionEnabledByDefault { get; } = AppContext.TryGetSwitch( From af0131be505f5516310f846458854279d327d974 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 5 Apr 2023 17:29:50 +0100 Subject: [PATCH 14/14] Add entry to feature-switches.md --- docs/workflow/trimming/feature-switches.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/workflow/trimming/feature-switches.md b/docs/workflow/trimming/feature-switches.md index 7600b3d7d480b..0aa44298f3c39 100644 --- a/docs/workflow/trimming/feature-switches.md +++ b/docs/workflow/trimming/feature-switches.md @@ -29,6 +29,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif | NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false | | DynamicCodeSupport | System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported | Changes RuntimeFeature.IsDynamicCodeSupported to false to allow testing AOT-safe fallback code without publishing for Native AOT. | | _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes | +| JsonSerializerIsReflectionEnabledByDefault | System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault | When set to false, disables using reflection as the default contract resolver in System.Text.Json | Any feature-switch which defines property can be set in csproj file or on the command line as any other MSBuild property. Those without predefined property name