fix(lsp): a panic caused by completion with decl_id out of range (#15576)

Fixes a bug caused by #15536 
Sorry about that, @fdncred 

# Description

I've made the panic reproducible in the test case.

TLDR: completer will sometimes return new decl_ids outside of the range
of the engine_state passed in.

# User-Facing Changes

bug fix

# Tests + Formatting

+1

# After Submitting
This commit is contained in:
zc he 2025-04-16 19:43:21 +08:00 committed by GitHub
parent 24cc2f9d87
commit cd4560e97a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 66 additions and 44 deletions

View File

@ -69,7 +69,8 @@ impl Completer for ExportableCompletion<'_> {
wrapped_name(name),
Some(cmd.description().to_string()),
None,
SuggestionKind::Command(cmd.command_type(), Some(*decl_id)),
// `None` here avoids arguments being expanded by snippet edit style for lsp
SuggestionKind::Command(cmd.command_type(), None),
);
}
}

View File

@ -68,52 +68,60 @@ impl LanguageServer {
let mut idx = 0;
// use snippet as `insert_text_format` for command argument completion
if let Some(SuggestionKind::Command(_, Some(decl_id))) = suggestion.kind {
let cmd = engine_state.get_decl(decl_id);
doc_string = Some(Self::get_decl_description(cmd, true));
insert_text_format = Some(InsertTextFormat::SNIPPET);
let signature = cmd.signature();
// add curly brackets around block arguments
// and keywords, e.g. `=` in `alias foo = bar`
let mut arg_wrapper = |arg: &PositionalArg, text: String, optional: bool| -> String {
idx += 1;
match &arg.shape {
SyntaxShape::Block | SyntaxShape::MatchBlock => {
format!("{{ ${{{}:{}}} }}", idx, text)
}
SyntaxShape::Keyword(kwd, _) => {
// NOTE: If optional, the keyword should also be in a placeholder so that it can be removed easily.
// Here we choose to use nested placeholders. Note that some editors don't fully support this format,
// but usually they will simply ignore the inner ones, so it should be fine.
if optional {
idx += 1;
format!(
"${{{}:{} ${{{}:{}}}}}",
idx - 1,
String::from_utf8_lossy(kwd),
idx,
text
)
} else {
format!("{} ${{{}:{}}}", String::from_utf8_lossy(kwd), idx, text)
// NOTE: for new commands defined in current context,
// which are not present in the engine state, skip the documentation and snippet.
if engine_state.num_decls() > decl_id.get() {
let cmd = engine_state.get_decl(decl_id);
doc_string = Some(Self::get_decl_description(cmd, true));
insert_text_format = Some(InsertTextFormat::SNIPPET);
let signature = cmd.signature();
// add curly brackets around block arguments
// and keywords, e.g. `=` in `alias foo = bar`
let mut arg_wrapper = |arg: &PositionalArg,
text: String,
optional: bool|
-> String {
idx += 1;
match &arg.shape {
SyntaxShape::Block | SyntaxShape::MatchBlock => {
format!("{{ ${{{}:{}}} }}", idx, text)
}
SyntaxShape::Keyword(kwd, _) => {
// NOTE: If optional, the keyword should also be in a placeholder so that it can be removed easily.
// Here we choose to use nested placeholders. Note that some editors don't fully support this format,
// but usually they will simply ignore the inner ones, so it should be fine.
if optional {
idx += 1;
format!(
"${{{}:{} ${{{}:{}}}}}",
idx - 1,
String::from_utf8_lossy(kwd),
idx,
text
)
} else {
format!("{} ${{{}:{}}}", String::from_utf8_lossy(kwd), idx, text)
}
}
_ => format!("${{{}:{}}}", idx, text),
}
_ => format!("${{{}:{}}}", idx, text),
}
};
};
for required in signature.required_positional {
snippet_text.push(' ');
snippet_text
.push_str(arg_wrapper(&required, required.name.clone(), false).as_str());
}
for optional in signature.optional_positional {
snippet_text.push(' ');
snippet_text
.push_str(arg_wrapper(&optional, format!("{}?", optional.name), true).as_str());
}
if let Some(rest) = signature.rest_positional {
idx += 1;
snippet_text.push_str(format!(" ${{{}:...{}}}", idx, rest.name).as_str());
for required in signature.required_positional {
snippet_text.push(' ');
snippet_text
.push_str(arg_wrapper(&required, required.name.clone(), false).as_str());
}
for optional in signature.optional_positional {
snippet_text.push(' ');
snippet_text.push_str(
arg_wrapper(&optional, format!("{}?", optional.name), true).as_str(),
);
}
if let Some(rest) = signature.rest_positional {
idx += 1;
snippet_text.push_str(format!(" ${{{}:...{}}}", idx, rest.name).as_str());
}
}
}
// no extra space for a command with args expanded in the snippet
@ -329,6 +337,17 @@ mod tests {
})
));
// fallback completion on a newly defined command,
// the decl_id is missing in the engine state, this shouldn't cause any panic.
let resp = send_complete_request(&client_connection, script.clone(), 13, 9);
assert_json_include!(
actual: result_from_message(resp),
expected: serde_json::json!([
// defined before the cursor
{ "label": "config n foo bar", "detail": detail_str, "kind": 2 },
])
);
// inside parenthesis
let resp = send_complete_request(&client_connection, script, 10, 34);
assert!(result_from_message(resp).as_array().unwrap().contains(

View File

@ -10,3 +10,5 @@ def "config n foo bar" [
echo "🤔🐘"
| str substring (str substring -)
}
config n # don't panic!