Skip to content
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

The System.Text.Json source generator doesn't populate JsonPropertyInfo.AttributeProvider metadata. #100095

Closed
Tracked by #100159
eiriktsarpalis opened this issue Mar 21, 2024 · 6 comments · Fixed by #100173

Comments

@eiriktsarpalis
Copy link
Member

The JsonPropertyInfo.AttributeProvider property isn't being populated by the source generator, making it impossible to resolve additional attributes from JsonPropertyInfo instances. This can create issues when mapping contract metadata to richer representations such as JSON schema (e.g. resolving DescriptionAttribute).

cc @captainsafia @stephentoub

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@gregsdennis
Copy link
Contributor

@eiriktsarpalis I'm interested how this could affect a JSON Schema representation. Where does this come from?

@eiriktsarpalis
Copy link
Member Author

@gregsdennis We want to be able to extract a JSON schema from the contract as specified in JsonTypeInfo instances. I'm working on a prototype for that here but at the moment we still need to fall back to reflection to extract some of the necessary information.

@gregsdennis
Copy link
Contributor

Happy to talk about this more offline if you have the time.

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Mar 25, 2024

Reopening, since apparently #100173 doesn't fully address the issue for the case of trimmed/Native AOT applications. The fact that it's using delayed MemberInfo resolution means that we're not providing any hints to the linker that these attributes shouldn't be trimmed.

One potential solution (already trialed in typeshape-csharp) is adding a delayed factory for ICustomAttributeProvider declared in the JsonPropertyInfoValues<T> class used by the source generator:

namespace System.Text.Json.Serialization.Metadata;

public partial class JsonPropertyInfoValues<T>
{
    public Func<ICustomAttributeProvider>? CustomAttributeProviderFactory { get; init; }
}

which would be populated by the source generator as follows:

new JsonPropertyInfoValues<string>
{
    PropertyName = "Name",
    CustomAttributeProviderFactory = static () => typeof(Person).GetProperty("Name"),
};

public record Person(string Name);

This should provide the linker with sufficient hints so that metadata for the particular properties will not be trimmed.

Given that this necessarily results in some reflection components being rooted by the source generator, we might additionally consider an opt-in flag in JsonSourceGenerationOptionsAttribute:

namespace System.Text.Json.Serialization;

public partial class JsonSourceGenerationOptionsAttribute
{
    public bool GenerateAttributeProviderFactories { get; set; } = null;
}

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented May 10, 2024

Superseded by #102078.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants