Skip to content

Commit 46b50b7

Browse files
committed
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 1b5295a commit 46b50b7

File tree

12 files changed

+480
-89
lines changed

12 files changed

+480
-89
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"
1718

.github/workflows/build.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,23 @@ jobs:
5151
key: ${{ runner.os }}-stable-v${{ env.CACHE_VERSION }}-tree-sitter-grammars-${{ hashFiles('languages.toml') }}
5252
restore-keys: ${{ runner.os }}-stable-v${{ env.CACHE_VERSION }}-tree-sitter-grammars-
5353

54+
- name: Install go lsp integration tests
55+
uses: actions/setup-go@v5
56+
with:
57+
go-version: '^1.22.0'
58+
59+
- name: Install gopls for lsp integration tests
60+
run: go install golang.org/x/tools/gopls@latest
61+
5462
- name: Run cargo test
5563
run: cargo test --workspace
5664

5765
- name: Run cargo integration-test
5866
run: cargo integration-test
5967

68+
- name: Run cargo integration-test-lsp
69+
run: cargo integration-test-lsp
70+
6071
strategy:
6172
matrix:
6273
os: [ubuntu-latest, macos-latest, windows-latest]

book/src/languages.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ These configuration keys are available:
7171
| `text-width` | Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set, defaults to `editor.text-width` |
7272
| `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. |
7373
| `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.
74+
| `code-actions-on-save` | List of LSP code actions to be run in order on save, for example `[{ code-action = "source.organizeImports", enabled = true }]` |
7475

7576
### File-type detection and the `file-types` key
7677

docs/CONTRIBUTING.md

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

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

6270
Helix follows the MSRV of Firefox.

helix-core/src/syntax.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ pub struct LanguageConfiguration {
118118
pub block_comment_tokens: Option<Vec<BlockCommentToken>>,
119119
pub text_width: Option<usize>,
120120
pub soft_wrap: Option<SoftWrap>,
121+
#[serde(default)]
122+
pub code_actions_on_save: Option<Vec<CodeActionsOnSave>>, // List of LSP code actions to be run in order upon saving
121123

122124
#[serde(default)]
123125
pub auto_format: bool,
@@ -490,6 +492,13 @@ pub struct AdvancedCompletion {
490492
pub default: Option<String>,
491493
}
492494

495+
#[derive(Debug, Clone, Serialize, Deserialize)]
496+
#[serde(rename_all = "kebab-case")]
497+
pub struct CodeActionsOnSave {
498+
pub code_action: String,
499+
pub enabled: bool,
500+
}
501+
493502
#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
494503
#[serde(rename_all = "kebab-case", untagged)]
495504
pub enum DebugConfigCompletion {

helix-term/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ homepage.workspace = true
1616
default = ["git"]
1717
unicode-lines = ["helix-core/unicode-lines", "helix-view/unicode-lines"]
1818
integration = ["helix-event/integration_test"]
19+
integration-lsp = ["integration"]
1920
git = ["helix-vcs/git"]
2021

2122
[[bin]]

helix-term/src/commands/lsp.rs

Lines changed: 170 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
1-
use futures_util::{stream::FuturesOrdered, FutureExt};
1+
use futures_util::{
2+
stream::{FuturesOrdered, FuturesUnordered},
3+
FutureExt,
4+
};
25
use helix_lsp::{
36
block_on,
47
lsp::{
5-
self, CodeAction, CodeActionOrCommand, CodeActionTriggerKind, DiagnosticSeverity,
6-
NumberOrString,
8+
self, CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionTriggerKind,
9+
DiagnosticSeverity, NumberOrString,
710
},
811
util::{diagnostic_to_lsp_diagnostic, lsp_range_to_range, range_to_lsp_range},
912
Client, LanguageServerId, OffsetEncoding,
1013
};
14+
use serde_json::Value;
1115
use tokio_stream::StreamExt;
1216
use tui::{text::Span, widgets::Row};
1317

1418
use super::{align_view, push_jump, Align, Context, Editor};
1519

1620
use helix_core::{
17-
syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, Uri,
21+
syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Range, Selection, Uri,
1822
};
1923
use helix_stdx::path;
2024
use helix_view::{
2125
document::{DocumentInlayHints, DocumentInlayHintsId},
2226
editor::Action,
2327
handlers::lsp::SignatureHelpInvoked,
2428
theme::Style,
25-
Document, View,
29+
Document, DocumentId, View,
2630
};
2731

2832
use crate::{
@@ -542,9 +546,9 @@ pub fn workspace_diagnostics_picker(cx: &mut Context) {
542546
cx.push_layer(Box::new(overlaid(picker)));
543547
}
544548

545-
struct CodeActionOrCommandItem {
546-
lsp_item: lsp::CodeActionOrCommand,
547-
language_server_id: LanguageServerId,
549+
pub struct CodeActionOrCommandItem {
550+
pub lsp_item: lsp::CodeActionOrCommand,
551+
pub language_server_id: LanguageServerId,
548552
}
549553

550554
impl ui::menu::Item for CodeActionOrCommandItem {
@@ -619,34 +623,8 @@ pub fn code_action(cx: &mut Context) {
619623

620624
let selection_range = doc.selection(view.id).primary();
621625

622-
let mut seen_language_servers = HashSet::new();
623-
624-
let mut futures: FuturesOrdered<_> = doc
625-
.language_servers_with_feature(LanguageServerFeature::CodeAction)
626-
.filter(|ls| seen_language_servers.insert(ls.id()))
627-
// TODO this should probably already been filtered in something like "language_servers_with_feature"
628-
.filter_map(|language_server| {
629-
let offset_encoding = language_server.offset_encoding();
630-
let language_server_id = language_server.id();
631-
let range = range_to_lsp_range(doc.text(), selection_range, offset_encoding);
632-
// Filter and convert overlapping diagnostics
633-
let code_action_context = lsp::CodeActionContext {
634-
diagnostics: doc
635-
.diagnostics()
636-
.iter()
637-
.filter(|&diag| {
638-
selection_range
639-
.overlaps(&helix_core::Range::new(diag.range.start, diag.range.end))
640-
})
641-
.map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding))
642-
.collect(),
643-
only: None,
644-
trigger_kind: Some(CodeActionTriggerKind::INVOKED),
645-
};
646-
let code_action_request =
647-
language_server.code_actions(doc.identifier(), range, code_action_context)?;
648-
Some((code_action_request, language_server_id))
649-
})
626+
let mut futures: FuturesUnordered<_> = code_actions_for_range(doc, selection_range, None)
627+
.into_iter()
650628
.map(|(request, ls_id)| async move {
651629
let json = request.await?;
652630
let response: Option<lsp::CodeActionResponse> = serde_json::from_value(json)?;
@@ -734,59 +712,174 @@ pub fn code_action(cx: &mut Context) {
734712

735713
// always present here
736714
let action = action.unwrap();
737-
let Some(language_server) = editor.language_server_by_id(action.language_server_id)
738-
else {
739-
editor.set_error("Language Server disappeared");
740-
return;
741-
};
742-
let offset_encoding = language_server.offset_encoding();
743715

744-
match &action.lsp_item {
745-
lsp::CodeActionOrCommand::Command(command) => {
746-
log::debug!("code action command: {:?}", command);
747-
execute_lsp_command(editor, action.language_server_id, command.clone());
748-
}
749-
lsp::CodeActionOrCommand::CodeAction(code_action) => {
750-
log::debug!("code action: {:?}", code_action);
751-
// we support lsp "codeAction/resolve" for `edit` and `command` fields
752-
let mut resolved_code_action = None;
753-
if code_action.edit.is_none() || code_action.command.is_none() {
754-
if let Some(future) =
755-
language_server.resolve_code_action(code_action.clone())
716+
apply_code_action(editor, action);
717+
});
718+
picker.move_down(); // pre-select the first item
719+
720+
let popup = Popup::new("code-action", picker).with_scrollbar(false);
721+
722+
compositor.replace_or_push("code-action", popup);
723+
};
724+
725+
Ok(Callback::EditorCompositor(Box::new(call)))
726+
});
727+
}
728+
729+
pub fn code_actions_for_range(
730+
doc: &Document,
731+
range: helix_core::Range,
732+
only: Option<Vec<CodeActionKind>>,
733+
) -> Vec<(
734+
impl Future<Output = Result<Value, helix_lsp::Error>>,
735+
LanguageServerId,
736+
)> {
737+
let mut seen_language_servers = HashSet::new();
738+
739+
doc.language_servers_with_feature(LanguageServerFeature::CodeAction)
740+
.filter(|ls| seen_language_servers.insert(ls.id()))
741+
// TODO this should probably already been filtered in something like "language_servers_with_feature"
742+
.filter_map(|language_server| {
743+
let offset_encoding = language_server.offset_encoding();
744+
let language_server_id = language_server.id();
745+
let lsp_range = range_to_lsp_range(doc.text(), range, offset_encoding);
746+
// Filter and convert overlapping diagnostics
747+
let code_action_context = lsp::CodeActionContext {
748+
diagnostics: doc
749+
.diagnostics()
750+
.iter()
751+
.filter(|&diag| {
752+
range.overlaps(&helix_core::Range::new(diag.range.start, diag.range.end))
753+
})
754+
.map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding))
755+
.collect(),
756+
only: only.clone(),
757+
trigger_kind: Some(CodeActionTriggerKind::INVOKED),
758+
};
759+
let code_action_request =
760+
language_server.code_actions(doc.identifier(), lsp_range, code_action_context)?;
761+
Some((code_action_request, language_server_id))
762+
})
763+
.collect::<Vec<_>>()
764+
}
765+
766+
/// Will apply the code actions on save from the language config for each language server
767+
pub fn code_actions_on_save(cx: &mut compositor::Context, doc_id: &DocumentId) {
768+
let doc = doc!(cx.editor, doc_id);
769+
770+
let code_actions_on_save_cfg = doc
771+
.language_config()
772+
.and_then(|c| c.code_actions_on_save.clone());
773+
774+
if let Some(code_actions_on_save_cfg) = code_actions_on_save_cfg {
775+
for code_action_kind in code_actions_on_save_cfg.into_iter().filter_map(|action| {
776+
action
777+
.enabled
778+
.then_some(CodeActionKind::from(action.code_action))
779+
}) {
780+
log::debug!("Attempting code action on save {:?}", code_action_kind);
781+
let doc = doc!(cx.editor, doc_id);
782+
let full_range = Range::new(0, doc.text().len_chars());
783+
let code_actions: Vec<CodeActionOrCommandItem> =
784+
code_actions_for_range(doc, full_range, Some(vec![code_action_kind.clone()]))
785+
.into_iter()
786+
.filter_map(|(future, language_server_id)| {
787+
if let Ok(json) = helix_lsp::block_on(future) {
788+
if let Ok(Some(mut actions)) = serde_json::from_value::<
789+
Option<helix_lsp::lsp::CodeActionResponse>,
790+
>(json)
756791
{
757-
if let Ok(response) = helix_lsp::block_on(future) {
758-
if let Ok(code_action) =
759-
serde_json::from_value::<CodeAction>(response)
760-
{
761-
resolved_code_action = Some(code_action);
762-
}
792+
// Retain only enabled code actions that do not have commands.
793+
//
794+
// Commands are deprecated and are not supported because they apply
795+
// workspace edits asynchronously and there is currently no mechanism
796+
// to handle waiting for the workspace edits to be applied before moving
797+
// on to the next code action (or auto-format).
798+
actions.retain(|action| {
799+
matches!(
800+
action,
801+
CodeActionOrCommand::CodeAction(CodeAction {
802+
disabled: None,
803+
command: None,
804+
..
805+
})
806+
)
807+
});
808+
809+
// Use the first matching code action
810+
if let Some(lsp_item) = actions.first() {
811+
return Some(CodeActionOrCommandItem {
812+
lsp_item: lsp_item.clone(),
813+
language_server_id,
814+
});
763815
}
764816
}
765817
}
766-
let resolved_code_action =
767-
resolved_code_action.as_ref().unwrap_or(code_action);
818+
None
819+
})
820+
.collect();
768821

769-
if let Some(ref workspace_edit) = resolved_code_action.edit {
770-
let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit);
771-
}
822+
if code_actions.is_empty() {
823+
log::debug!("Code action on save not found {:?}", code_action_kind);
824+
cx.editor
825+
.set_error(format!("Code Action not found: {:?}", code_action_kind));
826+
}
772827

773-
// if code action provides both edit and command first the edit
774-
// should be applied and then the command
775-
if let Some(command) = &code_action.command {
776-
execute_lsp_command(editor, action.language_server_id, command.clone());
828+
for code_action in code_actions {
829+
log::debug!(
830+
"Applying code action on save {:?} for language server {:?}",
831+
code_action.lsp_item,
832+
code_action.language_server_id
833+
);
834+
apply_code_action(cx.editor, &code_action);
835+
836+
// TODO: Find a better way to handle this
837+
// Sleep to avoid race condition between next code action/auto-format
838+
// and the textDocument/didChange notification
839+
std::thread::sleep(std::time::Duration::from_millis(10));
840+
}
841+
}
842+
}
843+
}
844+
845+
pub fn apply_code_action(editor: &mut Editor, action: &CodeActionOrCommandItem) {
846+
let Some(language_server) = editor.language_server_by_id(action.language_server_id) else {
847+
editor.set_error("Language Server disappeared");
848+
return;
849+
};
850+
let offset_encoding = language_server.offset_encoding();
851+
852+
match &action.lsp_item {
853+
lsp::CodeActionOrCommand::Command(command) => {
854+
log::debug!("code action command: {:?}", command);
855+
execute_lsp_command(editor, action.language_server_id, command.clone());
856+
}
857+
lsp::CodeActionOrCommand::CodeAction(code_action) => {
858+
log::debug!("code action: {:?}", code_action);
859+
// we support lsp "codeAction/resolve" for `edit` and `command` fields
860+
let mut resolved_code_action = None;
861+
if code_action.edit.is_none() || code_action.command.is_none() {
862+
if let Some(future) = language_server.resolve_code_action(code_action.clone()) {
863+
if let Ok(response) = helix_lsp::block_on(future) {
864+
if let Ok(code_action) = serde_json::from_value::<CodeAction>(response) {
865+
resolved_code_action = Some(code_action);
777866
}
778867
}
779868
}
780-
});
781-
picker.move_down(); // pre-select the first item
782-
783-
let popup = Popup::new("code-action", picker).with_scrollbar(false);
869+
}
870+
let resolved_code_action = resolved_code_action.as_ref().unwrap_or(code_action);
784871

785-
compositor.replace_or_push("code-action", popup);
786-
};
872+
if let Some(ref workspace_edit) = resolved_code_action.edit {
873+
let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit);
874+
}
787875

788-
Ok(Callback::EditorCompositor(Box::new(call)))
789-
});
876+
// if code action provides both edit and command first the edit
877+
// should be applied and then the command
878+
if let Some(command) = &code_action.command {
879+
execute_lsp_command(editor, action.language_server_id, command.clone());
880+
}
881+
}
882+
}
790883
}
791884

792885
pub fn execute_lsp_command(

0 commit comments

Comments
 (0)