Vscode-rust: RLS categorizes enums as arrays

Created on 29 Aug 2017  路  7Comments  路  Source: rust-lang/vscode-rust

I am not sure if it is an issue of the extension or the RLS itself, however @: command (Go to Symbol in File by Category) places enums in the array category.

#[derive(Clone, Debug)]
enum ChainState {
    // both front and back iterator are remaining
    Both,
    // only front is remaining
    Front,
    // only back is remaining
    Back,
}
bug

Most helpful comment

I think I like the second option better. Do you mind if we turn this into a tracking issue for implementing that? Here are the steps that I see so far:

All 7 comments

I looked into this a little, and it looks like it the root cause is in the compiler's save-analysis (rust-lang/rust). Though the enum itself is being classified correctly as an Enum, the enum values are currently being classified as a Tuple, which in VSCode renders the icon for the symbol like an array. I looked at language server implementations for languages like Typescript and C#, and it looks like they classify enum values as a Constant. Nonetheless, I think this would be better tracked as a rustc issue.

For reference: https://github.com/rust-lang/rust/blob/4cdb36262b93390c8733a1ce44665619d9348981/src/librustc_save_analysis/dump_visitor.rs#L647

@DSpeckhals Hey, thanks for looking into this! So the reason we classify it as a tuple is because there are three kinds of enum variants (with matching literal forms): tuple variants, struct variants, and unit variants. We try to make the kind in save-analysis reflect the kind of variant. We could just switch all three to have Enum kind, I'm not sure what benefit we get from the tuple/struct distinction. Alternatively we could be strictly more precise by introducing TupleVariant and StructVariant etc., for enum literals. Either would be a fairly minor change to the compiler, though the latter would need a change to rls-data too. I guess either way would allow us to use the Enum SymbolKind too.

I think I like the second option better. Do you mind if we turn this into a tracking issue for implementing that? Here are the steps that I see so far:

Sounds good! You'll want to do the 3rd and 4th steps in the opposite order though.

Note that there will be a bit of a dance to get the compiler and RLS to update their versions of rls-data. I think we might get away without doing this because transmuting from one version of the crate data to the other should still work. If you can compile the RLS locally after updating the version of rls-data in the rls, then you're good. If there is an error there, it is probably easier for me to get the changed version of rls-data into the compiler, since it requires changing multiple repositories 'at once'.

@nrc After a little work, I can get the RLS to compile locally with rls-data master. However, there are some stipulations that I think you may be alluding at above:

  • rls-analysis (raw.rs) had to patched to cover the three new DefKinds in rls-data
  • rls had to be patched similarly to above in lsp_data.rs. I currently have TupleVariant matching to SymbolKind::Constant and StructVariant to SymbolKind::Class. I also matched ExternType to SymbolKind::Module. Does this seem right?

What are the steps we should go about from here? From what I can tell, I think there has to be a new version of rls-data cut before opening PR's for rls-analysis and rls.

Yeah, I think that sounds right, you'll need to send PRs to rls-analysis and then the RLS. I'm making a release of rls-data - 0.11.0 - with your changes.

Was this page helpful?
0 / 5 - 0 ratings