From 4adec27a3d963fe155ea3cf927fab71be317b5a2 Mon Sep 17 00:00:00 2001 From: KyleBarton Date: Wed, 12 Nov 2025 10:32:46 -0800 Subject: [PATCH] Implement pretty TypeScript errors (#42494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #7844 This change uses tree-sitter highlights as a method of showing typescript errors prettily, keeping regex as simple as possible: Screenshot 2025-11-11 at 3 40 24 PM 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. --- crates/diagnostics/src/buffer_diagnostics.rs | 5 ++ crates/diagnostics/src/diagnostic_renderer.rs | 35 +++++++++-- crates/diagnostics/src/diagnostics.rs | 6 +- crates/editor/src/editor.rs | 21 +++++-- crates/editor/src/hover_popover.rs | 13 +++- .../languages/src/typescript/highlights.scm | 32 ++++++++++ crates/languages/src/vtsls.rs | 59 ++++++++++++++++++- 7 files changed, 159 insertions(+), 12 deletions(-) diff --git a/crates/diagnostics/src/buffer_diagnostics.rs b/crates/diagnostics/src/buffer_diagnostics.rs index 01626ddfd2a3f1a4773b2e88a9b8ff001b46680a..ed079c34864100238fd459cb2ec116bf21827fdd 100644 --- a/crates/diagnostics/src/buffer_diagnostics.rs +++ b/crates/diagnostics/src/buffer_diagnostics.rs @@ -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, ) })?; diff --git a/crates/diagnostics/src/diagnostic_renderer.rs b/crates/diagnostics/src/diagnostic_renderer.rs index 6204bf4b52ddb903773beac28627d53c3cce7765..2636b1aadc9708ff6832a5baa212277672dd305f 100644 --- a/crates/diagnostics/src/diagnostic_renderer.rs +++ b/crates/diagnostics/src/diagnostic_renderer.rs @@ -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>, buffer_id: BufferId, diagnostics_editor: Option>, + language_registry: Option>, cx: &mut App, ) -> Vec { 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, + language_registry: Option>, cx: &mut App, ) -> Vec> { - 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>, range: Range, buffer_id: BufferId, + language_registry: Option>, cx: &mut App, ) -> Option> { - 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 diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 344ce652969e9a6d54a22769741616def48ab3b1..92a4ba097f21d1f5894235bb2356c7ded9413359 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -73,7 +73,7 @@ pub fn init(cx: &mut App) { } pub(crate) struct ProjectDiagnosticsEditor { - project: Entity, + pub project: Entity, workspace: WeakEntity, focus_handle: FocusHandle, editor: Entity, @@ -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, ) })?; diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index cff337714b9619b54469e8915bfb36ff7a69111e..04d8794169ddcc0410f122c0d56124c7e5bcc254 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -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, + language_registry: Option>, cx: &mut App, ) -> Vec>; @@ -379,6 +381,7 @@ pub trait DiagnosticRenderer { diagnostic_group: Vec>, range: Range, buffer_id: BufferId, + language_registry: Option>, cx: &mut App, ) -> Option>; @@ -17947,8 +17950,18 @@ impl Editor { .diagnostic_group(buffer_id, diagnostic.diagnostic.group_id) .collect::>(); - 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() diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 1da3361f53853a5ea5a9d532b9ee2c05d6010a5d..721fce34c8c030322207cd74a69a266119596086 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -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) diff --git a/crates/languages/src/typescript/highlights.scm b/crates/languages/src/typescript/highlights.scm index 6474ba2a05af330b1a7bd2da8ed3411b9132fe22..14f6e2203476e5483af5bc21b69b6c62019c6b82 100644 --- a/crates/languages/src/typescript/highlights.scm +++ b/crates/languages/src/typescript/highlights.scm @@ -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 diff --git a/crates/languages/src/vtsls.rs b/crates/languages/src/vtsls.rs index 0766be24bab6a220748523c1107d40f5a58f03ae..fa1f47ff792265bd433ee82831dbd43f7500b289 100644 --- a/crates/languages/src/vtsls.rs +++ b/crates/languages/src/vtsls.rs @@ -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 { + static SINGLE_WORD_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"'([^\s']*)'").expect("Failed to create REGEX")); + + static MULTI_WORD_REGEX: LazyLock = + 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 { + VtslsLspAdapter::enhance_diagnostic_message(message) + } + fn language_ids(&self) -> HashMap { 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 + ); + } +}