Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

RLS categorizes enums as arrays #140

Closed
pravic opened this issue Aug 29, 2017 · 7 comments
Closed

RLS categorizes enums as arrays #140

pravic opened this issue Aug 29, 2017 · 7 comments
Labels

Comments

@pravic
Copy link

pravic commented Aug 29, 2017

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,
}
@nrc nrc added the bug label Aug 30, 2017
@DSpeckhals
Copy link
Contributor

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

@nrc
Copy link
Member

nrc commented Sep 20, 2017

@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.

@DSpeckhals
Copy link
Contributor

DSpeckhals commented Sep 21, 2017

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:

@nrc
Copy link
Member

nrc commented Sep 21, 2017

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'.

@DSpeckhals
Copy link
Contributor

@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.

@nrc
Copy link
Member

nrc commented Sep 24, 2017

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.

@DSpeckhals
Copy link
Contributor

This is resolved:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants