Support TimeSpan type in the RangeAttribute emitted code by options validation source generator

[tarekgh]

The RangeAttribute employs a TypeConverter, which renders it incompatible with AOT compilation. The Options source generator identifies the usage of the RangeAttribute and generates strongly typed attribute code. The generator currently supports this only for a predefined list of basic types, as indicated here: link.

TimeSpan is not included in the supported list, despite its use in real application scenarios. This issue is being tracked to add support for TimeSpan and possibly other types like DateTimeOffset.

[ghost]

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

Issue Details

---

The RangeAttribute employs a TypeConverter, which renders it incompatible with AOT compilation. The Options source generator identifies the usage of the RangeAttribute and generates strongly typed attribute code. The generator currently supports this only for a predefined list of basic types, as indicated here: link.

TimeSpan is not included in the supported list, despite its use in real application scenarios. This issue is being tracked to add support for TimeSpan and possibly other types like DateTimeOffset.

Author:	tarekgh
Assignees:	-
Labels:	area-Extensions-Options, source-generator
Milestone:	-

[tarekgh]

CC @eerhardt @ericstj

[Clockwork-Muse]

TimeSpan is going to be a problem due to the whole "the value 24+ is read as days instead of hours" thing.

[tarekgh]

is going to be a problem due to the whole "the value 24+ is read as days instead of hours" thing.

Why do you think this is a problem? The behavior is well defined, and users have control on the values they can use in the range attribute.

[Clockwork-Muse]

Principle of least surprise problems. The behavior is well defined, sure, but that assumes that the people doing the configuration know that whatever application has been written in C#, and that the element type follows the given behavior.
Config makes it more likely to obscure this requirement...

[tarekgh]

The change here is not going to add or introduce any behavior change. The RangeAttribute today can accept TimeSpan and have the defined behavior. The support we are talking about here is the source generator using RangeAttribute. Cannot have different behavior for source generator than the runtime.

[eiriktsarpalis]

The RangeAttribute employs a TypeConverter, which renders it incompatible with AOT compilation.

I believe this could be addressed in part using the API as originally proposed here: #82526. API review at the time indicated that we should focus on shipping a new generic range type instead, but wondering if this scenario is sufficient to reconsider. cc @jeffhandley

[tarekgh]

@eiriktsarpalis this will help except of users already using the constructor public [RangeAttribute(Type type, string minimum, string maximum) we need to handle this case in the source gen till we do other things. I can imagine we can run into other similar types too (like DateTimeOffset as another example).

[eiriktsarpalis]

Given that this constructor only works with a small number of known types, it is conceivable that the source generator could intercept it, parse the type symbol and strings at compile time, replacing it with a call to the proposed ICompable constructor.

[tarekgh]

This is not going to help in anything for this case because we must convert the string values to the object type (like TimeSpan in this case). The whole issue we are trying to handle here is avoid using type converter. This is why the source generator handles a limited set of types.

Another side question, can the C# attribute accept interface types in the constructors as the proposal has? This is interesting to me. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/attributes#2224-attribute-parameter-types

[tarekgh]

oh, I see now the proposal is changed to have a generic type attribute. that make sense now :-)

[eiriktsarpalis]

I think it should be possible to have it work for the case of the non-generic variant as well.

[eerhardt]

I added my thoughts to that proposal: #82526 (comment). If we went with something like that, we wouldn't need to change the source gen at all.