rust-analyzer icon indicating copy to clipboard operation
rust-analyzer copied to clipboard

rust-analyzer didn't return warning/hint diagnostics on `textDocument/diagnostic`

Open fannheyward opened this issue 1 year ago • 11 comments

  1. rust-analyzer publishDiagnostics after open rust file, containing the warning/hint level diagnostics
  2. switch windows in editor, then the client requests textDocument/diagnostic for the same file, rust-analyzer returns empty diagnostics

Server trace logs, same logs with VSCode+rust-analzyer too, more logs https://github.com/fannheyward/coc-rust-analyzer/issues/1276#issuecomment-2462451351

[Trace - 14:56:20] Received notification 'textDocument/publishDiagnostics'.
Params: {
    "uri": "file:///Users/fannheyward/src/hello-rust/src/main.rs",
    "diagnostics": [
        {
            "range": {...},
            "severity": 2,
            "code": "unused_imports",
            "source": "rustc",
            "message": "unused import: `std::env`\n`#[warn(unused_imports)]` on by default",
            "relatedInformation": [...],
            "tags": [1],
            "data": {...}
        },
        {
            "range": {...}
            },
            "severity": 4,
            "code": "unused_imports",
            "source": "rustc",
            "message": "remove the whole `use` item",
            "relatedInformation": [...]
        }
    ],
    "version": 1
}

...

[Trace - 14:57:31] Sending request 'textDocument/diagnostic - (18)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/fannheyward/src/hello-rust/src/main.rs"
    }
}


[Trace - 14:57:31] Received response 'textDocument/diagnostic - (18)' in 2ms.
Result: {
    "kind": "full",
    "items": []
}

rust-analyzer version: rust-analyzer 0.0.0 (27e824fad4 2024-12-15)

rustc version: rustc 1.83.0 (90b35a623 2024-11-26)

editor or extension: nvim + coc.nvim

code snippet to reproduce:

use std::env;

fn main() {
    println!("Hello, world!");
}

fannheyward avatar Dec 18 '24 07:12 fannheyward

We use publishDiagnostics for the rustc/cargo errors, the pull model for our native ones. This works just fine in VSCode and the spec doesn't state anything regarding it. Though according to https://github.com/microsoft/language-server-protocol/issues/1743#issuecomment-1584057181 this setup is discouraged.

cc @rchl @jwortmann tagging you because of your comments in https://github.com/rust-lang/rust-analyzer/pull/18404 (I forgot about this sorry)

Veykril avatar Dec 18 '24 13:12 Veykril

This problem is not that RA is us ing push and pull mode at same time, but: RA can publish diagnostics at pushing mode, but no diagnostics at pulling mode.

fannheyward avatar Dec 18 '24 14:12 fannheyward

This works just fine in VSCode

From the tracking logs in VSCode, publishDiagnostics has two diagnostics, but empty result in pull request.

fannheyward avatar Dec 18 '24 14:12 fannheyward

We use the push mode for cargo diagnostics where as pull only pull rust-analyzer native ones. So as currently implemented in r-a there is no overlap between the two (pull diagnostics will never list cargo check ones, push diagnostics will never list rust-analyzer native ones). So clients (except for VSCode for some reason?) will naturally replace push results for a document with the pull results for the given document (as we don't even populate the result-id #18717 fixes that part so there is no distinguishing to be done either way).

I personally do think using both pull and push is valid, but the spec leaves that (+ how to handle such a case) completely unspecified.

Veykril avatar Dec 18 '24 14:12 Veykril

except for VSCode for some reason?

See my previous comment, VSCode also pulls empty result, just that it didn't replace/update the PROBLEMS list.

fannheyward avatar Dec 18 '24 14:12 fannheyward

It is correct that the pull there is empty, because the pull only pulls rust-analyzer native diagnostics (the ones we compute), so they do not include the diagnostic computed by us via cargo check (triggered when you save a file). It is intentional by our implementation that pull and push diagnostics do not yield the same set of diagnostics for a document.

Veykril avatar Dec 18 '24 14:12 Veykril

I see, thank you for the clarification. Maybe I can make an improvement in the client side for the both modes.

fannheyward avatar Dec 18 '24 14:12 fannheyward

I've also started to see this when using remote vscode in Cursor in the last few days. I can report that it happens in v0.3.2228 but not in v0.3.2220. I do have a somewhat weird setup that I'm using bazel and run script to generate the diagnostic messages via rust-analyzer.check.overrideCommand.

[Trace - 1:12:31 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {
    "uri": "file:///home/ajwerner/src/<redacted>",
    "diagnostics": [
        "<redacted>
    ],
    "version": 3
}


[Trace - 1:12:31 PM] Received notification '$/progress'.
Params: {
    "token": "rust-analyzer/flycheck/0",
    "value": {
        "kind": "end"
    }
}


[Trace - 1:12:31 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {
    "uri": "file:///home/ajwerner/src/<redacted>",
    "diagnostics": [],
    "version": 3
}

ajwerner avatar Jan 05 '25 18:01 ajwerner

To me https://github.com/microsoft/language-server-protocol/issues/1743#issuecomment-1584057181 sounds like this setup is intended to be supported but r-a should provide identifier here to distinguish the two kinds of diagnostics: https://github.com/rust-lang/rust-analyzer/blob/76567ee28dacac2636aeb6e5b6a9560ffca21d87/crates/rust-analyzer/src/lsp/capabilities.rs#L168

I'm thinking this was the intention of #18717 but maybe the wrong field?

export interface DiagnosticOptions extends WorkDoneProgressOptions {
	/**
	 * An optional identifier under which the diagnostics are
	 * managed by the client.
	 */
	identifier?: string;
// ...

the-mikedavis avatar Mar 02 '25 02:03 the-mikedavis

Yes we should definitely set that field, that is an oversight

Veykril avatar Mar 02 '25 06:03 Veykril

We use publishDiagnostics for the rustc/cargo errors, the pull model for our native ones. This works just fine in VSCode and the spec doesn't state anything regarding it. Though according to microsoft/language-server-protocol#1743 (comment) this setup is discouraged.

cc @rchl @jwortmann tagging you because of your comments in #18404 (I forgot about this sorry)

Is there a plan for the pull model to obtain the same diagnostic information as that of textDocument/publishDiagnostics?

geeker-smallwhite avatar Apr 14 '25 11:04 geeker-smallwhite