From 9df78ec4a4e41ca94b25f292aba90e266f104f02 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 29 Mar 2021 21:23:45 +0200 Subject: [PATCH 1/5] Properly resolve intra doc links in hover and goto_definition --- crates/ide/src/doc_links.rs | 68 +++++++++++++++++-------------- crates/ide/src/goto_definition.rs | 24 ++++++++++- crates/ide/src/hover.rs | 29 +++++++++---- crates/ide_db/src/defs.rs | 23 +++++++++++ 4 files changed, 104 insertions(+), 40 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 99276168fb4..69442278b2f 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -26,12 +26,7 @@ pub(crate) type DocumentationLink = String; /// Rewrite documentation links in markdown to point to an online host (e.g. docs.rs) pub(crate) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> String { - let mut cb = |link: BrokenLink| { - Some(( - /*url*/ link.reference.to_owned().into(), - /*title*/ link.reference.to_owned().into(), - )) - }; + let mut cb = broken_link_clone_cb; let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb)); let doc = map_links(doc, |target, title: &str| { @@ -124,24 +119,24 @@ pub(crate) fn external_docs( 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_intra_doc_link(&link); - res.push((range, link.to_string(), ns)); - } - } - res + extract_definitions_from_markdown_(markdown, &mut broken_link_clone_cb).collect() +} + +fn extract_definitions_from_markdown_<'a>( + markdown: &'a str, + cb: &'a mut dyn FnMut(BrokenLink<'_>) -> Option<(CowStr<'a>, CowStr<'a>)>, +) -> impl Iterator, String, Option)> + 'a { + Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(cb)) + .into_offset_iter() + .filter_map(|(event, range)| { + if let Event::Start(Tag::Link(_, target, title)) = event { + let link = if target.is_empty() { title } else { target }; + let (link, ns) = parse_intra_doc_link(&link); + Some((range, link.to_string(), ns)) + } else { + None + } + }) } /// Extracts a link from a comment at the given position returning the spanning range, link and @@ -149,20 +144,24 @@ pub(crate) fn extract_definitions_from_markdown( pub(crate) fn extract_positioned_link_from_comment( position: TextSize, comment: &ast::Comment, + docs: hir::Documentation, ) -> Option<(TextRange, String, Option)> { - let doc_comment = comment.doc_comment()?; + let doc_comment = comment.doc_comment()?.to_string() + "\n" + docs.as_str(); 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 len = comment.syntax().text_range().len().into(); + let mut cb = broken_link_clone_cb; + // because pulldown_cmarks lifetimes are wrong we gotta dance around a few temporaries here + let res = extract_definitions_from_markdown_(&doc_comment, &mut cb) + .take_while(|&(Range { end, .. }, ..)| end < len) + .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)) + }); + res } /// Turns a syntax node into it's [`Definition`] if it can hold docs. @@ -220,6 +219,15 @@ pub(crate) fn resolve_doc_path_for_def( } } +fn broken_link_clone_cb<'a, 'b>(link: BrokenLink<'a>) -> Option<(CowStr<'b>, CowStr<'b>)> { + // 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(), + )) +} + // 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 c6556c4871e..4e4d1b2007c 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -31,7 +31,8 @@ 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 (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment)?; + let docs = doc_owner_to_def(&sema, &parent)?.docs(db)?; + let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment, docs)?; 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])); @@ -1158,4 +1159,25 @@ fn fn_macro() {} "#, ) } + + #[test] + fn goto_intra_doc_links() { + check( + r#" + +pub mod theitem { + /// This is the item. Cool! + pub struct TheItem; + //^^^^^^^ +} + +/// Gives you a [`TheItem$0`]. +/// +/// [`TheItem`]: theitem::TheItem +pub fn gimme() -> theitem::TheItem { + theitem::TheItem +} +"#, + ); + } } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 02a1a5b37f9..5a497e92d1a 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -115,10 +115,11 @@ pub(crate) fn hover( _ => 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)?; + let docs = def.docs(db)?; + let (idl_range, link, ns) = + extract_positioned_link_from_comment(position.offset, &comment, docs)?; + range = Some(idl_range); resolve_doc_path_for_def(db, def, &link, ns) }) .map(Definition::ModuleDef), @@ -3812,23 +3813,33 @@ fn main() { fn hover_intra_doc_links() { check( r#" -/// This is the [`foo`](foo$0) function. -fn foo() {} + +pub mod theitem { + /// This is the item. Cool! + pub struct TheItem; +} + +/// Gives you a [`TheItem$0`]. +/// +/// [`TheItem`]: theitem::TheItem +pub fn gimme() -> theitem::TheItem { + theitem::TheItem +} "#, expect![[r#" - *[`foo`](foo)* + *[`TheItem`]* ```rust - test + test::theitem ``` ```rust - fn foo() + pub struct TheItem ``` --- - This is the [`foo`](https://docs.rs/test/*/test/fn.foo.html) function. + This is the item. Cool! "#]], ); } diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs index de0dc2a40f0..378bac7b218 100644 --- a/crates/ide_db/src/defs.rs +++ b/crates/ide_db/src/defs.rs @@ -79,6 +79,29 @@ impl Definition { }; Some(name) } + + pub fn docs(&self, db: &RootDatabase) -> Option { + match self { + Definition::Macro(it) => it.docs(db), + Definition::Field(it) => it.docs(db), + Definition::ModuleDef(def) => match def { + hir::ModuleDef::Module(it) => it.docs(db), + hir::ModuleDef::Function(it) => it.docs(db), + hir::ModuleDef::Adt(def) => match def { + hir::Adt::Struct(it) => it.docs(db), + hir::Adt::Union(it) => it.docs(db), + hir::Adt::Enum(it) => it.docs(db), + }, + hir::ModuleDef::Variant(it) => it.docs(db), + hir::ModuleDef::Const(it) => it.docs(db), + hir::ModuleDef::Static(it) => it.docs(db), + hir::ModuleDef::Trait(it) => it.docs(db), + hir::ModuleDef::TypeAlias(it) => it.docs(db), + hir::ModuleDef::BuiltinType(_) => None, + }, + _ => None, + } + } } #[derive(Debug)] From 9a327311e4a9b9102528751e052c63266c00c6bd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 30 Mar 2021 17:20:43 +0200 Subject: [PATCH 2/5] Implement basic Documentation source to syntax range mapping --- crates/hir_def/src/attr.rs | 109 +++++++++++++++++- crates/ide/src/goto_definition.rs | 1 + crates/ide/src/syntax_highlighting/inject.rs | 55 ++++----- .../test_data/highlight_doctest.html | 10 +- crates/ide/src/syntax_highlighting/tests.rs | 10 +- 5 files changed, 149 insertions(+), 36 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 52a2bce9b10..7791402c96a 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -1,6 +1,10 @@ //! A higher level attributes based on TokenTree, with also some shortcuts. -use std::{ops, sync::Arc}; +use std::{ + cmp::Ordering, + ops::{self, Range}, + sync::Arc, +}; use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; @@ -12,7 +16,7 @@ use mbe::ast_to_token_tree; use smallvec::{smallvec, SmallVec}; use syntax::{ ast::{self, AstNode, AttrsOwner}, - match_ast, AstToken, SmolStr, SyntaxNode, + match_ast, AstToken, SmolStr, SyntaxNode, TextRange, TextSize, }; use tt::Subtree; @@ -451,6 +455,54 @@ impl AttrsWithOwner { .collect(), } } + + pub fn docs_with_rangemap( + &self, + db: &dyn DefDatabase, + ) -> Option<(Documentation, DocsRangeMap)> { + // FIXME: code duplication in `docs` above + let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_ref()? { + AttrInput::Literal(s) => Some((s, attr.index)), + AttrInput::TokenTree(_) => None, + }); + let indent = docs + .clone() + .flat_map(|(s, _)| s.lines()) + .filter(|line| !line.chars().all(|c| c.is_whitespace())) + .map(|line| line.chars().take_while(|c| c.is_whitespace()).count()) + .min() + .unwrap_or(0); + let mut buf = String::new(); + let mut mapping = Vec::new(); + for (doc, idx) in docs { + // str::lines doesn't yield anything for the empty string + if !doc.is_empty() { + for line in doc.split('\n') { + let line = line.trim_end(); + let (offset, line) = match line.char_indices().nth(indent) { + Some((offset, _)) => (offset, &line[offset..]), + None => (0, line), + }; + let buf_offset = buf.len(); + buf.push_str(line); + mapping.push(( + Range { start: buf_offset, end: buf.len() }, + idx, + Range { start: offset, end: line.len() }, + )); + buf.push('\n'); + } + } else { + buf.push('\n'); + } + } + buf.pop(); + if buf.is_empty() { + None + } else { + Some((Documentation(buf), DocsRangeMap { mapping, source: self.source_map(db).attrs })) + } + } } fn inner_attributes( @@ -507,6 +559,59 @@ impl AttrSourceMap { } } +/// A struct to map text ranges from [`Documentation`] back to TextRanges in the syntax tree. +pub struct DocsRangeMap { + source: Vec>>, + // (docstring-line-range, attr_index, attr-string-range) + // a mapping from the text range of a line of the [`Documentation`] to the attribute index and + // the original (untrimmed) syntax doc line + mapping: Vec<(Range, u32, Range)>, +} + +impl DocsRangeMap { + pub fn map(&self, range: Range) -> Option> { + let found = self + .mapping + .binary_search_by(|(probe, ..)| { + if probe.contains(&range.start) { + Ordering::Equal + } else { + probe.start.cmp(&range.end) + } + }) + .ok()?; + let (line_docs_range, idx, original_line_src_range) = self.mapping[found].clone(); + if range.end > line_docs_range.end { + return None; + } + + let relative_range = Range { + start: range.start - line_docs_range.start, + end: range.end - line_docs_range.start, + }; + let range_len = TextSize::from((range.end - range.start) as u32); + + let &InFile { file_id, value: ref source } = &self.source[idx as usize]; + match source { + Either::Left(_) => None, // FIXME, figure out a nice way to handle doc attributes here + // as well as for whats done in syntax highlight doc injection + Either::Right(comment) => { + let text_range = comment.syntax().text_range(); + let range = TextRange::at( + text_range.start() + + TextSize::from( + (comment.prefix().len() + + original_line_src_range.start + + relative_range.start) as u32, + ), + text_range.len().min(range_len), + ); + Some(InFile { file_id, value: range }) + } + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Attr { index: u32, diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 4e4d1b2007c..1951c599f22 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -32,6 +32,7 @@ pub(crate) fn goto_definition( let parent = token.parent()?; if let Some(comment) = ast::Comment::cast(token) { let docs = doc_owner_to_def(&sema, &parent)?.docs(db)?; + let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment, docs)?; let def = doc_owner_to_def(&sema, &parent)?; let nav = resolve_doc_path_for_def(db, def, &link, ns)?.try_to_nav(db)?; diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs index b62d43256cc..504783f3157 100644 --- a/crates/ide/src/syntax_highlighting/inject.rs +++ b/crates/ide/src/syntax_highlighting/inject.rs @@ -1,6 +1,6 @@ //! "Recursive" Syntax highlighting for code in doctests and fixtures. -use std::{mem, ops::Range}; +use std::mem; use either::Either; use hir::{HasAttrs, InFile, Semantics}; @@ -139,8 +139,28 @@ pub(super) fn doc_comment( // Replace the original, line-spanning comment ranges by new, only comment-prefix // spanning comment ranges. let mut new_comments = Vec::new(); - let mut intra_doc_links = Vec::new(); let mut string; + + if let Some((docs, doc_mapping)) = attributes.docs_with_rangemap(sema.db) { + extract_definitions_from_markdown(docs.as_str()) + .into_iter() + .filter_map(|(range, link, ns)| { + let def = resolve_doc_path_for_def(sema.db, def, &link, ns)?; + let InFile { file_id, value: range } = doc_mapping.map(range)?; + (file_id == node.file_id).then(|| (range, def)) + }) + .for_each(|(range, def)| { + hl.add(HlRange { + range, + highlight: module_def_to_hl_tag(def) + | HlMod::Documentation + | HlMod::Injected + | HlMod::IntraDocLink, + binding_hash: None, + }) + }); + } + for attr in attributes.by_key("doc").attrs() { let InFile { file_id, value: src } = attrs_source_map.source_of(&attr); if file_id != node.file_id { @@ -186,25 +206,7 @@ pub(super) fn doc_comment( is_doctest = is_codeblock && is_rust; continue; } - None if !is_doctest => { - intra_doc_links.extend( - extract_definitions_from_markdown(line) - .into_iter() - .filter_map(|(range, link, ns)| { - Some(range).zip(resolve_doc_path_for_def(sema.db, def, &link, ns)) - }) - .map(|(Range { start, end }, def)| { - ( - def, - TextRange::at( - prev_range_start + TextSize::from(start as u32), - TextSize::from((end - start) as u32), - ), - ) - }), - ); - continue; - } + None if !is_doctest => continue, None => (), } @@ -223,17 +225,6 @@ pub(super) fn doc_comment( } } - for (def, range) in intra_doc_links { - hl.add(HlRange { - range, - highlight: module_def_to_hl_tag(def) - | HlMod::Documentation - | HlMod::Injected - | HlMod::IntraDocLink, - binding_hash: None, - }); - } - if new_comments.is_empty() { return; // no need to run an analysis on an empty file } diff --git a/crates/ide/src/syntax_highlighting/test_data/highlight_doctest.html b/crates/ide/src/syntax_highlighting/test_data/highlight_doctest.html index 045162eb83b..b6d1cac4ea9 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlight_doctest.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlight_doctest.html @@ -100,10 +100,18 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd } /// [`Foo`](Foo) is a struct -/// [`all_the_links`](all_the_links) is this function +/// This function is > [`all_the_links`](all_the_links) < /// [`noop`](noop) is a macro below +/// [`Item`] is a struct in the module [`module`] +/// +/// [`Item`]: module::Item +/// [mix_and_match]: ThisShouldntResolve pub fn all_the_links() {} +pub mod module { + pub struct Item; +} + /// ``` /// noop!(1); /// ``` diff --git a/crates/ide/src/syntax_highlighting/tests.rs b/crates/ide/src/syntax_highlighting/tests.rs index 369ae0972b4..1b02857ecb9 100644 --- a/crates/ide/src/syntax_highlighting/tests.rs +++ b/crates/ide/src/syntax_highlighting/tests.rs @@ -544,10 +544,18 @@ impl Foo { } /// [`Foo`](Foo) is a struct -/// [`all_the_links`](all_the_links) is this function +/// This function is > [`all_the_links`](all_the_links) < /// [`noop`](noop) is a macro below +/// [`Item`] is a struct in the module [`module`] +/// +/// [`Item`]: module::Item +/// [mix_and_match]: ThisShouldntResolve pub fn all_the_links() {} +pub mod module { + pub struct Item; +} + /// ``` /// noop!(1); /// ``` From bb56b7a75cfae8297535d55fbddbee9875cbc756 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 30 Mar 2021 18:27:16 +0200 Subject: [PATCH 3/5] Use new new docs string source mapping in goto_def and hover --- crates/ide/src/doc_links.rs | 117 +++++++------------ crates/ide/src/goto_definition.rs | 17 ++- crates/ide/src/hover.rs | 21 ++-- crates/ide/src/syntax_highlighting/inject.rs | 35 +----- 4 files changed, 72 insertions(+), 118 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 69442278b2f..9a9a4111387 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -15,10 +15,7 @@ use ide_db::{ defs::{Definition, NameClass, NameRefClass}, RootDatabase, }; -use syntax::{ - ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxNode, SyntaxToken, TextRange, TextSize, - TokenAtOffset, T, -}; +use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, TokenAtOffset, T}; use crate::{FilePosition, Semantics}; @@ -119,77 +116,22 @@ pub(crate) fn external_docs( pub(crate) fn extract_definitions_from_markdown( markdown: &str, ) -> Vec<(Range, String, Option)> { - extract_definitions_from_markdown_(markdown, &mut broken_link_clone_cb).collect() -} - -fn extract_definitions_from_markdown_<'a>( - markdown: &'a str, - cb: &'a mut dyn FnMut(BrokenLink<'_>) -> Option<(CowStr<'a>, CowStr<'a>)>, -) -> impl Iterator, String, Option)> + 'a { - Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(cb)) - .into_offset_iter() - .filter_map(|(event, range)| { - if let Event::Start(Tag::Link(_, target, title)) = event { - let link = if target.is_empty() { title } else { target }; - let (link, ns) = parse_intra_doc_link(&link); - Some((range, link.to_string(), ns)) - } else { - None - } - }) -} - -/// 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, - docs: hir::Documentation, -) -> Option<(TextRange, String, Option)> { - let doc_comment = comment.doc_comment()?.to_string() + "\n" + docs.as_str(); - let comment_start = - comment.syntax().text_range().start() + TextSize::from(comment.prefix().len() as u32); - let len = comment.syntax().text_range().len().into(); - let mut cb = broken_link_clone_cb; - // because pulldown_cmarks lifetimes are wrong we gotta dance around a few temporaries here - let res = extract_definitions_from_markdown_(&doc_comment, &mut cb) - .take_while(|&(Range { end, .. }, ..)| end < len) - .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)) - }); - res -} - -/// 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::Macro(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, + Parser::new_with_broken_link_callback( + markdown, + Options::empty(), + Some(&mut broken_link_clone_cb), + ) + .into_offset_iter() + .filter_map(|(event, range)| { + if let Event::Start(Tag::Link(_, target, title)) = event { + let link = if target.is_empty() { title } else { target }; + let (link, ns) = parse_intra_doc_link(&link); + Some((range, link.to_string(), ns)) + } else { + None } - }; - Some(Definition::ModuleDef(res)) + }) + .collect() } pub(crate) fn resolve_doc_path_for_def( @@ -219,6 +161,33 @@ pub(crate) fn resolve_doc_path_for_def( } } +pub(crate) fn doc_attributes( + sema: &Semantics, + node: &SyntaxNode, +) -> Option<(hir::AttrsWithOwner, Definition)> { + match_ast! { + match node { + ast::SourceFile(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Module(def)))), + ast::Module(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Module(def)))), + ast::Fn(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Function(def)))), + ast::Struct(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Struct(def))))), + ast::Union(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Union(def))))), + ast::Enum(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Enum(def))))), + ast::Variant(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Variant(def)))), + ast::Trait(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Trait(def)))), + ast::Static(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Static(def)))), + ast::Const(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Const(def)))), + ast::TypeAlias(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::TypeAlias(def)))), + ast::Impl(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::SelfType(def))), + ast::RecordField(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Field(def))), + ast::TupleField(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Field(def))), + ast::Macro(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Macro(def))), + // ast::Use(it) => sema.to_def(&it).map(|def| (Box::new(it) as _, def.attrs(sema.db))), + _ => return None + } + } +} + fn broken_link_clone_cb<'a, 'b>(link: BrokenLink<'a>) -> Option<(CowStr<'b>, CowStr<'b>)> { // 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 diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 1951c599f22..780bdd40d81 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -1,5 +1,5 @@ use either::Either; -use hir::Semantics; +use hir::{InFile, Semantics}; use ide_db::{ defs::{NameClass, NameRefClass}, RootDatabase, @@ -8,7 +8,7 @@ use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, Toke use crate::{ display::TryToNav, - doc_links::{doc_owner_to_def, extract_positioned_link_from_comment, resolve_doc_path_for_def}, + doc_links::{doc_attributes, extract_definitions_from_markdown, resolve_doc_path_for_def}, FilePosition, NavigationTarget, RangeInfo, }; @@ -30,11 +30,16 @@ pub(crate) fn goto_definition( let original_token = pick_best(file.token_at_offset(position.offset))?; let token = sema.descend_into_macros(original_token.clone()); let parent = token.parent()?; - if let Some(comment) = ast::Comment::cast(token) { - let docs = doc_owner_to_def(&sema, &parent)?.docs(db)?; + if let Some(_) = ast::Comment::cast(token) { + let (attributes, def) = doc_attributes(&sema, &parent)?; - let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment, docs)?; - let def = doc_owner_to_def(&sema, &parent)?; + let (docs, doc_mapping) = attributes.docs_with_rangemap(db)?; + let (_, link, ns) = + extract_definitions_from_markdown(docs.as_str()).into_iter().find(|(range, ..)| { + doc_mapping.map(range.clone()).map_or(false, |InFile { file_id, value: range }| { + file_id == position.file_id.into() && range.contains(position.offset) + }) + })?; 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])); } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 5a497e92d1a..1e66219e4b1 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -1,6 +1,6 @@ use either::Either; use hir::{ - AsAssocItem, AssocItemContainer, GenericParam, HasAttrs, HasSource, HirDisplay, Module, + AsAssocItem, AssocItemContainer, GenericParam, HasAttrs, HasSource, HirDisplay, InFile, Module, ModuleDef, Semantics, }; use ide_db::{ @@ -16,8 +16,8 @@ use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, Toke use crate::{ display::{macro_label, TryToNav}, doc_links::{ - doc_owner_to_def, extract_positioned_link_from_comment, remove_links, - resolve_doc_path_for_def, rewrite_links, + doc_attributes, extract_definitions_from_markdown, remove_links, resolve_doc_path_for_def, + rewrite_links, }, markdown_remove::remove_markdown, markup::Markup, @@ -114,11 +114,18 @@ pub(crate) fn hover( ), _ => ast::Comment::cast(token.clone()) - .and_then(|comment| { - let def = doc_owner_to_def(&sema, &node)?; - let docs = def.docs(db)?; + .and_then(|_| { + let (attributes, def) = doc_attributes(&sema, &node)?; + let (docs, doc_mapping) = attributes.docs_with_rangemap(db)?; let (idl_range, link, ns) = - extract_positioned_link_from_comment(position.offset, &comment, docs)?; + extract_definitions_from_markdown(docs.as_str()).into_iter().find_map(|(range, link, ns)| { + let InFile { file_id, value: range } = doc_mapping.map(range.clone())?; + if file_id == position.file_id.into() && range.contains(position.offset) { + Some((range, link, ns)) + } else { + None + } + })?; range = Some(idl_range); resolve_doc_path_for_def(db, def, &link, ns) }) diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs index 504783f3157..04fafd244f8 100644 --- a/crates/ide/src/syntax_highlighting/inject.rs +++ b/crates/ide/src/syntax_highlighting/inject.rs @@ -3,15 +3,15 @@ use std::mem; use either::Either; -use hir::{HasAttrs, InFile, Semantics}; -use ide_db::{call_info::ActiveParameter, defs::Definition, SymbolKind}; +use hir::{InFile, Semantics}; +use ide_db::{call_info::ActiveParameter, SymbolKind}; use syntax::{ ast::{self, AstNode}, - match_ast, AstToken, NodeOrToken, SyntaxNode, SyntaxToken, TextRange, TextSize, + AstToken, NodeOrToken, SyntaxNode, SyntaxToken, TextRange, TextSize, }; use crate::{ - doc_links::{extract_definitions_from_markdown, resolve_doc_path_for_def}, + doc_links::{doc_attributes, extract_definitions_from_markdown, resolve_doc_path_for_def}, Analysis, HlMod, HlRange, HlTag, RootDatabase, }; @@ -90,33 +90,6 @@ const RUSTDOC_FENCE_TOKENS: &[&'static str] = &[ "edition2021", ]; -fn doc_attributes<'node>( - sema: &Semantics, - node: &'node SyntaxNode, -) -> Option<(hir::AttrsWithOwner, Definition)> { - match_ast! { - match node { - ast::SourceFile(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Module(def)))), - ast::Module(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Module(def)))), - ast::Fn(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Function(def)))), - ast::Struct(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Struct(def))))), - ast::Union(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Union(def))))), - ast::Enum(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Enum(def))))), - ast::Variant(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Variant(def)))), - ast::Trait(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Trait(def)))), - ast::Static(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Static(def)))), - ast::Const(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::Const(def)))), - ast::TypeAlias(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::ModuleDef(hir::ModuleDef::TypeAlias(def)))), - ast::Impl(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::SelfType(def))), - ast::RecordField(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Field(def))), - ast::TupleField(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Field(def))), - ast::Macro(it) => sema.to_def(&it).map(|def| (def.attrs(sema.db), Definition::Macro(def))), - // ast::Use(it) => sema.to_def(&it).map(|def| (Box::new(it) as _, def.attrs(sema.db))), - _ => return None - } - } -} - /// Injection of syntax highlighting of doctests. pub(super) fn doc_comment( hl: &mut Highlights, From c43359b64e98aeb73d3d63c883054644ba27e608 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 30 Mar 2021 18:30:42 +0200 Subject: [PATCH 4/5] Remove unused Definition::docs --- crates/ide_db/src/defs.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs index 378bac7b218..de0dc2a40f0 100644 --- a/crates/ide_db/src/defs.rs +++ b/crates/ide_db/src/defs.rs @@ -79,29 +79,6 @@ impl Definition { }; Some(name) } - - pub fn docs(&self, db: &RootDatabase) -> Option { - match self { - Definition::Macro(it) => it.docs(db), - Definition::Field(it) => it.docs(db), - Definition::ModuleDef(def) => match def { - hir::ModuleDef::Module(it) => it.docs(db), - hir::ModuleDef::Function(it) => it.docs(db), - hir::ModuleDef::Adt(def) => match def { - hir::Adt::Struct(it) => it.docs(db), - hir::Adt::Union(it) => it.docs(db), - hir::Adt::Enum(it) => it.docs(db), - }, - hir::ModuleDef::Variant(it) => it.docs(db), - hir::ModuleDef::Const(it) => it.docs(db), - hir::ModuleDef::Static(it) => it.docs(db), - hir::ModuleDef::Trait(it) => it.docs(db), - hir::ModuleDef::TypeAlias(it) => it.docs(db), - hir::ModuleDef::BuiltinType(_) => None, - }, - _ => None, - } - } } #[derive(Debug)] From 8d786dc4c3ce26dbb3432023c7461bd879993bfd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 30 Mar 2021 22:26:03 +0200 Subject: [PATCH 5/5] Replace Range usage with TextRange --- crates/hir_def/src/attr.rs | 42 +++++++++++++------------------------ crates/ide/src/doc_links.rs | 17 +++++++++++---- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 7791402c96a..74bb6de35b1 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -1,8 +1,8 @@ //! A higher level attributes based on TokenTree, with also some shortcuts. use std::{ - cmp::Ordering, - ops::{self, Range}, + convert::{TryFrom, TryInto}, + ops, sync::Arc, }; @@ -479,6 +479,7 @@ impl AttrsWithOwner { if !doc.is_empty() { for line in doc.split('\n') { let line = line.trim_end(); + let line_len = line.len(); let (offset, line) = match line.char_indices().nth(indent) { Some((offset, _)) => (offset, &line[offset..]), None => (0, line), @@ -486,9 +487,9 @@ impl AttrsWithOwner { let buf_offset = buf.len(); buf.push_str(line); mapping.push(( - Range { start: buf_offset, end: buf.len() }, + TextRange::new(buf_offset.try_into().ok()?, buf.len().try_into().ok()?), idx, - Range { start: offset, end: line.len() }, + TextRange::new(offset.try_into().ok()?, line_len.try_into().ok()?), )); buf.push('\n'); } @@ -565,31 +566,18 @@ pub struct DocsRangeMap { // (docstring-line-range, attr_index, attr-string-range) // a mapping from the text range of a line of the [`Documentation`] to the attribute index and // the original (untrimmed) syntax doc line - mapping: Vec<(Range, u32, Range)>, + mapping: Vec<(TextRange, u32, TextRange)>, } impl DocsRangeMap { - pub fn map(&self, range: Range) -> Option> { - let found = self - .mapping - .binary_search_by(|(probe, ..)| { - if probe.contains(&range.start) { - Ordering::Equal - } else { - probe.start.cmp(&range.end) - } - }) - .ok()?; + pub fn map(&self, range: TextRange) -> Option> { + let found = self.mapping.binary_search_by(|(probe, ..)| probe.ordering(range)).ok()?; let (line_docs_range, idx, original_line_src_range) = self.mapping[found].clone(); - if range.end > line_docs_range.end { + if !line_docs_range.contains_range(range) { return None; } - let relative_range = Range { - start: range.start - line_docs_range.start, - end: range.end - line_docs_range.start, - }; - let range_len = TextSize::from((range.end - range.start) as u32); + let relative_range = range - line_docs_range.start(); let &InFile { file_id, value: ref source } = &self.source[idx as usize]; match source { @@ -599,12 +587,10 @@ impl DocsRangeMap { let text_range = comment.syntax().text_range(); let range = TextRange::at( text_range.start() - + TextSize::from( - (comment.prefix().len() - + original_line_src_range.start - + relative_range.start) as u32, - ), - text_range.len().min(range_len), + + TextSize::try_from(comment.prefix().len()).ok()? + + original_line_src_range.start() + + relative_range.start(), + text_range.len().min(range.len()), ); Some(InFile { file_id, value: range }) } diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 9a9a4111387..2edd551cbca 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -1,6 +1,9 @@ //! Extracts, resolves and rewrites links and intra-doc links in markdown documentation. -use std::{convert::TryFrom, iter::once, ops::Range}; +use std::{ + convert::{TryFrom, TryInto}, + iter::once, +}; use itertools::Itertools; use pulldown_cmark::{BrokenLink, CowStr, Event, InlineStr, LinkType, Options, Parser, Tag}; @@ -15,7 +18,9 @@ use ide_db::{ defs::{Definition, NameClass, NameRefClass}, RootDatabase, }; -use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, TokenAtOffset, T}; +use syntax::{ + ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, TextRange, TokenAtOffset, T, +}; use crate::{FilePosition, Semantics}; @@ -115,7 +120,7 @@ pub(crate) fn external_docs( /// Extracts all links from a given markdown text. pub(crate) fn extract_definitions_from_markdown( markdown: &str, -) -> Vec<(Range, String, Option)> { +) -> Vec<(TextRange, String, Option)> { Parser::new_with_broken_link_callback( markdown, Options::empty(), @@ -126,7 +131,11 @@ pub(crate) fn extract_definitions_from_markdown( if let Event::Start(Tag::Link(_, target, title)) = event { let link = if target.is_empty() { title } else { target }; let (link, ns) = parse_intra_doc_link(&link); - Some((range, link.to_string(), ns)) + Some(( + TextRange::new(range.start.try_into().ok()?, range.end.try_into().ok()?), + link.to_string(), + ns, + )) } else { None }