-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fully annotate JsonNode for trimmability #53184
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<linker> | ||
<assembly fullname="System.Text.Json"> | ||
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute"> | ||
<argument>ILLink</argument> | ||
<argument>IL2026</argument> | ||
<property name="Scope">member</property> | ||
<property name="Target">M:System.Text.Json.Nodes.JsonNode.System#Dynamic#IDynamicMetaObjectProvider#GetMetaObject(System.Linq.Expressions.Expression)</property> | ||
<property name="Justification">System.Text.Json's integration with dynamic is not trim compatible. However, there isn't a direct API developers call. Instead they use the 'dynamic' keyword, which gets compiled into calls to Microsoft.CSharp. Microsoft.CSharp looks for IDynamicMetaObjectProvider implementations. Leaving this warning in the product so developers get a warning stating that using dynamic JsonValue code may be broken in trimmed apps.</property> | ||
</attribute> | ||
</assembly> | ||
</linker> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,7 +98,7 @@ internal JsonArray (JsonElement element, JsonNodeOptions? options = null) : base | |
/// <param name="value"> | ||
/// The object to be added to the end of the <see cref="JsonArray"/>. | ||
/// </param> | ||
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] | ||
[RequiresUnreferencedCode(JsonValue.CreateUnreferencedCodeMessage)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to trimming this was discussed as a method that we may want to remove, and require usage of the 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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:
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. |
||
public void Add<T>(T? value) | ||
{ | ||
if (value == null) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does usage of
dynamic
itself causes a warning (without STJ)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We marked basically all APIs in
Microsoft.CSharp
asRequiresUnreferencedCode
.runtime/src/libraries/Microsoft.CSharp/ref/Microsoft.CSharp.cs
Lines 10 to 34 in ffb095a
However, I didn't want to rely on those warnings here, because someone could cast a JsonNode to
IDynamicMetaObjectProvider
and callGetMetaObject
on it, and thus travelling down the path to creating aJsonValueNotTrimmable
object.