Records should allow attributes on the primary constructor

[RikkiGibson]

Slightly related to dotnet/roslyn#44691

Currently there is no way to declare attributes on the primary constructor of a record.

[Attr] // ... no target or distinct syntax exists for the primary constructor
record Rec(int X, int Y);

If we decide to allow declaring an explicit "body" for a primary constructor, we could make it so attributes may be applied to that syntax which solely corresponds to the constructor.

record Rec(int X, int Y)
{
    data int Z;

    // strawman "body" syntax with an attribute:
    [Attr]
    {
        Z = X + Y;
    }
}

[HaloFour]

Sounds like we need more attribute targets?

[constructor: Attr]
public record Rec(
    [property: Foo] int X,
    [field: NonSerialized] int Y
);

[raffaeler]

I second that because the intent is more clear in comparison to other proposals.

The practical use-case I had is when deserializing with System.Text.Json : dotnet/runtime#45373
I believe this issue will become more and more popular.

[333fred]

We had a discussion on the internal mailing list, and the sentiment seemed to be preferring method as the target. It's already the target you use on constructors today:

 using System;
[AttributeUsage(AttributeTargets.Constructor)]
public class Attr : Attribute{}
public class C {
    [method: Attr]
    public C()
    {
    }
}

[bbarry]

Would it make sense that [method:...] attributes added to the class would get attached to all constructors in general?

edit to be clear:

[method:Attr]
public class Foo { }

not just records. Perhaps to implicit constructors only?

[RikkiGibson]

I would not attach such attributes to any explicitly declared constructors within the record. One could argue that such attributes should be attached to the synthesized copy constructor as well as the primary constructor, though. I personally do not see utility in this because I see the copy constructor as an implementation detail of with-expressions that users should not need to customize.

[CyrusNajmabadi]

why would S.T.J need to call the synthesized copy constructor?

[raffaeler]

why would S.T.J need to call the synthesized copy constructor?

As I wrote in the provided link: «When a record declares other constructors in addition to the primary one, an exception is thrown and there is no way to put the JsonConstructor attribute on the primary.»

I believe it would be better for the dev to choose which ctor the library should use instead of throwing or assuming one to be the best choice.

[CyrusNajmabadi]

@raffaeler I don't see what that has to do with teh synthesized copy constructor though. The statement was: I believe there should be some way to attach attributes to the synthesized copy ctor. We're looking for a justification for why that constructor needs attributes.

[raffaeler]

Sorry @CyrusNajmabadi, I missed it was about the copy ctor. My fault.

[333fred]

Right. That constructor is a protected constructor that is (usually) only called by the synthesized clone virtual method used by with. I'm not sure why that ctor would need to be annotated with any info.

[qwertie]

I think I need this feature to fix an exception I'm getting from System.Text.Json on a record type with secondary constructors:

System.NotSupportedException: Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported.

[ugurpelister]

Exact same problem with MessagePack deserialization.

[DrPhil]

Is there a github issue or anything where we can track this, or is this discussion the official place for getting news about implementation status?

[HaloFour]

If this proposal becomes "championed" an issue will be opened to represent it and it would be linked to from this discussion. That doesn't seem to have happened yet.

[drieseng]

Unless I'm missing something, it's the same issue all over again with primary constructors. I started a new discussion for this here.

[RikkiGibson]

Thanks for pinging on this. I'd like to treat this discussion as an umbrella for both records and non-records. It will help us to be able to gather the feedback, reactions etc in one place.

[RikkiGibson]

I will champion this. I just don't have a proposal yet.

[poizan42]

See #7047