Skip to content

Add trim-safe ValidationContext constructor #113426

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

Merged
merged 6 commits into from
Mar 14, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ public ValidationContext(object instance) { }
public ValidationContext(object instance, System.Collections.Generic.IDictionary<object, object?>? items) { }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("The Type of instance cannot be statically discovered and the Type's properties can be trimmed.")]
public ValidationContext(object instance, System.IServiceProvider? serviceProvider, System.Collections.Generic.IDictionary<object, object?>? items) { }
public ValidationContext(object instance, string displayName, System.IServiceProvider? serviceProvider, System.Collections.Generic.IDictionary<object, object?>? items) { }
public string DisplayName { get { throw null; } set { } }
public System.Collections.Generic.IDictionary<object, object?> Items { get { throw null; } }
public string? MemberName { get { throw null; } set { } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,43 @@ public ValidationContext(object instance, IServiceProvider? serviceProvider, IDi
ObjectInstance = instance;
}

/// <summary>
/// Construct a <see cref="ValidationContext" /> for a given object instance with
/// a <paramref name="displayName" />, an optional <paramref name="serviceProvider" />,
/// and an optional property bag of <paramref name="items" />.
/// </summary>
/// <param name="instance">The object instance being validated. It cannot be null.</param>
/// <param name="displayName">The display name associated with the object instance.</param>
/// <param name="serviceProvider">
/// Optional <see cref="IServiceProvider" /> to use when <see cref="GetService" /> is called.
/// If it is null, <see cref="GetService" /> will always return null.
/// </param>
/// <param name="items">
/// Optional set of key/value pairs to make available to consumers via <see cref="Items" />.
/// If null, an empty dictionary will be created. If not null, the set of key/value pairs will be copied into a
/// new dictionary, preventing consumers from modifying the original dictionary.
/// </param>
/// <exception cref="ArgumentNullException">When <paramref name="instance" /> is <c>null</c></exception>
/// <remarks>
/// This constructor is trim-safe because it does not use reflection to resolve
/// the Type of the <paramref name="instance" /> to support setting the DisplayName.
/// </remarks>
public ValidationContext(object instance, string displayName, IServiceProvider? serviceProvider, IDictionary<object, object?>? items)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which part of the other constructor forces it to have an RUC annotation? This seems to be replicating most of the statements present in the previous ctor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is the reason why the other 2 constructors are marked RUC:

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "The ctors are marked with RequiresUnreferencedCode.")]
private string? GetDisplayName()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the RUC messages here to be a little bit clearer about the fact that the issue is not providing a display name forces us down the path of resolving the type from the object instance which is not trim-safe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty wild that getting the display name was the pitfall here. Very cool to overcome that, @captainsafia!

{
ArgumentException.ThrowIfNullOrEmpty(displayName);
ArgumentNullException.ThrowIfNull(instance);

if (serviceProvider != null)
{
IServiceProvider localServiceProvider = serviceProvider;
InitializeServiceProvider(localServiceProvider.GetService);
}

_items = items != null ? new Dictionary<object, object?>(items) : new Dictionary<object, object?>();
ObjectInstance = instance;
DisplayName = displayName;
}

#endregion

#region Properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public static void Constructor_creates_new_instance_for_three_arg_constructor()
new ValidationContext(testDataAnnotationsDerived, serviceProvider, items);
}

[Fact]
public static void Constructor_creates_new_instance_for_four_arg_constructor()
{
var testDataAnnotationsDerived = new TestClass();
var displayName = "testDisplayName";
new ValidationContext(testDataAnnotationsDerived, displayName, null, null);
var items = new Dictionary<object, object>();
new ValidationContext(testDataAnnotationsDerived, displayName, null, items);
var serviceProvider = new TestServiceProvider();
new ValidationContext(testDataAnnotationsDerived, displayName, serviceProvider, items);
}

[Fact]
public static void ObjectInstance_and_ObjectType_return_same_instance_and_type_as_passed()
{
Expand Down Expand Up @@ -88,6 +100,23 @@ public static void Can_get_and_set_DisplayName_to_existent_and_non_existent_memb
Assert.Equal("NonExistentDisplayName", validationContext.DisplayName);
}

[Fact]
public static void Can_set_DisplayName_via_constructor()
{
var displayName = "testDisplayName";
var testDataAnnotationsDerived = new TestClass();
var validationContext = new ValidationContext(testDataAnnotationsDerived, displayName, null, null);
Assert.Equal(displayName, validationContext.DisplayName);
}

[Fact]
public static void Setting_DisplayName_to_null_or_empty_via_constructor_throws()
{
var testDataAnnotationsDerived = new TestClass();
AssertExtensions.Throws<ArgumentNullException>("displayName", () => new ValidationContext(testDataAnnotationsDerived, null, null, null));
AssertExtensions.Throws<ArgumentException>("displayName", () => new ValidationContext(testDataAnnotationsDerived, string.Empty, null, null));
}

[Fact]
public static void Setting_DisplayName_to_null_or_empty_throws()
{
Expand Down
Loading