Roslyn: Custom record constructors are highlighted as struct

Created on 20 Aug 2020  路  13Comments  路  Source: dotnet/roslyn

Version Used:
VS 16.7.2

Steps to Reproduce:

Code like below:

public sealed record Record(int A, int B, int C, int D)
{
    public Record() : this(default, default, default, default)
    {
    }
}

Expected Behavior:
The constructor name Record will be highlighted in green (classes).

Actual Behavior:
The constructor is highlighted in pink (structs). In my machine, classes will be highlighted in green, while the structs are pink.

Area-IDE Bug New Language Feature - Records Urgency-Soon

Most helpful comment

We probably want a new classification for this entirely. A record is a record. If people want records to have a different color, we should likely support that :) Note, the default here can be for it to colored like a class (same as we do for interfaces/delegates/etc.).

All 13 comments

We probably want a new classification for this entirely. A record is a record. If people want records to have a different color, we should likely support that :) Note, the default here can be for it to colored like a class (same as we do for interfaces/delegates/etc.).

@CyrusNajmabadi 馃槂Thank you so much!!

The name of the record is also not correctly classified:
image

@jcouv Since we want to differentiate records from classes, does this require a change on the compiler side, i.e. adding 'Record' to the TypeKind enum?

@allisonchou we don't really need that necessarily on the compiler side. We do need some way to dsitinguish a class from a record (symbolically). Hopefully @jcouv can give us the right way to do that.

Yes, we are using the unspeakable clone method to recognize records. See https://github.com/dotnet/roslyn/pull/46338

can you expose this in a reasonable fashion for IDE consumption purposes. THat's a pretty unwieldy method to duplicate. note: i would be ok with an extension in a file that is source-linked-in if you don't want to make a public API.

We probably want a new classification for this entirely

it's also nice to somehow highlight "redefined record members" including properties, Equals and so on.

@alrz can you expand on that?

there are certain members in a record that can be replaced by user code. Equals(ContainingTyoe) being one. currently there's no indication that the member correctly replaced the compiler-generated one, or it's just another member on it's own.

it's a lot like new/override but this is none of them, it merely works by matching the signature (much like Java overrides).

So I think the IDE could help here, maybe a quick info that says "record member" or alike.

let me put it this way: as a consumer of the record, how does that help me and when would i want that? (Note: i'm not objecting thsi, i'm just asking to help understand your position so i can see if it's something we def should do). Thanks!

it helps to know that the signature correctly matched and replaced. there is no way for compiler to report it or otherwise make it appearant because there's no modifer requirements.

the quick info would appear on the decl-site showing that is indeed a record member being redefined.

Oh i see. You're asking that the decl be classified in a certain fashion, not that references be classified. That's a very interesting case. Would you be willing to open an issue asking for this? I can take it to a design meeting to see how the team feels on this.

Thanks!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

AceHack picture AceHack  路  3Comments

codingonHP picture codingonHP  路  3Comments

AdamSpeight2008 picture AdamSpeight2008  路  3Comments

MadsTorgersen picture MadsTorgersen  路  3Comments

asvishnyakov picture asvishnyakov  路  3Comments