Fix goto types to include records #52523
Conversation
| @@ -13,7 +13,6 @@ internal static class FSharpNavigateToItemKind | |||
| public static string Line => NavigateToItemKind.Line; | |||
| public static string File = NavigateToItemKind.File; | |||
| public static string Class => NavigateToItemKind.Class; | |||
| public static string Record => NavigateToItemKind.Record; | |||
There was a problem hiding this comment.
This is unfortunately external access, but it won't break FSharp since this isn't used on their end.
| @@ -288,8 +288,6 @@ private static string GetItemKind(DeclaredSymbolInfo declaredSymbolInfo) | |||
| { | |||
| case DeclaredSymbolInfoKind.Class: | |||
| return NavigateToItemKind.Class; | |||
| case DeclaredSymbolInfoKind.Record: | |||
| return NavigateToItemKind.Record; | |||
There was a problem hiding this comment.
i'm a little confused how this works. if you remove this entry, and we do run into a record, won't we crash in this switch?
| @@ -179,7 +179,7 @@ public override bool TryGetDeclaredSymbolInfo(StringTable stringTable, SyntaxNod | |||
| node.Kind() switch | |||
| { | |||
| SyntaxKind.ClassDeclaration => DeclaredSymbolInfoKind.Class, | |||
| SyntaxKind.RecordDeclaration => DeclaredSymbolInfoKind.Record, | |||
| SyntaxKind.RecordDeclaration => DeclaredSymbolInfoKind.Class, | |||
There was a problem hiding this comment.
so i liek the old way this worked. DeclaredSymbolInfoKind is roslyn specific, so it can represent Record here. What i think we should do though i just map this to Class at the boundaries of NavTo.
There was a problem hiding this comment.
@CyrusNajmabadi The current approach is more simpler. Just treat records as classes (and later, record structs as structs). Especially that we don't need any special handling for records. But let me know if you still think otherwise.
There was a problem hiding this comment.
Yeah. I don't like it. Within roslyn itself, I think we should represent fully. Just at it boundaries would we marshall differently
|
@CyrusNajmabadi I addressed your feedback. |
|
thanks! |
Fixes #51672
@jcouv FYI for record structs work.