Show diagnostic codes (#29296)

Closes #28135
Closes #4388
Closes #28136

Release Notes:

- diagnostics: Show the diagnostic code if available

---------

Co-authored-by: Neo Nie <nihgwu@live.com>
Co-authored-by: Zed AI <ai+claude-3.7@zed.dev>
This commit is contained in:
Conrad Irwin 2025-04-23 20:51:01 -06:00 committed by GitHub
parent 8836c6fb42
commit 9d10489607
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 517 additions and 327 deletions

View file

@ -28,6 +28,7 @@ impl DiagnosticRenderer {
diagnostic_group: Vec<DiagnosticEntry<Point>>,
buffer_id: BufferId,
diagnostics_editor: Option<WeakEntity<ProjectDiagnosticsEditor>>,
merge_same_row: bool,
cx: &mut App,
) -> Vec<DiagnosticBlock> {
let Some(primary_ix) = diagnostic_group
@ -45,7 +46,7 @@ impl DiagnosticRenderer {
if entry.diagnostic.is_primary {
continue;
}
if entry.range.start.row == primary.range.start.row {
if entry.range.start.row == primary.range.start.row && merge_same_row {
same_row.push(entry)
} else if entry.range.start.row.abs_diff(primary.range.start.row) < 5 {
close.push(entry)
@ -54,28 +55,48 @@ impl DiagnosticRenderer {
}
}
let mut markdown =
Markdown::escape(&if let Some(source) = primary.diagnostic.source.as_ref() {
format!("{}: {}", source, primary.diagnostic.message)
} else {
primary.diagnostic.message
})
.to_string();
let mut markdown = String::new();
let diagnostic = &primary.diagnostic;
markdown.push_str(&Markdown::escape(&diagnostic.message));
for entry in same_row {
markdown.push_str("\n- hint: ");
markdown.push_str(&Markdown::escape(&entry.diagnostic.message))
}
if diagnostic.source.is_some() || diagnostic.code.is_some() {
markdown.push_str(" (");
}
if let Some(source) = diagnostic.source.as_ref() {
markdown.push_str(&Markdown::escape(&source));
}
if diagnostic.source.is_some() && diagnostic.code.is_some() {
markdown.push(' ');
}
if let Some(code) = diagnostic.code.as_ref() {
if let Some(description) = diagnostic.code_description.as_ref() {
markdown.push('[');
markdown.push_str(&Markdown::escape(&code.to_string()));
markdown.push_str("](");
markdown.push_str(&Markdown::escape(description.as_ref()));
markdown.push(')');
} else {
markdown.push_str(&Markdown::escape(&code.to_string()));
}
}
if diagnostic.source.is_some() || diagnostic.code.is_some() {
markdown.push(')');
}
for (ix, entry) in &distant {
markdown.push_str("\n- hint: [");
markdown.push_str(&Markdown::escape(&entry.diagnostic.message));
markdown.push_str(&format!("](file://#diagnostic-{group_id}-{ix})\n",))
markdown.push_str(&format!(
"](file://#diagnostic-{buffer_id}-{group_id}-{ix})\n",
))
}
let mut results = vec![DiagnosticBlock {
initial_range: primary.range,
severity: primary.diagnostic.severity,
buffer_id,
diagnostics_editor: diagnostics_editor.clone(),
markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)),
}];
@ -91,7 +112,6 @@ impl DiagnosticRenderer {
results.push(DiagnosticBlock {
initial_range: entry.range,
severity: entry.diagnostic.severity,
buffer_id,
diagnostics_editor: diagnostics_editor.clone(),
markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)),
});
@ -105,15 +125,12 @@ impl DiagnosticRenderer {
};
let mut markdown = Markdown::escape(&markdown).to_string();
markdown.push_str(&format!(
" ([back](file://#diagnostic-{group_id}-{primary_ix}))"
" ([back](file://#diagnostic-{buffer_id}-{group_id}-{primary_ix}))"
));
// problem: group-id changes...
// - only an issue in diagnostics because caching
results.push(DiagnosticBlock {
initial_range: entry.range,
severity: entry.diagnostic.severity,
buffer_id,
diagnostics_editor: diagnostics_editor.clone(),
markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)),
});
@ -132,7 +149,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
editor: WeakEntity<Editor>,
cx: &mut App,
) -> Vec<BlockProperties<Anchor>> {
let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, cx);
let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, true, cx);
blocks
.into_iter()
.map(|block| {
@ -151,13 +168,40 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
})
.collect()
}
fn render_hover(
&self,
diagnostic_group: Vec<DiagnosticEntry<Point>>,
range: Range<Point>,
buffer_id: BufferId,
cx: &mut App,
) -> Option<Entity<Markdown>> {
let blocks =
Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, false, cx);
blocks.into_iter().find_map(|block| {
if block.initial_range == range {
Some(block.markdown)
} else {
None
}
})
}
fn open_link(
&self,
editor: &mut Editor,
link: SharedString,
window: &mut Window,
cx: &mut Context<Editor>,
) {
DiagnosticBlock::open_link(editor, &None, link, window, cx);
}
}
#[derive(Clone)]
pub(crate) struct DiagnosticBlock {
pub(crate) initial_range: Range<Point>,
pub(crate) severity: DiagnosticSeverity,
pub(crate) buffer_id: BufferId,
pub(crate) markdown: Entity<Markdown>,
pub(crate) diagnostics_editor: Option<WeakEntity<ProjectDiagnosticsEditor>>,
}
@ -181,7 +225,6 @@ impl DiagnosticBlock {
let settings = ThemeSettings::get_global(cx);
let editor_line_height = (settings.line_height() * settings.buffer_font_size(cx)).round();
let line_height = editor_line_height;
let buffer_id = self.buffer_id;
let diagnostics_editor = self.diagnostics_editor.clone();
div()
@ -195,14 +238,11 @@ impl DiagnosticBlock {
MarkdownElement::new(self.markdown.clone(), hover_markdown_style(bcx.window, cx))
.on_url_click({
move |link, window, cx| {
Self::open_link(
editor.clone(),
&diagnostics_editor,
link,
window,
buffer_id,
cx,
)
editor
.update(cx, |editor, cx| {
Self::open_link(editor, &diagnostics_editor, link, window, cx)
})
.ok();
}
}),
)
@ -210,79 +250,71 @@ impl DiagnosticBlock {
}
pub fn open_link(
editor: WeakEntity<Editor>,
editor: &mut Editor,
diagnostics_editor: &Option<WeakEntity<ProjectDiagnosticsEditor>>,
link: SharedString,
window: &mut Window,
buffer_id: BufferId,
cx: &mut App,
cx: &mut Context<Editor>,
) {
editor
.update(cx, |editor, cx| {
let Some(diagnostic_link) = link.strip_prefix("file://#diagnostic-") else {
editor::hover_popover::open_markdown_url(link, window, cx);
return;
};
let Some((group_id, ix)) = maybe!({
let (group_id, ix) = diagnostic_link.split_once('-')?;
let group_id: usize = group_id.parse().ok()?;
let ix: usize = ix.parse().ok()?;
Some((group_id, ix))
}) else {
return;
};
let Some(diagnostic_link) = link.strip_prefix("file://#diagnostic-") else {
editor::hover_popover::open_markdown_url(link, window, cx);
return;
};
let Some((buffer_id, group_id, ix)) = maybe!({
let mut parts = diagnostic_link.split('-');
let buffer_id: u64 = parts.next()?.parse().ok()?;
let group_id: usize = parts.next()?.parse().ok()?;
let ix: usize = parts.next()?.parse().ok()?;
Some((BufferId::new(buffer_id).ok()?, group_id, ix))
}) else {
return;
};
if let Some(diagnostics_editor) = diagnostics_editor {
if let Some(diagnostic) = diagnostics_editor
.update(cx, |diagnostics, _| {
diagnostics
.diagnostics
.get(&buffer_id)
.cloned()
.unwrap_or_default()
.into_iter()
.filter(|d| d.diagnostic.group_id == group_id)
.nth(ix)
})
.ok()
.flatten()
{
let multibuffer = editor.buffer().read(cx);
let Some(snapshot) = multibuffer
.buffer(buffer_id)
.map(|entity| entity.read(cx).snapshot())
else {
return;
};
for (excerpt_id, range) in multibuffer.excerpts_for_buffer(buffer_id, cx) {
if range.context.overlaps(&diagnostic.range, &snapshot) {
Self::jump_to(
editor,
Anchor::range_in_buffer(
excerpt_id,
buffer_id,
diagnostic.range,
),
window,
cx,
);
return;
}
}
}
} else {
if let Some(diagnostic) = editor
.snapshot(window, cx)
.buffer_snapshot
.diagnostic_group(buffer_id, group_id)
if let Some(diagnostics_editor) = diagnostics_editor {
if let Some(diagnostic) = diagnostics_editor
.update(cx, |diagnostics, _| {
diagnostics
.diagnostics
.get(&buffer_id)
.cloned()
.unwrap_or_default()
.into_iter()
.filter(|d| d.diagnostic.group_id == group_id)
.nth(ix)
{
Self::jump_to(editor, diagnostic.range, window, cx)
}
})
.ok()
.flatten()
{
let multibuffer = editor.buffer().read(cx);
let Some(snapshot) = multibuffer
.buffer(buffer_id)
.map(|entity| entity.read(cx).snapshot())
else {
return;
};
})
.ok();
for (excerpt_id, range) in multibuffer.excerpts_for_buffer(buffer_id, cx) {
if range.context.overlaps(&diagnostic.range, &snapshot) {
Self::jump_to(
editor,
Anchor::range_in_buffer(excerpt_id, buffer_id, diagnostic.range),
window,
cx,
);
return;
}
}
}
} else {
if let Some(diagnostic) = editor
.snapshot(window, cx)
.buffer_snapshot
.diagnostic_group(buffer_id, group_id)
.nth(ix)
{
Self::jump_to(editor, diagnostic.range, window, cx)
}
};
}
fn jump_to<T: ToOffset>(

View file

@ -416,6 +416,7 @@ impl ProjectDiagnosticsEditor {
group,
buffer_snapshot.remote_id(),
Some(this.clone()),
true,
cx,
)
})?;

View file

@ -1,10 +1,13 @@
use super::*;
use collections::{HashMap, HashSet};
use editor::{
DisplayPoint, InlayId,
actions::{GoToDiagnostic, GoToPreviousDiagnostic, MoveToBeginning},
DisplayPoint, EditorSettings, InlayId,
actions::{GoToDiagnostic, GoToPreviousDiagnostic, Hover, MoveToBeginning},
display_map::{DisplayRow, Inlay},
test::{editor_content_with_blocks, editor_test_context::EditorTestContext},
test::{
editor_content_with_blocks, editor_lsp_test_context::EditorLspTestContext,
editor_test_context::EditorTestContext,
},
};
use gpui::{TestAppContext, VisualTestContext};
use indoc::indoc;
@ -134,11 +137,13 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
// comment 1
// comment 2
c(y);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `y` has type `Vec<char>`, which does not
§ implement the `Copy` trait
d(x);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `x` has type `Vec<char>`, which does not
§ implement the `Copy` trait
§ hint: value moved here
@ -168,7 +173,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
lsp::Position::new(0, 15),
),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "mismatched types\nexpected `usize`, found `char`".to_string(),
message: "mismatched types expected `usize`, found `char`".to_string(),
..Default::default()
}],
version: None,
@ -206,11 +211,13 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
// comment 1
// comment 2
c(y);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `y` has type `Vec<char>`, which does not
§ implement the `Copy` trait
d(x);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `x` has type `Vec<char>`, which does not
§ implement the `Copy` trait
§ hint: value moved here
@ -241,7 +248,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
lsp::Position::new(0, 15),
),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "mismatched types\nexpected `usize`, found `char`".to_string(),
message: "mismatched types expected `usize`, found `char`".to_string(),
..Default::default()
},
lsp::Diagnostic {
@ -289,11 +296,13 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
// comment 1
// comment 2
c(y);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `y` has type `Vec<char>`, which does not
§ implement the `Copy` trait
d(x);
§ use of moved value value used here after move
§ use of moved value
§ value used here after move
§ hint: move occurs because `x` has type `Vec<char>`, which does not
§ implement the `Copy` trait
§ hint: value moved here
@ -1192,8 +1201,219 @@ async fn cycle_through_same_place_diagnostics(cx: &mut TestAppContext) {
"});
}
#[gpui::test]
async fn test_diagnostics_with_links(cx: &mut TestAppContext) {
init_test(cx);
let mut cx = EditorTestContext::new(cx).await;
cx.set_state(indoc! {"
fn func(abˇc def: i32) -> u32 {
}
"});
let lsp_store =
cx.update_editor(|editor, _, cx| editor.project.as_ref().unwrap().read(cx).lsp_store());
cx.update(|_, cx| {
lsp_store.update(cx, |lsp_store, cx| {
lsp_store.update_diagnostics(
LanguageServerId(0),
lsp::PublishDiagnosticsParams {
uri: lsp::Url::from_file_path(path!("/root/file")).unwrap(),
version: None,
diagnostics: vec![lsp::Diagnostic {
range: lsp::Range::new(lsp::Position::new(0, 8), lsp::Position::new(0, 12)),
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "we've had problems with <https://link.one>, and <https://link.two> is broken".to_string(),
..Default::default()
}],
},
&[],
cx,
)
})
}).unwrap();
cx.run_until_parked();
cx.update_editor(|editor, window, cx| {
editor::hover_popover::hover(editor, &Default::default(), window, cx)
});
cx.run_until_parked();
cx.update_editor(|editor, _, _| assert!(editor.hover_state.diagnostic_popover.is_some()))
}
#[gpui::test]
async fn test_hover_diagnostic_and_info_popovers(cx: &mut gpui::TestAppContext) {
init_test(cx);
let mut cx = EditorLspTestContext::new_rust(
lsp::ServerCapabilities {
hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
..Default::default()
},
cx,
)
.await;
// Hover with just diagnostic, pops DiagnosticPopover immediately and then
// info popover once request completes
cx.set_state(indoc! {"
fn teˇst() { println!(); }
"});
// Send diagnostic to client
let range = cx.lsp_range(indoc! {"
fn «test»() { println!(); }
"});
let lsp_store =
cx.update_editor(|editor, _, cx| editor.project.as_ref().unwrap().read(cx).lsp_store());
cx.update(|_, cx| {
lsp_store.update(cx, |lsp_store, cx| {
lsp_store.update_diagnostics(
LanguageServerId(0),
lsp::PublishDiagnosticsParams {
uri: lsp::Url::from_file_path(path!("/root/dir/file.rs")).unwrap(),
version: None,
diagnostics: vec![lsp::Diagnostic {
range,
severity: Some(lsp::DiagnosticSeverity::ERROR),
message: "A test diagnostic message.".to_string(),
..Default::default()
}],
},
&[],
cx,
)
})
})
.unwrap();
cx.run_until_parked();
// Hover pops diagnostic immediately
cx.update_editor(|editor, window, cx| editor::hover_popover::hover(editor, &Hover, window, cx));
cx.background_executor.run_until_parked();
cx.editor(|Editor { hover_state, .. }, _, _| {
assert!(hover_state.diagnostic_popover.is_some());
assert!(hover_state.info_popovers.is_empty());
});
// Info Popover shows after request responded to
let range = cx.lsp_range(indoc! {"
fn «test»() { println!(); }
"});
cx.set_request_handler::<lsp::request::HoverRequest, _, _>(move |_, _, _| async move {
Ok(Some(lsp::Hover {
contents: lsp::HoverContents::Markup(lsp::MarkupContent {
kind: lsp::MarkupKind::Markdown,
value: "some new docs".to_string(),
}),
range: Some(range),
}))
});
let delay = cx.update(|_, cx| EditorSettings::get_global(cx).hover_popover_delay + 1);
cx.background_executor
.advance_clock(Duration::from_millis(delay));
cx.background_executor.run_until_parked();
cx.editor(|Editor { hover_state, .. }, _, _| {
hover_state.diagnostic_popover.is_some() && hover_state.info_task.is_some()
});
}
#[gpui::test]
async fn test_diagnostics_with_code(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/root"),
json!({
"main.js": "
function test() {
const x = 10;
const y = 20;
return 1;
}
test();
"
.unindent(),
}),
)
.await;
let language_server_id = LanguageServerId(0);
let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*window, cx);
let workspace = window.root(cx).unwrap();
let uri = lsp::Url::from_file_path(path!("/root/main.js")).unwrap();
// Create diagnostics with code fields
lsp_store.update(cx, |lsp_store, cx| {
lsp_store
.update_diagnostics(
language_server_id,
lsp::PublishDiagnosticsParams {
uri: uri.clone(),
diagnostics: vec![
lsp::Diagnostic {
range: lsp::Range::new(
lsp::Position::new(1, 4),
lsp::Position::new(1, 14),
),
severity: Some(lsp::DiagnosticSeverity::WARNING),
code: Some(lsp::NumberOrString::String("no-unused-vars".to_string())),
source: Some("eslint".to_string()),
message: "'x' is assigned a value but never used".to_string(),
..Default::default()
},
lsp::Diagnostic {
range: lsp::Range::new(
lsp::Position::new(2, 4),
lsp::Position::new(2, 14),
),
severity: Some(lsp::DiagnosticSeverity::WARNING),
code: Some(lsp::NumberOrString::String("no-unused-vars".to_string())),
source: Some("eslint".to_string()),
message: "'y' is assigned a value but never used".to_string(),
..Default::default()
},
],
version: None,
},
&[],
cx,
)
.unwrap();
});
// Open the project diagnostics view
let diagnostics = window.build_entity(cx, |window, cx| {
ProjectDiagnosticsEditor::new(true, project.clone(), workspace.downgrade(), window, cx)
});
let editor = diagnostics.update(cx, |diagnostics, _| diagnostics.editor.clone());
diagnostics
.next_notification(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10), cx)
.await;
// Verify that the diagnostic codes are displayed correctly
pretty_assertions::assert_eq!(
editor_content_with_blocks(&editor, cx),
indoc::indoc! {
"§ main.js
§ -----
function test() {
const x = 10; § 'x' is assigned a value but never used (eslint no-unused-vars)
const y = 20; § 'y' is assigned a value but never used (eslint no-unused-vars)
return 1;
}"
}
);
}
fn init_test(cx: &mut TestAppContext) {
cx.update(|cx| {
env_logger::try_init().ok();
let settings = SettingsStore::test(cx);
cx.set_global(settings);
theme::init(theme::LoadThemes::JustBase, cx);