From 61360fdfec981eadef1eefb595c8b32c951771e8 Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Sun, 15 Dec 2019 01:20:07 +0800 Subject: [PATCH 1/3] Fix original_source find order --- crates/ra_hir/src/lib.rs | 3 +- crates/ra_hir_expand/src/lib.rs | 29 +++++-- .../ra_ide/src/display/navigation_target.rs | 19 ++++- crates/ra_ide/src/expand.rs | 79 ++++++++++--------- crates/ra_ide/src/goto_definition.rs | 34 ++++++-- 5 files changed, 109 insertions(+), 55 deletions(-) diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index e7602ee305..2bf729b6d6 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -58,6 +58,7 @@ pub use hir_def::{ type_ref::Mutability, }; pub use hir_expand::{ - name::Name, HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, + name::Name, ExpansionOrigin, HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, + MacroFile, }; pub use hir_ty::{display::HirDisplay, CallableDef}; diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index 94e1e466a5..d1a43fe6cf 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -214,7 +214,17 @@ pub struct ExpansionInfo { exp_map: Arc, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ExpansionOrigin { + Call, + Def, +} + impl ExpansionInfo { + pub fn call_node(&self) -> Option> { + Some(self.arg.with_value(self.arg.value.parent()?)) + } + pub fn map_token_down(&self, token: InFile<&SyntaxToken>) -> Option> { assert_eq!(token.file_id, self.arg.file_id); let range = token.value.text_range().checked_sub(self.arg.value.text_range().start())?; @@ -228,21 +238,26 @@ impl ExpansionInfo { Some(self.expanded.with_value(token)) } - pub fn map_token_up(&self, token: InFile<&SyntaxToken>) -> Option> { + pub fn map_token_up( + &self, + token: InFile<&SyntaxToken>, + ) -> Option<(InFile, ExpansionOrigin)> { let token_id = self.exp_map.token_by_range(token.value.text_range())?; let (token_id, origin) = self.macro_def.0.map_id_up(token_id); - let (token_map, tt) = match origin { - mbe::Origin::Call => (&self.macro_arg.1, self.arg.clone()), - mbe::Origin::Def => { - (&self.macro_def.1, self.def.as_ref().map(|tt| tt.syntax().clone())) - } + let (token_map, tt, origin) = match origin { + mbe::Origin::Call => (&self.macro_arg.1, self.arg.clone(), ExpansionOrigin::Call), + mbe::Origin::Def => ( + &self.macro_def.1, + self.def.as_ref().map(|tt| tt.syntax().clone()), + ExpansionOrigin::Def, + ), }; let range = token_map.range_by_token(token_id)?; let token = algo::find_covering_element(&tt.value, range + tt.value.text_range().start()) .into_token()?; - Some(tt.with_value(token)) + Some((tt.with_value(token), origin)) } } diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs index 6a6b49afdf..6a2bf72731 100644 --- a/crates/ra_ide/src/display/navigation_target.rs +++ b/crates/ra_ide/src/display/navigation_target.rs @@ -7,10 +7,14 @@ use ra_syntax::{ ast::{self, DocCommentsOwner, NameOwner}, match_ast, AstNode, SmolStr, SyntaxKind::{self, BIND_PAT, TYPE_PARAM}, - TextRange, + SyntaxNode, TextRange, }; -use crate::{db::RootDatabase, expand::original_range, FileSymbol}; +use crate::{ + db::RootDatabase, + expand::{original_range_by_kind, OriginalRangeKind}, + FileRange, FileSymbol, +}; use super::short_label::ShortLabel; @@ -416,3 +420,14 @@ pub(crate) fn description_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> } } } + +fn original_range(db: &RootDatabase, node: InFile<&SyntaxNode>) -> FileRange { + if let Some(range) = original_range_by_kind(db, node, OriginalRangeKind::CallToken) { + return range; + } + if let Some(range) = original_range_by_kind(db, node, OriginalRangeKind::WholeCall) { + return range; + } + + FileRange { file_id: node.file_id.original_file(db), range: node.value.text_range() } +} diff --git a/crates/ra_ide/src/expand.rs b/crates/ra_ide/src/expand.rs index 661628ae46..327393dbbe 100644 --- a/crates/ra_ide/src/expand.rs +++ b/crates/ra_ide/src/expand.rs @@ -1,57 +1,62 @@ //! Utilities to work with files, produced by macros. use std::iter::successors; -use hir::InFile; +use hir::{ExpansionOrigin, InFile}; use ra_db::FileId; use ra_syntax::{ast, AstNode, SyntaxNode, SyntaxToken, TextRange}; use crate::{db::RootDatabase, FileRange}; -pub(crate) fn original_range(db: &RootDatabase, node: InFile<&SyntaxNode>) -> FileRange { - let expansion = match node.file_id.expansion_info(db) { - None => { - return FileRange { - file_id: node.file_id.original_file(db), - range: node.value.text_range(), - } - } - Some(it) => it, - }; +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum OriginalRangeKind { + /// Return range if any token is matched + #[allow(dead_code)] + Any, + /// Return range if token is inside macro_call + CallToken, + /// Return whole macro call range if matched + WholeCall, +} + +pub(crate) fn original_range_by_kind( + db: &RootDatabase, + node: InFile<&SyntaxNode>, + kind: OriginalRangeKind, +) -> Option { + let expansion = node.file_id.expansion_info(db)?; + + // the input node has only one token ? + let single = node.value.first_token()? == node.value.last_token()?; + // FIXME: We should handle recurside macro expansions + let range = match kind { + OriginalRangeKind::WholeCall => expansion.call_node()?.map(|node| node.text_range()), + _ => node.value.descendants().find_map(|it| { + let first = it.first_token()?; + let last = it.last_token()?; - let range = node.value.descendants_with_tokens().find_map(|it| { - match it.as_token() { - // FIXME: Remove this branch after all `tt::TokenTree`s have a proper `TokenId`, - // and return the range of the overall macro expansions if mapping first and last tokens fails. - Some(token) => { - let token = expansion.map_token_up(node.with_value(&token))?; - Some(token.with_value(token.value.text_range())) + if !single && first == last { + return None; } - None => { - // Try to map first and last tokens of node, and, if success, return the union range of mapped tokens - let n = it.into_node()?; - let first = expansion.map_token_up(node.with_value(&n.first_token()?))?; - let last = expansion.map_token_up(node.with_value(&n.last_token()?))?; - // FIXME: Is is possible ? - if first.file_id != last.file_id { - return None; - } + // Try to map first and last tokens of node, and, if success, return the union range of mapped tokens + let (first, first_origin) = expansion.map_token_up(node.with_value(&first))?; + let (last, last_origin) = expansion.map_token_up(node.with_value(&last))?; - // FIXME: Add union method in TextRange - let range = union_range(first.value.text_range(), last.value.text_range()); - Some(first.with_value(range)) + if first.file_id != last.file_id + || first_origin != last_origin + || (kind == OriginalRangeKind::CallToken && first_origin != ExpansionOrigin::Call) + { + return None; } - } - }); - return match range { - Some(it) => FileRange { file_id: it.file_id.original_file(db), range: it.value }, - None => { - FileRange { file_id: node.file_id.original_file(db), range: node.value.text_range() } - } + // FIXME: Add union method in TextRange + Some(first.with_value(union_range(first.value.text_range(), last.value.text_range()))) + })?, }; + return Some(FileRange { file_id: range.file_id.original_file(db), range: range.value }); + fn union_range(a: TextRange, b: TextRange) -> TextRange { let start = a.start().min(b.start()); let end = a.end().max(b.end()); diff --git a/crates/ra_ide/src/goto_definition.rs b/crates/ra_ide/src/goto_definition.rs index cfe62037fc..2c634990da 100644 --- a/crates/ra_ide/src/goto_definition.rs +++ b/crates/ra_ide/src/goto_definition.rs @@ -209,7 +209,7 @@ fn named_target(db: &RootDatabase, node: InFile<&SyntaxNode>) -> Option (fn $name() {}) } - define_fn!( - foo - ) + define_fn!(foo); fn bar() { <|>foo(); } ", - "foo FN_DEF FileId(1) [80; 83) [80; 83)", + "foo FN_DEF FileId(1) [64; 80) [75; 78)", + "define_fn!(foo);|foo", ); } #[test] fn goto_definition_works_for_macro_defined_fn_no_arg() { - check_goto( + check_goto_with_range_content( " //- /lib.rs macro_rules! define_fn { @@ -373,7 +390,8 @@ mod tests { <|>foo(); } ", - "foo FN_DEF FileId(1) [39; 42) [39; 42)", + "foo FN_DEF FileId(1) [51; 64) [51; 64)", + "define_fn!();|define_fn!();", ); } From b53587c7bdd67c63bd33a745fdaeb22a847b6c2f Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Sun, 15 Dec 2019 01:46:39 +0800 Subject: [PATCH 2/3] Re-export Origin to replace ExpansionOrigin --- crates/ra_hir/src/lib.rs | 3 +-- crates/ra_hir_expand/src/lib.rs | 20 +++++++------------- crates/ra_ide/src/expand.rs | 4 ++-- crates/ra_mbe/src/lib.rs | 1 + 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 2bf729b6d6..8b9562722d 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -58,7 +58,6 @@ pub use hir_def::{ type_ref::Mutability, }; pub use hir_expand::{ - name::Name, ExpansionOrigin, HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, - MacroFile, + name::Name, HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, Origin, }; pub use hir_ty::{display::HirDisplay, CallableDef}; diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index d1a43fe6cf..cb4e1950bf 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -214,11 +214,7 @@ pub struct ExpansionInfo { exp_map: Arc, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum ExpansionOrigin { - Call, - Def, -} +pub use mbe::Origin; impl ExpansionInfo { pub fn call_node(&self) -> Option> { @@ -241,17 +237,15 @@ impl ExpansionInfo { pub fn map_token_up( &self, token: InFile<&SyntaxToken>, - ) -> Option<(InFile, ExpansionOrigin)> { + ) -> Option<(InFile, Origin)> { let token_id = self.exp_map.token_by_range(token.value.text_range())?; let (token_id, origin) = self.macro_def.0.map_id_up(token_id); - let (token_map, tt, origin) = match origin { - mbe::Origin::Call => (&self.macro_arg.1, self.arg.clone(), ExpansionOrigin::Call), - mbe::Origin::Def => ( - &self.macro_def.1, - self.def.as_ref().map(|tt| tt.syntax().clone()), - ExpansionOrigin::Def, - ), + let (token_map, tt) = match origin { + mbe::Origin::Call => (&self.macro_arg.1, self.arg.clone()), + mbe::Origin::Def => { + (&self.macro_def.1, self.def.as_ref().map(|tt| tt.syntax().clone())) + } }; let range = token_map.range_by_token(token_id)?; diff --git a/crates/ra_ide/src/expand.rs b/crates/ra_ide/src/expand.rs index 327393dbbe..258478bc13 100644 --- a/crates/ra_ide/src/expand.rs +++ b/crates/ra_ide/src/expand.rs @@ -1,7 +1,7 @@ //! Utilities to work with files, produced by macros. use std::iter::successors; -use hir::{ExpansionOrigin, InFile}; +use hir::{InFile, Origin}; use ra_db::FileId; use ra_syntax::{ast, AstNode, SyntaxNode, SyntaxToken, TextRange}; @@ -45,7 +45,7 @@ pub(crate) fn original_range_by_kind( if first.file_id != last.file_id || first_origin != last_origin - || (kind == OriginalRangeKind::CallToken && first_origin != ExpansionOrigin::Call) + || (kind == OriginalRangeKind::CallToken && first_origin != Origin::Call) { return None; } diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs index 0d2d43bef4..ce2deadf6e 100644 --- a/crates/ra_mbe/src/lib.rs +++ b/crates/ra_mbe/src/lib.rs @@ -104,6 +104,7 @@ impl Shift { } } +#[derive(Debug, Eq, PartialEq)] pub enum Origin { Def, Call, From 3ba4b3c554ee94cf96d62c57f9bb80eaff19beed Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Sun, 15 Dec 2019 02:34:16 +0800 Subject: [PATCH 3/3] Use simpler logic on original_range --- .../ra_ide/src/display/navigation_target.rs | 19 +---- crates/ra_ide/src/expand.rs | 73 ++++++++++--------- 2 files changed, 41 insertions(+), 51 deletions(-) diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs index 6a2bf72731..6a6b49afdf 100644 --- a/crates/ra_ide/src/display/navigation_target.rs +++ b/crates/ra_ide/src/display/navigation_target.rs @@ -7,14 +7,10 @@ use ra_syntax::{ ast::{self, DocCommentsOwner, NameOwner}, match_ast, AstNode, SmolStr, SyntaxKind::{self, BIND_PAT, TYPE_PARAM}, - SyntaxNode, TextRange, + TextRange, }; -use crate::{ - db::RootDatabase, - expand::{original_range_by_kind, OriginalRangeKind}, - FileRange, FileSymbol, -}; +use crate::{db::RootDatabase, expand::original_range, FileSymbol}; use super::short_label::ShortLabel; @@ -420,14 +416,3 @@ pub(crate) fn description_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> } } } - -fn original_range(db: &RootDatabase, node: InFile<&SyntaxNode>) -> FileRange { - if let Some(range) = original_range_by_kind(db, node, OriginalRangeKind::CallToken) { - return range; - } - if let Some(range) = original_range_by_kind(db, node, OriginalRangeKind::WholeCall) { - return range; - } - - FileRange { file_id: node.file_id.original_file(db), range: node.value.text_range() } -} diff --git a/crates/ra_ide/src/expand.rs b/crates/ra_ide/src/expand.rs index 258478bc13..7a22bb0a4e 100644 --- a/crates/ra_ide/src/expand.rs +++ b/crates/ra_ide/src/expand.rs @@ -7,55 +7,60 @@ use ra_syntax::{ast, AstNode, SyntaxNode, SyntaxToken, TextRange}; use crate::{db::RootDatabase, FileRange}; -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum OriginalRangeKind { - /// Return range if any token is matched - #[allow(dead_code)] - Any, - /// Return range if token is inside macro_call - CallToken, - /// Return whole macro call range if matched - WholeCall, +pub(crate) fn original_range(db: &RootDatabase, node: InFile<&SyntaxNode>) -> FileRange { + if let Some((range, Origin::Call)) = original_range_and_origin(db, node) { + return range; + } + + if let Some(expansion) = node.file_id.expansion_info(db) { + if let Some(call_node) = expansion.call_node() { + return FileRange { + file_id: call_node.file_id.original_file(db), + range: call_node.value.text_range(), + }; + } + } + + FileRange { file_id: node.file_id.original_file(db), range: node.value.text_range() } } -pub(crate) fn original_range_by_kind( +fn original_range_and_origin( db: &RootDatabase, node: InFile<&SyntaxNode>, - kind: OriginalRangeKind, -) -> Option { +) -> Option<(FileRange, Origin)> { let expansion = node.file_id.expansion_info(db)?; // the input node has only one token ? let single = node.value.first_token()? == node.value.last_token()?; // FIXME: We should handle recurside macro expansions - let range = match kind { - OriginalRangeKind::WholeCall => expansion.call_node()?.map(|node| node.text_range()), - _ => node.value.descendants().find_map(|it| { - let first = it.first_token()?; - let last = it.last_token()?; + let (range, origin) = node.value.descendants().find_map(|it| { + let first = it.first_token()?; + let last = it.last_token()?; - if !single && first == last { - return None; - } + if !single && first == last { + return None; + } - // Try to map first and last tokens of node, and, if success, return the union range of mapped tokens - let (first, first_origin) = expansion.map_token_up(node.with_value(&first))?; - let (last, last_origin) = expansion.map_token_up(node.with_value(&last))?; + // Try to map first and last tokens of node, and, if success, return the union range of mapped tokens + let (first, first_origin) = expansion.map_token_up(node.with_value(&first))?; + let (last, last_origin) = expansion.map_token_up(node.with_value(&last))?; - if first.file_id != last.file_id - || first_origin != last_origin - || (kind == OriginalRangeKind::CallToken && first_origin != Origin::Call) - { - return None; - } + if first.file_id != last.file_id || first_origin != last_origin { + return None; + } - // FIXME: Add union method in TextRange - Some(first.with_value(union_range(first.value.text_range(), last.value.text_range()))) - })?, - }; + // FIXME: Add union method in TextRange + Some(( + first.with_value(union_range(first.value.text_range(), last.value.text_range())), + first_origin, + )) + })?; - return Some(FileRange { file_id: range.file_id.original_file(db), range: range.value }); + return Some(( + FileRange { file_id: range.file_id.original_file(db), range: range.value }, + origin, + )); fn union_range(a: TextRange, b: TextRange) -> TextRange { let start = a.start().min(b.start());