Make anchor_in_excerpt Optional (cherry-pick #8975) (#8979)

gcp-cherry-pick-bot[bot] and Conrad Irwin created

Cherry-picked Make anchor_in_excerpt Optional (#8975)

We were seeing panics due to callers assuming they had valid
excerpt_ids, but that cannot easily be guaranteed across await points as
anyone may remove an excerpt.

Release Notes:

- Fixed a panic when hovering in a multibuffer

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

Change summary

crates/assistant/src/assistant_panel.rs       |  4 +
crates/diagnostics/src/diagnostics.rs         | 18 ++++---
crates/editor/src/hover_links.rs              | 27 ++++++-----
crates/editor/src/hover_popover.rs            | 25 +++++-----
crates/editor/src/inlay_hint_cache.rs         | 44 +++++++++++--------
crates/editor/src/items.rs                    |  4 
crates/language_tools/src/syntax_tree_view.rs |  8 ++
crates/multi_buffer/src/multi_buffer.rs       | 46 ++++++++++++--------
8 files changed, 102 insertions(+), 74 deletions(-)

Detailed changes

crates/assistant/src/assistant_panel.rs 🔗

@@ -2413,7 +2413,9 @@ impl ConversationEditor {
                 .read(cx)
                 .messages(cx)
                 .map(|message| BlockProperties {
-                    position: buffer.anchor_in_excerpt(excerpt_id, message.anchor),
+                    position: buffer
+                        .anchor_in_excerpt(excerpt_id, message.anchor)
+                        .unwrap(),
                     height: 2,
                     style: BlockStyle::Sticky,
                     render: Arc::new({

crates/diagnostics/src/diagnostics.rs 🔗

@@ -517,15 +517,15 @@ impl ProjectDiagnosticsEditor {
         self.editor.update(cx, |editor, cx| {
             editor.remove_blocks(blocks_to_remove, None, cx);
             let block_ids = editor.insert_blocks(
-                blocks_to_add.into_iter().map(|block| {
+                blocks_to_add.into_iter().flat_map(|block| {
                     let (excerpt_id, text_anchor) = block.position;
-                    BlockProperties {
-                        position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, text_anchor),
+                    Some(BlockProperties {
+                        position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, text_anchor)?,
                         height: block.height,
                         style: block.style,
                         render: block.render,
                         disposition: block.disposition,
-                    }
+                    })
                 }),
                 Some(Autoscroll::fit()),
                 cx,
@@ -589,14 +589,16 @@ impl ProjectDiagnosticsEditor {
                         Ok(ix) | Err(ix) => ix,
                     };
                     if let Some(group) = groups.get(group_ix) {
-                        let offset = excerpts_snapshot
+                        if let Some(offset) = excerpts_snapshot
                             .anchor_in_excerpt(
                                 group.excerpts[group.primary_excerpt_ix],
                                 group.primary_diagnostic.range.start,
                             )
-                            .to_offset(&excerpts_snapshot);
-                        selection.start = offset;
-                        selection.end = offset;
+                            .map(|anchor| anchor.to_offset(&excerpts_snapshot))
+                        {
+                            selection.start = offset;
+                            selection.end = offset;
+                        }
                     }
                 }
             }

crates/editor/src/hover_links.rs 🔗

@@ -13,7 +13,7 @@ use project::{
 };
 use std::ops::Range;
 use theme::ActiveTheme as _;
-use util::TryFutureExt;
+use util::{maybe, TryFutureExt};
 
 #[derive(Debug)]
 pub struct HoveredLinkState {
@@ -424,12 +424,13 @@ pub fn show_link_definition(
                 TriggerPoint::Text(_) => {
                     if let Some((url_range, url)) = find_url(&buffer, buffer_position, cx.clone()) {
                         this.update(&mut cx, |_, _| {
-                            let start = snapshot.anchor_in_excerpt(excerpt_id, url_range.start);
-                            let end = snapshot.anchor_in_excerpt(excerpt_id, url_range.end);
-                            (
-                                Some(RangeInEditor::Text(start..end)),
-                                vec![HoverLink::Url(url)],
-                            )
+                            let range = maybe!({
+                                let start =
+                                    snapshot.anchor_in_excerpt(excerpt_id, url_range.start)?;
+                                let end = snapshot.anchor_in_excerpt(excerpt_id, url_range.end)?;
+                                Some(RangeInEditor::Text(start..end))
+                            });
+                            (range, vec![HoverLink::Url(url)])
                         })
                         .ok()
                     } else if let Some(project) = project {
@@ -449,12 +450,14 @@ pub fn show_link_definition(
                             .map(|definition_result| {
                                 (
                                     definition_result.iter().find_map(|link| {
-                                        link.origin.as_ref().map(|origin| {
-                                            let start = snapshot
-                                                .anchor_in_excerpt(excerpt_id, origin.range.start);
+                                        link.origin.as_ref().and_then(|origin| {
+                                            let start = snapshot.anchor_in_excerpt(
+                                                excerpt_id,
+                                                origin.range.start,
+                                            )?;
                                             let end = snapshot
-                                                .anchor_in_excerpt(excerpt_id, origin.range.end);
-                                            RangeInEditor::Text(start..end)
+                                                .anchor_in_excerpt(excerpt_id, origin.range.end)?;
+                                            Some(RangeInEditor::Text(start..end))
                                         })
                                     }),
                                     definition_result.into_iter().map(HoverLink::Text).collect(),

crates/editor/src/hover_popover.rs 🔗

@@ -294,18 +294,19 @@ fn show_hover(
             let hover_popover = match hover_result {
                 Some(hover_result) if !hover_result.is_empty() => {
                     // Create symbol range of anchors for highlighting and filtering of future requests.
-                    let range = if let Some(range) = hover_result.range {
-                        let start = snapshot
-                            .buffer_snapshot
-                            .anchor_in_excerpt(excerpt_id, range.start);
-                        let end = snapshot
-                            .buffer_snapshot
-                            .anchor_in_excerpt(excerpt_id, range.end);
-
-                        start..end
-                    } else {
-                        anchor..anchor
-                    };
+                    let range = hover_result
+                        .range
+                        .and_then(|range| {
+                            let start = snapshot
+                                .buffer_snapshot
+                                .anchor_in_excerpt(excerpt_id, range.start)?;
+                            let end = snapshot
+                                .buffer_snapshot
+                                .anchor_in_excerpt(excerpt_id, range.end)?;
+
+                            Some(start..end)
+                        })
+                        .unwrap_or_else(|| anchor..anchor);
 
                     let language_registry =
                         project.update(&mut cx, |p, _| p.languages().clone())?;

crates/editor/src/inlay_hint_cache.rs 🔗

@@ -460,14 +460,15 @@ impl InlayHintCache {
                                     if !old_kinds.contains(&cached_hint.kind)
                                         && new_kinds.contains(&cached_hint.kind)
                                     {
-                                        to_insert.push(Inlay::hint(
-                                            cached_hint_id.id(),
-                                            multi_buffer_snapshot.anchor_in_excerpt(
-                                                *excerpt_id,
-                                                cached_hint.position,
-                                            ),
-                                            &cached_hint,
-                                        ));
+                                        if let Some(anchor) = multi_buffer_snapshot
+                                            .anchor_in_excerpt(*excerpt_id, cached_hint.position)
+                                        {
+                                            to_insert.push(Inlay::hint(
+                                                cached_hint_id.id(),
+                                                anchor,
+                                                &cached_hint,
+                                            ));
+                                        }
                                     }
                                     excerpt_cache.next();
                                 }
@@ -483,12 +484,15 @@ impl InlayHintCache {
                 let maybe_missed_cached_hint = &excerpt_cached_hints.hints_by_id[cached_hint_id];
                 let cached_hint_kind = maybe_missed_cached_hint.kind;
                 if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) {
-                    to_insert.push(Inlay::hint(
-                        cached_hint_id.id(),
-                        multi_buffer_snapshot
-                            .anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position),
-                        &maybe_missed_cached_hint,
-                    ));
+                    if let Some(anchor) = multi_buffer_snapshot
+                        .anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position)
+                    {
+                        to_insert.push(Inlay::hint(
+                            cached_hint_id.id(),
+                            anchor,
+                            &maybe_missed_cached_hint,
+                        ));
+                    }
                 }
             }
         }
@@ -1200,11 +1204,13 @@ fn apply_hint_update(
                 .allowed_hint_kinds
                 .contains(&new_hint.kind)
             {
-                let new_hint_position =
-                    multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position);
-                splice
-                    .to_insert
-                    .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint));
+                if let Some(new_hint_position) =
+                    multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position)
+                {
+                    splice
+                        .to_insert
+                        .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint));
+                }
             }
             let new_id = InlayId::Hint(new_inlay_id);
             cached_excerpt_hints.hints_by_id.insert(new_id, new_hint);

crates/editor/src/items.rs 🔗

@@ -1154,8 +1154,8 @@ impl SearchableItem for Editor {
                                 let end = excerpt
                                     .buffer
                                     .anchor_before(excerpt_range.start + range.end);
-                                buffer.anchor_in_excerpt(excerpt.id, start)
-                                    ..buffer.anchor_in_excerpt(excerpt.id, end)
+                                buffer.anchor_in_excerpt(excerpt.id, start).unwrap()
+                                    ..buffer.anchor_in_excerpt(excerpt.id, end).unwrap()
                             }),
                     );
                 }

crates/language_tools/src/syntax_tree_view.rs 🔗

@@ -227,8 +227,12 @@ impl SyntaxTreeView {
         let multibuffer = editor_state.editor.read(cx).buffer();
         let multibuffer = multibuffer.read(cx).snapshot(cx);
         let excerpt_id = buffer_state.excerpt_id;
-        let range = multibuffer.anchor_in_excerpt(excerpt_id, range.start)
-            ..multibuffer.anchor_in_excerpt(excerpt_id, range.end);
+        let range = multibuffer
+            .anchor_in_excerpt(excerpt_id, range.start)
+            .unwrap()
+            ..multibuffer
+                .anchor_in_excerpt(excerpt_id, range.end)
+                .unwrap();
 
         // Update the editor with the anchor range.
         editor_state.editor.update(cx, |editor, cx| {

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -2788,7 +2788,13 @@ impl MultiBufferSnapshot {
         }
     }
 
-    pub fn anchor_in_excerpt(&self, excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Anchor {
+    /// Returns an anchor for the given excerpt and text anchor,
+    /// returns None if the excerpt_id is no longer valid.
+    pub fn anchor_in_excerpt(
+        &self,
+        excerpt_id: ExcerptId,
+        text_anchor: text::Anchor,
+    ) -> Option<Anchor> {
         let locator = self.excerpt_locator_for_id(excerpt_id);
         let mut cursor = self.excerpts.cursor::<Option<&Locator>>();
         cursor.seek(locator, Bias::Left, &());
@@ -2796,14 +2802,14 @@ impl MultiBufferSnapshot {
             if excerpt.id == excerpt_id {
                 let text_anchor = excerpt.clip_anchor(text_anchor);
                 drop(cursor);
-                return Anchor {
+                return Some(Anchor {
                     buffer_id: Some(excerpt.buffer_id),
                     excerpt_id,
                     text_anchor,
-                };
+                });
             }
         }
-        panic!("excerpt not found");
+        None
     }
 
     pub fn can_resolve(&self, anchor: &Anchor) -> bool {
@@ -3283,13 +3289,15 @@ impl MultiBufferSnapshot {
             outline
                 .items
                 .into_iter()
-                .map(|item| OutlineItem {
-                    depth: item.depth,
-                    range: self.anchor_in_excerpt(*excerpt_id, item.range.start)
-                        ..self.anchor_in_excerpt(*excerpt_id, item.range.end),
-                    text: item.text,
-                    highlight_ranges: item.highlight_ranges,
-                    name_ranges: item.name_ranges,
+                .flat_map(|item| {
+                    Some(OutlineItem {
+                        depth: item.depth,
+                        range: self.anchor_in_excerpt(*excerpt_id, item.range.start)?
+                            ..self.anchor_in_excerpt(*excerpt_id, item.range.end)?,
+                        text: item.text,
+                        highlight_ranges: item.highlight_ranges,
+                        name_ranges: item.name_ranges,
+                    })
                 })
                 .collect(),
         ))
@@ -3310,13 +3318,15 @@ impl MultiBufferSnapshot {
                 .symbols_containing(anchor.text_anchor, theme)
                 .into_iter()
                 .flatten()
-                .map(|item| OutlineItem {
-                    depth: item.depth,
-                    range: self.anchor_in_excerpt(excerpt_id, item.range.start)
-                        ..self.anchor_in_excerpt(excerpt_id, item.range.end),
-                    text: item.text,
-                    highlight_ranges: item.highlight_ranges,
-                    name_ranges: item.name_ranges,
+                .flat_map(|item| {
+                    Some(OutlineItem {
+                        depth: item.depth,
+                        range: self.anchor_in_excerpt(excerpt_id, item.range.start)?
+                            ..self.anchor_in_excerpt(excerpt_id, item.range.end)?,
+                        text: item.text,
+                        highlight_ranges: item.highlight_ranges,
+                        name_ranges: item.name_ranges,
+                    })
                 })
                 .collect(),
         ))