From 41943f2328b6f1551e46effdd7bc5c9510b6b429 Mon Sep 17 00:00:00 2001 From: Alexander Gonzalez Date: Tue, 27 Jul 2021 17:50:26 -0400 Subject: [PATCH] refactor: Apply PR suggestions --- crates/ide/src/hover.rs | 87 +++++++++++++--------------- crates/ide/src/lib.rs | 11 +--- crates/rust-analyzer/src/handlers.rs | 19 ++---- docs/dev/lsp-extensions.md | 2 + 4 files changed, 49 insertions(+), 70 deletions(-) diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index ba71fb1ccf..56ea1951c0 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -71,19 +71,41 @@ pub struct HoverResult { pub actions: Vec, } -/// Feature: Hover -/// -/// Shows additional information, like the type of an expression or the documentation for a definition when "focusing" code. -/// Focusing is usually hovering with a mouse, but can also be triggered with a shortcut. -/// -/// image::https://user-images.githubusercontent.com/48062697/113020658-b5f98b80-917a-11eb-9f88-3dbc27320c95.gif +// Feature: Hover +// +// Shows additional information, like the type of an expression or the documentation for a definition when "focusing" code. +// Focusing is usually hovering with a mouse, but can also be triggered with a shortcut. +// +// image::https://user-images.githubusercontent.com/48062697/113020658-b5f98b80-917a-11eb-9f88-3dbc27320c95.gif pub(crate) fn hover( db: &RootDatabase, - position: FilePosition, + range: FileRange, config: &HoverConfig, ) -> Option> { let sema = hir::Semantics::new(db); - let file = sema.parse(position.file_id).syntax().clone(); + let file = sema.parse(range.file_id).syntax().clone(); + + // This means we're hovering over a range. + if !range.range.is_empty() { + let expr = find_node_at_range::(&file, range.range)?; + let ty = sema.type_of_expr(&expr)?; + + if ty.is_unknown() { + return None; + } + + let mut res = HoverResult::default(); + + res.markup = if config.markdown() { + Markup::fenced_block(&ty.display(db)) + } else { + ty.display(db).to_string().into() + }; + + return Some(RangeInfo::new(range.range, res)); + } + + let position = FilePosition { file_id: range.file_id, offset: range.range.start() }; let token = pick_best_token(file.token_at_offset(position.offset), |kind| match kind { IDENT | INT_NUMBER | LIFETIME_IDENT | T![self] | T![super] | T![crate] => 3, T!['('] | T![')'] => 2, @@ -197,6 +219,7 @@ pub(crate) fn hover( } else { ty.display(db).to_string().into() }; + let range = sema.original_range(&node).range; Some(RangeInfo::new(range, res)) } @@ -245,37 +268,6 @@ fn try_hover_for_lint(attr: &ast::Attr, token: &SyntaxToken) -> Option Option> { - let sema = hir::Semantics::new(db); - let file = sema.parse(range.file_id).syntax().clone(); - let expr = find_node_at_range::(&file, range.range)?; - let ty = sema.type_of_expr(&expr)?; - - if ty.is_unknown() { - return None; - } - - let mut res = HoverResult::default(); - - res.markup = if config.markdown() { - Markup::fenced_block(&ty.display(db)) - } else { - ty.display(db).to_string().into() - }; - - Some(RangeInfo::new(range.range, res)) -} - fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option { fn to_action(nav_target: NavigationTarget) -> HoverAction { HoverAction::Implementation(FilePosition { @@ -565,7 +557,8 @@ fn find_std_module(famous_defs: &FamousDefs, name: &str) -> Option #[cfg(test)] mod tests { use expect_test::{expect, Expect}; - use ide_db::base_db::FileLoader; + use ide_db::base_db::{FileLoader, FileRange}; + use syntax::TextRange; use crate::{fixture, hover::HoverDocFormat, HoverConfig}; @@ -577,7 +570,7 @@ mod tests { links_in_hover: true, documentation: Some(HoverDocFormat::Markdown), }, - position, + FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) }, ) .unwrap(); assert!(hover.is_none()); @@ -591,7 +584,7 @@ mod tests { links_in_hover: true, documentation: Some(HoverDocFormat::Markdown), }, - position, + FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) }, ) .unwrap() .unwrap(); @@ -611,7 +604,7 @@ mod tests { links_in_hover: false, documentation: Some(HoverDocFormat::Markdown), }, - position, + FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) }, ) .unwrap() .unwrap(); @@ -631,7 +624,7 @@ mod tests { links_in_hover: true, documentation: Some(HoverDocFormat::PlainText), }, - position, + FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) }, ) .unwrap() .unwrap(); @@ -651,7 +644,7 @@ mod tests { links_in_hover: true, documentation: Some(HoverDocFormat::Markdown), }, - position, + FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) }, ) .unwrap() .unwrap(); @@ -661,7 +654,7 @@ mod tests { fn check_hover_range(ra_fixture: &str, expect: Expect) { let (analysis, range) = fixture::range(ra_fixture); let hover = analysis - .hover_range( + .hover( &HoverConfig { links_in_hover: false, documentation: Some(HoverDocFormat::Markdown), @@ -676,7 +669,7 @@ mod tests { fn check_hover_range_no_results(ra_fixture: &str) { let (analysis, range) = fixture::range(ra_fixture); let hover = analysis - .hover_range( + .hover( &HoverConfig { links_in_hover: false, documentation: Some(HoverDocFormat::Markdown), diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 9154406712..d717c46057 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -416,20 +416,11 @@ impl Analysis { /// Returns a short text describing element at position. pub fn hover( - &self, - config: &HoverConfig, - position: FilePosition, - ) -> Cancellable>> { - self.with_db(|db| hover::hover(db, position, config)) - } - - /// Returns a short text displaying the type of the expression. - pub fn hover_range( &self, config: &HoverConfig, range: FileRange, ) -> Cancellable>> { - self.with_db(|db| hover::hover_range(db, range, config)) + self.with_db(|db| hover::hover(db, range, config)) } /// Return URL(s) for the documentation of the symbol under the cursor. diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 90202fdcec..4844b16c1a 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -874,21 +874,14 @@ pub(crate) fn handle_hover( ) -> Result> { let _p = profile::span("handle_hover"); let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; - let hover_result = match params.position { - PositionOrRange::Position(position) => { - let position = from_proto::file_position( - &snap, - lsp_types::TextDocumentPositionParams::new(params.text_document, position), - )?; - snap.analysis.hover(&snap.config.hover(), position)? - } - PositionOrRange::Range(range) => { - let range = from_proto::file_range(&snap, params.text_document, range)?; - snap.analysis.hover_range(&snap.config.hover(), range)? - } + + let range = match params.position { + PositionOrRange::Position(position) => Range::new(position, position), + PositionOrRange::Range(range) => range, }; - let info = match hover_result { + let file_range = from_proto::file_range(&snap, params.text_document, range)?; + let info = match snap.analysis.hover(&snap.config.hover(), file_range)? { None => return Ok(None), Some(info) => info, }; diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index dc8e93a645..e617153a6c 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -662,6 +662,8 @@ interface TestInfo { **Issue:** https://github.com/microsoft/language-server-protocol/issues/377 +**Experimental Server Capability:** { "hoverRange": boolean } + This request build upon the current `textDocument/hover` to show the type of the expression currently selected. ```typescript