Skip to content

Commit 4cdc5f1

Browse files
jpttrssnjunglerobba
authored andcommitted
Add code actions on save
* Add code-actions-on-save config * Match VS Code config to allow future flexibility * Refactor lsp commands to allow for code reuse * Attempt code actions for all language servers for the document * Add lsp specific integration tests * Update documentation in book * Canonicalize path argument when retrieving documents by path * Resolves issue when running lsp integration tests in windows
1 parent 26c340d commit 4cdc5f1

File tree

11 files changed

+481
-49
lines changed

11 files changed

+481
-49
lines changed

.cargo/config.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ rustflags = ["--cfg", "tokio_unstable", "-C", "target-feature=-crt-static"]
1414
[alias]
1515
xtask = "run --package xtask --"
1616
integration-test = "test --features integration --profile integration --workspace --test integration"
17+
integration-test-lsp = "test --features integration-lsp --profile integration --workspace --test integration-lsp -- --test-threads=1"
1718

.github/workflows/build.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,25 @@ jobs:
6969
key: ${{ runner.os }}-${{ runner.arch }}-stable-v${{ env.GRAMMAR_CACHE_VERSION }}-tree-sitter-grammars-${{ hashFiles('languages.toml') }}
7070
restore-keys: ${{ runner.os }}-${{ runner.arch }}-stable-v${{ env.GRAMMAR_CACHE_VERSION }}-tree-sitter-grammars-
7171

72+
- name: Install go lsp integration tests
73+
uses: actions/setup-go@v5
74+
with:
75+
go-version: '^1.22.0'
76+
77+
- name: Install gopls for lsp integration tests
78+
run: go install golang.org/x/tools/gopls@latest
79+
80+
81+
7282
- name: Run cargo test
7383
run: cargo test --workspace
7484

7585
- name: Run cargo integration-test
7686
run: cargo integration-test
7787

88+
- name: Run cargo integration-test-lsp
89+
run: cargo integration-test-lsp
90+
7891
strategy:
7992
matrix:
8093
os: [ubuntu-latest, macos-latest, windows-latest, ubuntu-24.04-arm]

book/src/languages.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ These configuration keys are available:
7575
| `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml`. Overwrites the setting of the same name in `config.toml` if set. |
7676
| `persistent-diagnostic-sources` | An array of LSP diagnostic sources assumed unchanged when the language server resends the same set of diagnostics. Helix can track the position for these diagnostics internally instead. Useful for diagnostics that are recomputed on save.
7777
| `rainbow-brackets` | Overrides the `editor.rainbow-brackets` config key for the language |
78+
| `code-actions-on-save` | List of LSP code actions to be run in order on save, for example `[{ code-action = "source.organizeImports", enabled = true }]` |
7879

7980
### File-type detection and the `file-types` key
8081

docs/CONTRIBUTING.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ Contributors using MacOS might encounter `Too many open files (os error 24)`
5959
failures while running integration tests. This can be resolved by increasing
6060
the default value (e.g. to `10240` from `256`) by running `ulimit -n 10240`.
6161

62+
### Language Server tests
63+
64+
There are integration tests specific for language server integration that can be
65+
run with `cargo integration-test-lsp` and have additional dependencies.
66+
67+
* [go](https://go.dev)
68+
* [gopls](https://pkg.go.dev/golang.org/x/tools/gopls)
69+
6270
## Minimum Stable Rust Version (MSRV) Policy
6371

6472
Helix keeps an intentionally low MSRV for the sake of easy building and packaging

helix-core/src/syntax/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ pub struct LanguageConfiguration {
5656
#[serde(default)]
5757
pub auto_format: bool,
5858

59+
#[serde(default)]
60+
pub code_actions_on_save: Option<Vec<CodeActionsOnSave>>,
61+
5962
#[serde(skip_serializing_if = "Option::is_none")]
6063
pub formatter: Option<FormatterConfiguration>,
6164

@@ -433,6 +436,13 @@ pub struct AdvancedCompletion {
433436
pub default: Option<String>,
434437
}
435438

439+
#[derive(Debug, Clone, Serialize, Deserialize)]
440+
#[serde(rename_all = "kebab-case")]
441+
pub struct CodeActionsOnSave {
442+
pub code_action: String,
443+
pub enabled: bool,
444+
}
445+
436446
#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
437447
#[serde(rename_all = "kebab-case", untagged)]
438448
pub enum DebugConfigCompletion {

helix-term/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ assets = [
3434
default = ["git"]
3535
unicode-lines = ["helix-core/unicode-lines", "helix-view/unicode-lines"]
3636
integration = ["helix-event/integration_test"]
37+
integration-lsp = ["integration"]
3738
git = ["helix-vcs/git"]
3839

3940
[[bin]]

helix-term/src/commands/lsp.rs

Lines changed: 164 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use futures_util::{stream::FuturesOrdered, FutureExt};
22
use helix_lsp::{
33
block_on,
44
lsp::{
5-
self, CodeAction, CodeActionOrCommand, CodeActionTriggerKind, DiagnosticSeverity,
6-
NumberOrString,
5+
self, CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionTriggerKind,
6+
DiagnosticSeverity, NumberOrString,
77
},
88
util::{diagnostic_to_lsp_diagnostic, lsp_range_to_range, range_to_lsp_range},
99
Client, LanguageServerId, OffsetEncoding,
@@ -23,7 +23,7 @@ use helix_view::{
2323
editor::Action,
2424
handlers::lsp::SignatureHelpInvoked,
2525
theme::Style,
26-
Document, View,
26+
Document, DocumentId, View,
2727
};
2828

2929
use crate::{
@@ -587,6 +587,11 @@ struct CodeActionOrCommandItem {
587587
language_server_id: LanguageServerId,
588588
}
589589

590+
struct CodeActionItem {
591+
lsp_item: lsp::CodeAction,
592+
language_server_id: LanguageServerId,
593+
}
594+
590595
impl ui::menu::Item for CodeActionOrCommandItem {
591596
type Data = ();
592597
fn format(&self, _data: &Self::Data) -> Row<'_> {
@@ -659,34 +664,8 @@ pub fn code_action(cx: &mut Context) {
659664

660665
let selection_range = doc.selection(view.id).primary();
661666

662-
let mut seen_language_servers = HashSet::new();
663-
664-
let mut futures: FuturesOrdered<_> = doc
665-
.language_servers_with_feature(LanguageServerFeature::CodeAction)
666-
.filter(|ls| seen_language_servers.insert(ls.id()))
667-
// TODO this should probably already been filtered in something like "language_servers_with_feature"
668-
.filter_map(|language_server| {
669-
let offset_encoding = language_server.offset_encoding();
670-
let language_server_id = language_server.id();
671-
let range = range_to_lsp_range(doc.text(), selection_range, offset_encoding);
672-
// Filter and convert overlapping diagnostics
673-
let code_action_context = lsp::CodeActionContext {
674-
diagnostics: doc
675-
.diagnostics()
676-
.iter()
677-
.filter(|&diag| {
678-
selection_range
679-
.overlaps(&helix_core::Range::new(diag.range.start, diag.range.end))
680-
})
681-
.map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding))
682-
.collect(),
683-
only: None,
684-
trigger_kind: Some(CodeActionTriggerKind::INVOKED),
685-
};
686-
let code_action_request =
687-
language_server.code_actions(doc.identifier(), range, code_action_context)?;
688-
Some((code_action_request, language_server_id))
689-
})
667+
let mut futures: FuturesOrdered<_> = code_actions_for_range(doc, selection_range, None)
668+
.into_iter()
690669
.map(|(request, ls_id)| async move {
691670
let Some(mut actions) = request.await? else {
692671
return anyhow::Ok(Vec::new());
@@ -788,19 +767,12 @@ pub fn code_action(cx: &mut Context) {
788767
}
789768
lsp::CodeActionOrCommand::CodeAction(code_action) => {
790769
log::debug!("code action: {:?}", code_action);
791-
// we support lsp "codeAction/resolve" for `edit` and `command` fields
792-
let mut resolved_code_action = None;
793-
if code_action.edit.is_none() || code_action.command.is_none() {
794-
if let Some(future) = language_server.resolve_code_action(code_action) {
795-
if let Ok(code_action) = helix_lsp::block_on(future) {
796-
resolved_code_action = Some(code_action);
797-
}
798-
}
799-
}
800770
let resolved_code_action =
801-
resolved_code_action.as_ref().unwrap_or(code_action);
771+
resolve_code_action(code_action, language_server);
802772

803-
if let Some(ref workspace_edit) = resolved_code_action.edit {
773+
if let Some(ref workspace_edit) =
774+
resolved_code_action.as_ref().unwrap_or(code_action).edit
775+
{
804776
let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit);
805777
}
806778

@@ -825,6 +797,156 @@ pub fn code_action(cx: &mut Context) {
825797
});
826798
}
827799

800+
pub fn code_actions_for_range(
801+
doc: &Document,
802+
range: helix_core::Range,
803+
only: Option<Vec<CodeActionKind>>,
804+
) -> Vec<(
805+
impl Future<Output = Result<Option<Vec<CodeActionOrCommand>>, helix_lsp::Error>>,
806+
LanguageServerId,
807+
)> {
808+
let mut seen_language_servers = HashSet::new();
809+
810+
doc.language_servers_with_feature(LanguageServerFeature::CodeAction)
811+
.filter(|ls| seen_language_servers.insert(ls.id()))
812+
// TODO this should probably already been filtered in something like "language_servers_with_feature"
813+
.filter_map(|language_server| {
814+
let offset_encoding = language_server.offset_encoding();
815+
let language_server_id = language_server.id();
816+
let lsp_range = range_to_lsp_range(doc.text(), range, offset_encoding);
817+
// Filter and convert overlapping diagnostics
818+
let code_action_context = lsp::CodeActionContext {
819+
diagnostics: doc
820+
.diagnostics()
821+
.iter()
822+
.filter(|&diag| {
823+
range.overlaps(&helix_core::Range::new(diag.range.start, diag.range.end))
824+
})
825+
.map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding))
826+
.collect(),
827+
only: only.clone(),
828+
trigger_kind: Some(CodeActionTriggerKind::INVOKED),
829+
};
830+
let code_action_request =
831+
language_server.code_actions(doc.identifier(), lsp_range, code_action_context)?;
832+
Some((code_action_request, language_server_id))
833+
})
834+
.collect::<Vec<_>>()
835+
}
836+
837+
/// Will apply the code actions on save from the language config for each language server
838+
pub fn code_actions_on_save(cx: &mut compositor::Context, doc_id: &DocumentId) {
839+
let doc = doc!(cx.editor, doc_id);
840+
841+
let code_actions_on_save_cfg = doc
842+
.language_config()
843+
.and_then(|c| c.code_actions_on_save.clone());
844+
845+
if let Some(code_actions_on_save_cfg) = code_actions_on_save_cfg {
846+
for code_action_kind in code_actions_on_save_cfg.into_iter().filter_map(|action| {
847+
action
848+
.enabled
849+
.then_some(CodeActionKind::from(action.code_action))
850+
}) {
851+
log::debug!("Attempting code action on save {:?}", code_action_kind);
852+
let doc = doc!(cx.editor, doc_id);
853+
let full_range = helix_core::Range::new(0, doc.text().len_chars());
854+
let code_actions: Vec<CodeActionItem> =
855+
code_actions_for_range(doc, full_range, Some(vec![code_action_kind.clone()]))
856+
.into_iter()
857+
.filter_map(|(future, language_server_id)| {
858+
if let Ok(Some(mut actions)) = helix_lsp::block_on(future) {
859+
// Retain only enabled code actions that do not have commands.
860+
//
861+
// Commands are deprecated and are not supported because they apply
862+
// workspace edits asynchronously and there is currently no mechanism
863+
// to handle waiting for the workspace edits to be applied before moving
864+
// on to the next code action (or auto-format).
865+
actions.retain(|action| {
866+
matches!(
867+
action,
868+
CodeActionOrCommand::CodeAction(CodeAction {
869+
disabled: None,
870+
command: None,
871+
..
872+
})
873+
)
874+
});
875+
876+
// Use the first matching code action
877+
if let Some(lsp_item) = actions.first() {
878+
return match lsp_item {
879+
CodeActionOrCommand::CodeAction(code_action) => {
880+
Some(CodeActionItem {
881+
lsp_item: code_action.clone(),
882+
language_server_id,
883+
})
884+
}
885+
_ => None,
886+
};
887+
}
888+
}
889+
None
890+
})
891+
.collect();
892+
893+
if code_actions.is_empty() {
894+
log::debug!("Code action on save not found {:?}", code_action_kind);
895+
cx.editor
896+
.set_error(format!("Code Action not found: {:?}", code_action_kind));
897+
}
898+
899+
for code_action in code_actions {
900+
log::debug!(
901+
"Applying code action on save {:?} for language server {:?}",
902+
code_action.lsp_item,
903+
code_action.language_server_id
904+
);
905+
let Some(language_server) = cx
906+
.editor
907+
.language_server_by_id(code_action.language_server_id)
908+
else {
909+
log::error!(
910+
"Language server disappeared {:?}",
911+
code_action.language_server_id
912+
);
913+
continue;
914+
};
915+
916+
let offset_encoding = language_server.offset_encoding();
917+
918+
let resolved_code_action =
919+
resolve_code_action(&code_action.lsp_item, language_server);
920+
921+
if let Some(ref workspace_edit) = resolved_code_action
922+
.as_ref()
923+
.unwrap_or(&code_action.lsp_item)
924+
.edit
925+
{
926+
let _ = cx
927+
.editor
928+
.apply_workspace_edit_sync(offset_encoding, workspace_edit);
929+
}
930+
}
931+
}
932+
}
933+
}
934+
935+
pub fn resolve_code_action(
936+
code_action: &CodeAction,
937+
language_server: &Client,
938+
) -> Option<CodeAction> {
939+
let mut resolved_code_action = None;
940+
if code_action.edit.is_none() || code_action.command.is_none() {
941+
if let Some(future) = language_server.resolve_code_action(code_action) {
942+
if let Ok(code_action) = helix_lsp::block_on(future) {
943+
resolved_code_action = Some(code_action);
944+
}
945+
}
946+
}
947+
resolved_code_action
948+
}
949+
828950
#[derive(Debug)]
829951
pub struct ApplyEditError {
830952
pub kind: ApplyEditErrorKind,

0 commit comments

Comments
 (0)