Looking up diagnostics by URL is ambigous

[Blaze2305]

Summary

Errors do not show up in the local diagnostic picker (Shift g) while writing rust code with the rust analyzer. But they do show up in the global diagnostic picker (Shift G). Screenshots below as an example.

Note that issues are shown in the top right of the screen if my cursor is on the error. As well as indicating the error presence on the left diagnostic gutter.

Code with obvious issues

Empty local diagnostic picker

Global picker has all the errors and warnings.

I found this issue only with rust and rust analyzer. Other languages work well, with file issues in local and workspace issues in the global diagnostic picker.

Helix :

• built from source ( dfc31e7 )
• cargo version : cargo 1.62.0 (a748cf5a3 2022-06-08)
• rustc version : rustc 1.62.0 (a8314ef7d 2022-06-27)

Rust Analyzer:

• built from source ( 897a7ec4b )

I have attached helix logs for today and the last time I used helix while omitting the rest as those would be too much. If you need more comprehensive date ranged logs, I'd be happy to provide them.

I don't have much experience with rust so I am not sure if this is a rust issue, a rust analyzer issue, or a helix issue.

Reproduction Steps

I tried this:

1. hx
2. Open a rust file.
3. Add some code with errors (any kind which rust analyzer will notify)
4. Use local diagnostic picker (Shift g)

I expected this to happen:

The issues in the currently open file to show up there

Instead, this happened:

no errors listed in the local diagnostic picker.

Opening global diagnostic picker (Shift G) shows the errors as expected.

Helix log

~/.cache/helix/helix.log

2022-07-24T00:50:06.819 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:53:07.436 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:53:33.455 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:54:19.304 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T00:54:19.305 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T00:54:27.368 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:57:34.012 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:57:52.634 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T00:58:32.992 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T00:59:50.604 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T00:59:50.605 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T01:01:26.954 helix_view::editor [ERROR] Failed to initialize the LSP for `source.toml` { cannot find binary path }
2022-07-24T01:05:15.300 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-24T01:05:15.302 helix_lsp::transport [ERROR] err: <- StreamClosed
2022-07-31T01:16:44.031 helix_lsp::transport [ERROR] <- ServerError(-32801): waiting for cargo metadata or cargo check
2022-07-31T01:16:45.551 helix_lsp::transport [ERROR] <- ServerError(-32801): waiting for cargo metadata or cargo check
2022-07-31T01:16:46.149 helix_lsp::transport [ERROR] <- ServerError(-32801): waiting for cargo metadata or cargo check
2022-07-31T01:16:47.801 helix_lsp::transport [ERROR] err <- "[ERROR project_model::workspace] cyclic deps: helix_view(CrateId(66)) -> helix_tui(CrateId(60)), alternative path: helix_tui(CrateId(60)) -> helix_view(CrateId(66))\n"
2022-07-31T01:17:10.680 helix_lsp::transport [ERROR] err <- "[ERROR project_model::workspace] cyclic deps: helix_view(CrateId(66)) -> helix_tui(CrateId(60)), alternative path: helix_tui(CrateId(60)) -> helix_view(CrateId(66))\n"
2022-07-31T01:18:54.795 helix_term::application [WARN] unhandled window/showMessage: ShowMessageParams { typ: Error, message: "overly long loop turn: 137.2607ms" }

Platform

Windows

Terminal Emulator

windows terminal 1.14.1963.0

Helix Version

22.05-272-gdfc31e74

[the-mikedavis]

Can you post the log when running in verbose mode (-vv)?

The difference between the workspace and non-workspace pickers is only the filtering of the current LSP URI:

helix/helix-term/src/commands/lsp.rs

Lines 376 to 422 in e405e88

	pub fn diagnostics_picker(cx: &mut Context) {
	let doc = doc!(cx.editor);
	let language_server = language_server!(cx.editor, doc);
	if let Some(current_url) = doc.url() {
	let offset_encoding = language_server.offset_encoding();
	let diagnostics = cx
	.editor
	.diagnostics
	.get(&current_url)
	.cloned()
	.unwrap_or_default();
	let picker = diag_picker(
	cx,
	[(current_url.clone(), diagnostics)].into(),
	Some(current_url),
	DiagnosticsFormat::HideSourcePath,
	offset_encoding,
	);
	cx.push_layer(Box::new(overlayed(picker)));
	}
	}
	pub fn workspace_diagnostics_picker(cx: &mut Context) {
	let doc = doc!(cx.editor);
	let language_server = language_server!(cx.editor, doc);
	let current_url = doc.url();
	let offset_encoding = language_server.offset_encoding();
	let diagnostics = cx.editor.diagnostics.clone();
	let picker = diag_picker(
	cx,
	diagnostics,
	current_url,
	DiagnosticsFormat::ShowSourcePath,
	offset_encoding,
	);
	cx.push_layer(Box::new(overlayed(picker)));
	}
	impl ui::menu::Item for lsp::CodeActionOrCommand {
	type Data = ();
	fn label(&self, _data: &Self::Data) -> Spans {
	match self {
	lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str().into(),
	lsp::CodeActionOrCommand::Command(command) => command.title.as_str().into(),
	}
	}
	}

Seems like the current documents URI is not being detected correctly for you

[Blaze2305]

I ran helix in verbose mode and have uploaded the log of the run.

helix.log

[the-mikedavis]

It looks like this file is outside a cargo project's module tree, right? You might be running into rust-analyzer's limitations on non-cargo projects (without extra build info in rust-project.json): https://rust-analyzer.github.io/manual.html#non-cargo-based-projects

Can you reproduce this on a file in a cargo project's module tree?

[Blaze2305]

But the example I showed was in a cargo project ( see screenshot below)

The file with the error is test.rs.

EDIT :

I tested the same in a non cargo directory. For those files, the rust analyzer does not work at all.
No errors \ warnings , even the gutter diagnostic is missing.

[the-mikedavis]

Ah ok. I can't reproduce this on linux so I think it may be a difference in windows with how we (or rust-analyzer) is tracking URIs. Probably / vs \ :P. I don't have a windows machine to debug this but you may be able to add some log::error!s to the above code-blocks to dump out the URIs and see why the filtering isn't working as expected

[poliorcetics]

I have not managed to reproduce either on macOS and have been using both pickers since they were added to master, so I second the-mikedavis, it's probably a windows-specific bug

[Frojdholm]

I've managed to reproduce this. I think it's due to the fact that the drive letter has differing case internally in Helix compared to what is being sent by rust-analyzer. This causes the URLs to not be compared equal so the diagnostics are not found.

In the general case it will be difficult to know which case the sending LSP will use. I think drive letters in Windows are always upper case, and either way paths are case insensitive.

This seems like something that is better solved upstream, the URL crate is simply comparing the serialized URLs (the string representation seen in the logs in this case) for efficiency, but it's unfortunate that this causes problems on Windows. In the meantime if comparing paths is the only way to solve the filtering problem we should probably override the comparisons of URLs and always lower case them.

Below is a relevant excerpt from the logs, note the differing case of the drive letter.

2022-08-06T14:51:15.710 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///c:/<...>/helix/helix-core/src/shellwords.rs","diagnostics":[<...>],"version":1}}
2022-08-06T14:51:16.608 helix_view::document [ERROR] doc.url() C:\<...>\helix\helix-core\src\shellwords.rs
2022-08-06T14:51:16.608 helix_term::commands::lsp [ERROR] current_url file:///C:/<...>/helix/helix-core/src/shellwords.rs
2022-08-06T14:51:16.608 helix_term::commands::lsp [ERROR] url file:///c:/<...>/helix/helix-core/src/shellwords.rs, diagnostics 5, urls equal? false

[Frojdholm]

I went to make an issue on the URL crate and sadly it already implements the spec as written regarding URL comparison.

https://url.spec.whatwg.org/#url-equivalence

4.6. URL equivalence

To determine whether a URL A equals URL B, with an optional boolean exclude fragments
(default false), run these steps:

Let serializedA be the result of serializing A, with exclude fragment set to exclude fragments.

Let serializedB be the result of serializing B, with exclude fragment set to exclude fragments.

Return true if serializedA is serializedB; otherwise false.

The remaining options as I see it are either:

1. Make rust-analyzer change to use upper case drive letter
2. Change the drive letter internally in Helix
3. Write custom URL comparison logic that takes this into account

Option 1. and 2. might break for other LSP servers.

[poliorcetics]

Wow good find, that explains why reproducing it failed on macOS and Linux.

On the std::path, the docs say:

Case sensitivity

Unless otherwise indicated path methods that do not access the filesystem, such as Path::starts_with and Path::ends_with, are case sensitive no matter the platform or filesystem. An exception to this is made for Windows drive letters.

We don't need to lower all the times, just use Path comparison:

use std::path::Path;

fn main() {
    let lower = Path::new("c:\\MyPath\\Is\\Here");
    let upper = Path::new("C:\\MyPath\\Is\\Here");

    dbg!(upper == lower);
    for (a, b) in lower.components().zip(upper.components()) {
        dbg!(a, b, a == b);
    }
}

Running this on a Windows VM gives true everywhere for me. That would avoid doing a lowering operation everytime, since it would only be done for the first character (and I'm not sure it's not simply a giant match, not really a lowering)

[Frojdholm]

Yes, using std::path might be a good solution. I don't know if using a URL has any specific purpose, is there a usecase for accessing files through other types of URLs than file:///? I guess technically it's possible to access remote files..

There is an open issue in the URL standard repo for the windows drive letter whatwg/url#515, but it hasn't seen much activity.

[poliorcetics]

After some exploration, it could be somewhat hard to fix properly. The following program consider the C:/c: a Normal path component on Windows, not a Prefix:

use std::path::Path;

fn main() {
    let url = url::Url::parse("file://C:\\test\\Path").unwrap();
    let path = Path::new(url.path());
    dbg!(path.components().collect::<Vec<_>>());

    let url = url::Url::parse("file://c:\\test\\Path").unwrap();
    let path = Path::new(url.path());
    dbg!(path.components().collect::<Vec<_>>());
}

Run in Windows Powershell:

[src/main.rs:6] path.components().collect::<Vec<_>>() = [
    RootDir,
    Normal(
        "C:",
    ),
    Normal(
        "test",
    ),
    Normal(
        "Path",
    ),
]
[src/main.rs:10] path.components().collect::<Vec<_>>() = [
    RootDir,
    Normal(
        "c:",
    ),
    Normal(
        "test",
    ),
    Normal(
        "Path",
    ),
]

Simply lowercasing everything is not valid since filesystems respecting case exist.

[Frojdholm]

Simply lowercasing everything is not valid since filesystems respecting case exist.

Yeah, either way there's no way of knowing if the URL is targeting a filesystem that is case-insensitive. The only "guaranteed" way of comparing two URLs is to compare whatever they're pointing too, but that is obviously not feasible here.

The format for the drive letter is written in the spec and will pretty much always be of the form <ASCII alpha + :> (I've never seen the pipe character be used instead of a colon). If we changed that part of the path to normalize it the rest of the code should work as is.

Basically check if the path contains a drive letter and in that case always change it to either upper or lower case.

[poliorcetics]

I just submitted a PR to fix this. I don't use a Windows machine outside of VMs so if someone who does could test this works for them that would be a nice bonus :)

[aminnairi]

I had the issue where Rust LSP did not pick anything.

This was because I was not in a cargo project (I guess you call that like that, I'm a newbie in Rust for now).

Using a cargo project Helix is able to display errors correctly.

Using Arch Linux with rust-analyzer installed from the community repository.

[crtado]

I am experiencing this same issue. I don't see new commits related to this (correct me if I'm wrong), but I can spend some time looking into this if the discussion in #3347 is still relevant, i.e. the suggestion to convert a URI from the language server to Path with to_file_path for local use.