Cleanup

Cargo.lock

@@ -645,6 +645,7 @@ dependencies = [
 name = "ide_db"
 version = "0.0.0"
 dependencies = [
+ "arrayvec",
  "base_db",
  "cov-mark",
  "either",

crates/hir_def/src/attr.rs

@@ -547,6 +547,7 @@ fn inner_attributes(
     Some((attrs, docs))
 }
 
+#[derive(Debug)]
 pub struct AttrSourceMap {
     attrs: Vec<InFile<ast::Attr>>,
     doc_comments: Vec<InFile<ast::Comment>>,
@@ -599,6 +600,7 @@ impl AttrSourceMap {
 }
 
 /// A struct to map text ranges from [`Documentation`] back to TextRanges in the syntax tree.
+#[derive(Debug)]
 pub struct DocsRangeMap {
     source_map: AttrSourceMap,
     // (docstring-line-range, attr_index, attr-string-range)

crates/ide/src/goto_definition.rs

@@ -64,7 +64,7 @@ pub(crate) fn goto_definition(
                 }
             }
             Some(
-                Definition::from_node(&sema, &token)
+                Definition::from_token(&sema, &token)
                     .into_iter()
                     .flat_map(|def| {
                         try_find_trait_item_definition(sema.db, &def)

crates/ide/src/hover.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashSet, ops::ControlFlow};
+use std::iter;
 
 use either::Either;
 use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, Semantics, TypeInfo};
@@ -15,7 +15,7 @@ use itertools::Itertools;
 use stdx::format_to;
 use syntax::{
     algo, ast, display::fn_as_proc_macro_label, match_ast, AstNode, Direction, SyntaxKind::*,
-    SyntaxNode, SyntaxToken, TextRange, TextSize, T,
+    SyntaxNode, SyntaxToken, T,
 };
 
 use crate::{
@@ -99,7 +99,7 @@ pub(crate) fn hover(
     FileRange { file_id, range }: FileRange,
     config: &HoverConfig,
 ) -> Option<RangeInfo<HoverResult>> {
-    let sema = hir::Semantics::new(db);
+    let sema = &hir::Semantics::new(db);
     let file = sema.parse(file_id).syntax().clone();
 
     if !range.is_empty() {
@@ -114,135 +114,75 @@ pub(crate) fn hover(
         _ => 1,
     })?;
 
-    let mut seen = HashSet::default();
+    let descended = sema.descend_into_macros_many(original_token.clone());
 
-    let mut fallback = None;
+    // FIXME handle doc attributes? TokenMap currently doesn't work with comments
-    // attributes, require special machinery as they are mere ident tokens
+    if original_token.kind() == COMMENT {
-
+        let relative_comment_offset = offset - original_token.text_range().start();
-    let descend_macros = sema.descend_into_macros_many(original_token.clone());
-
-    for token in &descend_macros {
-        if token.kind() != COMMENT {
-            if let Some(attr) = token.ancestors().find_map(ast::Attr::cast) {
-                // lints
-                if let Some(res) = try_hover_for_lint(&attr, &token) {
-                    return Some(res);
-                }
-            }
-        }
-    }
-
-    descend_macros
-        .iter()
-        .filter_map(|token| match token.parent() {
-            Some(node) => {
-                match find_hover_result(
-                    &sema,
-                    file_id,
-                    offset,
-                    config,
-                    &original_token,
-                    token,
-                    &node,
-                    &mut seen,
-                ) {
-                    Some(res) => match res {
-                        ControlFlow::Break(inner) => Some(inner),
-                        ControlFlow::Continue(_) => {
-                            if fallback.is_none() {
-                                // FIXME we're only taking the first fallback into account that's not `None`
-                                fallback = hover_for_keyword(&sema, config, &token)
-                                    .or(type_hover(&sema, config, &token));
-                            }
-                            None
-                        }
-                    },
-                    None => None,
-                }
-            }
-            None => None,
-        })
-        // reduce all descends into a single `RangeInfo`
-        // that spans from the earliest start to the latest end (fishy/FIXME),
-        // concatenates all `Markup`s with `\n---\n`,
-        // and accumulates all actions into its `actions` vector.
-        .reduce(|mut acc, RangeInfo { range, mut info }| {
-            let start = acc.range.start().min(range.start());
-            let end = acc.range.end().max(range.end());
-
-            acc.range = TextRange::new(start, end);
-            acc.info.actions.append(&mut info.actions);
-            acc.info.markup = Markup::from(format!("{}\n---\n{}", acc.info.markup, info.markup));
-            acc
-        })
-        .or(fallback)
-}
-
-fn find_hover_result(
-    sema: &Semantics<RootDatabase>,
-    file_id: FileId,
-    offset: TextSize,
-    config: &HoverConfig,
-    original_token: &SyntaxToken,
-    token: &SyntaxToken,
-    node: &SyntaxNode,
-    seen: &mut HashSet<Definition>,
-) -> Option<ControlFlow<RangeInfo<HoverResult>>> {
-    let mut range_override = None;
-
-    // intra-doc links and attributes are special cased
-    // so don't add them to the `seen` duplicate check
-    let mut add_to_seen_definitions = true;
-
-    let definition = Definition::from_node(sema, token).into_iter().next().or_else(|| {
         // intra-doc links
-        // FIXME: move comment + attribute special cases somewhere else to simplify control flow,
+        cov_mark::hit!(no_highlight_on_comment_hover);
-        // hopefully simplifying the return type of this function in the process
+        return descended.iter().find_map(|t| {
-        // (the `Break`/`Continue` distinction is needed to decide whether to use fallback hovers)
+            match t.kind() {
-        //
+                COMMENT => (),
-        // FIXME: hovering the intra doc link to `Foo` not working:
+                TOKEN_TREE => {}
-        //
+                _ => return None,
-        // #[identity]
+            }
-        // trait Foo {
+            let node = t.parent()?;
-        //    /// [`Foo`]
+            let absolute_comment_offset = t.text_range().start() + relative_comment_offset;
-        // fn foo() {}
+            let (attributes, def) = doc_attributes(sema, &node)?;
-        if token.kind() == COMMENT {
-            add_to_seen_definitions = false;
-            cov_mark::hit!(no_highlight_on_comment_hover);
-            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(offset))
+                    (mapped.file_id == file_id.into()
-                        .then(|| (mapped.value, link, ns))
+                        && mapped.value.contains(absolute_comment_offset))
+                    .then(|| (mapped.value, link, ns))
                 },
             )?;
-            range_override = Some(idl_range);
+            let def = match resolve_doc_path_for_def(sema.db, def, &link, ns)? {
-            Some(match resolve_doc_path_for_def(sema.db, def, &link, ns)? {
                 Either::Left(it) => Definition::ModuleDef(it),
                 Either::Right(it) => Definition::Macro(it),
-            })
+            };
-        } else {
+            let res = hover_for_definition(sema, file_id, def, &node, config)?;
-            None
+            Some(RangeInfo::new(idl_range, res))
-        }
+        });
-    });
-
-    if let Some(definition) = definition {
-        // skip duplicates
-        if seen.contains(&definition) {
-            return None;
-        }
-        if add_to_seen_definitions {
-            seen.insert(definition);
-        }
-        if let Some(res) = hover_for_definition(sema, file_id, definition, &node, config) {
-            let range = range_override.unwrap_or_else(|| original_token.text_range());
-            return Some(ControlFlow::Break(RangeInfo::new(range, res)));
-        }
     }
 
-    Some(ControlFlow::Continue(()))
+    // 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
+    if let res @ Some(_) = descended.iter().find_map(|token| {
+        let attr = token.ancestors().find_map(ast::Attr::cast)?;
+        try_hover_for_lint(&attr, &token)
+    }) {
+        return res;
+    }
+
+    let result = descended
+        .iter()
+        .filter_map(|token| {
+            let node = token.parent()?;
+            let defs = Definition::from_token(sema, token);
+            Some(defs.into_iter().zip(iter::once(node).cycle()))
+        })
+        .flatten()
+        .unique_by(|&(def, _)| def)
+        .filter_map(|(def, node)| hover_for_definition(sema, file_id, def, &node, config))
+        .reduce(|mut acc, HoverResult { markup, actions }| {
+            acc.actions.extend(actions);
+            acc.markup = Markup::from(format!("{}\n---\n{}", acc.markup, markup));
+            acc
+        });
+    if result.is_none() {
+        // fallbacks, show keywords or types
+        if let res @ Some(_) = hover_for_keyword(sema, config, &original_token) {
+            return res;
+        }
+        if let res @ Some(_) = descended.iter().find_map(|token| type_hover(sema, config, token)) {
+            return res;
+        }
+    }
+    result.map(|res| RangeInfo::new(original_token.text_range(), res))
 }
 
 fn type_hover(
@@ -250,9 +190,6 @@ fn type_hover(
     config: &HoverConfig,
     token: &SyntaxToken,
 ) -> Option<RangeInfo<HoverResult>> {
-    if token.kind() == COMMENT {
-        return None;
-    }
     let node = token
         .ancestors()
         .take_while(|it| !ast::Item::can_cast(it.kind()))
@@ -3626,6 +3563,15 @@ fn main() {
                 ```rust
                 f: &i32
                 ```
+                ---
+
+                ```rust
+                test::S
+                ```
+
+                ```rust
+                f: i32
+                ```
             "#]],
         );
     }

crates/ide_db/Cargo.toml

@@ -17,6 +17,7 @@ rustc-hash = "1.1.0"
 once_cell = "1.3.1"
 either = "1.6.1"
 itertools = "0.10.0"
+arrayvec = "0.7"
 
 stdx = { path = "../stdx", version = "0.0.0" }
 syntax = { path = "../syntax", version = "0.0.0" }

crates/ide_db/src/defs.rs

@@ -5,13 +5,14 @@
 
 // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06).
 
+use arrayvec::ArrayVec;
 use hir::{
     Field, GenericParam, HasVisibility, Impl, Label, Local, MacroDef, Module, ModuleDef, Name,
     PathResolution, Semantics, Visibility,
 };
 use syntax::{
     ast::{self, AstNode},
-    match_ast, SyntaxKind, SyntaxToken,
+    match_ast, SyntaxKind, SyntaxNode, SyntaxToken,
 };
 
 use crate::{helpers::try_resolve_derive_input_at, RootDatabase};
@@ -29,60 +30,71 @@ pub enum Definition {
 }
 
 impl Definition {
-    pub fn from_node(sema: &Semantics<RootDatabase>, token: &SyntaxToken) -> Vec<Definition> {
+    pub fn from_token(
-        let node = if let Some(x) = token.parent() {
+        sema: &Semantics<RootDatabase>,
-            x
+        token: &SyntaxToken,
-        } else {
+    ) -> ArrayVec<Definition, 2> {
-            return vec![];
+        let parent = match token.parent() {
+            Some(parent) => parent,
+            None => return Default::default(),
         };
-        if token.kind() != SyntaxKind::COMMENT {
+        let attr = parent
-            if let Some(attr) = token.ancestors().find_map(ast::Attr::cast) {
+            .ancestors()
-                // derives
+            .find_map(ast::TokenTree::cast)
-                let def = try_resolve_derive_input_at(&sema, &attr, &token).map(Definition::Macro);
+            .and_then(|tt| tt.parent_meta())
-                if let Some(def) = def {
+            .and_then(|meta| meta.parent_attr());
-                    return vec![def];
+        if let Some(attr) = attr {
+            try_resolve_derive_input_at(&sema, &attr, &token)
+                .map(Definition::Macro)
+                .into_iter()
+                .collect()
+        } else {
+            Self::from_node(sema, &parent)
+        }
+    }
+
+    pub fn from_node(sema: &Semantics<RootDatabase>, node: &SyntaxNode) -> ArrayVec<Definition, 2> {
+        let mut res = ArrayVec::new();
+        (|| {
+            match_ast! {
+                match node {
+                    ast::Name(name) => {
+                        match NameClass::classify(&sema, &name)? {
+                            NameClass::Definition(it) | NameClass::ConstReference(it) => res.push(it),
+                            NameClass::PatFieldShorthand { local_def, field_ref } => {
+                                res.push(Definition::Local(local_def));
+                                res.push(Definition::Field(field_ref));
+                            }
+                        }
+                    },
+                    ast::NameRef(name_ref) => {
+                        match NameRefClass::classify(sema, &name_ref)? {
+                            NameRefClass::Definition(it) => res.push(it),
+                            NameRefClass::FieldShorthand { local_ref, field_ref } => {
+                                res.push(Definition::Local(local_ref));
+                                res.push(Definition::Field(field_ref));
+                            }
+                        }
+                    },
+                    ast::Lifetime(lifetime) => {
+                        let def = if let Some(x) = NameClass::classify_lifetime(&sema, &lifetime) {
+                            NameClass::defined(x)
+                        } else {
+                            NameRefClass::classify_lifetime(&sema, &lifetime).and_then(|class| match class {
+                                NameRefClass::Definition(it) => Some(it),
+                                _ => None,
+                            })
+                        };
+                        if let Some(def) = def {
+                            res.push(def);
+                        }
+                    },
+                    _ => (),
                 }
             }
-        }
+            Some(())
-        match_ast! {
+        })();
-            match node {
+        res
-                ast::Name(name) => {
-                    let class = if let Some(x) = NameClass::classify(&sema, &name) {
-                        x
-                    } else {
-                        return vec![];
-                    };
-                    match class {
-                        NameClass::Definition(it) | NameClass::ConstReference(it) => vec![it],
-                        NameClass::PatFieldShorthand { local_def, field_ref } => vec![Definition::Local(local_def), Definition::Field(field_ref)],
-                    }
-                },
-                ast::NameRef(name_ref) => {
-                    let class = if let Some(x) = NameRefClass::classify(sema, &name_ref) {
-                        x
-                    } else {
-                        return vec![];
-                    };
-                    match class {
-                        NameRefClass::Definition(def) => vec![def],
-                        NameRefClass::FieldShorthand { local_ref, field_ref } => {
-                            vec![Definition::Field(field_ref), Definition::Local(local_ref)]
-                        }
-                    }
-                },
-                ast::Lifetime(lifetime) => {
-                    (if let Some(x) = NameClass::classify_lifetime(&sema, &lifetime) {
-                        NameClass::defined(x)
-                    } else {
-                        NameRefClass::classify_lifetime(&sema, &lifetime).and_then(|class| match class {
-                            NameRefClass::Definition(it) => Some(it),
-                            _ => None,
-                        })
-                    }).into_iter().collect()
-                },
-                _ => vec![],
-            }
-        }
     }
 
     pub fn module(&self, db: &RootDatabase) -> Option<Module> {

crates/syntax/src/ast/node_ext.rs

@@ -796,6 +796,16 @@ impl ast::TokenTree {
             .into_token()
             .filter(|it| matches!(it.kind(), T!['}'] | T![')'] | T![']']))
     }
+
+    pub fn parent_meta(&self) -> Option<ast::Meta> {
+        self.syntax().parent().and_then(ast::Meta::cast)
+    }
+}
+
+impl ast::Meta {
+    pub fn parent_attr(&self) -> Option<ast::Attr> {
+        self.syntax().parent().and_then(ast::Attr::cast)
+    }
 }
 
 impl ast::GenericParamList {