Skip to content

Commit 85656bf

Browse files
Techatrixosiewicz
authored andcommitted
Filter LSP code actions based on the requested kinds (zed-industries#20847)
I've observed that Zed's implementation of [Code Actions On Format](https://zed.dev/docs/configuring-zed#code-actions-on-format) uses the [CodeActionContext.only](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionContext) parameter to request specific code action kinds from the server. The issue is that it does not filter out code actions from the response, believing that the server will do it. The [LSP specification](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionContext) says that the client is responsible for filtering out unwanted code actions: ```js /** * Requested kind of actions to return. * * Actions not of this kind are filtered out by the client before being * shown. So servers can omit computing them. */ only?: CodeActionKind[]; ``` This PR will filter out unwanted code action on the client side. I have initially encountered this issue because the [ZLS language server](https://github.com/zigtools/zls) (until zigtools/zls#2087) does not filter code action based on `CodeActionContext.only` so Zed runs all received code actions even if they are explicitly disabled in the `code_actions_on_format` setting. Release Notes: - Fix the `code_actions_on_format` setting when used with a language server like ZLS --------- Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
1 parent 3cc51b1 commit 85656bf

File tree

6 files changed

+116
-18
lines changed

6 files changed

+116
-18
lines changed

crates/collab/src/tests/random_project_collaboration_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,7 @@ impl RandomizedTest for ProjectCollaborationTest {
835835
.map_ok(|_| ())
836836
.boxed(),
837837
LspRequestKind::CodeAction => project
838-
.code_actions(&buffer, offset..offset, cx)
838+
.code_actions(&buffer, offset..offset, None, cx)
839839
.map(|_| Ok(()))
840840
.boxed(),
841841
LspRequestKind::Definition => project

crates/editor/src/editor.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13811,7 +13811,9 @@ impl CodeActionProvider for Model<Project> {
1381113811
range: Range<text::Anchor>,
1381213812
cx: &mut WindowContext,
1381313813
) -> Task<Result<Vec<CodeAction>>> {
13814-
self.update(cx, |project, cx| project.code_actions(buffer, range, cx))
13814+
self.update(cx, |project, cx| {
13815+
project.code_actions(buffer, range, None, cx)
13816+
})
1381513817
}
1381613818

1381713819
fn apply_code_action(

crates/project/src/lsp_command.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,19 +2090,33 @@ impl LspCommand for GetCodeActions {
20902090
server_id: LanguageServerId,
20912091
_: AsyncAppContext,
20922092
) -> Result<Vec<CodeAction>> {
2093+
let requested_kinds_set = if let Some(kinds) = self.kinds {
2094+
Some(kinds.into_iter().collect::<HashSet<_>>())
2095+
} else {
2096+
None
2097+
};
2098+
20932099
Ok(actions
20942100
.unwrap_or_default()
20952101
.into_iter()
20962102
.filter_map(|entry| {
2097-
if let lsp::CodeActionOrCommand::CodeAction(lsp_action) = entry {
2098-
Some(CodeAction {
2099-
server_id,
2100-
range: self.range.clone(),
2101-
lsp_action,
2102-
})
2103-
} else {
2104-
None
2103+
let lsp::CodeActionOrCommand::CodeAction(lsp_action) = entry else {
2104+
return None;
2105+
};
2106+
2107+
if let Some((requested_kinds, kind)) =
2108+
requested_kinds_set.as_ref().zip(lsp_action.kind.as_ref())
2109+
{
2110+
if !requested_kinds.contains(kind) {
2111+
return None;
2112+
}
21052113
}
2114+
2115+
Some(CodeAction {
2116+
server_id,
2117+
range: self.range.clone(),
2118+
lsp_action,
2119+
})
21062120
})
21072121
.collect())
21082122
}

crates/project/src/lsp_store.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,6 +2015,7 @@ impl LspStore {
20152015
&mut self,
20162016
buffer_handle: &Model<Buffer>,
20172017
range: Range<Anchor>,
2018+
kinds: Option<Vec<CodeActionKind>>,
20182019
cx: &mut ModelContext<Self>,
20192020
) -> Task<Result<Vec<CodeAction>>> {
20202021
if let Some((upstream_client, project_id)) = self.upstream_client() {
@@ -2028,7 +2029,7 @@ impl LspStore {
20282029
request: Some(proto::multi_lsp_query::Request::GetCodeActions(
20292030
GetCodeActions {
20302031
range: range.clone(),
2031-
kinds: None,
2032+
kinds: kinds.clone(),
20322033
}
20332034
.to_proto(project_id, buffer_handle.read(cx)),
20342035
)),
@@ -2054,7 +2055,7 @@ impl LspStore {
20542055
.map(|code_actions_response| {
20552056
GetCodeActions {
20562057
range: range.clone(),
2057-
kinds: None,
2058+
kinds: kinds.clone(),
20582059
}
20592060
.response_from_proto(
20602061
code_actions_response,
@@ -2079,7 +2080,7 @@ impl LspStore {
20792080
Some(range.start),
20802081
GetCodeActions {
20812082
range: range.clone(),
2082-
kinds: None,
2083+
kinds: kinds.clone(),
20832084
},
20842085
cx,
20852086
);

crates/project/src/project.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ use language::{
5252
Transaction, Unclipped,
5353
};
5454
use lsp::{
55-
CompletionContext, CompletionItemKind, DocumentHighlightKind, LanguageServer, LanguageServerId,
56-
LanguageServerName, MessageActionItem,
55+
CodeActionKind, CompletionContext, CompletionItemKind, DocumentHighlightKind, LanguageServer,
56+
LanguageServerId, LanguageServerName, MessageActionItem,
5757
};
5858
use lsp_command::*;
5959
use node_runtime::NodeRuntime;
@@ -2843,12 +2843,13 @@ impl Project {
28432843
&mut self,
28442844
buffer_handle: &Model<Buffer>,
28452845
range: Range<T>,
2846+
kinds: Option<Vec<CodeActionKind>>,
28462847
cx: &mut ModelContext<Self>,
28472848
) -> Task<Result<Vec<CodeAction>>> {
28482849
let buffer = buffer_handle.read(cx);
28492850
let range = buffer.anchor_before(range.start)..buffer.anchor_before(range.end);
28502851
self.lsp_store.update(cx, |lsp_store, cx| {
2851-
lsp_store.code_actions(buffer_handle, range, cx)
2852+
lsp_store.code_actions(buffer_handle, range, kinds, cx)
28522853
})
28532854
}
28542855

crates/project/src/project_tests.rs

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2792,7 +2792,9 @@ async fn test_apply_code_actions_with_commands(cx: &mut gpui::TestAppContext) {
27922792
let fake_server = fake_language_servers.next().await.unwrap();
27932793

27942794
// Language server returns code actions that contain commands, and not edits.
2795-
let actions = project.update(cx, |project, cx| project.code_actions(&buffer, 0..0, cx));
2795+
let actions = project.update(cx, |project, cx| {
2796+
project.code_actions(&buffer, 0..0, None, cx)
2797+
});
27962798
fake_server
27972799
.handle_request::<lsp::request::CodeActionRequest, _, _>(|_, _| async move {
27982800
Ok(Some(vec![
@@ -4961,6 +4963,84 @@ async fn test_hovers_with_empty_parts(cx: &mut gpui::TestAppContext) {
49614963
);
49624964
}
49634965

4966+
#[gpui::test]
4967+
async fn test_code_actions_only_kinds(cx: &mut gpui::TestAppContext) {
4968+
init_test(cx);
4969+
4970+
let fs = FakeFs::new(cx.executor());
4971+
fs.insert_tree(
4972+
"/dir",
4973+
json!({
4974+
"a.ts": "a",
4975+
}),
4976+
)
4977+
.await;
4978+
4979+
let project = Project::test(fs, ["/dir".as_ref()], cx).await;
4980+
4981+
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
4982+
language_registry.add(typescript_lang());
4983+
let mut fake_language_servers = language_registry.register_fake_lsp(
4984+
"TypeScript",
4985+
FakeLspAdapter {
4986+
capabilities: lsp::ServerCapabilities {
4987+
code_action_provider: Some(lsp::CodeActionProviderCapability::Simple(true)),
4988+
..lsp::ServerCapabilities::default()
4989+
},
4990+
..FakeLspAdapter::default()
4991+
},
4992+
);
4993+
4994+
let buffer = project
4995+
.update(cx, |p, cx| p.open_local_buffer("/dir/a.ts", cx))
4996+
.await
4997+
.unwrap();
4998+
cx.executor().run_until_parked();
4999+
5000+
let fake_server = fake_language_servers
5001+
.next()
5002+
.await
5003+
.expect("failed to get the language server");
5004+
5005+
let mut request_handled = fake_server.handle_request::<lsp::request::CodeActionRequest, _, _>(
5006+
move |_, _| async move {
5007+
Ok(Some(vec![
5008+
lsp::CodeActionOrCommand::CodeAction(lsp::CodeAction {
5009+
title: "organize imports".to_string(),
5010+
kind: Some(CodeActionKind::SOURCE_ORGANIZE_IMPORTS),
5011+
..lsp::CodeAction::default()
5012+
}),
5013+
lsp::CodeActionOrCommand::CodeAction(lsp::CodeAction {
5014+
title: "fix code".to_string(),
5015+
kind: Some(CodeActionKind::SOURCE_FIX_ALL),
5016+
..lsp::CodeAction::default()
5017+
}),
5018+
]))
5019+
},
5020+
);
5021+
5022+
let code_actions_task = project.update(cx, |project, cx| {
5023+
project.code_actions(
5024+
&buffer,
5025+
0..buffer.read(cx).len(),
5026+
Some(vec![CodeActionKind::SOURCE_ORGANIZE_IMPORTS]),
5027+
cx,
5028+
)
5029+
});
5030+
5031+
let () = request_handled
5032+
.next()
5033+
.await
5034+
.expect("The code action request should have been triggered");
5035+
5036+
let code_actions = code_actions_task.await.unwrap();
5037+
assert_eq!(code_actions.len(), 1);
5038+
assert_eq!(
5039+
code_actions[0].lsp_action.kind,
5040+
Some(CodeActionKind::SOURCE_ORGANIZE_IMPORTS)
5041+
);
5042+
}
5043+
49645044
#[gpui::test]
49655045
async fn test_multiple_language_server_actions(cx: &mut gpui::TestAppContext) {
49665046
init_test(cx);
@@ -5092,7 +5172,7 @@ async fn test_multiple_language_server_actions(cx: &mut gpui::TestAppContext) {
50925172
}
50935173

50945174
let code_actions_task = project.update(cx, |project, cx| {
5095-
project.code_actions(&buffer, 0..buffer.read(cx).len(), cx)
5175+
project.code_actions(&buffer, 0..buffer.read(cx).len(), None, cx)
50965176
});
50975177

50985178
// cx.run_until_parked();

0 commit comments

Comments
 (0)