Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix panic with completion ranges and autoclose regions interop (cherry-pick #35408) (#35414)

Browse source 
Cherry-picked Fix panic with completion ranges and autoclose regions
interop (#35408)

As reported [in

Discord](https://discord.com/channels/869392257814519848/1106226198494859355/1398470747227426948)
C projects with `"` as "brackets" that autoclose, may invoke panics when
edited at the end of the file.

With a single selection-caret (`ˇ`), at the end of the file,
```c
ifndef BAR_H
#define BAR_H

#include <stdbool.h>

int fn_branch(bool do_branch1, bool do_branch2);

#endif // BAR_H
#include"ˇ"
```
gets an LSP response from clangd
```jsonc
{
  "filterText": "AGL/",
  "insertText": "AGL/",
  "insertTextFormat": 1,
  "kind": 17,
  "label": " AGL/",
  "labelDetails": {},
  "score": 0.78725427389144897,
  "sortText": "40b67681AGL/",
  "textEdit": {
    "newText": "AGL/",
    "range": { "end": { "character": 11, "line": 8 }, "start": { "character": 10, "line": 8 } }
  }
}
```

which replaces `"` after the caret (character/column 11, 0-indexed).
This is reasonable, as regular follow-up (proposed in further
completions), is a suffix + a closing `"`:

<img width="842" height="259" alt="image"

src="https://github.com/user-attachments/assets/ea56f621-7008-4ce2-99ba-87344ddf33d2"
/>

Yet when Zed handles user input of `"`, it panics due to multiple
reasons:

* after applying any snippet text edit, Zed did a selection change:

5537987630/crates/editor/src/editor.rs (L9539-L9545)
which caused eventual autoclose region invalidation:

5537987630/crates/editor/src/editor.rs (L2970)

This covers all cases that insert the `include""` text.

* after applying any user input and "plain" text edit, Zed did not
invalidate any autoclose regions at all, relying on the "bracket" (which
includes `"`) autoclose logic to rule edge cases out

* bracket autoclose logic detects previous `"` and considers the new
user input as a valid closure, hence no autoclose region needed.
But there is an autoclose bracket data after the plaintext completion
insertion (`AGL/`) really, and it's not invalidated after `"` handling

* in addition to that, `Anchor::is_valid` method in `text` panicked, and
required `fn try_fragment_id_for_anchor` to handle "pointing at odd,
after the end of the file, offset" cases as `false`

A test reproducing the feedback and 2 fixes added: proper, autoclose
region invalidation call which required the invalidation logic tweaked a
bit, and "superficial", "do not apply bad selections that cause panics"
fix in the editor to be more robust

Release Notes:

- Fixed panic with completion ranges and autoclose regions interop

---------

Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
This commit is contained in:
gcp-cherry-pick-bot[bot] 2025-08-01 08:39:10 +03:00 • committed by GitHub
parent 5c450693fa
commit 059a409235
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 258 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

172
crates/editor/src/editor_tests.rs
View file

@ -13400,6 +13400,178 @@ async fn test_as_is_completions(cx: &mut TestAppContext) {
cx.assert_editor_state("fn a() {}\n unsafeˇ");
}
#[gpui::test]
async fn test_panic_during_c_completions(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let language =
Arc::try_unwrap(languages::language("c", tree_sitter_c::LANGUAGE.into())).unwrap();
let mut cx = EditorLspTestContext::new(
language,
lsp::ServerCapabilities {
completion_provider: Some(lsp::CompletionOptions {
..lsp::CompletionOptions::default()
}),
..lsp::ServerCapabilities::default()
},
cx,
)
.await;
cx.set_state(
"#ifndef BAR_H
#define BAR_H
#include <stdbool.h>
int fn_branch(bool do_branch1, bool do_branch2);
#endif // BAR_H
ˇ",
);
cx.executor().run_until_parked();
cx.update_editor(|editor, window, cx| {
editor.handle_input("#", window, cx);
});
cx.executor().run_until_parked();
cx.update_editor(|editor, window, cx| {
editor.handle_input("i", window, cx);
});
cx.executor().run_until_parked();
cx.update_editor(|editor, window, cx| {
editor.handle_input("n", window, cx);
});
cx.executor().run_until_parked();
cx.assert_editor_state(
"#ifndef BAR_H
#define BAR_H
#include <stdbool.h>
int fn_branch(bool do_branch1, bool do_branch2);
#endif // BAR_H
#inˇ",
);
cx.lsp
.set_request_handler::<lsp::request::Completion, _, _>(move |_, _| async move {
Ok(Some(lsp::CompletionResponse::List(lsp::CompletionList {
is_incomplete: false,
item_defaults: None,
items: vec![lsp::CompletionItem {
kind: Some(lsp::CompletionItemKind::SNIPPET),
label_details: Some(lsp::CompletionItemLabelDetails {
detail: Some("header".to_string()),
description: None,
}),
label: " include".to_string(),
text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
range: lsp::Range {
start: lsp::Position {
line: 8,
character: 1,
},
end: lsp::Position {
line: 8,
character: 1,
},
},
new_text: "include \"$0\"".to_string(),
})),
sort_text: Some("40b67681include".to_string()),
insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
filter_text: Some("include".to_string()),
insert_text: Some("include \"$0\"".to_string()),
..lsp::CompletionItem::default()
}],
})))
});
cx.update_editor(|editor, window, cx| {
editor.show_completions(&ShowCompletions { trigger: None }, window, cx);
});
cx.executor().run_until_parked();
cx.update_editor(|editor, window, cx| {
editor.confirm_completion(&ConfirmCompletion::default(), window, cx)
});
cx.executor().run_until_parked();
cx.assert_editor_state(
"#ifndef BAR_H
#define BAR_H
#include <stdbool.h>
int fn_branch(bool do_branch1, bool do_branch2);
#endif // BAR_H
#include \"ˇ\"",
);
cx.lsp
.set_request_handler::<lsp::request::Completion, _, _>(move |_, _| async move {
Ok(Some(lsp::CompletionResponse::List(lsp::CompletionList {
is_incomplete: true,
item_defaults: None,
items: vec![lsp::CompletionItem {
kind: Some(lsp::CompletionItemKind::FILE),
label: "AGL/".to_string(),
text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
range: lsp::Range {
start: lsp::Position {
line: 8,
character: 10,
},
end: lsp::Position {
line: 8,
character: 11,
},
},
new_text: "AGL/".to_string(),
})),
sort_text: Some("40b67681AGL/".to_string()),
insert_text_format: Some(lsp::InsertTextFormat::PLAIN_TEXT),
filter_text: Some("AGL/".to_string()),
insert_text: Some("AGL/".to_string()),
..lsp::CompletionItem::default()
}],
})))
});
cx.update_editor(|editor, window, cx| {
editor.show_completions(&ShowCompletions { trigger: None }, window, cx);
});
cx.executor().run_until_parked();
cx.update_editor(|editor, window, cx| {
editor.confirm_completion(&ConfirmCompletion::default(), window, cx)
});
cx.executor().run_until_parked();
cx.assert_editor_state(
r##"#ifndef BAR_H
#define BAR_H
#include <stdbool.h>
int fn_branch(bool do_branch1, bool do_branch2);
#endif // BAR_H
#include "AGL/ˇ"##,
);
cx.update_editor(|editor, window, cx| {
editor.handle_input("\"", window, cx);
});
cx.executor().run_until_parked();
cx.assert_editor_state(
r##"#ifndef BAR_H
#define BAR_H
#include <stdbool.h>
int fn_branch(bool do_branch1, bool do_branch2);
#endif // BAR_H
#include "AGL/"ˇ"##,
);
}
#[gpui::test]
async fn test_no_duplicated_completion_requests(cx: &mut TestAppContext) {
init_test(cx, |_| {});