Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

positional record can't accept Attribute. #3644

Closed
moh-hassan opened this issue Jul 6, 2020 · 9 comments
Closed

positional record can't accept Attribute. #3644

moh-hassan opened this issue Jul 6, 2020 · 9 comments

Comments

@moh-hassan
Copy link

Records with attributes can be declared as

record Point 
{
    [Option]
    public int X { get; }
    [Option]
    public int Y { get; }
}

Nominal record can be declared as:

record Point(int X, int Y);

Trying to add attribute, like

record Point([Option]int X, [Option]int Y); //invalid syntax

Compiler raise an exception:

CS0592 Attribute 'Option' is not valid on this declaration type. It is only valid on 'property, indexer' declarations.

As I see from this doc, there is no limitation on the nominal record.
I'm using Net 5 Preview 5.
How to define attributes for the nominal record?

@YairHalberstadt
Copy link
Contributor

Perhaps there should be a [property: Option] syntax similar to the existing [field: Option] syntax for auto properties?

@moh-hassan
Copy link
Author

moh-hassan commented Jul 8, 2020

@YairHalberstadt ,
How your syntax match the proposal(now it is implemented) of Nominal records.
Attributes are allowed for positional records but not for Nominal records.

@333fred
Copy link
Member

333fred commented Jul 8, 2020

@moh-hassan, you seem to be confusing nominal and positional records. This syntax is a nominal record:

record Point 
{
    [Option]
    public int X { get; }
    [Option]
    public int Y { get; }
}

This is a positional record:

record Point(int X, int Y);

Yair is proposing an additional attribute scope, presumably something like

record Point([property: Option]int X, [property: Option]int Y);

for the positional case.

@moh-hassan
Copy link
Author

Thanks @333fred for clarification.

@RikkiGibson
Copy link
Contributor

See dotnet/roslyn#44691. I believe the expectation is that attributes will go on the parameter by default but if a property: target is specified we will put them on the synthesized property instead. This will require a diagnostic in the case that no property is synthesized for the positional parameter (like when the property is explicitly declared in the record body).

@moh-hassan moh-hassan changed the title Nominal record can't accept Attribute. positional record can't accept Attribute. Jul 20, 2020
@moh-hassan
Copy link
Author

Net 5 version 5.0.100-rc.1.20452.10 is now supporting positional record with attributes as described by @YairHalberstadt

public record Test(
    [field: A]
    [property: B]
    [param: C]
    [D]     
    [property: Option]
    int P1
    )
{
}

@333fred
Copy link
Member

333fred commented Sep 15, 2020

@moh-hassan is there anything else you were requesting? Or can this be closed?

@moh-hassan
Copy link
Author

@333fred ,
Just, I want to do more checks for the feature using Net5 Rc1.
Although VScode IDE didn't show any compilation warning, I was surprised to get a compilation warning after cleaning for the field and property attributes, but passed for [param ] and [D].
I'll feed back when test is completed.

@moh-hassan
Copy link
Author

I can confirm that attributes for positional record are implemented in 5.0.100-rc.1.20452.10. No compilation warning in VScode and I can access the attributes at runtime:

The attributes for the member 'Int32 P1' are:
The type of the attribute is 'B'.
The type of the attribute is 'CommandLine.OptionAttribute'.

@333fred
Thanks,
You can close this issue.

@333fred 333fred closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants