Fix diagnostic fixes showing up everywhere #253
Conversation
The LSP code action request always returned the fixes for all diagnostics anywhere in the file, because of a shadowed variable.
|
Excellent catch! As for the testing, I am not quite sure myself. I think it makes sense to write a small amount of integration tests for That probably means that yes, we need a heavy test here. OTOH, I am not entirely sure that heavy tests is the right approach here. A possible alternative would be to write a client-side test (that is, spawn a headless vs code process, spawn an lsp_server binary, etc). This seems more trustworthy, b/c it actually tests interaction between the client and the server, but it should be hard to write reliably: we need to do some contortions (internal_mode) to synchronize the All things considered, using heavy tests for the time being seems like the best option. The "file names are not determenistic" problem should be solved by improving the test fixture. The way we solve this in Cargo is that you can use Here's the code that does this: https://github.com/rust-lang/cargo/blob/485670b3983b52289a2f353d589c57fae2f60f82/tests/testsuite/support/mod.rs#L1223-L1276 I suggest copy-pasting it to the test-utils crate, or, better, fishing on crates.io for something which allows to approximately compare JSON. |
|
Let's merge this as is for now, but a test would be very welcome here! bors r+ |
253: Fix diagnostic fixes showing up everywhere r=matklad a=flodiebold The LSP code action request always returned the fixes for all diagnostics anywhere in the file, because of a shadowed variable. There's no test yet; I wasn't sure where to add it. I tried adding one in `heavy_tests`, but that's a bit uncomfortable because the code action response contains the (random) file paths. I could make it work, but wanted to ask beforehand what you think. 256: Improve/add use_item documentation r=matklad a=DJMcNab Adds some documentation to use_item explaining all code paths (use imports are hard, especially with the ongoing discussion of anchored v. uniform paths - see rust-lang/rust#55618 for what appears to be the latest developments) Co-authored-by: Florian Diebold <flodiebold@gmail.com> Co-authored-by: DJMcNab <36049421+djmcnab@users.noreply.github.com>
Build failed (retrying...) |
253: Fix diagnostic fixes showing up everywhere r=matklad a=flodiebold The LSP code action request always returned the fixes for all diagnostics anywhere in the file, because of a shadowed variable. There's no test yet; I wasn't sure where to add it. I tried adding one in `heavy_tests`, but that's a bit uncomfortable because the code action response contains the (random) file paths. I could make it work, but wanted to ask beforehand what you think. Co-authored-by: Florian Diebold <flodiebold@gmail.com>
Build succeeded |
The LSP code action request always returned the fixes for all diagnostics anywhere in the file, because of a shadowed variable.
There's no test yet; I wasn't sure where to add it. I tried adding one in
heavy_tests, but that's a bit uncomfortable because the code action response contains the (random) file paths. I could make it work, but wanted to ask beforehand what you think.