Skip to content

Commit

Permalink
Use Result for failed text document retrieval in LSP requests (#14579)
Browse files Browse the repository at this point in the history
## Summary

Ref:
astral-sh/ruff-vscode#644 (comment)

## Test Plan

Not sure how to test this as this is mainly to get more context on the
panic that the server is raising.
  • Loading branch information
dhruvmanila authored Nov 25, 2024
1 parent 9f61474 commit 0c9165f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 8 deletions.
4 changes: 3 additions & 1 deletion crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Context;
use lsp_types::{self as types, request as req};
use types::TextEdit;

Expand Down Expand Up @@ -64,7 +65,8 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form
let text_document = snapshot
.query()
.as_single_document()
.expect("format should only be called on text documents or notebook cells");
.context("Failed to get text document for the format request")
.unwrap();
let query = snapshot.query();
format_text_document(
text_document,
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Context;
use lsp_types::{self as types, request as req, Range};

use crate::edit::{RangeExt, ToRangeExt};
Expand Down Expand Up @@ -32,7 +33,8 @@ fn format_document_range(
let text_document = snapshot
.query()
.as_single_document()
.expect("format should only be called on text documents or notebook cells");
.context("Failed to get text document for the format range request")
.unwrap();
let query = snapshot.query();
format_text_document_range(text_document, range, query, snapshot.encoding())
}
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff_server/src/server/api/requests/hover.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot;
use anyhow::Context;
use lsp_types::{self as types, request as req};
use regex::Regex;
use ruff_diagnostics::FixAvailability;
Expand Down Expand Up @@ -33,7 +34,8 @@ pub(crate) fn hover(
let document = snapshot
.query()
.as_single_document()
.expect("hover should only be called on text documents or notebook cells");
.context("Failed to get text document for the hover request")
.unwrap();
let line_number: usize = position
.position
.line
Expand Down
27 changes: 22 additions & 5 deletions crates/ruff_server/src/session/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{collections::BTreeMap, path::Path, sync::Arc};
use anyhow::anyhow;
use lsp_types::Url;
use rustc_hash::FxHashMap;
use thiserror::Error;

pub(crate) use ruff_settings::RuffSettings;

Expand Down Expand Up @@ -597,16 +598,24 @@ impl DocumentQuery {

/// Attempt to access the single inner text document selected by the query.
/// If this query is selecting an entire notebook document, this will return `None`.
pub(crate) fn as_single_document(&self) -> Option<&TextDocument> {
pub(crate) fn as_single_document(&self) -> Result<&TextDocument, SingleDocumentError> {
match self {
Self::Text { document, .. } => Some(document),
Self::Text { document, .. } => Ok(document),
Self::Notebook {
notebook,
file_url,
cell_url: cell_uri,
..
} => cell_uri
.as_ref()
.and_then(|cell_uri| notebook.cell_document_by_uri(cell_uri)),
} => {
if let Some(cell_uri) = cell_uri {
let cell = notebook
.cell_document_by_uri(cell_uri)
.ok_or_else(|| SingleDocumentError::CellDoesNotExist(cell_uri.clone()))?;
Ok(cell)
} else {
Err(SingleDocumentError::Notebook(file_url.clone()))
}
}
}
}

Expand All @@ -618,3 +627,11 @@ impl DocumentQuery {
}
}
}

#[derive(Debug, Error)]
pub(crate) enum SingleDocumentError {
#[error("Expected a single text document, but found a notebook document: {0}")]
Notebook(Url),
#[error("Cell with URL {0} does not exist in the internal notebook document")]
CellDoesNotExist(Url),
}

0 comments on commit 0c9165f

Please sign in to comment.