From af44821804ff52b8f723cc494e9772dcc98e2b8a Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 2 Jun 2026 12:51:53 +0200 Subject: [PATCH] fix(hover): avoid panics for unsupported nodes --- crates/pgls_hover/src/lib.rs | 42 ++++++++++++++++--- .../tests/hover_integration_tests.rs | 3 +- crates/pgls_lsp/src/handlers/hover.rs | 22 ++++++---- crates/pgls_lsp/src/server.rs | 4 +- crates/pgls_workspace/src/workspace/server.rs | 3 +- 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/crates/pgls_hover/src/lib.rs b/crates/pgls_hover/src/lib.rs index dbfdac68d..3ce570ec4 100644 --- a/crates/pgls_hover/src/lib.rs +++ b/crates/pgls_hover/src/lib.rs @@ -24,7 +24,7 @@ pub struct OnHoverParams<'a> { text = params.stmt_sql, position = params.position.to_string() ))] -pub fn on_hover(params: OnHoverParams) -> Vec { +pub fn on_hover(params: OnHoverParams) -> Option> { let ctx = pgls_treesitter::context::TreesitterContext::new(TreeSitterContextParams { position: params.position, text: params.stmt_sql, @@ -123,15 +123,47 @@ pub fn on_hover(params: OnHoverParams) -> Vec { .unwrap_or_default(), }, - _ => todo!(), + HoveredNode::Policy(_) | HoveredNode::Trigger(_) => return None, }; - prioritize_by_context(items, &ctx) + let markdown_blocks: Vec = prioritize_by_context(items, &ctx) .into_iter() .map(|item| format_hover_markdown(&item, params.schema_cache)) .filter_map(Result::ok) - .collect() + .collect(); + + (!markdown_blocks.is_empty()).then_some(markdown_blocks) } else { - Default::default() + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hover_over_drop_policy_name_returns_none() { + let query = + r#"drop policy if exists "Av{}atar images are publicly readable" on storage.objects;"#; + let position = query.find("{}").unwrap(); + let sql = query.replace("{}", ""); + + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&pgls_treesitter_grammar::LANGUAGE.into()) + .unwrap(); + let tree = parser.parse(&sql, None).unwrap(); + let schema_cache = SchemaCache::default(); + + let hover = on_hover(OnHoverParams { + position: TextSize::new(position as u32), + schema_cache: &schema_cache, + stmt_sql: &sql, + ast: None, + ts_tree: &tree, + }); + + assert!(hover.is_none()); } } diff --git a/crates/pgls_hover/tests/hover_integration_tests.rs b/crates/pgls_hover/tests/hover_integration_tests.rs index b8d9f864b..a40d97933 100644 --- a/crates/pgls_hover/tests/hover_integration_tests.rs +++ b/crates/pgls_hover/tests/hover_integration_tests.rs @@ -34,7 +34,8 @@ async fn test_hover_at_cursor(name: &str, query: String, setup: Option<&str>, te stmt_sql: &sql, ast: ast.as_ref(), ts_tree: &tree, - }); + }) + .unwrap_or_default(); let mut snapshot = String::new(); snapshot.push_str("# Input\n"); diff --git a/crates/pgls_lsp/src/handlers/hover.rs b/crates/pgls_lsp/src/handlers/hover.rs index 9155207f0..75f938516 100644 --- a/crates/pgls_lsp/src/handlers/hover.rs +++ b/crates/pgls_lsp/src/handlers/hover.rs @@ -7,7 +7,7 @@ use crate::{adapters::get_cursor_position, diagnostics::LspError, session::Sessi pub(crate) fn on_hover( session: &Session, params: lsp_types::HoverParams, -) -> Result { +) -> Result, LspError> { let url = params.text_document_position_params.text_document.uri; let position = params.text_document_position_params.position; let path = session.file_path(&url)?; @@ -19,20 +19,24 @@ pub(crate) fn on_hover( Ok(result) => { tracing::debug!("Found hover items: {:#?}", result); - Ok(lsp_types::HoverContents::Array( - result - .into_iter() - .map(MarkedString::from_markdown) - .collect(), - )) + let hover_items: Vec = result + .into_iter() + .map(MarkedString::from_markdown) + .collect(); + + if hover_items.is_empty() { + Ok(None) + } else { + Ok(Some(lsp_types::HoverContents::Array(hover_items))) + } } Err(e) => match e { WorkspaceError::DatabaseConnectionError(_) => { - Ok(lsp_types::HoverContents::Markup(MarkupContent { + Ok(Some(lsp_types::HoverContents::Markup(MarkupContent { kind: lsp_types::MarkupKind::PlainText, value: "Cannot connect to database.".into(), - })) + }))) } _ => { tracing::error!("Received an error: {:#?}", e); diff --git a/crates/pgls_lsp/src/server.rs b/crates/pgls_lsp/src/server.rs index 437924043..062d02460 100644 --- a/crates/pgls_lsp/src/server.rs +++ b/crates/pgls_lsp/src/server.rs @@ -270,8 +270,8 @@ impl LanguageServer for LSPServer { #[tracing::instrument(level = "trace", skip_all)] async fn hover(&self, params: HoverParams) -> LspResult> { match handlers::hover::on_hover(&self.session, params) { - Ok(result) => LspResult::Ok(Some(Hover { - contents: result, + Ok(result) => LspResult::Ok(result.map(|contents| Hover { + contents, range: None, })), Err(e) => LspResult::Err(into_lsp_error(e)), diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 30dca4c6d..123702450 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -1153,7 +1153,8 @@ impl Workspace for WorkspaceServer { ast: maybe_ast.as_ref(), position: position_in_stmt, stmt_sql: stmt_id.content(), - }); + }) + .unwrap_or_default(); Ok(OnHoverResult { markdown_blocks }) }