Use Result for failed text document retrieval in LSP requests (#14579)

## Summary

Ref:
https://github.com/astral-sh/ruff-vscode/issues/644#issuecomment-2496588452

## Test Plan

Not sure how to test this as this is mainly to get more context on the
panic that the server is raising.
This commit is contained in:
Dhruv Manilawala 2024-11-25 15:14:30 +05:30 committed by GitHub
parent 9f6147490b
commit 0c9165fc3a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 31 additions and 8 deletions

View file

@ -1,3 +1,4 @@
use anyhow::Context;
use lsp_types::{self as types, request as req}; use lsp_types::{self as types, request as req};
use types::TextEdit; use types::TextEdit;
@ -64,7 +65,8 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form
let text_document = snapshot let text_document = snapshot
.query() .query()
.as_single_document() .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(); let query = snapshot.query();
format_text_document( format_text_document(
text_document, text_document,

View file

@ -1,3 +1,4 @@
use anyhow::Context;
use lsp_types::{self as types, request as req, Range}; use lsp_types::{self as types, request as req, Range};
use crate::edit::{RangeExt, ToRangeExt}; use crate::edit::{RangeExt, ToRangeExt};
@ -32,7 +33,8 @@ fn format_document_range(
let text_document = snapshot let text_document = snapshot
.query() .query()
.as_single_document() .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(); let query = snapshot.query();
format_text_document_range(text_document, range, query, snapshot.encoding()) format_text_document_range(text_document, range, query, snapshot.encoding())
} }

View file

@ -1,5 +1,6 @@
use crate::server::{client::Notifier, Result}; use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot; use crate::session::DocumentSnapshot;
use anyhow::Context;
use lsp_types::{self as types, request as req}; use lsp_types::{self as types, request as req};
use regex::Regex; use regex::Regex;
use ruff_diagnostics::FixAvailability; use ruff_diagnostics::FixAvailability;
@ -33,7 +34,8 @@ pub(crate) fn hover(
let document = snapshot let document = snapshot
.query() .query()
.as_single_document() .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 let line_number: usize = position
.position .position
.line .line

View file

@ -6,6 +6,7 @@ use std::{collections::BTreeMap, path::Path, sync::Arc};
use anyhow::anyhow; use anyhow::anyhow;
use lsp_types::Url; use lsp_types::Url;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use thiserror::Error;
pub(crate) use ruff_settings::RuffSettings; pub(crate) use ruff_settings::RuffSettings;
@ -597,16 +598,24 @@ impl DocumentQuery {
/// Attempt to access the single inner text document selected by the query. /// 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`. /// 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 { match self {
Self::Text { document, .. } => Some(document), Self::Text { document, .. } => Ok(document),
Self::Notebook { Self::Notebook {
notebook, notebook,
file_url,
cell_url: cell_uri, cell_url: cell_uri,
.. ..
} => cell_uri } => {
.as_ref() if let Some(cell_uri) = cell_uri {
.and_then(|cell_uri| notebook.cell_document_by_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()))
}
}
} }
} }
@ -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),
}