Trim warnings for MinLengthAttribute

[sbomer]

The following produces warnings on the MinLength attribute instance:

using Microsoft.Extensions.Options;
using System.ComponentModel.DataAnnotations;

var model = new Model { Name = "abcd" };
var result = model.Validate(null, model);
Console.WriteLine(result.Succeeded);

[OptionsValidator]
partial class Model : IValidateOptions<Model> {
    [MinLength(4)]
    public required string Name { get; set; }
}

Program.cs(10,6): warning IL2026: Using member 'System.ComponentModel.DataAnnotations.MinLengthAttribute.MinLengthAttribute(Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved

(This is true even when using the options validation source generator - even though the generator generates its own attribute types, the original attribute still exists in user code.)

It seems like the best pattern we can recommend at the moment is to add UnconditionalSuppressMessage to the property, but it seems like there should be a built-in way to do length validation that doesn't require suppressing warnings.

The MinLengthAttribute ctor is ruc because its IsValid implementation reflects over the instance type to get the Count property. Maybe that could be represented using generics, for example:

class MinLengthAttribute<T> : MinLengthAttribute
    where T : ICollection
{
    public MinLengthAttribute(int length) {}
    public override bool IsValid(object? value)
    {
        if (value is ICollection collection)
        {
            return collection.Count >= Length;
        }
        throw ...;
    }
}

Somewhat related to #82526.

@eerhardt @tarekgh @eiriktsarpalis

[dotnet-policy-service]

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

[tarekgh]

This may assist with types that implement the ICollection interface (which is likely the common case), but it will not work for types that only have a Count property. Also, this will require exposing two types (for min and max length). For now, it is easy for the users to suppress this warning.

[eiriktsarpalis]

The MinLengthAttribute ctor is ruc because its IsValid implementation reflects over the instance type to get the Count property. Maybe that could be represented using generics, for example:

I presume you mean marking the generic parameter with a DynamicallyAccessedMembers annotation so that the reflection-based lookup doesn't warn. That could work, although we would need to replicate every single validation attribute type that needs to access the length of a value. I suspect the better approach might be to update our annotation strategy -- we could perhaps mark the IsValid() method as being wholesale linker-unsafe so that constructors (and validation libraries only relying on the constructors) do not warn.

[sbomer]

I presume you mean marking the generic parameter with a DynamicallyAccessedMembers annotation so that the reflection-based lookup doesn't warn.

That could work, but what I meant was to constrain the generic parameter to implement ICollection and not support the reflection fallback.

we could perhaps mark the IsValid() method as being wholesale linker-unsafe so that constructors (and validation libraries only relying on the constructors) do not warn.

That might work - though we generally try to avoid marking virtual methods with RequiresUnreferencedCode because it becomes viral quickly. In this case, ValidationAttribute.IsValid isn't inherently problematic, so marking it unsafe would exclude some legitimate derived attribute definitions.