Fully annotate JsonNode for trimmability

[eerhardt]

Follow up to #52934.

• Using JsonNode in dynamic statements is not trim compatible. Add a LibraryBuild warning since there isn't a direct API to put the warning on.
• Mark JsonValueNotTrimmable's ctor as unsafe
• Fix up a few warning messages
• minor doc fixup

Contributes to #45623

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Follow up to #52934.

• Using JsonNode in dynamic statements is not trim compatible. Add a LibraryBuild warning since there isn't a direct API to put the warning on.
• Mark JsonValueNotTrimmable's ctor as unsafe
• Fix up a few warning messages
• minor doc fixup

Contributes to #45623

Author:	eerhardt
Assignees:	-
Labels:	area-System.Text.Json
Milestone:	-

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[eerhardt]

@steveharter - thoughts on this change?

(Note: The runtime-staging builds appear to be broken due to infra issues.)

[steveharter]

Due to trimming this was discussed as a method that we may want to remove, and require usage of the IList<T>.Add<T>(T value) implementation. However since that requires an extra call to JsonValue.Create(), it affects usability.

Have there been other non-JSON areas that we've discouraged API use from in this manner in favor of forcing or encouraging a trimmable API?

[eerhardt]

Have there been other non-JSON areas that we've discouraged API use from in this manner in favor of forcing or encouraging a trimmable API?

Yes. A main difference between JSON and those APIs is that we are actively adding features to JSON, so that's why it seems JSON is trying to be "trim-compatible" from the start. And also because any API we've already shipped we can't change. But since we are defining these JSON APIs at the same time as making the BCL trimmable, we might as well at least consider it in the API design.

Other examples include:

• System.Linq.Expressions (ex. Preserving operator methods necessary for System.Linq.Expressions linker#1821 - see the full discussion, but the current recommended approach is to add a new overload that doesn't do the scanning for operator methods (which is the trim-incompatible issue)
• TypeDescriptor is virtually all RequiresUnreferencedCode, because of its heavy use of Reflection.
• System.Data.Common is getting a lot of RequiresUnreferencedCode attributes in Add trimmer annotations to System.Data.Common #52046

discouraged API use

Note though, this only matters if you care about trimmability and AOT'ing your code. If a developer is only concerned about the "current deployment models" of their code, they can easily use these APIs and their code will work. They only get warnings once they attempt to trim unused code away.

[steveharter]

Does usage of dynamic itself causes a warning (without STJ)?

[eerhardt]

Yes. We marked basically all APIs in Microsoft.CSharp as RequiresUnreferencedCode.

runtime/src/libraries/Microsoft.CSharp/ref/Microsoft.CSharp.cs

Lines 10 to 34 in ffb095a

	public static partial class Binder
	{
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder BinaryOperation(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Linq.Expressions.ExpressionType operation, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder Convert(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Type type, System.Type? context) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder GetIndex(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder GetMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, string name, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder Invoke(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder InvokeConstructor(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder InvokeMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, string name, System.Collections.Generic.IEnumerable<System.Type>? typeArguments, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder IsEvent(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, string name, System.Type? context) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder SetIndex(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder SetMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, string name, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Using dynamic types might cause types or members to be removed by trimmer.")]
	public static System.Runtime.CompilerServices.CallSiteBinder UnaryOperation(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags flags, System.Linq.Expressions.ExpressionType operation, System.Type? context, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>? argumentInfo) { throw null; }
	}

However, I didn't want to rely on those warnings here, because someone could cast a JsonNode to IDynamicMetaObjectProvider and call GetMetaObject on it, and thus travelling down the path to creating a JsonValueNotTrimmable object.