From a61e2609c18d459a8f236cbbd0ce5a04a06d28da Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 28 Aug 2026 18:06:41 +0000 Subject: [PATCH] Normalize ranges returned by language servers in more cases (#63372) Previously, we'd panic when constructing multibuffer excerpts in response to "find all references" if a language server returned a reversed range. This PR makes it so we normalize those ranges using the existing `range_from_lsp` helper before passing them to `set_excerpts_for_path`. It also replaces several other places where we were calling `point_from_lsp` on each endpoint of a range separately with `range_from_lsp`. Closes ZED-79W Co-authored-by: Mikayla Release Notes: - Fixed a crash with some languages servers when handling `editor::FindAllReferences`. --- crates/copilot/src/copilot.rs | 24 +++------ crates/editor/src/document_colors.rs | 7 +-- crates/editor/src/editor.rs | 2 +- crates/editor/src/editor_tests.rs | 80 ++++++++++++++++++++++++++++ crates/editor/src/navigation.rs | 7 ++- crates/project/src/lsp_command.rs | 61 ++++++++++----------- 6 files changed, 123 insertions(+), 58 deletions(-) diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index fed2f4bc9a..bd28e4f047 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -19,7 +19,7 @@ use language::language_settings::{AllLanguageSettings, CopilotSettings}; use language::{ Anchor, Bias, Buffer, BufferSnapshot, Language, PointUtf16, ToPointUtf16, language_settings::{EditPredictionProvider, all_language_settings}, - point_from_lsp, point_to_lsp, + point_to_lsp, range_from_lsp, }; use lsp::{LanguageServer, LanguageServerBinary, LanguageServerId, LanguageServerName}; use node_runtime::{NodeRuntime, VersionStrategy}; @@ -1068,14 +1068,9 @@ impl Copilot { .edits .into_iter() .map(|completion| { - let start = snapshot.clip_point_utf16( - point_from_lsp(completion.range.start), - Bias::Left, - ); - let end = snapshot.clip_point_utf16( - point_from_lsp(completion.range.end), - Bias::Left, - ); + let range = range_from_lsp(completion.range); + let start = snapshot.clip_point_utf16(range.start, Bias::Left); + let end = snapshot.clip_point_utf16(range.end, Bias::Left); CopilotEditPrediction { buffer: buffer_entity.clone(), range: snapshot.anchor_before(start) @@ -1124,14 +1119,9 @@ impl Copilot { .items .into_iter() .map(|item| { - let start = snapshot.clip_point_utf16( - point_from_lsp(item.range.start), - Bias::Left, - ); - let end = snapshot.clip_point_utf16( - point_from_lsp(item.range.end), - Bias::Left, - ); + let range = range_from_lsp(item.range); + let start = snapshot.clip_point_utf16(range.start, Bias::Left); + let end = snapshot.clip_point_utf16(range.end, Bias::Left); CopilotEditPrediction { buffer: buffer_entity.clone(), range: snapshot.anchor_before(start) diff --git a/crates/editor/src/document_colors.rs b/crates/editor/src/document_colors.rs index 4ce89f90c9..d413d3c1ca 100644 --- a/crates/editor/src/document_colors.rs +++ b/crates/editor/src/document_colors.rs @@ -4,7 +4,7 @@ use collections::HashMap; use futures::future::join_all; use gpui::{Hsla, Rgba}; use itertools::Itertools; -use language::point_from_lsp; +use language::range_from_lsp; use multi_buffer::Anchor; use project::{DocumentColor, InlayId}; use settings::Settings as _; @@ -219,8 +219,9 @@ impl Editor { .clear(); } else { for color in colors.colors { - let color_start = point_from_lsp(color.lsp_range.start); - let color_end = point_from_lsp(color.lsp_range.end); + let color_range = range_from_lsp(color.lsp_range); + let color_start = color_range.start; + let color_end = color_range.end; let Some(range) = multi_buffer_snapshot .buffer_anchor_range_to_anchor_range( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2e5506e206..1209b8dfb7 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -186,7 +186,7 @@ use language::{ self, AllLanguageSettings, LanguageSettings, LspInsertMode, RewrapBehavior, WordsCompletionMode, all_language_settings, }, - point_from_lsp, point_to_lsp, text_diff_with_options, + point_to_lsp, text_diff_with_options, }; use language_detection::detect_language; use linked_editing_ranges::refresh_linked_ranges; diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 5ffa82dc04..4a47f5beeb 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -30868,6 +30868,86 @@ async fn test_goto_definition_with_find_all_references_fallback(cx: &mut TestApp }); } +/// End-to-end regression test for ZED-79W ("cannot summarize backward"): +/// language servers can return a location whose range is reversed (start +/// after end). `GetReferences::response_from_lsp` used to convert the +/// endpoints individually, bypassing `range_from_lsp`'s normalization, so +/// the reversed range reached excerpt construction intact, and when the +/// reversal exceeded twice the excerpt context line count, the results +/// multibuffer built an excerpt whose context anchors resolve backward, +/// panicking the rope layer. +#[gpui::test] +async fn test_find_all_references_with_reversed_server_range(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + references_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + + cx.set_state( + &r#"fn one() { + let mut a = ˇtwo(); + } + + fn two() {} + + fn three() { + two(); + two(); + two(); + }"# + .unindent(), + ); + cx.lsp + .set_request_handler::(move |params, _| async move { + Ok(Some(vec![ + lsp::Location { + uri: params.text_document_position.text_document.uri.clone(), + range: lsp::Range::new(lsp::Position::new(1, 16), lsp::Position::new(1, 19)), + }, + // A reversed range, as returned by some language servers. + lsp::Location { + uri: params.text_document_position.text_document.uri, + range: lsp::Range::new(lsp::Position::new(9, 8), lsp::Position::new(1, 16)), + }, + ])) + }); + + let navigated = cx + .update_editor(|editor, window, cx| { + editor.find_all_references(&FindAllReferences::default(), window, cx) + }) + .expect("should have spawned a references request") + .await + .expect("references request should succeed"); + assert_eq!(navigated, Navigated::Yes); + + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, test_editor_cx| { + assert_eq!( + editors.len(), + 2, + "references should open in a new multibuffer editor" + ); + let references_text = editors + .into_iter() + .find(|new_editor| *new_editor != test_editor_cx.entity()) + .expect("should have one non-test editor") + .read(test_editor_cx) + .text(test_editor_cx); + assert!( + references_text.contains("fn three()"), + "the reversed range's rows should be excerpted, got: {references_text:?}" + ); + }); +} + #[gpui::test] async fn test_goto_definition_no_fallback(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/navigation.rs b/crates/editor/src/navigation.rs index 76108fdd7d..3766811b76 100644 --- a/crates/editor/src/navigation.rs +++ b/crates/editor/src/navigation.rs @@ -2445,10 +2445,9 @@ impl Editor { let location = Some({ let target_buffer_handle = location_task.await.context("open local buffer")?; let range = target_buffer_handle.read_with(cx, |target_buffer, _| { - let target_start = target_buffer - .clip_point_utf16(point_from_lsp(lsp_location.range.start), Bias::Left); - let target_end = target_buffer - .clip_point_utf16(point_from_lsp(lsp_location.range.end), Bias::Left); + let range = language::range_from_lsp(lsp_location.range); + let target_start = target_buffer.clip_point_utf16(range.start, Bias::Left); + let target_end = target_buffer.clip_point_utf16(range.end, Bias::Left); target_buffer.anchor_after(target_start) ..target_buffer.anchor_before(target_end) }); diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 3f8ba1ab84..ddc74a4a32 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -391,8 +391,9 @@ async fn call_hierarchy_item_from_lsp( } fn anchor_range_from_lsp(range: lsp::Range, buffer: &Buffer) -> Range { - let start = buffer.clip_point_utf16(point_from_lsp(range.start), Bias::Left); - let end = buffer.clip_point_utf16(point_from_lsp(range.end), Bias::Left); + let range = range_from_lsp(range); + let start = buffer.clip_point_utf16(range.start, Bias::Left); + let end = buffer.clip_point_utf16(range.end, Bias::Left); buffer.anchor_after(start)..buffer.anchor_before(end) } @@ -1997,10 +1998,9 @@ pub async fn location_links_from_lsp( cx.update(|cx| { let origin_location = origin_range.map(|origin_range| { let origin_buffer = buffer.read(cx); - let origin_start = - origin_buffer.clip_point_utf16(point_from_lsp(origin_range.start), Bias::Left); - let origin_end = - origin_buffer.clip_point_utf16(point_from_lsp(origin_range.end), Bias::Left); + let origin_range = range_from_lsp(origin_range); + let origin_start = origin_buffer.clip_point_utf16(origin_range.start, Bias::Left); + let origin_end = origin_buffer.clip_point_utf16(origin_range.end, Bias::Left); Location { buffer: buffer.clone(), range: origin_buffer.anchor_after(origin_start) @@ -2009,10 +2009,9 @@ pub async fn location_links_from_lsp( }); let target_buffer = target_buffer_handle.read(cx); - let target_start = - target_buffer.clip_point_utf16(point_from_lsp(target_range.start), Bias::Left); - let target_end = - target_buffer.clip_point_utf16(point_from_lsp(target_range.end), Bias::Left); + let target_range = range_from_lsp(target_range); + let target_start = target_buffer.clip_point_utf16(target_range.start, Bias::Left); + let target_end = target_buffer.clip_point_utf16(target_range.end, Bias::Left); let target_location = Location { buffer: target_buffer_handle, range: target_buffer.anchor_after(target_start) @@ -2091,7 +2090,7 @@ fn edit_prediction_definitions_from_lsp( worktree_id: worktree.id(), path: relative_path, }, - range: point_from_lsp(range.start)..point_from_lsp(range.end), + range: range_from_lsp(range), }); } @@ -2140,10 +2139,9 @@ pub async fn location_link_from_lsp( Ok(cx.update(|cx| { let origin_location = origin_range.map(|origin_range| { let origin_buffer = buffer.read(cx); - let origin_start = - origin_buffer.clip_point_utf16(point_from_lsp(origin_range.start), Bias::Left); - let origin_end = - origin_buffer.clip_point_utf16(point_from_lsp(origin_range.end), Bias::Left); + let origin_range = range_from_lsp(origin_range); + let origin_start = origin_buffer.clip_point_utf16(origin_range.start, Bias::Left); + let origin_end = origin_buffer.clip_point_utf16(origin_range.end, Bias::Left); Location { buffer: buffer.clone(), range: origin_buffer.anchor_after(origin_start) @@ -2152,10 +2150,9 @@ pub async fn location_link_from_lsp( }); let target_buffer = target_buffer_handle.read(cx); - let target_start = - target_buffer.clip_point_utf16(point_from_lsp(target_range.start), Bias::Left); - let target_end = - target_buffer.clip_point_utf16(point_from_lsp(target_range.end), Bias::Left); + let target_range = range_from_lsp(target_range); + let target_start = target_buffer.clip_point_utf16(target_range.start, Bias::Left); + let target_end = target_buffer.clip_point_utf16(target_range.end, Bias::Left); let target_location = Location { buffer: target_buffer_handle, range: target_buffer.anchor_after(target_start) @@ -2331,10 +2328,9 @@ impl LspCommand for GetReferences { target_buffer_handle .clone() .read_with(&cx, |target_buffer, _| { - let target_start = target_buffer - .clip_point_utf16(point_from_lsp(lsp_location.range.start), Bias::Left); - let target_end = target_buffer - .clip_point_utf16(point_from_lsp(lsp_location.range.end), Bias::Left); + let range = range_from_lsp(lsp_location.range); + let target_start = target_buffer.clip_point_utf16(range.start, Bias::Left); + let target_end = target_buffer.clip_point_utf16(range.end, Bias::Left); references.push(Location { buffer: target_buffer_handle, range: target_buffer.anchor_after(target_start) @@ -2492,10 +2488,9 @@ impl LspCommand for GetDocumentHighlights { lsp_highlights .into_iter() .map(|lsp_highlight| { - let start = buffer - .clip_point_utf16(point_from_lsp(lsp_highlight.range.start), Bias::Left); - let end = buffer - .clip_point_utf16(point_from_lsp(lsp_highlight.range.end), Bias::Left); + let range = range_from_lsp(lsp_highlight.range); + let start = buffer.clip_point_utf16(range.start, Bias::Left); + let end = buffer.clip_point_utf16(range.end, Bias::Left); DocumentHighlight { range: buffer.anchor_after(start)..buffer.anchor_before(end), kind: lsp_highlight @@ -2968,9 +2963,9 @@ impl LspCommand for GetHover { ( buffer.language().cloned(), hover.range.map(|range| { - let token_start = - buffer.clip_point_utf16(point_from_lsp(range.start), Bias::Left); - let token_end = buffer.clip_point_utf16(point_from_lsp(range.end), Bias::Left); + let range = range_from_lsp(range); + let token_start = buffer.clip_point_utf16(range.start, Bias::Left); + let token_end = buffer.clip_point_utf16(range.end, Bias::Left); buffer.anchor_after(token_start)..buffer.anchor_before(token_end) }), ) @@ -4946,9 +4941,9 @@ impl LspCommand for LinkedEditingRange { ranges .into_iter() .map(|range| { - let start = - buffer.clip_point_utf16(point_from_lsp(range.start), Bias::Left); - let end = buffer.clip_point_utf16(point_from_lsp(range.end), Bias::Left); + let range = range_from_lsp(range); + let start = buffer.clip_point_utf16(range.start, Bias::Left); + let end = buffer.clip_point_utf16(range.end, Bias::Left); buffer.anchor_before(start)..buffer.anchor_after(end) }) .collect()