From 42eb4efb5b58d1e91787eed0c23cc26157908474 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 23 Sep 2021 15:37:52 +0200 Subject: [PATCH] Cleanup --- Cargo.lock | 1 + crates/hir_def/src/attr.rs | 2 + crates/ide/src/goto_definition.rs | 2 +- crates/ide/src/hover.rs | 192 +++++++++++------------------- crates/ide_db/Cargo.toml | 1 + crates/ide_db/src/defs.rs | 116 ++++++++++-------- crates/syntax/src/ast/node_ext.rs | 10 ++ 7 files changed, 148 insertions(+), 176 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47e88e877d..9170ec3a66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -645,6 +645,7 @@ dependencies = [ name = "ide_db" version = "0.0.0" dependencies = [ + "arrayvec", "base_db", "cov-mark", "either", diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 0470b9510f..ef86572fe5 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -547,6 +547,7 @@ fn inner_attributes( Some((attrs, docs)) } +#[derive(Debug)] pub struct AttrSourceMap { attrs: Vec>, doc_comments: Vec>, @@ -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) diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 4c01231f4a..9acea114d6 100644 --- a/crates/ide/src/goto_definition.rs +++ b/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) diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 80e91f303c..4e57484c78 100644 --- a/crates/ide/src/hover.rs +++ b/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> { - 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; - // attributes, require special machinery as they are mere ident tokens - - 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, - file_id: FileId, - offset: TextSize, - config: &HoverConfig, - original_token: &SyntaxToken, - token: &SyntaxToken, - node: &SyntaxNode, - seen: &mut HashSet, -) -> Option>> { - 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(|| { + // 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 - // FIXME: move comment + attribute special cases somewhere else to simplify control flow, - // hopefully simplifying the return type of this function in the process - // (the `Break`/`Continue` distinction is needed to decide whether to use fallback hovers) - // - // FIXME: hovering the intra doc link to `Foo` not working: - // - // #[identity] - // trait Foo { - // /// [`Foo`] - // fn foo() {} - if token.kind() == COMMENT { - add_to_seen_definitions = false; - cov_mark::hit!(no_highlight_on_comment_hover); - let (attributes, def) = doc_attributes(sema, node)?; + 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(offset)) - .then(|| (mapped.value, link, ns)) + (mapped.file_id == file_id.into() + && mapped.value.contains(absolute_comment_offset)) + .then(|| (mapped.value, link, ns)) }, )?; - range_override = Some(idl_range); - Some(match resolve_doc_path_for_def(sema.db, def, &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), - }) - } else { - None - } - }); - - 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))); - } + }; + let res = hover_for_definition(sema, file_id, def, &node, config)?; + Some(RangeInfo::new(idl_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> { - 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 + ``` "#]], ); } diff --git a/crates/ide_db/Cargo.toml b/crates/ide_db/Cargo.toml index 981a58347a..cdaff0ce39 100644 --- a/crates/ide_db/Cargo.toml +++ b/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" } diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs index d42af9c38f..effa694aae 100644 --- a/crates/ide_db/src/defs.rs +++ b/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, token: &SyntaxToken) -> Vec { - let node = if let Some(x) = token.parent() { - x - } else { - return vec![]; + pub fn from_token( + sema: &Semantics, + token: &SyntaxToken, + ) -> ArrayVec { + let parent = match token.parent() { + Some(parent) => parent, + None => return Default::default(), }; - if token.kind() != SyntaxKind::COMMENT { - if let Some(attr) = token.ancestors().find_map(ast::Attr::cast) { - // derives - let def = try_resolve_derive_input_at(&sema, &attr, &token).map(Definition::Macro); - if let Some(def) = def { - return vec![def]; + let attr = parent + .ancestors() + .find_map(ast::TokenTree::cast) + .and_then(|tt| tt.parent_meta()) + .and_then(|meta| meta.parent_attr()); + 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, node: &SyntaxNode) -> ArrayVec { + 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); + } + }, + _ => (), } } - } - match_ast! { - match node { - 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![], - } - } + Some(()) + })(); + res } pub fn module(&self, db: &RootDatabase) -> Option { diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index 930e5ce74f..4070406702 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/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 { + self.syntax().parent().and_then(ast::Meta::cast) + } +} + +impl ast::Meta { + pub fn parent_attr(&self) -> Option { + self.syntax().parent().and_then(ast::Attr::cast) + } } impl ast::GenericParamList {