From 4943ef085d55a722c7f70b34de6a4c8a57dcd7d9 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 8 Dec 2020 19:01:27 +0100 Subject: Make `original_range` a method on `InFile<&SyntaxNode>` --- crates/hir/src/lib.rs | 2 +- crates/hir/src/semantics.rs | 76 ++------------------------ crates/hir_expand/src/lib.rs | 72 +++++++++++++++++++++++- crates/ide/src/display/navigation_target.rs | 23 ++++---- crates/rust-analyzer/src/cli/analysis_stats.rs | 4 +- 5 files changed, 88 insertions(+), 89 deletions(-) (limited to 'crates') diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c7c7377d7..302a52491 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -39,7 +39,7 @@ pub use crate::{ Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, }, has_source::HasSource, - semantics::{original_range, PathResolution, Semantics, SemanticsScope}, + semantics::{PathResolution, Semantics, SemanticsScope}, }; pub use hir_def::{ diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index c61a430e1..4315ad48b 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -13,10 +13,7 @@ use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo}; use hir_ty::associated_type_shorthand_candidates; use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; -use syntax::{ - algo::{find_node_at_offset, skip_trivia_token}, - ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextSize, -}; +use syntax::{algo::find_node_at_offset, ast, AstNode, SyntaxNode, SyntaxToken, TextSize}; use crate::{ code_model::Access, @@ -25,7 +22,7 @@ use crate::{ semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, source_analyzer::{resolve_hir_path, SourceAnalyzer}, AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, - Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef, + Module, ModuleDef, Name, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -372,7 +369,7 @@ impl<'db> SemanticsImpl<'db> { fn original_range(&self, node: &SyntaxNode) -> FileRange { let node = self.find_file(node.clone()); - original_range(self.db, node.as_ref()) + node.as_ref().original_file_range(self.db.upcast()) } fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { @@ -380,7 +377,7 @@ impl<'db> SemanticsImpl<'db> { let root = self.db.parse_or_expand(src.file_id).unwrap(); let node = src.value.to_node(&root); self.cache(root, src.file_id); - original_range(self.db, src.with_value(&node)) + src.with_value(&node).original_file_range(self.db.upcast()) } fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator + '_ { @@ -771,68 +768,3 @@ impl<'a> SemanticsScope<'a> { resolve_hir_path(self.db, &self.resolver, &path) } } - -// FIXME: Change `HasSource` trait to work with `Semantics` and remove this? -pub fn original_range(db: &dyn HirDatabase, node: InFile<&SyntaxNode>) -> FileRange { - if let Some(range) = original_range_opt(db, node) { - let original_file = range.file_id.original_file(db.upcast()); - if range.file_id == original_file.into() { - return FileRange { file_id: original_file, range: range.value }; - } - - log::error!("Fail to mapping up more for {:?}", range); - return FileRange { file_id: range.file_id.original_file(db.upcast()), range: range.value }; - } - - // Fall back to whole macro call - if let Some(expansion) = node.file_id.expansion_info(db.upcast()) { - if let Some(call_node) = expansion.call_node() { - return FileRange { - file_id: call_node.file_id.original_file(db.upcast()), - range: call_node.value.text_range(), - }; - } - } - - FileRange { file_id: node.file_id.original_file(db.upcast()), range: node.value.text_range() } -} - -fn original_range_opt( - db: &dyn HirDatabase, - node: InFile<&SyntaxNode>, -) -> Option> { - let expansion = node.file_id.expansion_info(db.upcast())?; - - // the input node has only one token ? - let single = skip_trivia_token(node.value.first_token()?, Direction::Next)? - == skip_trivia_token(node.value.last_token()?, Direction::Prev)?; - - Some(node.value.descendants().find_map(|it| { - let first = skip_trivia_token(it.first_token()?, Direction::Next)?; - let first = ascend_call_token(db, &expansion, node.with_value(first))?; - - let last = skip_trivia_token(it.last_token()?, Direction::Prev)?; - let last = ascend_call_token(db, &expansion, node.with_value(last))?; - - if (!single && first == last) || (first.file_id != last.file_id) { - return None; - } - - Some(first.with_value(first.value.text_range().cover(last.value.text_range()))) - })?) -} - -fn ascend_call_token( - db: &dyn HirDatabase, - expansion: &ExpansionInfo, - token: InFile, -) -> Option> { - let (mapped, origin) = expansion.map_token_up(token.as_ref())?; - if origin != Origin::Call { - return None; - } - if let Some(info) = mapped.file_id.expansion_info(db.upcast()) { - return ascend_call_token(db, &info, mapped); - } - Some(mapped) -} diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 2633fd8f7..3edcadf0d 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -20,11 +20,11 @@ pub use mbe::{ExpandError, ExpandResult}; use std::hash::Hash; use std::sync::Arc; -use base_db::{impl_intern_key, salsa, CrateId, FileId}; +use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange}; use syntax::{ - algo, + algo::{self, skip_trivia_token}, ast::{self, AstNode}, - SyntaxNode, SyntaxToken, TextSize, + Direction, SyntaxNode, SyntaxToken, TextRange, TextSize, }; use crate::ast_id_map::FileAstId; @@ -445,6 +445,72 @@ impl InFile { } } +impl<'a> InFile<&'a SyntaxNode> { + pub fn original_file_range(self, db: &dyn db::AstDatabase) -> FileRange { + if let Some(range) = original_range_opt(db, self) { + let original_file = range.file_id.original_file(db); + if range.file_id == original_file.into() { + return FileRange { file_id: original_file, range: range.value }; + } + + log::error!("Fail to mapping up more for {:?}", range); + return FileRange { file_id: range.file_id.original_file(db), range: range.value }; + } + + // Fall back to whole macro call + if let Some(expansion) = self.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: self.file_id.original_file(db), range: self.value.text_range() } + } +} + +fn original_range_opt( + db: &dyn db::AstDatabase, + node: InFile<&SyntaxNode>, +) -> Option> { + let expansion = node.file_id.expansion_info(db)?; + + // the input node has only one token ? + let single = skip_trivia_token(node.value.first_token()?, Direction::Next)? + == skip_trivia_token(node.value.last_token()?, Direction::Prev)?; + + Some(node.value.descendants().find_map(|it| { + let first = skip_trivia_token(it.first_token()?, Direction::Next)?; + let first = ascend_call_token(db, &expansion, node.with_value(first))?; + + let last = skip_trivia_token(it.last_token()?, Direction::Prev)?; + let last = ascend_call_token(db, &expansion, node.with_value(last))?; + + if (!single && first == last) || (first.file_id != last.file_id) { + return None; + } + + Some(first.with_value(first.value.text_range().cover(last.value.text_range()))) + })?) +} + +fn ascend_call_token( + db: &dyn db::AstDatabase, + expansion: &ExpansionInfo, + token: InFile, +) -> Option> { + let (mapped, origin) = expansion.map_token_up(token.as_ref())?; + if origin != Origin::Call { + return None; + } + if let Some(info) = mapped.file_id.expansion_info(db) { + return ascend_call_token(db, &info, mapped); + } + Some(mapped) +} + impl InFile { pub fn ancestors_with_macros( self, diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index 0c429a262..4790d648a 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here use either::Either; -use hir::{original_range, AssocItem, FieldSource, HasSource, InFile, ModuleSource}; +use hir::{AssocItem, FieldSource, HasSource, InFile, ModuleSource}; use ide_db::base_db::{FileId, SourceDatabase}; use ide_db::{defs::Definition, RootDatabase}; use syntax::{ @@ -62,7 +62,8 @@ impl NavigationTarget { pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget { let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default(); if let Some(src) = module.declaration_source(db) { - let frange = original_range(db, src.as_ref().map(|it| it.syntax())); + let node = src.as_ref().map(|it| it.syntax()); + let frange = node.original_file_range(db); let mut res = NavigationTarget::from_syntax( frange.file_id, name, @@ -104,8 +105,8 @@ impl NavigationTarget { let name = node.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_")); let focus_range = - node.value.name().map(|it| original_range(db, node.with_value(it.syntax())).range); - let frange = original_range(db, node.map(|it| it.syntax())); + node.value.name().map(|it| node.with_value(it.syntax()).original_file_range(db).range); + let frange = node.map(|it| it.syntax()).original_file_range(db); NavigationTarget::from_syntax( frange.file_id, @@ -124,7 +125,7 @@ impl NavigationTarget { ) -> NavigationTarget { let name = named.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_")); - let frange = original_range(db, node.map(|it| it.syntax())); + let frange = node.map(|it| it.syntax()).original_file_range(db); NavigationTarget::from_syntax( frange.file_id, @@ -236,7 +237,7 @@ impl ToNav for hir::Module { (node.syntax(), node.name().map(|it| it.syntax().text_range())) } }; - let frange = original_range(db, src.with_value(syntax)); + let frange = src.with_value(syntax).original_file_range(db); NavigationTarget::from_syntax(frange.file_id, name, focus, frange.range, syntax.kind()) } } @@ -246,14 +247,14 @@ impl ToNav for hir::ImplDef { let src = self.source(db); let derive_attr = self.is_builtin_derive(db); let frange = if let Some(item) = &derive_attr { - original_range(db, item.syntax()) + item.syntax().original_file_range(db) } else { - original_range(db, src.as_ref().map(|it| it.syntax())) + src.as_ref().map(|it| it.syntax()).original_file_range(db) }; let focus_range = if derive_attr.is_some() { None } else { - src.value.self_ty().map(|ty| original_range(db, src.with_value(ty.syntax())).range) + src.value.self_ty().map(|ty| src.with_value(ty.syntax()).original_file_range(db).range) }; NavigationTarget::from_syntax( @@ -278,7 +279,7 @@ impl ToNav for hir::Field { res } FieldSource::Pos(it) => { - let frange = original_range(db, src.with_value(it.syntax())); + let frange = src.with_value(it.syntax()).original_file_range(db); NavigationTarget::from_syntax( frange.file_id, "".into(), @@ -331,7 +332,7 @@ impl ToNav for hir::Local { } Either::Right(it) => it.syntax().clone(), }; - let full_range = original_range(db, src.with_value(&node)); + let full_range = src.with_value(&node).original_file_range(db); let name = match self.name(db) { Some(it) => it.to_string().into(), None => "".into(), diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index 98ef0cd68..58d284d47 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -8,7 +8,7 @@ use std::{ use hir::{ db::{AstDatabase, DefDatabase, HirDatabase}, - original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef, + AssocItem, Crate, HasSource, HirDisplay, ModuleDef, }; use hir_def::FunctionId; use hir_ty::{Ty, TypeWalk}; @@ -232,7 +232,7 @@ impl AnalysisStatsCmd { // But also, we should just turn the type mismatches into diagnostics and provide these let root = db.parse_or_expand(src.file_id).unwrap(); let node = src.map(|e| e.to_node(&root).syntax().clone()); - let original_range = original_range(db, node.as_ref()); + let original_range = node.as_ref().original_file_range(db); let path = vfs.file_path(original_range.file_id); let line_index = host.analysis().file_line_index(original_range.file_id).unwrap(); -- cgit v1.2.3 From 306c6cbaac692bd5fec66173cb7fd9f2d7de9e98 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 8 Dec 2020 19:03:24 +0100 Subject: Use `original_file_range` in `TestDB` --- crates/hir_def/src/test_db.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index 59c788b18..f8b150850 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -158,17 +158,11 @@ impl TestDB { let src = d.display_source(); let root = db.parse_or_expand(src.file_id).unwrap(); - // Place all diagnostics emitted in macro files on the original caller. - // Note that this does *not* match IDE behavior. - let mut src = src.map(|ptr| ptr.to_node(&root)); - while let Some(exp) = src.file_id.call_node(db) { - src = exp; - } + let node = src.map(|ptr| ptr.to_node(&root)); + let frange = node.as_ref().original_file_range(db); - let file_id = src.file_id.original_file(db); - let range = src.value.text_range(); let message = d.message().to_owned(); - actual.entry(file_id).or_default().push((range, message)); + actual.entry(frange.file_id).or_default().push((frange.range, message)); }); for (file_id, diags) in actual.iter_mut() { -- cgit v1.2.3 From da5027138d93c59aeb25aab5021276969f074992 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 8 Dec 2020 19:11:12 +0100 Subject: Fix logic for determining macro calls I believe this currently goes back all the way to the initial user-written call, but that seems better than the current broken behavior. --- crates/hir_expand/src/lib.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'crates') diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 3edcadf0d..1a9428514 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -457,17 +457,15 @@ impl<'a> InFile<&'a SyntaxNode> { return FileRange { file_id: range.file_id.original_file(db), range: range.value }; } - // Fall back to whole macro call - if let Some(expansion) = self.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(), - }; - } + // Fall back to whole macro call. + let mut node = self.cloned(); + while let Some(call_node) = node.file_id.call_node(db) { + node = call_node; } - FileRange { file_id: self.file_id.original_file(db), range: self.value.text_range() } + let orig_file = node.file_id.original_file(db); + assert_eq!(node.file_id, orig_file.into()); + FileRange { file_id: orig_file, range: node.value.text_range() } } } -- cgit v1.2.3