refactor: Apply PR suggestions

This commit is contained in:
Alexander Gonzalez 2021-07-27 17:50:26 -04:00
parent 743c037a34
commit 41943f2328
4 changed files with 49 additions and 70 deletions

View file

@ -71,19 +71,41 @@ pub struct HoverResult {
pub actions: Vec<HoverAction>, pub actions: Vec<HoverAction>,
} }
/// Feature: Hover // Feature: Hover
/// //
/// Shows additional information, like the type of an expression or the documentation for a definition when "focusing" code. // 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. // 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 // image::https://user-images.githubusercontent.com/48062697/113020658-b5f98b80-917a-11eb-9f88-3dbc27320c95.gif
pub(crate) fn hover( pub(crate) fn hover(
db: &RootDatabase, db: &RootDatabase,
position: FilePosition, range: FileRange,
config: &HoverConfig, config: &HoverConfig,
) -> Option<RangeInfo<HoverResult>> { ) -> Option<RangeInfo<HoverResult>> {
let sema = hir::Semantics::new(db); 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::<ast::Expr>(&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 { 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, IDENT | INT_NUMBER | LIFETIME_IDENT | T![self] | T![super] | T![crate] => 3,
T!['('] | T![')'] => 2, T!['('] | T![')'] => 2,
@ -197,6 +219,7 @@ pub(crate) fn hover(
} else { } else {
ty.display(db).to_string().into() ty.display(db).to_string().into()
}; };
let range = sema.original_range(&node).range; let range = sema.original_range(&node).range;
Some(RangeInfo::new(range, res)) Some(RangeInfo::new(range, res))
} }
@ -245,37 +268,6 @@ fn try_hover_for_lint(attr: &ast::Attr, token: &SyntaxToken) -> Option<RangeInfo
)) ))
} }
/// LSP Extension: Hover over a range
///
/// Gets the type of the expression closest to the selection if the user is hovering inside
/// the selection. If not, it is handled by `handle_hover`.
///
/// https://user-images.githubusercontent.com/22298999/126914293-0ce49a92-545d-4005-a59e-9294fa2330d6.gif
pub(crate) fn hover_range(
db: &RootDatabase,
range: FileRange,
config: &HoverConfig,
) -> Option<RangeInfo<HoverResult>> {
let sema = hir::Semantics::new(db);
let file = sema.parse(range.file_id).syntax().clone();
let expr = find_node_at_range::<ast::Expr>(&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<HoverAction> { fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option<HoverAction> {
fn to_action(nav_target: NavigationTarget) -> HoverAction { fn to_action(nav_target: NavigationTarget) -> HoverAction {
HoverAction::Implementation(FilePosition { HoverAction::Implementation(FilePosition {
@ -565,7 +557,8 @@ fn find_std_module(famous_defs: &FamousDefs, name: &str) -> Option<hir::Module>
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use expect_test::{expect, Expect}; 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}; use crate::{fixture, hover::HoverDocFormat, HoverConfig};
@ -577,7 +570,7 @@ mod tests {
links_in_hover: true, links_in_hover: true,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),
}, },
position, FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
) )
.unwrap(); .unwrap();
assert!(hover.is_none()); assert!(hover.is_none());
@ -591,7 +584,7 @@ mod tests {
links_in_hover: true, links_in_hover: true,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),
}, },
position, FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
) )
.unwrap() .unwrap()
.unwrap(); .unwrap();
@ -611,7 +604,7 @@ mod tests {
links_in_hover: false, links_in_hover: false,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),
}, },
position, FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
) )
.unwrap() .unwrap()
.unwrap(); .unwrap();
@ -631,7 +624,7 @@ mod tests {
links_in_hover: true, links_in_hover: true,
documentation: Some(HoverDocFormat::PlainText), documentation: Some(HoverDocFormat::PlainText),
}, },
position, FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
) )
.unwrap() .unwrap()
.unwrap(); .unwrap();
@ -651,7 +644,7 @@ mod tests {
links_in_hover: true, links_in_hover: true,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),
}, },
position, FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
) )
.unwrap() .unwrap()
.unwrap(); .unwrap();
@ -661,7 +654,7 @@ mod tests {
fn check_hover_range(ra_fixture: &str, expect: Expect) { fn check_hover_range(ra_fixture: &str, expect: Expect) {
let (analysis, range) = fixture::range(ra_fixture); let (analysis, range) = fixture::range(ra_fixture);
let hover = analysis let hover = analysis
.hover_range( .hover(
&HoverConfig { &HoverConfig {
links_in_hover: false, links_in_hover: false,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),
@ -676,7 +669,7 @@ mod tests {
fn check_hover_range_no_results(ra_fixture: &str) { fn check_hover_range_no_results(ra_fixture: &str) {
let (analysis, range) = fixture::range(ra_fixture); let (analysis, range) = fixture::range(ra_fixture);
let hover = analysis let hover = analysis
.hover_range( .hover(
&HoverConfig { &HoverConfig {
links_in_hover: false, links_in_hover: false,
documentation: Some(HoverDocFormat::Markdown), documentation: Some(HoverDocFormat::Markdown),

View file

@ -416,20 +416,11 @@ impl Analysis {
/// Returns a short text describing element at position. /// Returns a short text describing element at position.
pub fn hover( pub fn hover(
&self,
config: &HoverConfig,
position: FilePosition,
) -> Cancellable<Option<RangeInfo<HoverResult>>> {
self.with_db(|db| hover::hover(db, position, config))
}
/// Returns a short text displaying the type of the expression.
pub fn hover_range(
&self, &self,
config: &HoverConfig, config: &HoverConfig,
range: FileRange, range: FileRange,
) -> Cancellable<Option<RangeInfo<HoverResult>>> { ) -> Cancellable<Option<RangeInfo<HoverResult>>> {
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. /// Return URL(s) for the documentation of the symbol under the cursor.

View file

@ -874,21 +874,14 @@ pub(crate) fn handle_hover(
) -> Result<Option<lsp_ext::Hover>> { ) -> Result<Option<lsp_ext::Hover>> {
let _p = profile::span("handle_hover"); let _p = profile::span("handle_hover");
let file_id = from_proto::file_id(&snap, &params.text_document.uri)?; let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
let hover_result = match params.position {
PositionOrRange::Position(position) => { let range = match params.position {
let position = from_proto::file_position( PositionOrRange::Position(position) => Range::new(position, position),
&snap, PositionOrRange::Range(range) => range,
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 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), None => return Ok(None),
Some(info) => info, Some(info) => info,
}; };

View file

@ -662,6 +662,8 @@ interface TestInfo {
**Issue:** https://github.com/microsoft/language-server-protocol/issues/377 **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. This request build upon the current `textDocument/hover` to show the type of the expression currently selected.
```typescript ```typescript