Allow common types in attribute values (nullable, decimal)

[GSPP]

Values in attributes currently do not support nullable values. That is annoying because it forces us back to obsolete techniques used pre-.NET-2.0. Decimal support also would be nice.

Maybe we can even have a general way we can add "JSON-like" object trees. e.g.

[ActionMethod(
  name: "abc",
  routes: new [] { "/a", "/b" },
  connectionString: new ConnectionString() { Server = "s", Port = 42 })]

I hope you get what I mean. I made up a contrived, yet plausible, example.

That way we can have very high generality in attributes. We no longer need to make up rules and add piecemeal support for certain things.

[DixonDs]

This issue seems to be related to my request as well: https://github.com/dotnet/coreclr/issues/1534

[robertmclaws]

Hi @terrajobst! Any chance we could get this worked into the schedule for .NET 7.0?

[terrajobst]

Tagging @jkotas @tmat.

My understanding is that this is a major work item as it requires to rev the underlying metadata format. Attributes are essentially stored as binary blobs and the set of supported types is a closed set.

This issue pops up from time to time but so far there wasn't compelling enough evidence that warrants that level of investment yet.

That's not to say it's a bad idea (it would certainly be neat) just that this would mean we get to spend less time on features that seem more compelling.

[jzabroski]

• Decimal.
  • Current workarounds: (1) Encode value in string, call decimal.Parse. (2) Use doubles, risk lossy precision when casting double to decimal. (3) doesn't work in all cases, see below for example of open data type closed at run-time.
• DateTime
  • Example: for FluentMigrator, we recommend people use the ISO 8601 prefixed long integer encoding of DateTime values to represent a migration version number. [Migration(202201040001, "DOTNET-4525: Demonstrate DateTime for attribute values")] would instead be written as, perhaps, [Migration(new DateTime(2022, 1, 4), sequence: 1, "DOTNET-4525: Demonstrate DateTime for attribute values")] - this is maybe not the best example, because I can't confirm people would prefer writing it this way, which brings me to the next point.
  • Example: DateTime constructors are noisy as hell and a pain to write. Why can't we simply write dt"20220104" and .NET infer the ISO 8601 value from a DateTime string literal? With modern IDEs it can probably even be color-coded, and code-lens can probably display a compile-time constant value help tip on what the constructed value would be.
• static readonly properties, especially for things that have no free variables in the initialization logic.
  • I'm sure there are compiler limitations I am not thinking about for why the ECMA standard disallows this, but it is a really annoying restriction. Our workaround is to use Constants, and Constants don't propagate across assemblies, they're copy-pasted by the compiler, so there is no way to dynamically link values.
  • Perhaps there should be a different concept than static readonly here - like Robert Harper's notion of assignable vs. mobile values http://www.cs.cmu.edu/~rwh/talks/hope13.pdf

That said, my most frequent use case is xunit.net theories. I use theories a lot to guarantee 100% code coverage. In these circumstances, I often have to use MemberData instead of InlineData because decimal and other values are not supported.

Here is a good example xunit test I would also love to be able to generalize to more native data types like Decimal:

        [Theory]
        [InlineData(1)]
        [InlineData(1L)]
        [SuppressMessage("Usage", "xUnit1026:Theory methods should use all of their parameters", Justification = "<Pending>")]
        public void TestGetNullableWhenNull<T>(T _) where T : struct
        {
            var fieldName = Fixture.Create<string>();
            var ordinal = Fixture.Create<int>();
            var fieldValue = DBNull.Value;
            var sqlDataReaderMock = new Mock<IDataRecord>();
            sqlDataReaderMock.Setup(s => s.GetOrdinal(fieldName))
                .Returns(ordinal);
            sqlDataReaderMock.Setup(s => s.GetFieldType(ordinal))
                .Returns(typeof(T));
            sqlDataReaderMock.Setup(s => s[It.IsAny<string>()])
                .Returns(fieldValue);

            var sqlDataReader = sqlDataReaderMock.Object;

            var result = sqlDataReader.Get<T?>(fieldName);
            Assert.Null(result);

            Mock.VerifyAll(sqlDataReaderMock);
        }

        [Theory]
        [InlineData(1)]
        [InlineData(1L)]
        [InlineData("")]
        [SuppressMessage("Usage", "xUnit1026:Theory methods should use all of their parameters", Justification = "<Pending>")]
        public void TestGetMockUserDefinedTypeThrowsException<T>(T _)
        {
            var fieldName = Fixture.Create<string>();
            var ordinal = Fixture.Create<int>();
            var sqlDataReaderMock = new Mock<IDataRecord>();
            sqlDataReaderMock.Setup(s => s.GetOrdinal(fieldName))
                .Returns(ordinal);
            // The documentation for SqlDataReader says that GetFieldType returns null for SQL columns with user-defined data types.
            sqlDataReaderMock.Setup(s => s.GetFieldType(ordinal))
                .Returns((Type)null);

            var sqlDataReader = sqlDataReaderMock.Object;

            Assert.Throws<NotSupportedException>(() => sqlDataReader.Get<T>(fieldName));
            
            Mock.VerifyAll(sqlDataReaderMock);
        }

        [Theory]
        [InlineData(1, 1L)]
        [InlineData(1L, 1)]
        [SuppressMessage("Usage", "xUnit1026:Theory methods should use all of their parameters", Justification = "<Pending>")]
        public void TestGetWhenDatabaseTypeFieldTypeAndClrTypeMismatch<TFieldType, TClrType>(TFieldType _, TClrType __)
        {
            var fieldName = Fixture.Create<string>();
            var ordinal = Fixture.Create<int>();
            var fieldValue = Fixture.Create<TFieldType>();
            var sqlDataReaderMock = new Mock<IDataRecord>();
            sqlDataReaderMock.Setup(s => s.GetOrdinal(fieldName))
                .Returns(ordinal);
            sqlDataReaderMock.Setup(s => s.GetFieldType(ordinal))
                .Returns(typeof(TFieldType));

            var sqlDataReader = sqlDataReaderMock.Object;

            var exception = Assert.Throws<Exception>(() => sqlDataReader.Get<TClrType>(fieldName));
            Assert.Equal($"Database fieldType {typeof(TFieldType).FullName} does not match C# destination: {typeof(TClrType).FullName}.  fieldName: {fieldName}", exception.Message);

            Mock.VerifyAll(sqlDataReaderMock);
        }

This example is not representative of every test I write, but it is a decent example where I can't simply call decimal.Parse on a string, because T is an open data type that is closed at run-time by the test runner.

[jzabroski]

Attributes are essentially stored as binary blobs and the set of supported types is a closed set.

This issue pops up from time to time but so far there wasn't compelling enough evidence that warrants that level of investment yet.

Just a thought - why not implement some sort of "Gradual Attributes" as a workaround to this? They would simply derive a different superclass than System.Attribute. Perhaps System.Annotation. A compiler transform similar to nameof would then effectively implement the manual transform most of us use in frameworks like xunit in order to be able to read data at run-time. The two APIs, System.Attribute and System.Annotation, could be source-level compatible for fields, methods and other APIs (reflection), such that it could eventually be merged into a single type in the future.

I'm not saying this is even a good idea - just a workaround proposal to why this feature would have a heavy complexity cost budget as well as "Paying the Language Design UI Tax".

[jzabroski]

@jcouv @RikkiGibson How does this proposal intersect with C# 10.0 Generic Attributes dotnet/csharplang#124 viz a viz dotnet/csharplang#4936 ? I haven't played around with the Generic Attributes feature yet, but it would seem like either Generic Attributes would workaround these limitations or resolve them directly for non-generic attributes.

If a generic type parameter in an attribute constructor isn't storable, then how are generic attributes even possible?

e.g., it's not clear from the spec if any of these should compile:

// Freeze the application clock so that all test fixture requests for date time resolve to 1/1/2020
[Freeze<DateTime>(new DateTime(2020, 1, 1))]

This is not my favorite example, but I imagine it's something some people might enjoy writing:

/* Expression Trees */
// This property is a ForeignKey to entity Foo via Bar navigation property.
[ForeignKey<Foo>(f => f.Bar)]

[RikkiGibson]

Use of generic attributes doesn't change which types are allowed as attribute arguments. So [Attr<DateTime>(new DateTime(..))] as of now would not work.

[robertmclaws]

@RikkiGibson That's going to be a massive point of failure for a lot of people, I think. Enabling generic attributes is going to exacerbate this problem quite a bit because T in Attribute is unconstrained, and people are going to want to know why they can put a massively-complex type in there, but not something simple a decimal.

Also, in the pull request, there are no unit tests to outline which types are expected to fail, only happy-path tests. So there is no indication that decimal will fail, except the time they will waste trying to figure it out.

[jzabroski]

@robertmclaws It's a good catch that the unit tests don't call out negative test cases, but it's not exactly clear how those would be caught via unit test (to me). You can't even express as valid syntax some of these examples, so it would have to be fairly high level compilation tests for C#, vs. CLR tests.

In general, having generic attributes does enable some more type-safe programming. For example, Imagine closing SomeSourceGeneratorAttribute<TDataTransferObject> vs. SomeSourceGeneratorAttribute(typeof(DataTransferObject)) - the former is more safe than the latter and easier to use from an API builder perspective, as it allows directly referencing constraints from TDataTransferObject's generic interface.

The broader reason I raised this point is that I wonder to what extent C# and the CLR would be best off implementing support for higher-kinded polymorphism. F# would certainly benefit as a functional language where higher-kinded polymorphism makes for very powerful functional abstractions. And it is pretty common to implement data constructors through some higher-kinded mechanism. The other major benefit of higher-kinded polymorphism is it eliminates a lot of edge cases composing open generic types, like open generic delegates.

We could also just do the simple thing and implement my System.Annotation hack via Source Generators, especially now that we have Generic Attributes in preview mode. An Annotation would just lift data constructor terms to a closed type. So you could have a "generic attribute" whose sole purpose is to unwrap a data transfer object that carries the real attribute.

My particular desire in resurfacing this old issue is rather silly - I want to have a clearer boundary between Input Test Data and Expected Result Test Data in my xunit.net theory test's InlineData attributes. I've never liked how I have had no clear boundary between the two sides of Arrange and Assert in my theory specifications.

[RikkiGibson]

people are going to want to know why they can put a massively-complex type in there, but not something simple a decimal.

You can use most any concrete type, including decimal, as a type argument to an attribute type. It's just that if this causes a parameter on the attribute constructor to have a disallowed type after substitution, you'll get an error. So, for example, [Attr<DateTime>] or [Attr<decimal>] are just fine if Attr has an accessible parameterless constructor.

[jzabroski]

So, for example, [Attr<DateTime>] or [Attr<decimal>] are just fine if Attr has an accessible parameterless constructor.
-- @RikkiGibson

To elaborate on my Annotation hack, you can imagine something like this:

public class AnnotationAttribute<T> : Attribute where T : new()
{
  T Unwrap<T>() => new T();
}

public class Cargo
{
  public DateTime Date => new DateTime(2020, 1, 1);
}

[Annotation<Cargo>]
public class Foo
{
  public Cargo GetAttributeData()
  {
    // Read attribute into some variable  named 'annotation' ....
    return annotation.Unwrap<Cargo>();
  }
}

and, in my case of extending xunit's InlineData into something nicer to work with, you can imagine tupling the annotation:

public class AnnotationAttribute<T1, T2> : Attribute where T1 : new(), T2 : new()
{
  (T1, T2) Unwrap<T1, T2>() => (new T1(), new T2());
}