Diagnostics break for the entire file when an empty message is posted

[lnicola]

Does this issue occur when all extensions are disabled?: No

• VS Code Version: 1.69.1
• OS Version: Linux

Spotted in rust-lang/rust-analyzer#11404. I suspect the undefined in

vscode/src/vs/platform/markers/common/markerService.ts

Lines 201 to 203 in 5acd950

	if (!message) {
	return undefined;
	}

breaks the caller.

CC #5952

Repro (with rust-analyzer):

fn main() {
    $;
    compile_error!("");
    $;
}

[jrieken]

This is thrown in the extension host (here) and indicates bad extension behaviour. Depending on how the extension is written this likely prevent further errors but not errors from other extension.

Closing as extension-caused since they must provide valid diagnostic objects

[lnicola]

I don't think this is a bug in the extension. Rust code can legitimately emit compilation errors without a message. And the compiler does that too (for the related diagnostics), if I recall correctly.

[Veykril]

Curious but is it documented somewhere that the diagnostic message is not allowed to be empty? If not that should be at least written down somewhere.

Fwiw the language server protocol also does not say that a message has to be non-empty, so either that should be added to that there as well or in my eyes VSCode is missing something to handle that case there.

[lnicola]

@jrieken would you mind reconsidering this, given the two comments above?

		if (!message) {
			throw new TypeError('message must be set');
		}

I don't even know if this is intentional. The check might be for a missing message, but given the weakly-typed nature of JS, it will also trigger on empty-but-present messages. What breaks if we replace the condition by if (message == null)?

[jrieken]

Yes, this is very much intentional. The one change I should probably do is to also throw with whitespace-only strings. Diagnostics are user-facing elements, they represent an issue with source code and doing that without a message feels illogical.

Maybe I am not understanding your use-case but when would like to see an error squiggle (or entry in the problems views) without stating what the problem is? What's the use-case and how is that desirable?

[lnicola]

The one change I should probably do is to also throw with whitespace-only strings.

You're not throwing away the empty diagnostics, you're completely breaking them for the whole extension.

What's the use-case and how is that desirable?

1. Rust code, including libraries, can emit compiler errors with an empty message. That might not be a great experience for the users, but it's fully allowed and used. Sample (click the Run button). Some popular libraries do or did this at one point.
2. Rust compiler errors include multiple spans. Notice how the one on &'a i32 has no message:

We expand them to multiple diagnostics, in order to show them nicely:

If you delete the first span, you throw away information from the compiler:

And of course, by throwing that exception, you're preventing diagnostics from working in the whole file (or even project/workspace, I don't remember), even those that do have an associated message.

We could duplicate the message from &'b i32 in this case, but I'm not sure that works every time, and it would crowd out the diagnostic list. I think we did something like this at one point.

This was reported to multiple times and in different forms, so it's not just a theoretical concern.

[jrieken]

Why aren't you using DiagnosticRelatedInformation for such cases?

[lnicola]

We are. As far as I know, this was/is also an issue for those.

And that doesn't fix the compiler_error! case.