From 8b0d0bd9c7488fedfd0bdd34a0a9cb04da3f143a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 23 Mar 2021 19:19:44 +0100 Subject: Show item info when hovering intra doc links --- crates/ide/src/call_hierarchy.rs | 11 ++- crates/ide/src/doc_links.rs | 129 +++++++++++++++++++++------ crates/ide/src/goto_definition.rs | 66 ++------------ crates/ide/src/hover.rs | 63 +++++++++++-- crates/ide/src/runnables.rs | 28 +----- crates/ide/src/syntax_highlighting/inject.rs | 6 +- 6 files changed, 175 insertions(+), 128 deletions(-) (limited to 'crates/ide/src') diff --git a/crates/ide/src/call_hierarchy.rs b/crates/ide/src/call_hierarchy.rs index 96021f677..5cd186565 100644 --- a/crates/ide/src/call_hierarchy.rs +++ b/crates/ide/src/call_hierarchy.rs @@ -50,16 +50,16 @@ pub(crate) fn incoming_calls(db: &RootDatabase, position: FilePosition) -> Optio for (file_id, references) in refs.references { let file = sema.parse(file_id); let file = file.syntax(); - for (r_range, _) in references { - let token = file.token_at_offset(r_range.start()).next()?; + for (relative_range, token) in references + .into_iter() + .filter_map(|(range, _)| Some(range).zip(file.token_at_offset(range.start()).next())) + { let token = sema.descend_into_macros(token); // This target is the containing function if let Some(nav) = token.ancestors().find_map(|node| { - let fn_ = ast::Fn::cast(node)?; - let def = sema.to_def(&fn_)?; + let def = ast::Fn::cast(node).and_then(|fn_| sema.to_def(&fn_))?; def.try_to_nav(sema.db) }) { - let relative_range = r_range; calls.add(&nav, relative_range); } } @@ -87,7 +87,6 @@ pub(crate) fn outgoing_calls(db: &RootDatabase, position: FilePosition) -> Optio let name_ref = call_node.name_ref()?; let func_target = match call_node { FnCallNode::CallExpr(expr) => { - //FIXME: Type::as_callable is broken let callable = sema.type_of_expr(&expr.expr()?)?.as_callable(db)?; match callable.kind() { hir::CallableKind::Function(it) => it.try_to_nav(db), diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index c7c1f4fee..f12c9d442 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -1,4 +1,4 @@ -//! Resolves and rewrites links in markdown documentation. +//! Extracts, resolves and rewrites links and intra-doc links in markdown documentation. use std::{convert::TryFrom, iter::once, ops::Range}; @@ -15,7 +15,10 @@ use ide_db::{ defs::{Definition, NameClass, NameRefClass}, RootDatabase, }; -use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; +use syntax::{ + ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxNode, SyntaxToken, TextRange, TextSize, + TokenAtOffset, T, +}; use crate::{FilePosition, Semantics}; @@ -60,29 +63,6 @@ pub(crate) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Defi out } -pub(crate) fn extract_definitions_from_markdown( - markdown: &str, -) -> Vec<(String, Option, Range)> { - let mut res = vec![]; - let mut cb = |link: BrokenLink| { - // These allocations are actually unnecessary but the lifetimes on BrokenLinkCallback are wrong - // this is fixed in the repo but not on the crates.io release yet - Some(( - /*url*/ link.reference.to_owned().into(), - /*title*/ link.reference.to_owned().into(), - )) - }; - let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb)); - for (event, range) in doc.into_offset_iter() { - if let Event::Start(Tag::Link(_, target, title)) = event { - let link = if target.is_empty() { title } else { target }; - let (link, ns) = parse_link(&link); - res.push((link.to_string(), ns, range)); - } - } - res -} - /// Remove all links in markdown documentation. pub(crate) fn remove_links(markdown: &str) -> String { let mut drop_link = false; @@ -118,6 +98,105 @@ pub(crate) fn remove_links(markdown: &str) -> String { out } +pub(crate) fn extract_definitions_from_markdown( + markdown: &str, +) -> Vec<(Range, String, Option)> { + let mut res = vec![]; + let mut cb = |link: BrokenLink| { + // These allocations are actually unnecessary but the lifetimes on BrokenLinkCallback are wrong + // this is fixed in the repo but not on the crates.io release yet + Some(( + /*url*/ link.reference.to_owned().into(), + /*title*/ link.reference.to_owned().into(), + )) + }; + let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb)); + for (event, range) in doc.into_offset_iter() { + if let Event::Start(Tag::Link(_, target, title)) = event { + let link = if target.is_empty() { title } else { target }; + let (link, ns) = parse_link(&link); + res.push((range, link.to_string(), ns)); + } + } + res +} + +/// Extracts a link from a comment at the given position returning the spanning range, link and +/// optionally it's namespace. +pub(crate) fn extract_positioned_link_from_comment( + position: TextSize, + comment: &ast::Comment, +) -> Option<(TextRange, String, Option)> { + let doc_comment = comment.doc_comment()?; + let comment_start = + comment.syntax().text_range().start() + TextSize::from(comment.prefix().len() as u32); + let def_links = extract_definitions_from_markdown(doc_comment); + let (range, def_link, ns) = + def_links.into_iter().find_map(|(Range { start, end }, def_link, ns)| { + let range = TextRange::at( + comment_start + TextSize::from(start as u32), + TextSize::from((end - start) as u32), + ); + range.contains(position).then(|| (range, def_link, ns)) + })?; + Some((range, def_link, ns)) +} + +/// Turns a syntax node into it's [`Definition`] if it can hold docs. +pub(crate) fn doc_owner_to_def( + sema: &Semantics, + item: &SyntaxNode, +) -> Option { + let res: hir::ModuleDef = match_ast! { + match item { + ast::SourceFile(_it) => sema.scope(item).module()?.into(), + ast::Fn(it) => sema.to_def(&it)?.into(), + ast::Struct(it) => sema.to_def(&it)?.into(), + ast::Enum(it) => sema.to_def(&it)?.into(), + ast::Union(it) => sema.to_def(&it)?.into(), + ast::Trait(it) => sema.to_def(&it)?.into(), + ast::Const(it) => sema.to_def(&it)?.into(), + ast::Static(it) => sema.to_def(&it)?.into(), + ast::TypeAlias(it) => sema.to_def(&it)?.into(), + ast::Variant(it) => sema.to_def(&it)?.into(), + ast::Trait(it) => sema.to_def(&it)?.into(), + ast::Impl(it) => return sema.to_def(&it).map(Definition::SelfType), + ast::MacroRules(it) => return sema.to_def(&it).map(Definition::Macro), + ast::TupleField(it) => return sema.to_def(&it).map(Definition::Field), + ast::RecordField(it) => return sema.to_def(&it).map(Definition::Field), + _ => return None, + } + }; + Some(Definition::ModuleDef(res)) +} + +pub(crate) fn resolve_doc_path_for_def( + db: &dyn HirDatabase, + def: Definition, + link: &str, + ns: Option, +) -> Option { + match def { + Definition::ModuleDef(def) => match def { + ModuleDef::Module(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Function(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Adt(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Variant(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Const(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Static(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::Trait(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::TypeAlias(it) => it.resolve_doc_path(db, &link, ns), + ModuleDef::BuiltinType(_) => None, + }, + Definition::Macro(it) => it.resolve_doc_path(db, &link, ns), + Definition::Field(it) => it.resolve_doc_path(db, &link, ns), + Definition::SelfType(_) + | Definition::Local(_) + | Definition::GenericParam(_) + | Definition::Label(_) => None, + } +} + // FIXME: // BUG: For Option::Some // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 473d48c2f..a2c97061f 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -1,18 +1,14 @@ -use std::ops::Range; - use either::Either; -use hir::{HasAttrs, ModuleDef, Semantics}; +use hir::Semantics; use ide_db::{ - defs::{Definition, NameClass, NameRefClass}, + defs::{NameClass, NameRefClass}, RootDatabase, }; -use syntax::{ - ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TextRange, TextSize, - TokenAtOffset, T, -}; +use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; use crate::{ - display::TryToNav, doc_links::extract_definitions_from_markdown, runnables::doc_owner_to_def, + display::TryToNav, + doc_links::{doc_owner_to_def, extract_positioned_link_from_comment, resolve_doc_path_for_def}, FilePosition, NavigationTarget, RangeInfo, }; @@ -35,7 +31,9 @@ pub(crate) fn goto_definition( let token = sema.descend_into_macros(original_token.clone()); let parent = token.parent()?; if let Some(comment) = ast::Comment::cast(token) { - let nav = def_for_doc_comment(&sema, position, &comment)?.try_to_nav(db)?; + let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment)?; + let def = doc_owner_to_def(&sema, &parent)?; + let nav = resolve_doc_path_for_def(db, def, &link, ns)?.try_to_nav(db)?; return Some(RangeInfo::new(original_token.text_range(), vec![nav])); } @@ -61,54 +59,6 @@ pub(crate) fn goto_definition( Some(RangeInfo::new(original_token.text_range(), nav.into_iter().collect())) } -fn def_for_doc_comment( - sema: &Semantics, - position: FilePosition, - doc_comment: &ast::Comment, -) -> Option { - let parent = doc_comment.syntax().parent()?; - let (link, ns) = extract_positioned_link_from_comment(position, doc_comment)?; - - let def = doc_owner_to_def(sema, parent)?; - match def { - Definition::ModuleDef(def) => match def { - ModuleDef::Module(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Function(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Adt(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Variant(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Const(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Static(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::Trait(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::TypeAlias(it) => it.resolve_doc_path(sema.db, &link, ns), - ModuleDef::BuiltinType(_) => return None, - }, - Definition::Macro(it) => it.resolve_doc_path(sema.db, &link, ns), - Definition::Field(it) => it.resolve_doc_path(sema.db, &link, ns), - Definition::SelfType(_) - | Definition::Local(_) - | Definition::GenericParam(_) - | Definition::Label(_) => return None, - } -} - -fn extract_positioned_link_from_comment( - position: FilePosition, - comment: &ast::Comment, -) -> Option<(String, Option)> { - let doc_comment = comment.doc_comment()?; - let comment_start = - comment.syntax().text_range().start() + TextSize::from(comment.prefix().len() as u32); - let def_links = extract_definitions_from_markdown(doc_comment); - let (def_link, ns, _) = def_links.into_iter().find(|&(_, _, Range { start, end })| { - TextRange::at( - comment_start + TextSize::from(start as u32), - TextSize::from((end - start) as u32), - ) - .contains(position.offset) - })?; - Some((def_link, ns)) -} - fn pick_best(tokens: TokenAtOffset) -> Option { return tokens.max_by_key(priority); fn priority(n: &SyntaxToken) -> usize { diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index a3fb17c0a..c43089476 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -11,11 +11,14 @@ use ide_db::{ }; use itertools::Itertools; use stdx::format_to; -use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; +use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; use crate::{ display::{macro_label, TryToNav}, - doc_links::{remove_links, rewrite_links}, + doc_links::{ + doc_owner_to_def, extract_positioned_link_from_comment, remove_links, + resolve_doc_path_for_def, rewrite_links, + }, markdown_remove::remove_markdown, markup::Markup, runnables::{runnable_fn, runnable_mod}, @@ -93,20 +96,35 @@ pub(crate) fn hover( let mut res = HoverResult::default(); let node = token.parent()?; + let mut range = None; let definition = match_ast! { match node { // we don't use NameClass::referenced_or_defined here as we do not want to resolve // field pattern shorthands to their definition ast::Name(name) => NameClass::classify(&sema, &name).and_then(|class| match class { NameClass::ConstReference(def) => Some(def), - def => def.defined(sema.db), + def => def.defined(db), }), - ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(sema.db)), - ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime) - .map_or_else(|| NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(sema.db)), |d| d.defined(sema.db)), - _ => None, + ast::NameRef(name_ref) => { + NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(db)) + }, + ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime).map_or_else( + || NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(db)), + |d| d.defined(db), + ), + + _ => ast::Comment::cast(token.clone()) + .and_then(|comment| { + let (idl_range, link, ns) = + extract_positioned_link_from_comment(position.offset, &comment)?; + range = Some(idl_range); + let def = doc_owner_to_def(&sema, &node)?; + resolve_doc_path_for_def(db, def, &link, ns) + }) + .map(Definition::ModuleDef), } }; + if let Some(definition) = definition { let famous_defs = match &definition { Definition::ModuleDef(ModuleDef::BuiltinType(_)) => { @@ -128,15 +146,16 @@ pub(crate) fn hover( res.actions.push(action); } - let range = sema.original_range(&node).range; + let range = range.unwrap_or_else(|| sema.original_range(&node).range); return Some(RangeInfo::new(range, res)); } } if token.kind() == syntax::SyntaxKind::COMMENT { - // don't highlight the entire parent node on comment hover + cov_mark::hit!(no_highlight_on_comment_hover); return None; } + if let res @ Some(_) = hover_for_keyword(&sema, links_in_hover, markdown, &token) { return res; } @@ -3483,6 +3502,7 @@ fn foo$0() {} #[test] fn hover_comments_dont_highlight_parent() { + cov_mark::check!(no_highlight_on_comment_hover); check_hover_no_result( r#" fn no_hover() { @@ -3755,4 +3775,29 @@ fn main() { "#]], ) } + + #[test] + fn hover_intra_doc_links() { + check( + r#" +/// This is the [`foo`](foo$0) function. +fn foo() {} +"#, + expect![[r#" + *[`foo`](foo)* + + ```rust + test + ``` + + ```rust + fn foo() + ``` + + --- + + This is the [`foo`](https://docs.rs/test/*/test/fn.foo.html) function. + "#]], + ); + } } diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index bea020b06..5b488e2c5 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -7,17 +7,13 @@ use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, Semantics}; use ide_assists::utils::test_related_attribute; use ide_db::{ base_db::{FilePosition, FileRange}, - defs::Definition, helpers::visit_file_defs, search::SearchScope, RootDatabase, SymbolKind, }; use itertools::Itertools; use rustc_hash::FxHashSet; -use syntax::{ - ast::{self, AstNode, AttrsOwner}, - match_ast, SyntaxNode, -}; +use syntax::ast::{self, AstNode, AttrsOwner}; use crate::{ display::{ToNav, TryToNav}, @@ -271,28 +267,6 @@ pub(crate) fn runnable_mod(sema: &Semantics, def: hir::Module) -> Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg }) } -// FIXME: figure out a proper API here. -pub(crate) fn doc_owner_to_def( - sema: &Semantics, - item: SyntaxNode, -) -> Option { - let res: hir::ModuleDef = match_ast! { - match item { - ast::SourceFile(_it) => sema.scope(&item).module()?.into(), - ast::Fn(it) => sema.to_def(&it)?.into(), - ast::Struct(it) => sema.to_def(&it)?.into(), - ast::Enum(it) => sema.to_def(&it)?.into(), - ast::Union(it) => sema.to_def(&it)?.into(), - ast::Trait(it) => sema.to_def(&it)?.into(), - ast::Const(it) => sema.to_def(&it)?.into(), - ast::Static(it) => sema.to_def(&it)?.into(), - ast::TypeAlias(it) => sema.to_def(&it)?.into(), - _ => return None, - } - }; - Some(Definition::ModuleDef(res)) -} - fn module_def_doctest(sema: &Semantics, def: hir::ModuleDef) -> Option { let attrs = match def { hir::ModuleDef::Module(it) => it.attrs(sema.db), diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs index 8e0940184..38bf49348 100644 --- a/crates/ide/src/syntax_highlighting/inject.rs +++ b/crates/ide/src/syntax_highlighting/inject.rs @@ -190,10 +190,10 @@ pub(super) fn doc_comment( intra_doc_links.extend( extract_definitions_from_markdown(line) .into_iter() - .filter_map(|(link, ns, range)| { - validate_intra_doc_link(sema.db, &def, &link, ns).zip(Some(range)) + .filter_map(|(range, link, ns)| { + Some(range).zip(validate_intra_doc_link(sema.db, &def, &link, ns)) }) - .map(|(def, Range { start, end })| { + .map(|(Range { start, end }, def)| { ( def, TextRange::at( -- cgit v1.2.3