From f1d80b715aa4cf5bb80785384870b33fcb7043f3 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 29 Sep 2025 14:01:34 -0600 Subject: [PATCH] Fix panic in UnwrapSyntaxNode (#39139) Closes #39139 Fixes ZED-1HY Release Notes: - Fixed a panic in UnwrapSyntaxNode in multi-buffers --- crates/editor/src/actions.rs | 2 + crates/editor/src/editor.rs | 20 +++------ crates/editor/src/editor_tests.rs | 58 +++++++++++++++++++++++++ crates/editor/src/hover_popover.rs | 11 ++--- crates/multi_buffer/src/multi_buffer.rs | 16 ++----- 5 files changed, 72 insertions(+), 35 deletions(-) diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 9dac77970a4560d4e9684c925b5ac4878157941f..8e682888031c238323540f718efb59e63787665d 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -776,6 +776,8 @@ actions!( UniqueLinesCaseInsensitive, /// Removes duplicate lines (case-sensitive). UniqueLinesCaseSensitive, + /// Removes the surrounding syntax node (for example brackets, or closures) + /// from the current selections. UnwrapSyntaxNode, /// Wraps selections in tag specified by language. WrapSelectionsInTag diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1c440c2192b56d4cb8e16bdf0002eca1d3679745..9ab7f857dad3d33b2f4eaaa5298dbee8a307f05d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -142,7 +142,7 @@ use mouse_context_menu::MouseContextMenu; use movement::TextLayoutDetails; use multi_buffer::{ ExcerptInfo, ExpandExcerptDirection, MultiBufferDiffHunk, MultiBufferPoint, MultiBufferRow, - MultiOrSingleBufferOffsetRange, ToOffsetUtf16, + ToOffsetUtf16, }; use parking_lot::Mutex; use persistence::DB; @@ -15175,12 +15175,8 @@ impl Editor { } let mut new_range = old_range.clone(); - while let Some((node, containing_range)) = buffer.syntax_ancestor(new_range.clone()) - { - new_range = match containing_range { - MultiOrSingleBufferOffsetRange::Single(_) => break, - MultiOrSingleBufferOffsetRange::Multi(range) => range, - }; + while let Some((node, range)) = buffer.syntax_ancestor(new_range.clone()) { + new_range = range; if !node.is_named() { continue; } @@ -15310,20 +15306,14 @@ impl Editor { && let Some((_, ancestor_range)) = buffer.syntax_ancestor(selection.start..selection.end) { - match ancestor_range { - MultiOrSingleBufferOffsetRange::Single(range) => range, - MultiOrSingleBufferOffsetRange::Multi(range) => range, - } + ancestor_range } else { selection.range() }; let mut parent = child.clone(); while let Some((_, ancestor_range)) = buffer.syntax_ancestor(parent.clone()) { - parent = match ancestor_range { - MultiOrSingleBufferOffsetRange::Single(range) => range, - MultiOrSingleBufferOffsetRange::Multi(range) => range, - }; + parent = ancestor_range; if parent.start < child.start || parent.end > child.end { break; } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index c3a527899e4c509047fbf2d8273f1fb1c5a6c2a7..46b7ee6a6522579d5872868fcb173bfe3f0297ba 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9161,6 +9161,64 @@ async fn test_unwrap_syntax_nodes(cx: &mut gpui::TestAppContext) { }); cx.assert_editor_state(indoc! { r#"use mod1::{mod2::«mod3ˇ», mod5::«mod7ˇ»};"# }); + + cx.set_state(indoc! { r#"fn a() { + // what + // a + // ˇlong + // method + // I + // sure + // hope + // it + // works + }"# }); + + let buffer = cx.update_multibuffer(|multibuffer, _| multibuffer.as_singleton().unwrap()); + let multi_buffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); + cx.update(|_, cx| { + multi_buffer.update(cx, |multi_buffer, cx| { + multi_buffer.set_excerpts_for_path( + PathKey::for_buffer(&buffer, cx), + buffer, + [Point::new(1, 0)..Point::new(1, 0)], + 3, + cx, + ); + }); + }); + + let editor2 = cx.new_window_entity(|window, cx| { + Editor::new(EditorMode::full(), multi_buffer, None, window, cx) + }); + + let mut cx = EditorTestContext::for_editor_in(editor2, &mut cx).await; + cx.update_editor(|editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(3, 0)..Point::new(3, 0)]); + }) + }); + + cx.assert_editor_state(indoc! { " + fn a() { + // what + // a + ˇ // long + // method"}); + + cx.update_editor(|editor, window, cx| { + editor.unwrap_syntax_node(&UnwrapSyntaxNode, window, cx); + }); + + // Although we could potentially make the action work when the syntax node + // is half-hidden, it seems a bit dangerous as you can't easily tell what it + // did. Maybe we could also expand the excerpt to contain the range? + cx.assert_editor_state(indoc! { " + fn a() { + // what + // a + ˇ // long + // method"}); } #[gpui::test] diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 1815029207cf485292277861cd8d18ed482d6077..bbc01d725d23ef65e8c2660a8635023be9223ed4 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -16,7 +16,7 @@ use itertools::Itertools; use language::{DiagnosticEntry, Language, LanguageRegistry}; use lsp::DiagnosticSeverity; use markdown::{Markdown, MarkdownElement, MarkdownStyle}; -use multi_buffer::{MultiOrSingleBufferOffsetRange, ToOffset, ToPoint}; +use multi_buffer::{ToOffset, ToPoint}; use project::{HoverBlock, HoverBlockKind, InlayHintLabelPart}; use settings::Settings; use std::{borrow::Cow, cell::RefCell}; @@ -477,13 +477,8 @@ fn show_hover( }) .or_else(|| { let snapshot = &snapshot.buffer_snapshot; - match snapshot.syntax_ancestor(anchor..anchor)?.1 { - MultiOrSingleBufferOffsetRange::Multi(range) => Some( - snapshot.anchor_before(range.start) - ..snapshot.anchor_after(range.end), - ), - MultiOrSingleBufferOffsetRange::Single(_) => None, - } + let range = snapshot.syntax_ancestor(anchor..anchor)?.1; + Some(snapshot.anchor_before(range.start)..snapshot.anchor_after(range.end)) }) .unwrap_or_else(|| anchor..anchor); diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 4132666c628925aceb57ee5033600f64ffa5f6a8..01bf1ee36990981b8de6f771855e916412163cf8 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -79,12 +79,6 @@ pub struct MultiBuffer { buffer_changed_since_sync: Rc>, } -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum MultiOrSingleBufferOffsetRange { - Single(Range), - Multi(Range), -} - #[derive(Clone, Debug, PartialEq, Eq)] pub enum Event { ExcerptsAdded { @@ -6077,19 +6071,17 @@ impl MultiBufferSnapshot { pub fn syntax_ancestor( &self, range: Range, - ) -> Option<(tree_sitter::Node<'_>, MultiOrSingleBufferOffsetRange)> { + ) -> Option<(tree_sitter::Node<'_>, Range)> { let range = range.start.to_offset(self)..range.end.to_offset(self); let mut excerpt = self.excerpt_containing(range.clone())?; let node = excerpt .buffer() .syntax_ancestor(excerpt.map_range_to_buffer(range))?; let node_range = node.byte_range(); - let range = if excerpt.contains_buffer_range(node_range.clone()) { - MultiOrSingleBufferOffsetRange::Multi(excerpt.map_range_from_buffer(node_range)) - } else { - MultiOrSingleBufferOffsetRange::Single(node_range) + if !excerpt.contains_buffer_range(node_range.clone()) { + return None; }; - Some((node, range)) + Some((node, excerpt.map_range_from_buffer(node_range))) } pub fn syntax_next_sibling(