Merge #10320

10320: feat: Hover/GotoDef works in macro invocations and on doc attribute strings r=Veykril a=Veykril ![image](https://user-images.githubusercontent.com/3757771/134554781-b903d33d-674f-4ed4-8acb-71ff5913f1cb.png) cc https://github.com/rust-analyzer/rust-analyzer/issues/10271 Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
2021-09-23 17:56:55 +00:00 · 2021-09-23 17:56:55 +00:00 · 61bb0cfbbe
commit 61bb0cfbbe
parent eb727c7222 22c6f0a8a5
5 changed files with 177 additions and 55 deletions
--- a/crates/hir_def/src/attr.rs
+++ b/crates/hir_def/src/attr.rs
@ -17,7 +17,7 @@
 use mbe::{syntax_node_to_token_tree, DelimiterKind};
 use smallvec::{smallvec, SmallVec};
 use syntax::{
-    ast::{self, AstNode, AttrsOwner},
+    ast::{self, AstNode, AttrsOwner, IsString},
    match_ast, AstPtr, AstToken, SmolStr, SyntaxNode, TextRange, TextSize,
 };
 use tt::Subtree;
@ -610,6 +610,7 @@ pub struct DocsRangeMap {
 }

 impl DocsRangeMap {
+    /// Maps a [`TextRange`] relative to the documentation string back to its AST range
    pub fn map(&self, range: TextRange) -> Option<InFile<TextRange>> {
        let found = self.mapping.binary_search_by(|(probe, ..)| probe.ordering(range)).ok()?;
        let (line_docs_range, idx, original_line_src_range) = self.mapping[found];
@ -621,8 +622,15 @@ pub fn map(&self, range: TextRange) -> Option<InFile<TextRange>> {

        let InFile { file_id, value: source } = self.source_map.source_of_id(idx);
        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::Left(attr) => {
+                let string = get_doc_string_in_attr(&attr)?;
+                let text_range = string.open_quote_text_range()?;
+                let range = TextRange::at(
+                    text_range.end() + original_line_src_range.start() + relative_range.start(),
+                    string.syntax().text_range().len().min(range.len()),
+                );
+                Some(InFile { file_id, value: range })
+            }
            Either::Right(comment) => {
                let text_range = comment.syntax().text_range();
                let range = TextRange::at(
@ -638,6 +646,22 @@ pub fn map(&self, range: TextRange) -> Option<InFile<TextRange>> {
    }
 }

+fn get_doc_string_in_attr(it: &ast::Attr) -> Option<ast::String> {
+    match it.expr() {
+        // #[doc = lit]
+        Some(ast::Expr::Literal(lit)) => match lit.kind() {
+            ast::LiteralKind::String(it) => Some(it),
+            _ => None,
+        },
+        // #[cfg_attr(..., doc = "", ...)]
+        None => {
+            // FIXME: See highlight injection for what to do here
+            None
+        }
+        _ => None,
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub(crate) struct AttrId {
    is_doc_comment: bool,
--- a/crates/ide/src/doc_links.rs
+++ b/crates/ide/src/doc_links.rs
@ -19,7 +19,12 @@
    helpers::pick_best_token,
    RootDatabase,
 };
-use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode, TextRange, T};
+use syntax::{
+    ast::{self, IsString},
+    match_ast, AstNode, AstToken,
+    SyntaxKind::*,
+    SyntaxNode, SyntaxToken, TextRange, TextSize, T,
+};

 use crate::{
    doc_links::intra_doc_links::{parse_intra_doc_link, strip_prefixes_suffixes},
@ -220,6 +225,66 @@ pub(crate) fn doc_attributes(
    }
 }

+pub(crate) struct DocCommentToken {
+    doc_token: SyntaxToken,
+    prefix_len: TextSize,
+}
+
+pub(crate) fn token_as_doc_comment(doc_token: &SyntaxToken) -> Option<DocCommentToken> {
+    (match_ast! {
+        match doc_token {
+            ast::Comment(comment) => TextSize::try_from(comment.prefix().len()).ok(),
+            ast::String(string) => doc_token.ancestors().find_map(ast::Attr::cast)
+                .filter(|attr| attr.simple_name().as_deref() == Some("doc")).and_then(|_| string.open_quote_text_range().map(|it| it.len())),
+            _ => None,
+        }
+    }).map(|prefix_len| DocCommentToken { prefix_len, doc_token: doc_token.clone() })
+}
+
+impl DocCommentToken {
+    pub(crate) fn get_definition_with_descend_at<T>(
+        self,
+        sema: &Semantics<RootDatabase>,
+        offset: TextSize,
+        // Definition, CommentOwner, range of intra doc link in original file
+        mut cb: impl FnMut(Definition, SyntaxNode, TextRange) -> Option<T>,
+    ) -> Option<T> {
+        let DocCommentToken { prefix_len, doc_token } = self;
+        // offset relative to the comments contents
+        let original_start = doc_token.text_range().start();
+        let relative_comment_offset = offset - original_start - prefix_len;
+
+        sema.descend_into_macros_many(doc_token.clone()).into_iter().find_map(|t| {
+            let (node, descended_prefix_len) = match_ast! {
+                match t {
+                    ast::Comment(comment) => (t.parent()?, TextSize::try_from(comment.prefix().len()).ok()?),
+                    ast::String(string) => (t.ancestors().skip_while(|n| n.kind() != ATTR).nth(1)?, string.open_quote_text_range()?.len()),
+                    _ => return None,
+                }
+            };
+            let token_start = t.text_range().start();
+            let abs_in_expansion_offset = token_start + relative_comment_offset + descended_prefix_len;
+
+            let (attributes, def) = doc_attributes(sema, &node)?;
+            let (docs, doc_mapping) = attributes.docs_with_rangemap(sema.db)?;
+            let (in_expansion_range, link, ns) =
+                extract_definitions_from_docs(&docs).into_iter().find_map(|(range, link, ns)| {
+                    let mapped = doc_mapping.map(range)?;
+                    (mapped.value.contains(abs_in_expansion_offset)).then(|| (mapped.value, link, ns))
+                })?;
+            // get the relative range to the doc/attribute in the expansion
+            let in_expansion_relative_range = in_expansion_range - descended_prefix_len - token_start;
+            // Apply relative range to the original input comment
+            let absolute_range = in_expansion_relative_range + original_start + prefix_len;
+            let def = match resolve_doc_path_for_def(sema.db, def, &link, ns)? {
+                Either::Left(it) => Definition::ModuleDef(it),
+                Either::Right(it) => Definition::Macro(it),
+            };
+            cb(def, node, absolute_range)
+        })
+    }
+}
+
 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
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@ -1,11 +1,9 @@
 use std::convert::TryInto;

 use crate::{
-    display::TryToNav,
-    doc_links::{doc_attributes, extract_definitions_from_docs, resolve_doc_path_for_def},
-    FilePosition, NavigationTarget, RangeInfo,
+    display::TryToNav, doc_links::token_as_doc_comment, FilePosition, NavigationTarget, RangeInfo,
 };
-use hir::{AsAssocItem, InFile, ModuleDef, Semantics};
+use hir::{AsAssocItem, ModuleDef, Semantics};
 use ide_db::{
    base_db::{AnchoredPath, FileId, FileLoader},
    defs::Definition,
@ -30,7 +28,7 @@ pub(crate) fn goto_definition(
    db: &RootDatabase,
    position: FilePosition,
 ) -> Option<RangeInfo<Vec<NavigationTarget>>> {
-    let sema = Semantics::new(db);
+    let sema = &Semantics::new(db);
    let file = sema.parse(position.file_id).syntax().clone();
    let original_token =
        pick_best_token(file.token_at_offset(position.offset), |kind| match kind {
@ -38,18 +36,11 @@ pub(crate) fn goto_definition(
            kind if kind.is_trivia() => 0,
            _ => 1,
        })?;
-    if let Some(_) = ast::Comment::cast(original_token.clone()) {
-        let parent = original_token.parent()?;
-        let (attributes, def) = doc_attributes(&sema, &parent)?;
-        let (docs, doc_mapping) = attributes.docs_with_rangemap(db)?;
-        let (_, link, ns) =
-            extract_definitions_from_docs(&docs).into_iter().find(|&(range, ..)| {
-                doc_mapping.map(range).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]));
+    if let Some(doc_comment) = token_as_doc_comment(&original_token) {
+        return doc_comment.get_definition_with_descend_at(sema, position.offset, |def, _, _| {
+            let nav = def.try_to_nav(db)?;
+            Some(RangeInfo::new(original_token.text_range(), vec![nav]))
+        });
    }
    let navs = sema
        .descend_into_macros_many(original_token.clone())
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@ -20,10 +20,7 @@

 use crate::{
    display::{macro_label, TryToNav},
-    doc_links::{
-        doc_attributes, extract_definitions_from_docs, remove_links, resolve_doc_path_for_def,
-        rewrite_links,
-    },
+    doc_links::{remove_links, rewrite_links, token_as_doc_comment},
    markdown_remove::remove_markdown,
    markup::Markup,
    runnables::{runnable_fn, runnable_mod},
@ -114,40 +111,15 @@ pub(crate) fn hover(
        _ => 1,
    })?;

-    let descended = sema.descend_into_macros_many(original_token.clone());
-
-    // FIXME handle doc attributes? TokenMap currently doesn't work with comments
-    if original_token.kind() == COMMENT {
-        let relative_comment_offset = offset - original_token.text_range().start();
-        // intra-doc links
+    if let Some(doc_comment) = token_as_doc_comment(&original_token) {
        cov_mark::hit!(no_highlight_on_comment_hover);
-        return descended.iter().find_map(|t| {
-            match t.kind() {
-                COMMENT => (),
-                TOKEN_TREE => {}
-                _ => return None,
-            }
-            let node = t.parent()?;
-            let absolute_comment_offset = t.text_range().start() + relative_comment_offset;
-            let (attributes, def) = doc_attributes(sema, &node)?;
-            let (docs, doc_mapping) = attributes.docs_with_rangemap(sema.db)?;
-            let (idl_range, link, ns) = extract_definitions_from_docs(&docs).into_iter().find_map(
-                |(range, link, ns)| {
-                    let mapped = doc_mapping.map(range)?;
-                    (mapped.file_id == file_id.into()
-                        && mapped.value.contains(absolute_comment_offset))
-                    .then(|| (mapped.value, link, ns))
-                },
-            )?;
-            let def = match resolve_doc_path_for_def(sema.db, def, &link, ns)? {
-                Either::Left(it) => Definition::ModuleDef(it),
-                Either::Right(it) => Definition::Macro(it),
-            };
+        return doc_comment.get_definition_with_descend_at(sema, offset, |def, node, range| {
            let res = hover_for_definition(sema, file_id, def, &node, config)?;
-            Some(RangeInfo::new(idl_range, res))
+            Some(RangeInfo::new(range, res))
        });
    }

+    let descended = sema.descend_into_macros_many(original_token.clone());
    // attributes, require special machinery as they are mere ident tokens

    // FIXME: Definition should include known lints and the like instead of having this special case here
@ -4941,4 +4913,63 @@ fn foo() {
            "#]],
        );
    }
+
+    #[test]
+    fn hover_intra_in_macro() {
+        check(
+            r#"
+macro_rules! foo_macro {
+    ($(#[$attr:meta])* $name:ident) => {
+        $(#[$attr])*
+        pub struct $name;
+    }
+}
+
+foo_macro!(
+    /// Doc comment for [`Foo$0`]
+    Foo
+);
+"#,
+            expect![[r#"
+                *[`Foo`]*
+
+                ```rust
+                test
+                ```
+
+                ```rust
+                pub struct Foo
+                ```
+
+                ---
+
+                Doc comment for [`Foo`](https://docs.rs/test/*/test/struct.Foo.html)
+            "#]],
+        );
+    }
+
+    #[test]
+    fn hover_intra_in_attr() {
+        check(
+            r#"
+#[doc = "Doc comment for [`Foo$0`]"]
+pub struct Foo;
+"#,
+            expect![[r#"
+                *[`Foo`]*
+
+                ```rust
+                test
+                ```
+
+                ```rust
+                pub struct Foo
+                ```
+
+                ---
+
+                Doc comment for [`Foo`](https://docs.rs/test/*/test/struct.Foo.html)
+            "#]],
+        );
+    }
 }
--- a/crates/mbe/src/syntax_bridge.rs
+++ b/crates/mbe/src/syntax_bridge.rs
@ -149,7 +149,18 @@ struct StackEntry {
        let k: SyntaxKind = token.kind();
        if k == COMMENT {
            if let Some(tokens) = conv.convert_doc_comment(&token) {
-                result.extend(tokens);
+                // FIXME: There has to be a better way to do this
+                // Add the comments token id to the converted doc string
+                let id = conv.id_alloc().alloc(range);
+                result.extend(tokens.into_iter().map(|mut tt| {
+                    if let tt::TokenTree::Subtree(sub) = &mut tt {
+                        if let tt::TokenTree::Leaf(tt::Leaf::Literal(lit)) = &mut sub.token_trees[2]
+                        {
+                            lit.id = id
+                        }
+                    }
+                    tt
+                }));
            }
            continue;
        }