Implement pretty TypeScript errors (#42494)
Closes #7844 This change uses tree-sitter highlights as a method of showing typescript errors prettily, keeping regex as simple as possible: <img width="832" height="446" alt="Screenshot 2025-11-11 at 3 40 24 PM" src="https://github.com/user-attachments/assets/0b3b6cf1-4d4d-4398-b89b-ef5ec0df87ec" /> It covers three main areas: 1. Diagnostics Diagnostics are now rendered with language-aware typescript, by providing the project's language registry. 2. Vtsls The LSP provider for typescript now implements the `diagnostic_message_to_markdown` function in the `LspAdapter` trait, so as to provide Diagnostics with \`\`\`typescript...\`\`\`-style code blocks for any selection of typescript longer than one word. In the single-word case, it simply wraps with \`\` 3. Typescript's `highlights.scm` `vtsls` doesn't provide strictly valid typescript in much of its messaging. Rather, it returns a message with snippets of typescript values which are invalid. Tree-sitter was not properly highlighting these snippets because it was expecting key-value formats. For instance: ``` type foo = { foo: string; bar: string; baz: number[] } ``` is valid, whereas simply ``` { foo: string; bar: string; baz: number[] } ``` is not. Therefore, highlights.scm needed to be adjusted in order to pattern-match on literal values that might be returned from the vtsls diagnostics messages. This was done by a) identifying arrow functions on their own, and b) augmenting the `statment_block` pattern matching in order to match on values which were clearly object literals. This approach may not be exhaustive - I'm happy to work on any additional cases we might identify from `vtsls` here - but hopefully demonstrates an extensible approach to making these messages look nice, without taking on the technical burden of extensive regex. Release Notes: - Show pretty TypeScript errors with language-aware Markdown.
This commit is contained in:
KyleBarton
@@ -370,11 +370,16 @@ impl BufferDiagnosticsEditor {
|
||||
continue;
|
||||
}
|
||||
|
||||
let languages = buffer_diagnostics_editor
|
||||
.read_with(cx, |b, cx| b.project.read(cx).languages().clone())
|
||||
.ok();
|
||||
|
||||
let diagnostic_blocks = cx.update(|_window, cx| {
|
||||
DiagnosticRenderer::diagnostic_blocks_for_group(
|
||||
group,
|
||||
buffer_snapshot.remote_id(),
|
||||
Some(Arc::new(buffer_diagnostics_editor.clone())),
|
||||
languages,
|
||||
cx,
|
||||
)
|
||||
})?;
|
||||
|
||||
@@ -6,7 +6,7 @@ use editor::{
|
||||
hover_popover::diagnostics_markdown_style,
|
||||
};
|
||||
use gpui::{AppContext, Entity, Focusable, WeakEntity};
|
||||
use language::{BufferId, Diagnostic, DiagnosticEntryRef};
|
||||
use language::{BufferId, Diagnostic, DiagnosticEntryRef, LanguageRegistry};
|
||||
use lsp::DiagnosticSeverity;
|
||||
use markdown::{Markdown, MarkdownElement};
|
||||
use settings::Settings;
|
||||
@@ -27,6 +27,7 @@ impl DiagnosticRenderer {
|
||||
diagnostic_group: Vec<DiagnosticEntryRef<'_, Point>>,
|
||||
buffer_id: BufferId,
|
||||
diagnostics_editor: Option<Arc<dyn DiagnosticsToolbarEditor>>,
|
||||
language_registry: Option<Arc<LanguageRegistry>>,
|
||||
cx: &mut App,
|
||||
) -> Vec<DiagnosticBlock> {
|
||||
let Some(primary_ix) = diagnostic_group
|
||||
@@ -75,11 +76,14 @@ impl DiagnosticRenderer {
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
results.push(DiagnosticBlock {
|
||||
initial_range: primary.range.clone(),
|
||||
severity: primary.diagnostic.severity,
|
||||
diagnostics_editor: diagnostics_editor.clone(),
|
||||
markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)),
|
||||
markdown: cx.new(|cx| {
|
||||
Markdown::new(markdown.into(), language_registry.clone(), None, cx)
|
||||
}),
|
||||
});
|
||||
} else {
|
||||
if entry.range.start.row.abs_diff(primary.range.start.row) >= 5 {
|
||||
@@ -91,7 +95,9 @@ impl DiagnosticRenderer {
|
||||
initial_range: entry.range.clone(),
|
||||
severity: entry.diagnostic.severity,
|
||||
diagnostics_editor: diagnostics_editor.clone(),
|
||||
markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)),
|
||||
markdown: cx.new(|cx| {
|
||||
Markdown::new(markdown.into(), language_registry.clone(), None, cx)
|
||||
}),
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -118,9 +124,16 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
|
||||
buffer_id: BufferId,
|
||||
snapshot: EditorSnapshot,
|
||||
editor: WeakEntity<Editor>,
|
||||
language_registry: Option<Arc<LanguageRegistry>>,
|
||||
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,
|
||||
language_registry,
|
||||
cx,
|
||||
);
|
||||
|
||||
blocks
|
||||
.into_iter()
|
||||
@@ -146,9 +159,16 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
|
||||
diagnostic_group: Vec<DiagnosticEntryRef<'_, Point>>,
|
||||
range: Range<Point>,
|
||||
buffer_id: BufferId,
|
||||
language_registry: Option<Arc<LanguageRegistry>>,
|
||||
cx: &mut App,
|
||||
) -> Option<Entity<Markdown>> {
|
||||
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,
|
||||
language_registry,
|
||||
cx,
|
||||
);
|
||||
blocks
|
||||
.into_iter()
|
||||
.find_map(|block| (block.initial_range == range).then(|| block.markdown))
|
||||
@@ -206,6 +226,11 @@ impl DiagnosticBlock {
|
||||
self.markdown.clone(),
|
||||
diagnostics_markdown_style(bcx.window, cx),
|
||||
)
|
||||
.code_block_renderer(markdown::CodeBlockRenderer::Default {
|
||||
copy_button: false,
|
||||
copy_button_on_hover: false,
|
||||
border: false,
|
||||
})
|
||||
.on_url_click({
|
||||
move |link, window, cx| {
|
||||
editor
|
||||
|
||||
@@ -73,7 +73,7 @@ pub fn init(cx: &mut App) {
|
||||
}
|
||||
|
||||
pub(crate) struct ProjectDiagnosticsEditor {
|
||||
project: Entity<Project>,
|
||||
pub project: Entity<Project>,
|
||||
workspace: WeakEntity<Workspace>,
|
||||
focus_handle: FocusHandle,
|
||||
editor: Entity<Editor>,
|
||||
@@ -545,11 +545,15 @@ impl ProjectDiagnosticsEditor {
|
||||
if group_severity.is_none_or(|s| s > max_severity) {
|
||||
continue;
|
||||
}
|
||||
let languages = this
|
||||
.read_with(cx, |t, cx| t.project.read(cx).languages().clone())
|
||||
.ok();
|
||||
let more = cx.update(|_, cx| {
|
||||
crate::diagnostic_renderer::DiagnosticRenderer::diagnostic_blocks_for_group(
|
||||
group,
|
||||
buffer_snapshot.remote_id(),
|
||||
Some(diagnostics_toolbar_editor.clone()),
|
||||
languages,
|
||||
cx,
|
||||
)
|
||||
})?;
|
||||
|
||||
@@ -117,8 +117,9 @@ use language::{
|
||||
AutoindentMode, BlockCommentConfig, BracketMatch, BracketPair, Buffer, BufferRow,
|
||||
BufferSnapshot, Capability, CharClassifier, CharKind, CharScopeContext, CodeLabel, CursorShape,
|
||||
DiagnosticEntryRef, DiffOptions, EditPredictionsMode, EditPreview, HighlightedText, IndentKind,
|
||||
IndentSize, Language, OffsetRangeExt, OutlineItem, Point, Runnable, RunnableRange, Selection,
|
||||
SelectionGoal, TextObject, TransactionId, TreeSitterOptions, WordsQuery,
|
||||
IndentSize, Language, LanguageRegistry, OffsetRangeExt, OutlineItem, Point, Runnable,
|
||||
RunnableRange, Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions,
|
||||
WordsQuery,
|
||||
language_settings::{
|
||||
self, LspInsertMode, RewrapBehavior, WordsCompletionMode, all_language_settings,
|
||||
language_settings,
|
||||
@@ -371,6 +372,7 @@ pub trait DiagnosticRenderer {
|
||||
buffer_id: BufferId,
|
||||
snapshot: EditorSnapshot,
|
||||
editor: WeakEntity<Editor>,
|
||||
language_registry: Option<Arc<LanguageRegistry>>,
|
||||
cx: &mut App,
|
||||
) -> Vec<BlockProperties<Anchor>>;
|
||||
|
||||
@@ -379,6 +381,7 @@ pub trait DiagnosticRenderer {
|
||||
diagnostic_group: Vec<DiagnosticEntryRef<'_, Point>>,
|
||||
range: Range<Point>,
|
||||
buffer_id: BufferId,
|
||||
language_registry: Option<Arc<LanguageRegistry>>,
|
||||
cx: &mut App,
|
||||
) -> Option<Entity<markdown::Markdown>>;
|
||||
|
||||
@@ -17947,8 +17950,18 @@ impl Editor {
|
||||
.diagnostic_group(buffer_id, diagnostic.diagnostic.group_id)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let blocks =
|
||||
renderer.render_group(diagnostic_group, buffer_id, snapshot, cx.weak_entity(), cx);
|
||||
let language_registry = self
|
||||
.project()
|
||||
.map(|project| project.read(cx).languages().clone());
|
||||
|
||||
let blocks = renderer.render_group(
|
||||
diagnostic_group,
|
||||
buffer_id,
|
||||
snapshot,
|
||||
cx.weak_entity(),
|
||||
language_registry,
|
||||
cx,
|
||||
);
|
||||
|
||||
let blocks = self.display_map.update(cx, |display_map, cx| {
|
||||
display_map.insert_blocks(blocks, cx).into_iter().collect()
|
||||
|
||||
@@ -341,7 +341,13 @@ fn show_hover(
|
||||
renderer
|
||||
.as_ref()
|
||||
.and_then(|renderer| {
|
||||
renderer.render_hover(group, point_range, buffer_id, cx)
|
||||
renderer.render_hover(
|
||||
group,
|
||||
point_range,
|
||||
buffer_id,
|
||||
language_registry.clone(),
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.context("no rendered diagnostic")
|
||||
})??;
|
||||
@@ -986,6 +992,11 @@ impl DiagnosticPopover {
|
||||
self.markdown.clone(),
|
||||
diagnostics_markdown_style(window, cx),
|
||||
)
|
||||
.code_block_renderer(markdown::CodeBlockRenderer::Default {
|
||||
copy_button: false,
|
||||
copy_button_on_hover: false,
|
||||
border: false,
|
||||
})
|
||||
.on_url_click(
|
||||
move |link, window, cx| {
|
||||
if let Some(renderer) = GlobalDiagnosticRenderer::global(cx)
|
||||
|
||||
@@ -9,6 +9,36 @@
|
||||
(type_identifier) @type
|
||||
(predefined_type) @type.builtin
|
||||
|
||||
;; Highlights object literals by hijacking the statement_block pattern, but only if
|
||||
;; the statement block follows an object literal pattern
|
||||
((statement_block
|
||||
(labeled_statement
|
||||
;; highlight the label like a property name
|
||||
label: (statement_identifier) @property.name
|
||||
body: [
|
||||
;; match a terminating expression statement
|
||||
(expression_statement
|
||||
;; single identifier - treat as a type name
|
||||
[(identifier) @type.name
|
||||
;; object - treat as a property - type pair
|
||||
(object
|
||||
(pair
|
||||
key: (_) @property.name
|
||||
value: (_) @type.name))
|
||||
;; subscript_expression - treat as an array declaration
|
||||
(subscript_expression
|
||||
object: (_) @type.name
|
||||
index: (_)
|
||||
)
|
||||
;; templated string - treat each identifier contained as a type name
|
||||
(template_string
|
||||
(template_substitution
|
||||
(identifier) @type.name))
|
||||
])
|
||||
;; match a nested statement block
|
||||
(statement_block) @nested
|
||||
])))
|
||||
|
||||
(import_specifier
|
||||
"type"
|
||||
name: (identifier) @type
|
||||
@@ -79,6 +109,8 @@
|
||||
left: (identifier) @function
|
||||
right: [(function_expression) (arrow_function)])
|
||||
|
||||
(arrow_function) @function
|
||||
|
||||
; Literals
|
||||
|
||||
(this) @variable.special
|
||||
|
||||
@@ -6,11 +6,12 @@ use language::{LanguageName, LspAdapter, LspAdapterDelegate, LspInstaller, Toolc
|
||||
use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName};
|
||||
use node_runtime::{NodeRuntime, VersionStrategy};
|
||||
use project::{Fs, lsp_store::language_server_settings};
|
||||
use regex::Regex;
|
||||
use serde_json::Value;
|
||||
use std::{
|
||||
ffi::OsString,
|
||||
path::{Path, PathBuf},
|
||||
sync::Arc,
|
||||
sync::{Arc, LazyLock},
|
||||
};
|
||||
use util::{ResultExt, maybe, merge_json_value_into};
|
||||
|
||||
@@ -56,6 +57,20 @@ impl VtslsLspAdapter {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
pub fn enhance_diagnostic_message(message: &str) -> Option<String> {
|
||||
static SINGLE_WORD_REGEX: LazyLock<Regex> =
|
||||
LazyLock::new(|| Regex::new(r"'([^\s']*)'").expect("Failed to create REGEX"));
|
||||
|
||||
static MULTI_WORD_REGEX: LazyLock<Regex> =
|
||||
LazyLock::new(|| Regex::new(r"'([^']+\s+[^']*)'").expect("Failed to create REGEX"));
|
||||
|
||||
let first = SINGLE_WORD_REGEX.replace_all(message, "`$1`").to_string();
|
||||
let second = MULTI_WORD_REGEX
|
||||
.replace_all(&first, "\n```typescript\n$1\n```\n")
|
||||
.to_string();
|
||||
Some(second)
|
||||
}
|
||||
}
|
||||
|
||||
pub struct TypeScriptVersions {
|
||||
@@ -274,6 +289,10 @@ impl LspAdapter for VtslsLspAdapter {
|
||||
Ok(default_workspace_configuration)
|
||||
}
|
||||
|
||||
fn diagnostic_message_to_markdown(&self, message: &str) -> Option<String> {
|
||||
VtslsLspAdapter::enhance_diagnostic_message(message)
|
||||
}
|
||||
|
||||
fn language_ids(&self) -> HashMap<LanguageName, String> {
|
||||
HashMap::from_iter([
|
||||
(LanguageName::new("TypeScript"), "typescript".into()),
|
||||
@@ -302,3 +321,41 @@ async fn get_cached_ts_server_binary(
|
||||
.await
|
||||
.log_err()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::vtsls::VtslsLspAdapter;
|
||||
|
||||
#[test]
|
||||
fn test_diagnostic_message_to_markdown() {
|
||||
// Leaves simple messages unchanged
|
||||
let message = "The expected type comes from the return type of this signature.";
|
||||
|
||||
let expected = "The expected type comes from the return type of this signature.";
|
||||
|
||||
assert_eq!(
|
||||
VtslsLspAdapter::enhance_diagnostic_message(message).expect("Should be some"),
|
||||
expected
|
||||
);
|
||||
|
||||
// Parses both multi-word and single-word correctly
|
||||
let message = "Property 'baz' is missing in type '{ foo: string; bar: string; }' but required in type 'User'.";
|
||||
|
||||
let expected = "Property `baz` is missing in type \n```typescript\n{ foo: string; bar: string; }\n```\n but required in type `User`.";
|
||||
|
||||
assert_eq!(
|
||||
VtslsLspAdapter::enhance_diagnostic_message(message).expect("Should be some"),
|
||||
expected
|
||||
);
|
||||
|
||||
// Parses multi-and-single word in any order, and ignores existing newlines
|
||||
let message = "Type '() => { foo: string; bar: string; }' is not assignable to type 'GetUserFunction'.\n Property 'baz' is missing in type '{ foo: string; bar: string; }' but required in type 'User'.";
|
||||
|
||||
let expected = "Type \n```typescript\n() => { foo: string; bar: string; }\n```\n is not assignable to type `GetUserFunction`.\n Property `baz` is missing in type \n```typescript\n{ foo: string; bar: string; }\n```\n but required in type `User`.";
|
||||
|
||||
assert_eq!(
|
||||
VtslsLspAdapter::enhance_diagnostic_message(message).expect("Should be some"),
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user