Skip to content

Commit 82eae51

Browse files
authored
Ignore source code actions for a notebook cell (#16154)
## Summary Related to astral-sh/ruff-vscode#686, this PR ignores handling source code actions for notebooks which are not prefixed with `notebook`. The main motivation is that the native server does not actually handle it well which results in gibberish code. There's some context about this in astral-sh/ruff-vscode#680 (comment) and the following comments. closes: astral-sh/ruff-vscode#680 ## Test Plan Running a notebook with the following does nothing except log the message: ```json "notebook.codeActionsOnSave": { "source.organizeImports.ruff": "explicit", }, ``` while, including the `notebook` code actions does make the edit (as usual): ```json "notebook.codeActionsOnSave": { "notebook.source.organizeImports.ruff": "explicit" }, ```
1 parent b5cd4f2 commit 82eae51

File tree

3 files changed

+48
-3
lines changed

3 files changed

+48
-3
lines changed

crates/ruff_server/src/server/api/requests/code_action.rs

+22-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,15 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
4848

4949
if snapshot.client_settings().fix_all() {
5050
if supported_code_actions.contains(&SupportedCodeAction::SourceFixAll) {
51-
response.push(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?);
51+
if snapshot.is_notebook_cell() {
52+
// This is ignore here because the client requests this code action for each
53+
// cell in parallel and the server would send a workspace edit with the same
54+
// content which would result in applying the same edit multiple times
55+
// resulting in (possibly) duplicate code.
56+
tracing::debug!("Ignoring `source.fixAll` code action for a notebook cell");
57+
} else {
58+
response.push(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?);
59+
}
5260
} else if supported_code_actions.contains(&SupportedCodeAction::NotebookSourceFixAll) {
5361
response
5462
.push(notebook_fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?);
@@ -57,8 +65,19 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
5765

5866
if snapshot.client_settings().organize_imports() {
5967
if supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) {
60-
response
61-
.push(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?);
68+
if snapshot.is_notebook_cell() {
69+
// This is ignore here because the client requests this code action for each
70+
// cell in parallel and the server would send a workspace edit with the same
71+
// content which would result in applying the same edit multiple times
72+
// resulting in (possibly) duplicate code.
73+
tracing::debug!(
74+
"Ignoring `source.organizeImports` code action for a notebook cell"
75+
);
76+
} else {
77+
response.push(
78+
organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?,
79+
);
80+
}
6281
} else if supported_code_actions
6382
.contains(&SupportedCodeAction::NotebookSourceOrganizeImports)
6483
{

crates/ruff_server/src/server/api/requests/code_action_resolve.rs

+15
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,21 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
5050
.with_failure_code(ErrorCode::InvalidParams);
5151
};
5252

53+
match action_kind {
54+
SupportedCodeAction::SourceFixAll | SupportedCodeAction::SourceOrganizeImports
55+
if snapshot.is_notebook_cell() =>
56+
{
57+
// This should never occur because we ignore generating these code actions for a
58+
// notebook cell in the `textDocument/codeAction` request handler.
59+
return Err(anyhow::anyhow!(
60+
"Code action resolver cannot resolve {:?} for a notebook cell",
61+
action_kind.to_kind().as_str()
62+
))
63+
.with_failure_code(ErrorCode::InvalidParams);
64+
}
65+
_ => {}
66+
}
67+
5368
action.edit = match action_kind {
5469
SupportedCodeAction::SourceFixAll | SupportedCodeAction::NotebookSourceFixAll => Some(
5570
resolve_edit_for_fix_all(

crates/ruff_server/src/session.rs

+11
Original file line numberDiff line numberDiff line change
@@ -184,4 +184,15 @@ impl DocumentSnapshot {
184184
pub(crate) fn encoding(&self) -> PositionEncoding {
185185
self.position_encoding
186186
}
187+
188+
/// Returns `true` if this snapshot represents a notebook cell.
189+
pub(crate) const fn is_notebook_cell(&self) -> bool {
190+
matches!(
191+
&self.document_ref,
192+
index::DocumentQuery::Notebook {
193+
cell_url: Some(_),
194+
..
195+
}
196+
)
197+
}
187198
}

0 commit comments

Comments
 (0)