Skip to content

Commit a20ee6f

Browse files
github-actions[bot]eiriktsarpaliscarlossanlopViktorHofer
authored
[release/8.0-staging] Fix JsonArray.Add and ReplaceWith regressions. (#94882)
* Fix JsonArray.Add and ReplaceWith regressions. * Add comment * Address feedback. * Update packaging metadata * Bring back removed using statement * Fix the build error that was showing up after bumping the ServicingVersion prop: Assembly 'System.Text.Json.TestLibrary.Roslyn3.11' with identity 'System.Text.Json.TestLibrary.Roslyn3.11, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' uses 'System.Text.Json, Version=8.0.0.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' which has a higher version than referenced assembly 'System.Text.Json' with identity 'System.Text.Json, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' * Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.TestLibrary/System.Text.Json.TestLibrary.targets * Fix incremental servicing props ordering issue --------- Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com> Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
1 parent 4fc3df2 commit a20ee6f

File tree

5 files changed

+119
-21
lines changed

5 files changed

+119
-21
lines changed

‎src/libraries/Directory.Build.targets

+12-8
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@
77
<StrongNameKeyId Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true'">$(TestStrongNameKeyId)</StrongNameKeyId>
88
</PropertyGroup>
99

10+
<!-- Need to be defined before packaging.targets is imported. -->
11+
<PropertyGroup>
12+
<!-- The source of truth for these IsNETCoreApp* properties is NetCoreAppLibrary.props. -->
13+
<IsNETCoreAppSrc Condition="'$(IsSourceProject)' == 'true' and
14+
$(NetCoreAppLibrary.Contains('$(AssemblyName);'))">true</IsNETCoreAppSrc>
15+
<IsNETCoreAppRef Condition="('$(IsReferenceAssemblyProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true') and
16+
$(NetCoreAppLibrary.Contains('$(AssemblyName);')) and
17+
'$(IsPrivateAssembly)' != 'true'">true</IsNETCoreAppRef>
18+
<IsNETCoreAppAnalyzer Condition="'$(IsGeneratorProject)' == 'true' and
19+
$(NetCoreAppLibraryGenerator.Contains('$(MSBuildProjectName);'))">true</IsNETCoreAppAnalyzer>
20+
</PropertyGroup>
21+
1022
<!-- resources.targets need to be imported before the Arcade SDK. -->
1123
<Import Project="$(RepositoryEngineeringDir)resources.targets" />
1224
<Import Project="..\..\Directory.Build.targets" />
@@ -47,14 +59,6 @@
4759
-->
4860
<WarningsAsErrors>$(WarningsAsErrors.Replace('NU1605', ''))</WarningsAsErrors>
4961

50-
<!-- The source of truth for these IsNETCoreApp* properties is NetCoreAppLibrary.props. -->
51-
<IsNETCoreAppSrc Condition="'$(IsSourceProject)' == 'true' and
52-
$(NetCoreAppLibrary.Contains('$(AssemblyName);'))">true</IsNETCoreAppSrc>
53-
<IsNETCoreAppRef Condition="('$(IsReferenceAssemblyProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true') and
54-
$(NetCoreAppLibrary.Contains('$(AssemblyName);')) and
55-
'$(IsPrivateAssembly)' != 'true'">true</IsNETCoreAppRef>
56-
<IsNETCoreAppAnalyzer Condition="'$(IsGeneratorProject)' == 'true' and
57-
$(NetCoreAppLibraryGenerator.Contains('$(MSBuildProjectName);'))">true</IsNETCoreAppAnalyzer>
5862
<!-- Inbox analyzers shouldn't use the live targeting / runtime pack. They better depend on an LKG to avoid layering concerns. -->
5963
<UseLocalTargetingRuntimePack Condition="'$(IsNETCoreAppAnalyzer)' == 'true'">false</UseLocalTargetingRuntimePack>
6064
<!-- By default, disable implicit framework references for NetCoreAppCurrent libraries. -->

‎src/libraries/System.Text.Json/src/System.Text.Json.csproj

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
<NoWarn>CS8969</NoWarn>
99
<IncludeInternalObsoleteAttribute>true</IncludeInternalObsoleteAttribute>
1010
<IsPackable>true</IsPackable>
11+
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
12+
<ServicingVersion>1</ServicingVersion>
1113
<PackageDescription>Provides high-performance and low-allocating types that serialize objects to JavaScript Object Notation (JSON) text and deserialize JSON text to objects, with UTF-8 support built-in. Also provides types to read and write JSON text encoded as UTF-8, and to create an in-memory document object model (DOM), that is read-only, for random access of the JSON elements within a structured view of the data.
1214

1315
The System.Text.Json library is built-in as part of the shared framework in .NET Runtime. The package can be installed when you need to use it in other target frameworks.</PackageDescription>

‎src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs

+1-7
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,7 @@ internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base(
174174
[RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
175175
public void Add<T>(T? value)
176176
{
177-
JsonNode? nodeToAdd = value switch
178-
{
179-
null => null,
180-
JsonNode node => node,
181-
_ => JsonValue.Create(value, Options)
182-
};
183-
177+
JsonNode? nodeToAdd = ConvertFromValue(value, Options);
184178
Add(nodeToAdd);
185179
}
186180

‎src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs

+35-6
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
using System.Collections.Generic;
55
using System.Diagnostics.CodeAnalysis;
6+
using System.Text.Json.Serialization.Converters;
7+
using System.Text.Json.Serialization.Metadata;
68

79
namespace System.Text.Json.Nodes
810
{
@@ -316,17 +318,16 @@ public static bool DeepEquals(JsonNode? node1, JsonNode? node2)
316318
[RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
317319
public void ReplaceWith<T>(T value)
318320
{
321+
JsonNode? node;
319322
switch (_parent)
320323
{
321-
case null:
322-
return;
323324
case JsonObject jsonObject:
324-
JsonValue? jsonValue = JsonValue.Create(value);
325-
jsonObject.SetItem(GetPropertyName(), jsonValue);
325+
node = ConvertFromValue(value);
326+
jsonObject.SetItem(GetPropertyName(), node);
326327
return;
327328
case JsonArray jsonArray:
328-
JsonValue? jValue = JsonValue.Create(value);
329-
jsonArray.SetItem(GetElementIndex(), jValue);
329+
node = ConvertFromValue(value);
330+
jsonArray.SetItem(GetElementIndex(), node);
330331
return;
331332
}
332333
}
@@ -351,5 +352,33 @@ internal void AssignParent(JsonNode parent)
351352

352353
Parent = parent;
353354
}
355+
356+
/// <summary>
357+
/// Adaptation of the equivalent JsonValue.Create factory method extended
358+
/// to support arbitrary <see cref="JsonElement"/> and <see cref="JsonNode"/> values.
359+
/// TODO consider making public cf. https://github.com/dotnet/runtime/issues/70427
360+
/// </summary>
361+
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
362+
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
363+
internal static JsonNode? ConvertFromValue<T>(T? value, JsonNodeOptions? options = null)
364+
{
365+
if (value is null)
366+
{
367+
return null;
368+
}
369+
370+
if (value is JsonNode node)
371+
{
372+
return node;
373+
}
374+
375+
if (value is JsonElement element)
376+
{
377+
return JsonNodeConverter.Create(element, options);
378+
}
379+
380+
var jsonTypeInfo = (JsonTypeInfo<T>)JsonSerializerOptions.Default.GetTypeInfo(typeof(T));
381+
return new JsonValueCustomized<T>(value, jsonTypeInfo, options);
382+
}
354383
}
355384
}

‎src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs

+69
Original file line numberDiff line numberDiff line change
@@ -680,5 +680,74 @@ public static void ReplaceWith()
680680
Assert.Null(jValue.Parent);
681681
Assert.Equal("[5]", jArray.ToJsonString());
682682
}
683+
684+
[Theory]
685+
[InlineData("null")]
686+
[InlineData("1")]
687+
[InlineData("false")]
688+
[InlineData("\"str\"")]
689+
[InlineData("""{"test":"hello world"}""")]
690+
[InlineData("[1,2,3]")]
691+
public static void AddJsonElement(string json)
692+
{
693+
// Regression test for https://github.com/dotnet/runtime/issues/94842
694+
using var jdoc = JsonDocument.Parse(json);
695+
var array = new JsonArray();
696+
697+
array.Add(jdoc.RootElement);
698+
699+
JsonNode arrayElement = Assert.Single(array);
700+
switch (jdoc.RootElement.ValueKind)
701+
{
702+
case JsonValueKind.Object:
703+
Assert.IsAssignableFrom<JsonObject>(arrayElement);
704+
break;
705+
case JsonValueKind.Array:
706+
Assert.IsAssignableFrom<JsonArray>(arrayElement);
707+
break;
708+
case JsonValueKind.Null:
709+
Assert.Null(arrayElement);
710+
break;
711+
default:
712+
Assert.IsAssignableFrom<JsonValue>(arrayElement);
713+
break;
714+
}
715+
Assert.Equal($"[{json}]", array.ToJsonString());
716+
}
717+
718+
[Theory]
719+
[InlineData("null")]
720+
[InlineData("1")]
721+
[InlineData("false")]
722+
[InlineData("\"str\"")]
723+
[InlineData("""{"test":"hello world"}""")]
724+
[InlineData("[1,2,3]")]
725+
public static void ReplaceWithJsonElement(string json)
726+
{
727+
// Regression test for https://github.com/dotnet/runtime/issues/94842
728+
using var jdoc = JsonDocument.Parse(json);
729+
var array = new JsonArray { 1 };
730+
731+
array[0].ReplaceWith(jdoc.RootElement);
732+
733+
JsonNode arrayElement = Assert.Single(array);
734+
switch (jdoc.RootElement.ValueKind)
735+
{
736+
case JsonValueKind.Object:
737+
Assert.IsAssignableFrom<JsonObject>(arrayElement);
738+
break;
739+
case JsonValueKind.Array:
740+
Assert.IsAssignableFrom<JsonArray>(arrayElement);
741+
break;
742+
case JsonValueKind.Null:
743+
Assert.Null(arrayElement);
744+
break;
745+
default:
746+
Assert.IsAssignableFrom<JsonValue>(arrayElement);
747+
break;
748+
}
749+
750+
Assert.Equal($"[{json}]", array.ToJsonString());
751+
}
683752
}
684753
}

0 commit comments

Comments
 (0)