Skip to content

[API Proposal]: Provide trim-safe overload for ValidationContext construction #113134

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

Closed
Tracked by #109396
captainsafia opened this issue Mar 4, 2025 · 4 comments · Fixed by #113426
Closed
Tracked by #109396

[API Proposal]: Provide trim-safe overload for ValidationContext construction #113134

captainsafia opened this issue Mar 4, 2025 · 4 comments · Fixed by #113426
Labels
api-approved API was approved in API review, it can be implemented area-System.ComponentModel.DataAnnotations in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@captainsafia
Copy link
Member

captainsafia commented Mar 4, 2025

Background and motivation

The only trim-unsafe codepaths in the ValidationContext currently are invoked when no explicit Display is set on a ValidationContext. In this case, the ValidationContext will use unbounded reflection to discover the Display attribute on a type to support resolving it.

This presents a challenge to components that always initialize the DisplayName property as they always have to mark the construction of the ValidationContext as trim-safe even when it is not.

API Proposal

namespace System.ComponentModel.DataAnnotations;

public sealed class ValidationContext
{
    public ValidationContext(object instance, string displayName, IServiceProvider? serviceProvider = null, IDictionary<object, object?>? items = null)
}

API Usage

// Trim-safe initializtion of ValidationContext
var validationContext = new ValidationContext(42, "MyMagicNumber", null, null);

Alternative Designs

The proposed API only adds the displayName parameter to the most-specific constructor on the ValidationContext type. We can consider adding constructors with the other permutations of this type.

Risks

No response

@captainsafia captainsafia added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 4, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 4, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

@tarekgh tarekgh added this to the 10.0.0 milestone Mar 4, 2025
@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Mar 4, 2025
@tarekgh
Copy link
Member

tarekgh commented Mar 4, 2025

Would be better to default the serviceProvider and items to nulls?

@captainsafia
Copy link
Member Author

Would be better to default the serviceProvider and items to nulls?

We can. I over-indexed on the existing constructor overloads for this but we can use a default value in these new overloads. I'll edit the proposal.

@tarekgh tarekgh added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 6, 2025
@bartonjs
Copy link
Member

bartonjs commented Mar 11, 2025

Video

  • The type has three existing constructors, which did suggest using optional parameters on this fouth one was reasonable, but usage data says that the average caller is using the serviceProvider+items constructor, so no defaults are really needed.
namespace System.ComponentModel.DataAnnotations;

public sealed class ValidationContext
{
    public ValidationContext(object instance, string displayName, IServiceProvider? serviceProvider, IDictionary<object, object?>? items)
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 11, 2025
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 12, 2025
@tarekgh tarekgh mentioned this issue Mar 14, 2025
14 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.ComponentModel.DataAnnotations in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants