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 <mikayla@zed.dev> Release Notes: - Fixed a crash with some languages servers when handling `editor::FindAllReferences`.
This commit is contained in:
1 parent
49469000f0
commit
a61e2609c1
6 files changed
+123
-58
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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::<lsp::request::References, _, _>(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::<Editor>(cx).collect::<Vec<_>>()
|
||||
});
|
||||
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, |_| {});
|
||||
|
||||
@@ -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)
|
||||
});
|
||||
|
||||
@@ -391,8 +391,9 @@ async fn call_hierarchy_item_from_lsp(
|
||||
}
|
||||
|
||||
fn anchor_range_from_lsp(range: lsp::Range, buffer: &Buffer) -> Range<Anchor> {
|
||||
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()
|
||||
|
||||
Reference in new issue
Block a user