Skip to content

Duplicate references in wasm_bindgen types #13407

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

Closed
abseee opened this issue Oct 13, 2022 · 16 comments · Fixed by #13571
Closed

Duplicate references in wasm_bindgen types #13407

abseee opened this issue Oct 13, 2022 · 16 comments · Fixed by #13571
Labels
Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug E-has-instructions Issue has some instructions and pointers to code to get started

Comments

@abseee
Copy link

abseee commented Oct 13, 2022

When I issue a textDocument/references request on MyStruct in lib.rs, I expect 3 references. Instead I get 288! VSCode seems to dedupe these results before surfacing them in the UI but other editors may not. Dupes are also returned for MyEnum and its variants. Non-wasm-bindgen tagged types work fine; presumably other macros also trigger this issue but I don't know enough to debug further.

VSCode <-> Rust analyze trace
Expanded lib.rs

rust-analyzer version: 0.3.1238-standalone

rustc version: rustc 1.63.0 (4b91a6ea7 2022-08-08)

@vimalk78
Copy link

I am seeing same issue in nvim

@vimalk78
Copy link

i just upgraded to latest master and this issue is super annoying.
do we know which commit caused this? i would want to revert from latest master. Is there a way to de-dup in nvim?

@lnicola lnicola added Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug labels Oct 17, 2022
@Veykril
Copy link
Member

Veykril commented Oct 17, 2022

This behavior is there for at least a year judging from the commits (if not longer), fix is adding a .unique() after this here

.map(|file_ref| (file_ref.range, file_ref.category))

@Veykril Veykril added the E-has-instructions Issue has some instructions and pointers to code to get started label Oct 17, 2022
@vimalk78
Copy link

wow, is this something that editors need to take care of, and fixing it in rust-analyzer will cause additional issues?

@Veykril
Copy link
Member

Veykril commented Oct 17, 2022

No, we can easily fix this in r-a, and I think it makes sense to do so as well. Though I would expect clients to dedup this as well.

@vimalk78
Copy link

shall i test it and open a PR?

@Veykril
Copy link
Member

Veykril commented Oct 17, 2022

You are more then welcome to!

@abseee
Copy link
Author

abseee commented Oct 17, 2022

FWIW I bisected the issue to ebfbb31

@lnicola
Copy link
Member

lnicola commented Oct 17, 2022

That's not right. Prior to that commit, proc macros didn't work on your rustc version.

@lnicola
Copy link
Member

lnicola commented Oct 17, 2022

You can use the expansion output directly if you want to bisect it further.

@abseee
Copy link
Author

abseee commented Oct 17, 2022

Ah I thought VS Code would show a warning if the proc_macros weren't expanded in rust-analyzer. Was I supposed to build r-a with a matching older version of rustc? Unfortunately I can't repro the duplicate references in the macro expanded source file.

@lnicola
Copy link
Member

lnicola commented Oct 17, 2022

Ah I thought VS Code would show a warning if the proc_macros weren't expanded in rust-analyzer.

I think it should print an error in the logs, at least, not sure when that was added. You can check by running Expand macro, if that's easy to do from Helix.

Was I supposed to build r-a with a matching older version of rustc?

No, you need to change your project toolchain (rustup override or rustup default) to a supported version. The RA build version doesn't matter.

@vimalk78
Copy link

fix is adding a .unique() after this here

i tried adding tracing::info!() lines at multiple places in function find_all_refs but it didn't seem to get logged

@lnicola
Copy link
Member

lnicola commented Oct 18, 2022

You'll need to do something like https://github.com/rust-lang/rust-analyzer/tree/master/docs/dev#logging (replace lsp_server with the module you want).

@vimalk78
Copy link

i am using RA_LOG=rust_analyzer=debug ra-multiplex-server

ra-multiplex-server is from https://github.com/pr2502/ra-multiplex/

@lnicola
Copy link
Member

lnicola commented Oct 18, 2022

Try ide=debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug E-has-instructions Issue has some instructions and pointers to code to get started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants