rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.08k stars 1.57k forks source link

ASCII color codes are displayed inline for for editors #15443

Open lindhe opened 1 year ago

lindhe commented 1 year ago

Sometimes, the error shown in Vim (and apparently VS Code too) is printed with ASCII color codes showing in the string from rust_analyzer. It makes the error very hard to read. See https://github.com/neovim/nvim-lspconfig/issues/2760.

It seems to me like rust-analyzer does not properly distinguish between being executed in a terminal environment or not, and decide from that whether to print ASCII color codes or skip them.

In terminal:

Screenshot from 2023-08-11 16-16-58

In editor:

Screenshot from 2023-08-11 10-51-47

Steps to reproduce

  1. Create the following crate:
cargo new -q foo
cd foo
cargo add -q rocket@0.5.0-rc.3
cat <<EOF > src/main.rs
use rocket::UriDisplayPath;

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

#[derive(UriDisplayPath)]
pub enum Foo {
    Bar,
}
EOF
  1. Optionally run cargo check.
  2. Open src/main.rs with Vim.

Other info

rust-analyzer version: (eg. output of "rust-analyzer: Show RA Version" command, accessible in VSCode via Ctrl/⌘+Shift+P)

Mason reports rust-analyzer 2023-08-07.

rustc version: (eg. output of rustc -V)

$ rustc -V
rustc 1.71.1 (eb26296b5 2023-08-03)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)

Not sure, nothing I've set explicitly at least. I run NeoVim with rust-lang/rust.vim and rust_analyzer for lspconfig installed via Mason.

Veykril commented 1 year ago

Your error message for some reason contains a green [note] part, which is what you are seeing inline. That should not be there as that is not the normal diagnostic output of the compiler. This is what I am getting here image I don't know what could cause this for you but something seems to fiddle with your compiler output?

tadeokondrak commented 1 year ago

I think the derive macro is emitting escape codes (when using a stable compiler): https://github.com/SergioBenitez/proc-macro2-diagnostics/blob/master/src/line.rs#L79

Veykril commented 1 year ago

Ah good catch, that's indeed gonna be the issue. There isn't really anything we can do here then, r-a just spits out the error messages we get in json verbatim as the expectation is for compiler diagnostics to be just text. This might be something to request as a feature for the proc-macro api, since the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

lindhe commented 1 year ago

Ah, there it is! Thanks so much for identifying the real culprit for me! I'll continue my hunt over there.

lindhe commented 1 year ago

Interestingly, it seems like cargo build and proc-macro properly toggles color output sometimes!

Here's what it looks like in my terminal, and the color codes are not printed!

Screenshot from 2023-08-12 10-01-05

It seems like proc-macro do attempt to use different logic depending on whether or not color is set:

#[cfg(all(feature = "colors", not(nightly_diagnostics)))]

@Veykril, you mentioned "the error messages we get in json". It seems like cargo build --message-format has a few different options that might be of relevance here: json, json-diagnostic-short, json-diagnostic-rendered-ansi and json-render-diagnostics.

What field do you render? Is it message.message?

The full JSON object

```json { "reason": "compiler-message", "package_id": "foo 0.1.0 (path+file:///home/andreas/tmp/foo)", "manifest_path": "/home/andreas/tmp/foo/Cargo.toml", "target": { "kind": [ "bin" ], "crate_types": [ "bin" ], "name": "foo", "src_path": "/home/andreas/tmp/foo/src/main.rs", "edition": "2021", "doc": true, "doctest": false, "test": true }, "message": { "rendered": "error: [note] error occurred while deriving `UriDisplay`\n --> src/main.rs:7:10\n |\n7 | #[derive(UriDisplayPath)]\n | ^^^^^^^^^^^^^^\n |\n = note: this error originates in the derive macro `UriDisplayPath` (in Nightly builds, run with -Z macro-backtrace for more info)\n\n", "children": [], "code": null, "level": "error", "message": "\u001b[1;34m[\u001b[0m\u001b[1;32mnote\u001b[0m\u001b[1;34m] \u001b[0m\u001b[1;39merror occurred while deriving `UriDisplay`\u001b[0m", "spans": [ { "byte_end": 98, "byte_start": 84, "column_end": 24, "column_start": 10, "expansion": { "def_site_span": { "byte_end": 36109, "byte_start": 36044, "column_end": 66, "column_start": 1, "expansion": null, "file_name": "/home/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rocket_codegen-0.5.0-rc.3/src/lib.rs", "is_primary": false, "label": null, "line_end": 1060, "line_start": 1060, "suggested_replacement": null, "suggestion_applicability": null, "text": [ { "highlight_end": 66, "highlight_start": 1, "text": "pub fn derive_uri_display_path(input: TokenStream) -> TokenStream {" } ] }, "macro_decl_name": "#[derive(UriDisplayPath)]", "span": { "byte_end": 98, "byte_start": 84, "column_end": 24, "column_start": 10, "expansion": null, "file_name": "src/main.rs", "is_primary": false, "label": null, "line_end": 7, "line_start": 7, "suggested_replacement": null, "suggestion_applicability": null, "text": [ { "highlight_end": 24, "highlight_start": 10, "text": "#[derive(UriDisplayPath)]" } ] } }, "file_name": "src/main.rs", "is_primary": true, "label": null, "line_end": 7, "line_start": 7, "suggested_replacement": null, "suggestion_applicability": null, "text": [ { "highlight_end": 24, "highlight_start": 10, "text": "#[derive(UriDisplayPath)]" } ] } ] } } ```

BTW, I notice that cargo build prints the [note] while rustc does not. What do rust-analyzer use internally to build/check?

EDIT 1: To my surprise, json and json-diagnostic-rendered-ansi has the same behavior:

Screenshot from 2023-08-12 10-48-39

lindhe commented 1 year ago

Rats! I think this might be a bug in Cargo. Even with --color=never, the color feature appears to be set when building using --json:

Screenshot from 2023-08-12 11-50-16

Veykril commented 1 year ago

No, there is no bug here. The colored [note] is created by the proc-macro itself, no tool has control over that proc-macro emitting color codes or not. Hence why I said

This might be something to request as a feature for the proc-macro api, since the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

earlier

lindhe commented 1 year ago

the compiler itself has a flag that toggles whether the json output has colors in it or not

What flag is that, if not --color?

Veykril commented 1 year ago

That flag controls the color of the compilers output colors for diagnostic, the color you are seeing here (that is the [note] part) is created by the proc-macro though, not by the compiler. The compiler merely emits the message that the proc-macro tells it to.

lindhe commented 1 year ago

Yes, but proc-macro emits that conditionally on whether or not the color feature is set:

https://github.com/SergioBenitez/proc-macro2-diagnostics/blob/45fa2d68669d10daeaf0f9b664a96576626fae16/src/line.rs#L78C1-L78C1

Veykril commented 1 year ago

Yes, that is a feature flag. Passing --color or not to the compiler is rrelevant to that though, as that is not a flag that sets the feature. In fact, crates cannot observe --color being passed or not whatsoever

lindhe commented 1 year ago

Oh, that's interesting. Any idea how on earth proc-macro is able to distinguish between colorized and plain output then? Because, as you can see in my screenshots, it does not colorize "[note]" when I pipe it to cat.

lindhe commented 1 year ago

the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

Do you have any pointers to how one checks that flag?

lindhe commented 1 year ago

That flag controls the color of the compilers output colors for diagnostic, the color you are seeing here (that is the [note] part) is created by the proc-macro though, not by the compiler. The compiler merely emits the message that the proc-macro tells it to.

Passing --color or not to the compiler is rrelevant to that though, as that is not a flag that sets the feature. In fact, crates cannot observe --color being passed or not whatsoever

I am sure you are right, I just don't understand. What trips me up here is that [note] is printed in color when setting --color=always but printed not in color when setting --color=never (see screenshot below). Unless Cargo or the compiler does a text replacement to strip color codes from the output, which I very much doubt, I fail to see how it can be the case that --color=always is not picked up by the proc-macro.

Screenshot from 2023-08-12 19-03-46

SergioBenitez commented 11 months ago

All, copying my comments from https://github.com/SergioBenitez/proc-macro2-diagnostics/issues/3#issuecomment-1712314620 here for discourse:

The issue [...] is that as far as I can tell, there's no way for a proc-macro to know if it should emit colors or not. The proxy is typically to check whether we're connected to a TTY, but Cargo captures all output, so we're never connected to a TTY, and detecting via this manner would mean never emitting colors, which would be a shame.

Ideally, there would be some way to query 1) whether cargo itself will be emitting colors and/or 2) what message-format is expected. Alternatively, if we could inquire about how the compiler is invoked, we might be able to patch this minimally for rust-analyzer. It doesn't seem like 1) or 2) are viable options now, but the alternative might be, given that rust-analyzer appears to set RUSTC_WRAPPER to rust-analyzer by default. If we can detect this, we can stop emitting colors when rust-analyzer invokes rustc.

[...] the above didn't work. First, the wrapper is only set for build scripts, which is fine-ish as we can forward the env-var along via Cargo. Second, and more importantly, the wrapper is only set the first time the build script is built/run, so the solution only really works once in a given workspace, if it works at all.

Until there's some reliable mechanism to detect whether we should color, I think the "correct" solution here is for rust-analyzer to strip ANSI escapes from any output before it re-emits it. Cargo itself uses https://crates.io/crates/strip-ansi-escapes, which is why using --color=never works as expected.

Veykril commented 11 months ago

Interesting, so cargo already handles this specially itself as well? In that case you are right, we should do the same as cargo here when processing compile_error! invocations (and also we should pass --color=never to cargo check, I don't think we are doing that currently)

DriedYellowPeach commented 10 months ago

Is there any solution I can avoid this right now? I tried add color=never in my .toml, but it only avoid color output in cargo build, not in diagnostics. Can I setup rust-analyzer or something?

lindhe commented 10 months ago

I know of no work-around until they fix this.

SergioBenitez commented 10 months ago

One "work-around" is to use a nightly toolchain, at least for rust-analyzer.