editor: Fix diagnostic rendering when semantic tokens set to full

[lingyaochu]

Self-Review Checklist:

• I've reviewed my own diff for quality, security, and reliability
• Unsafe blocks (if any) have justifying comments
• The content is consistent with the UI/UX checklist
• Tests cover the new/changed behavior
• Performance impact has been considered and is acceptable

Closes #50212

There are two unreasonable coupling account for this issue, the coupling of use_tree_sitter with languge_aware in

zed/crates/editor/src/element.rs

Lines 3820 to 3822 in 7892b93

	let use_tree_sitter = !snapshot.semantic_tokens_enabled
	|| snapshot.use_tree_sitter_for_syntax(rows.start, cx);
	let chunks = snapshot.highlighted_chunks(rows.clone(), use_tree_sitter, style);

and the coupling of language_aware with diagnostics in

zed/crates/language/src/buffer.rs

Lines 3736 to 3746 in 7892b93

	pub fn chunks<T: ToOffset>(&self, range: Range<T>, language_aware: bool) -> BufferChunks<'_> {
	let range = range.start.to_offset(self)..range.end.to_offset(self);
	let mut syntax = None;
	if language_aware {
	syntax = Some(self.get_highlights(range.clone()));
	}
	// We want to look at diagnostic spans only when iterating over language-annotated chunks.
	let diagnostics = language_aware;
	BufferChunks::new(self.text.as_rope(), range, syntax, diagnostics, Some(self))
	}

Because of these couplings, when the editor stops using Tree-sitter highlighting when "semantic_tokens" set to "full", it also accidentally stops fetching diagnostic information. This is why error and warning underlines disappear.

I’ve fixed this by adding a separate use_tree_sitter parameter to highlighted_chunks. This way, we can keep language_aware true to get the diagnostic data we need, but still decide whether or not to apply Tree-sitter highlights. I chose to fix this at the highlighted_chunks level because I’m worried that changing the logic in the deeper layers of the DisplayMap or Buffer might have too many side effects that are hard to predict. This approach feels like a safer way to solve the problem.

Release Notes:

• Fixed a bug where diagnostic underlines disappeared when "semantic_tokens" set to "full"

[SomeoneToIgnore]

Great catch, thank you.

One question: why skip the most important part?

[SomeoneToIgnore]

Oh, I see now, sorry: it's the rendering code that we're hitting here purely, so it's hard to test — would be amazing if we can get it tested somehow still, but not insisting now.

What's confusing though is this bit:

zed/crates/language/src/buffer.rs

Lines 3736 to 3746 in cb99ab4

	pub fn chunks<T: ToOffset>(&self, range: Range<T>, language_aware: bool) -> BufferChunks<'_> {
	let range = range.start.to_offset(self)..range.end.to_offset(self);
	let mut syntax = None;
	if language_aware {
	syntax = Some(self.get_highlights(range.clone()));
	}
	// We want to look at diagnostic spans only when iterating over language-annotated chunks.
	let diagnostics = language_aware;
	BufferChunks::new(self.text.as_rope(), range, syntax, diagnostics, Some(self))
	}

it seems wasteful to get language highlights for the full case here — which we will do due to the new code, won't we?
Can we rework the code somehow to avoid getting the highlights but do get the diagnostics?

Would be good to get tests, if possible.

[lingyaochu]

it seems wasteful to get language highlights for the full case here — which we will do due to the new code, won't we?
Can we rework the code somehow to avoid getting the highlights but do get the diagnostics?

Very insightful point! Ideally, when using the "full" mode, Tree-sitter highlighting is unnecessary, and skipping it would indeed be a performance win and also make the architecture cleaner.

However, achieving this would require decoupling diagnostics from syntax highlighting at a very low level. The language_aware parameter currently travels through a long chain of layers: highlighted_chunks -> BlockSnapshot -> WrapSnapshot -> TabSnapshot -> FoldSnapshot -> InlaySnapshot -> MultiBufferSnapshot, and finally into Buffer::chunks().

Modifying the signature of chunks() to independently control diagnostics would force changes across all these intermediate snapshots, not to mention other parts of the codebase that depend on them. I also looked into extending the Highlights struct to pass these settings down, but it is also heavily used and wouldn't lead to a much cleaner fix.

I agree the current fix is a bit "dirty". While it correctly breaks the coupling between use_tree_sitter and language_aware at the orchestration level, it leaves the underlying coupling between syntax and diagnostics intact. Properly decoupling them at the bottom would require a full-stack refactor of the snapshot chain. In my view, performing such an invasive change to every core map layer is a high-risk approach for this issue. I chose this level of fix to restore diagnostics safely while avoiding any potential for regressions in the critical rendering path.

---

One question: why skip the most important part?

I intended to add a GPUI test, but found that I would have to manually call highlighted_chunks to verify the result. Since this PR adds a new parameter to that function, manually passing the "correct" parameter in a test would only be testing my own new logic in isolation. It would feel like a redundant unit test instead of a real integration test that mimics actual system behavior, so I decided not to include it.

[SomeoneToIgnore]

In my view, performing such an invasive change to every core map layer is a high-risk approach for this issue.

Yes, that's a 7-level chain but overall it's triggered in a particular, highlighted above level only, to disable the highlights on one level, the rest is just passing the parameter through, so I find it hard to agree with this phrase.

Do I miss something?
If not, let's take some more time and do things properly.

but found that I would have to manually call highlighted_chunks to verify the result

Is it a bad thing?

If we can have a test with both diagnostics and tokens and the full semantic tokens' level that produces different results on main (without diagnostics) and this branch (with the diagnostics) after calling the highlighted_chunks — that is superb.

[lingyaochu]

Truly thanks for the review! The fix now addresses the root cause directly, and I'm pretty satisfied with how it turned out.

About the test I added: it's a bit tricky because it cannot be easily tested against the current main branch. It will fail to compile rather than failing at the assertion, simply because it passes the new LanguageAware parameter which is only introduced in this fix. Hopefully, it can still provide some value in preventing future regressions.

[SomeoneToIgnore]

Thank you a lot for the fix and the test!

[lingyaochu]

May I ask why you chose to pass the full struct in every location instead of using From<bool> and .into()? Is it for better semantic readability?

[SomeoneToIgnore]

Yes, it's totally not clear to me by looking at false.into() what does it convert into, "explicit is better than implicit", etc., etc.
I guess even better would be to have a named method(s) for this, but this way works too as it's mostly about test code changes.

To emphasize, the more important fix was to pass false in the chunks method in the new test, to be 100% sure we test what we want.

[lingyaochu]

Understood. I'm transitioning from Python and still learning the 'Rust way.' Thanks for the explanation, it helps me keep my future contributions idiomatic.

[SomeoneToIgnore]

/cherry-pick preview
/cherry-pick stable

[zed-zippy]

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/24087248925

[zed-zippy]

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/24087251066