This repository has been archived by the owner on Dec 29, 2022. It is now read-only.
Don't return symbols with empty names #1193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xanewok
suggested changes
Dec 17, 2018
src/actions/requests.rs
Outdated
| @@ -112,6 +112,7 @@ impl RequestAction for Symbols { | |||
|
|
|||
| Ok(symbols | |||
| .into_iter() | |||
| .filter(|s| !s.name.is_empty()) | |||
There was a problem hiding this comment.
Could we add a HACK stating that this is working around VSCode symbol handling logic?
There was a problem hiding this comment.
Good idea, especially since LSP is ambiguous.
Although LSP does not define what constitutes a valid symbol name, VS Code's client implementation explicitly prohibits symbols with empty names. Currently, RLS returns such symbols in many cases (for example, the root module of a binary crate). These symbols not only fail to parse in VS Code, but in fact cause it to throw an exception that prevents the symbol list from being populated *at all*. To fix this, filter out symbols with empty names before returning them to the client.
tchebb
force-pushed
the
no-empty-symbols
branch
from
9e31676
to
1210105
Compare
|
Filed LSP issue at microsoft/language-server-protocol#641. |
Xanewok
approved these changes
Dec 17, 2018
|
Thank you! bors r+ EDIT: I forgot we didn't update bors config after CI update 🤦♂️ I'll merge this when CI is green. |
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Dec 29, 2018
Update cargo, rls, miri Update cargo, rls, miri Added `rustc-workspace-hack` to miri so that it shares the same features for serde as other tools. cc @alexcrichton ## cargo 25 commits in 2cf1f5dda2f7ed84e94c4d32f643e0f1f15352f0..0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4 2018-12-11 03:44:04 +0000 to 2018-12-19 14:45:14 +0000 - Remove Stale bot's configuration (rust-lang/cargo#6463) - Add labels to issue templates (rust-lang/cargo#6464) - Fix new man page links. (rust-lang/cargo#6459) - Fix metabuild compile errors with --message-format=json. (rust-lang/cargo#6432) - Support alt-registry names in [patch] table. (rust-lang/cargo#6456) - Update the rustup URL (rust-lang/cargo#6455) - New man pages. (rust-lang/cargo#6405) - Reify the DepFingerprint type (rust-lang/cargo#6451) - Extract Fingerprint::new (rust-lang/cargo#6449) - Upgrade the metabuild to Rust 2018 (rust-lang/cargo#6448) - Make edition comparing code consistent (rust-lang/cargo#6450) - Document `name` and `authors` in [package] (rust-lang/cargo#6447) - Travis: only use mdbook 0.1.7. (rust-lang/cargo#6443) - Update git2-curl requirement from 0.8.1 to 0.9.0 (rust-lang/cargo#6439) - Update git2 requirement from 0.7.5 to 0.8.0 (rust-lang/cargo#6438) - Display errors when `cargo fix` fails. (rust-lang/cargo#6419) - cargo fix: fix targets with shared sources. (rust-lang/cargo#6434) - Fix panic-in-panic in tests. (rust-lang/cargo#6431) - More Rust 2018 edition cleanups (rust-lang/cargo#6422) - Cleanup some trait impls for SourceId (rust-lang/cargo#6429) - Remove a nightly check from doc tests (rust-lang/cargo#6427) - Replace CargoError with failure::Error (rust-lang/cargo#6425) - Allow testsuite warnings in dev (rust-lang/cargo#6426) - add `--dry-run` option to cargo update (rust-lang/cargo#6371) - Migrate to some Rust 2018 idioms (rust-lang/cargo#6416) ## rls 16 commits in bd5b899afb05e14d33e210ede3da241ca1ca088f..6f5e4bba7b1586fca6e0ea7724cadb5683b2f308 2018-12-10 08:53:00 +0100 to 2018-12-21 17:11:08 +0100 - Update jsonrpc-core (rust-lang/rls#1206) - Use `home_dir` from `home` crate (rust-lang/rls#1207) - Update cargo. (rust-lang/rls#1204) - Fix deprecated `trim_{left,right}` warnings (rust-lang/rls#1203) - Respect ${CARGO,RUSTUP}_HOME for tooltip relative dirs (rust-lang/rls#1201) - Separate tooltip tests that require Racer fallback (rust-lang/rls#1200) - tests: Don't generate tooltip results in tests/fixtures (rust-lang/rls#1199) - Overhaul fixture handling in tests (rust-lang/rls#1190) - Don't return symbols with empty names (rust-lang/rls#1193) - Don't check AppVeyor CI status for bors - Properly infer full_docs (rust-lang/rls#1192) - Update cargo (rust-lang/rls#1191) - Improve hover test_tooltip tests (rust-lang/rls#1175) - Fix unused warnings (rust-lang/rls#1185) - Workaround rust-lang/rls#703 to prevent obscure failures due to sccache. (rust-lang/rls#1177) - Disable travis cache (rust-lang/rls#1182) ## miri 14 commits in bccadeb..6c2fc6d 2018-12-08 11:07:22 +0100 to 2018-12-26 14:28:25 +0100 - use memory::check_bounds_ptr for offset check (rust-lang/miri#589) - Fix comparing function pointers (rust-lang/miri#587) - fix for infallible allocation (rust-lang/miri#586) - fix test for latest nightly (rust-lang/miri#585) - Treat ref-to-raw cast like a reborrow: do a special kind of retag (rust-lang/miri#572) - Test cargo-miri on Windows (rust-lang/miri#578) - Cargo miri tweaks and test that we can exclude tests (rust-lang/miri#580) - Fix cargo miri test (rust-lang/miri#550) - fix for latest nightly (rust-lang/miri#574) - Add rustc-workspace-hack. (rust-lang/miri#575) - use RUSTC_WRAPPER for the cargo hook (rust-lang/miri#573) - do not auto-detect the targets in the sysroot, instead specify target manually through env var (rust-lang/miri#570) - Cleanup: Avoid repeating signatures, get rid of to_bytes hack (rust-lang/miri#568) - Support building and running with full MIR on foreign architectures, drop support for missing MIR (rust-lang/miri#566)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Although LSP does not define what constitutes a valid symbol name, VS Code's client implementation explicitly prohibits symbols with empty names. Currently, RLS returns such symbols in many cases (for example, the root module of a binary crate). These symbols not only fail to parse in VS Code, but in fact cause it to throw an exception that prevents the symbol list from being populated at all.
To fix this, filter out symbols with empty names before returning them to the client.