[API Proposal]: Have AllowedValuesAttribute and DeniedValuesAttribute support case-insensitive strings

[rjperes]

Background and motivation

One of the most common uses of AllowedValues/DeniedValues is with strings, however, they are checked case-sensitive, and we probably want to allow case-insensitive comparisons too. Maybe a case of adding an extra optional constructor argument of type StringComparison, which, of course, would only apply to string values.

API Proposal

namespace System.ComponentModel.DataAnnotations;

public class AllowedValuesAttribute : ValidationAttribute
{
    public AllowedValuesAttribute(StringComparison comparer, params string?[] values) : this(values.OfType<object>().ToArray())
    {
    }

    public AllowedValuesAttribute(params string?[] values) : this(StringComparison.CurrentCulture, values)
    {
    }
}

public class DeniedValuesAttribute : ValidationAttribute
{
    public DeniedValuesAttribute(StringComparison comparer, params string?[] values) : this(values.OfType<object>().ToArray())
    {
    }

    public DeniedValuesAttribute(params string?[] values) : this(StringComparison.CurrentCulture, values)
    {
    }
}

API Usage

[AllowedValues(StringComparison.InvariantCultureIgnoreCase, "red", "green", "blue")]
public string Color { get; set; }

[DeniedValues("a", "e", "i", "o", "u")]
public string Consonants { get; set; }

Alternative Designs

No response

Risks

No response

[dotnet-policy-service[bot]]

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

[rjperes]

Not really, it’s actually an enum…

---

De: Juliano Leal Goncalves @.> Enviado: Thursday, August 22, 2024 3:20:36 PM Para: dotnet/runtime @.> Cc: Ricardo Peres @.>; Author @.> Assunto: Re: [dotnet/runtime] [API Proposal]: Have AllowedValuesAttribute and DeniedValuesAttribute support case-insensitive strings (Issue #106806)

[AllowedValues(“red”, “green”, “blue”, comparer: StringComparison.InvariantCultureIgnoreCase)]

Pretty sure this will not compile since StringComparison.InvariantCultureIgnoreCase is not a const value (it is static).

Attributes only allow consts to be passed into them.

— Reply to this email directly, view it on GitHubhttps://github.com/dotnet/runtime/issues/106806#issuecomment-2304798467, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AABG5SNRZG6MBWM62FLCDH3ZSXXTJAVCNFSM6AAAAABM534R5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBUG44TQNBWG4. You are receiving this because you authored the thread.Message ID: @.***>

[julealgon]

Not really, it’s actually an enum…

Which is why I deleted my post right after posting @rjperes .

[eiriktsarpalis]

The attributes don't only work with strings, so I'm not sure what should happen in the following scenario:

[AllowedValues(null, false, 1, "value", comparer: StringComparison.OrdinalIgnoreCase)]
public object? Values { get; set; }

This might sound like a niche example, but from a maintainer's perspective we need to make sure this doesn't result in unexpected behavior or unexpected exceptions. A more general-purpose solution might involve accepting an IEqualityComparer parameter instead (although granted you can't inline a StringComparer value in an attribute annotations, you would still need to subclass the attribute to support case-insensitive string values).

[rjperes]

As I mentioned in my example, this would apply to strings only, for all other cases this new parameter should be ignored. I think this is important because by large, in my use cases, I need to compare values case-insensitive.

[eiriktsarpalis]

As I mentioned in my example, this would apply to strings only, for all other cases this new parameter should be ignored.

Silently ignoring configuration parameters on the basis of input type is not a workable design, and is not consistent with how the attribute currently works or how similar APIs dealing with customized equality work (c.f. LINQ overloads accepting IEqualityComparer<T>). If we did consider this, it would need to accept an IEqualityComparer parameter which could then be subclassed.

[rjperes]

Or, provide a constructor overload that takes strings and the StringComparison parameter.

[eiriktsarpalis]

I like that idea. Could you update the proposal to reflect that?

[eiriktsarpalis]

Another issue with the proposal as it stands is that params must always be the last parameter so the constructor should look as follows:

public AllowedValuesAttribute(StringComparison comparer, params string?[] values);

Which would be invoked as follows:

[AllowValues(StringComparison.OrdinalIgnoreCase, "x", "y")]

[rjperes]

Will do! Would a PR also be useful?

[eiriktsarpalis]

Not yet, this would need to go through API review first, although providing a prototype branch with working unit tests should provide a useful proof of concept.

[rjperes]

Proposed changes for AllowedValuesAttribute:

public class AllowedValuesAttribute : ValidationAttribute
{
    private readonly StringComparison? _comparer;

    public AllowedValuesAttribute(StringComparison comparer, params string?[] values) : this(values.OfType<object>().ToArray())
    {
        _comparer = comparer;
    }

    public AllowedValuesAttribute(params string?[] values) : this(StringComparison.CurrentCulture, values)
    {
    }

    public override bool IsValid(object? value)
    {
        foreach (object? allowed in Values)
        {
            if (allowed is null ? value is null : Compare(allowed, value))
            {
                return true;
            }
        }

        return false;
    }

    private bool Compare(object allowed, object? value)
    {
        if (_comparer != null)
        {
            return string.Equals(allowed as string, value as string, _comparer.Value);
        }

        return allowed.Equals(value);
    }
}

The only way _comparer can be set is by using one of the constructors that take strings. Added an overload without StringComparison to allow skipping this parameter while supplying only strings.