aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-12-08 18:13:54 +0000
committerGitHub <[email protected]>2020-12-08 18:13:54 +0000
commit87e35006362686b281caf46550c0fff35cddefb4 (patch)
tree67b9f5e41749a89b85b3f4ce49fec62b4a85da82
parentb01981e6364a9de26a52adefac10e7aebfb85874 (diff)
parentda5027138d93c59aeb25aab5021276969f074992 (diff)
Merge #6765
6765: Fix file range computation in macros r=jonas-schievink a=jonas-schievink This also aligns the diagnostics behavior of `TestDB` with the one from the real IDE (by making the logic from `semantics.rs` a method on `InFile<&SyntaxNode>`), which makes bugs like this easier to find. This should fix the misplaced diagnostics seen in https://github.com/rust-analyzer/rust-analyzer/issues/6747 and other issues. bors r+ Co-authored-by: Jonas Schievink <[email protected]>
-rw-r--r--crates/hir/src/lib.rs2
-rw-r--r--crates/hir/src/semantics.rs76
-rw-r--r--crates/hir_def/src/test_db.rs12
-rw-r--r--crates/hir_expand/src/lib.rs70
-rw-r--r--crates/ide/src/display/navigation_target.rs23
-rw-r--r--crates/rust-analyzer/src/cli/analysis_stats.rs4
6 files changed, 89 insertions, 98 deletions
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::{
39 Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, 39 Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef,
40 }, 40 },
41 has_source::HasSource, 41 has_source::HasSource,
42 semantics::{original_range, PathResolution, Semantics, SemanticsScope}, 42 semantics::{PathResolution, Semantics, SemanticsScope},
43}; 43};
44 44
45pub use hir_def::{ 45pub 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};
13use hir_ty::associated_type_shorthand_candidates; 13use hir_ty::associated_type_shorthand_candidates;
14use itertools::Itertools; 14use itertools::Itertools;
15use rustc_hash::{FxHashMap, FxHashSet}; 15use rustc_hash::{FxHashMap, FxHashSet};
16use syntax::{ 16use syntax::{algo::find_node_at_offset, ast, AstNode, SyntaxNode, SyntaxToken, TextSize};
17 algo::{find_node_at_offset, skip_trivia_token},
18 ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextSize,
19};
20 17
21use crate::{ 18use crate::{
22 code_model::Access, 19 code_model::Access,
@@ -25,7 +22,7 @@ use crate::{
25 semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, 22 semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
26 source_analyzer::{resolve_hir_path, SourceAnalyzer}, 23 source_analyzer::{resolve_hir_path, SourceAnalyzer},
27 AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, 24 AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef,
28 Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef, 25 Module, ModuleDef, Name, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef,
29}; 26};
30 27
31#[derive(Debug, Clone, PartialEq, Eq)] 28#[derive(Debug, Clone, PartialEq, Eq)]
@@ -372,7 +369,7 @@ impl<'db> SemanticsImpl<'db> {
372 369
373 fn original_range(&self, node: &SyntaxNode) -> FileRange { 370 fn original_range(&self, node: &SyntaxNode) -> FileRange {
374 let node = self.find_file(node.clone()); 371 let node = self.find_file(node.clone());
375 original_range(self.db, node.as_ref()) 372 node.as_ref().original_file_range(self.db.upcast())
376 } 373 }
377 374
378 fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { 375 fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange {
@@ -380,7 +377,7 @@ impl<'db> SemanticsImpl<'db> {
380 let root = self.db.parse_or_expand(src.file_id).unwrap(); 377 let root = self.db.parse_or_expand(src.file_id).unwrap();
381 let node = src.value.to_node(&root); 378 let node = src.value.to_node(&root);
382 self.cache(root, src.file_id); 379 self.cache(root, src.file_id);
383 original_range(self.db, src.with_value(&node)) 380 src.with_value(&node).original_file_range(self.db.upcast())
384 } 381 }
385 382
386 fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator<Item = SyntaxNode> + '_ { 383 fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator<Item = SyntaxNode> + '_ {
@@ -771,68 +768,3 @@ impl<'a> SemanticsScope<'a> {
771 resolve_hir_path(self.db, &self.resolver, &path) 768 resolve_hir_path(self.db, &self.resolver, &path)
772 } 769 }
773} 770}
774
775// FIXME: Change `HasSource` trait to work with `Semantics` and remove this?
776pub fn original_range(db: &dyn HirDatabase, node: InFile<&SyntaxNode>) -> FileRange {
777 if let Some(range) = original_range_opt(db, node) {
778 let original_file = range.file_id.original_file(db.upcast());
779 if range.file_id == original_file.into() {
780 return FileRange { file_id: original_file, range: range.value };
781 }
782
783 log::error!("Fail to mapping up more for {:?}", range);
784 return FileRange { file_id: range.file_id.original_file(db.upcast()), range: range.value };
785 }
786
787 // Fall back to whole macro call
788 if let Some(expansion) = node.file_id.expansion_info(db.upcast()) {
789 if let Some(call_node) = expansion.call_node() {
790 return FileRange {
791 file_id: call_node.file_id.original_file(db.upcast()),
792 range: call_node.value.text_range(),
793 };
794 }
795 }
796
797 FileRange { file_id: node.file_id.original_file(db.upcast()), range: node.value.text_range() }
798}
799
800fn original_range_opt(
801 db: &dyn HirDatabase,
802 node: InFile<&SyntaxNode>,
803) -> Option<InFile<TextRange>> {
804 let expansion = node.file_id.expansion_info(db.upcast())?;
805
806 // the input node has only one token ?
807 let single = skip_trivia_token(node.value.first_token()?, Direction::Next)?
808 == skip_trivia_token(node.value.last_token()?, Direction::Prev)?;
809
810 Some(node.value.descendants().find_map(|it| {
811 let first = skip_trivia_token(it.first_token()?, Direction::Next)?;
812 let first = ascend_call_token(db, &expansion, node.with_value(first))?;
813
814 let last = skip_trivia_token(it.last_token()?, Direction::Prev)?;
815 let last = ascend_call_token(db, &expansion, node.with_value(last))?;
816
817 if (!single && first == last) || (first.file_id != last.file_id) {
818 return None;
819 }
820
821 Some(first.with_value(first.value.text_range().cover(last.value.text_range())))
822 })?)
823}
824
825fn ascend_call_token(
826 db: &dyn HirDatabase,
827 expansion: &ExpansionInfo,
828 token: InFile<SyntaxToken>,
829) -> Option<InFile<SyntaxToken>> {
830 let (mapped, origin) = expansion.map_token_up(token.as_ref())?;
831 if origin != Origin::Call {
832 return None;
833 }
834 if let Some(info) = mapped.file_id.expansion_info(db.upcast()) {
835 return ascend_call_token(db, &info, mapped);
836 }
837 Some(mapped)
838}
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 {
158 let src = d.display_source(); 158 let src = d.display_source();
159 let root = db.parse_or_expand(src.file_id).unwrap(); 159 let root = db.parse_or_expand(src.file_id).unwrap();
160 160
161 // Place all diagnostics emitted in macro files on the original caller. 161 let node = src.map(|ptr| ptr.to_node(&root));
162 // Note that this does *not* match IDE behavior. 162 let frange = node.as_ref().original_file_range(db);
163 let mut src = src.map(|ptr| ptr.to_node(&root));
164 while let Some(exp) = src.file_id.call_node(db) {
165 src = exp;
166 }
167 163
168 let file_id = src.file_id.original_file(db);
169 let range = src.value.text_range();
170 let message = d.message().to_owned(); 164 let message = d.message().to_owned();
171 actual.entry(file_id).or_default().push((range, message)); 165 actual.entry(frange.file_id).or_default().push((frange.range, message));
172 }); 166 });
173 167
174 for (file_id, diags) in actual.iter_mut() { 168 for (file_id, diags) in actual.iter_mut() {
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index 2633fd8f7..1a9428514 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -20,11 +20,11 @@ pub use mbe::{ExpandError, ExpandResult};
20use std::hash::Hash; 20use std::hash::Hash;
21use std::sync::Arc; 21use std::sync::Arc;
22 22
23use base_db::{impl_intern_key, salsa, CrateId, FileId}; 23use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange};
24use syntax::{ 24use syntax::{
25 algo, 25 algo::{self, skip_trivia_token},
26 ast::{self, AstNode}, 26 ast::{self, AstNode},
27 SyntaxNode, SyntaxToken, TextSize, 27 Direction, SyntaxNode, SyntaxToken, TextRange, TextSize,
28}; 28};
29 29
30use crate::ast_id_map::FileAstId; 30use crate::ast_id_map::FileAstId;
@@ -445,6 +445,70 @@ impl InFile<SyntaxNode> {
445 } 445 }
446} 446}
447 447
448impl<'a> InFile<&'a SyntaxNode> {
449 pub fn original_file_range(self, db: &dyn db::AstDatabase) -> FileRange {
450 if let Some(range) = original_range_opt(db, self) {
451 let original_file = range.file_id.original_file(db);
452 if range.file_id == original_file.into() {
453 return FileRange { file_id: original_file, range: range.value };
454 }
455
456 log::error!("Fail to mapping up more for {:?}", range);
457 return FileRange { file_id: range.file_id.original_file(db), range: range.value };
458 }
459
460 // Fall back to whole macro call.
461 let mut node = self.cloned();
462 while let Some(call_node) = node.file_id.call_node(db) {
463 node = call_node;
464 }
465
466 let orig_file = node.file_id.original_file(db);
467 assert_eq!(node.file_id, orig_file.into());
468 FileRange { file_id: orig_file, range: node.value.text_range() }
469 }
470}
471
472fn original_range_opt(
473 db: &dyn db::AstDatabase,
474 node: InFile<&SyntaxNode>,
475) -> Option<InFile<TextRange>> {
476 let expansion = node.file_id.expansion_info(db)?;
477
478 // the input node has only one token ?
479 let single = skip_trivia_token(node.value.first_token()?, Direction::Next)?
480 == skip_trivia_token(node.value.last_token()?, Direction::Prev)?;
481
482 Some(node.value.descendants().find_map(|it| {
483 let first = skip_trivia_token(it.first_token()?, Direction::Next)?;
484 let first = ascend_call_token(db, &expansion, node.with_value(first))?;
485
486 let last = skip_trivia_token(it.last_token()?, Direction::Prev)?;
487 let last = ascend_call_token(db, &expansion, node.with_value(last))?;
488
489 if (!single && first == last) || (first.file_id != last.file_id) {
490 return None;
491 }
492
493 Some(first.with_value(first.value.text_range().cover(last.value.text_range())))
494 })?)
495}
496
497fn ascend_call_token(
498 db: &dyn db::AstDatabase,
499 expansion: &ExpansionInfo,
500 token: InFile<SyntaxToken>,
501) -> Option<InFile<SyntaxToken>> {
502 let (mapped, origin) = expansion.map_token_up(token.as_ref())?;
503 if origin != Origin::Call {
504 return None;
505 }
506 if let Some(info) = mapped.file_id.expansion_info(db) {
507 return ascend_call_token(db, &info, mapped);
508 }
509 Some(mapped)
510}
511
448impl InFile<SyntaxToken> { 512impl InFile<SyntaxToken> {
449 pub fn ancestors_with_macros( 513 pub fn ancestors_with_macros(
450 self, 514 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 @@
1//! FIXME: write short doc here 1//! FIXME: write short doc here
2 2
3use either::Either; 3use either::Either;
4use hir::{original_range, AssocItem, FieldSource, HasSource, InFile, ModuleSource}; 4use hir::{AssocItem, FieldSource, HasSource, InFile, ModuleSource};
5use ide_db::base_db::{FileId, SourceDatabase}; 5use ide_db::base_db::{FileId, SourceDatabase};
6use ide_db::{defs::Definition, RootDatabase}; 6use ide_db::{defs::Definition, RootDatabase};
7use syntax::{ 7use syntax::{
@@ -62,7 +62,8 @@ impl NavigationTarget {
62 pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget { 62 pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget {
63 let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default(); 63 let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default();
64 if let Some(src) = module.declaration_source(db) { 64 if let Some(src) = module.declaration_source(db) {
65 let frange = original_range(db, src.as_ref().map(|it| it.syntax())); 65 let node = src.as_ref().map(|it| it.syntax());
66 let frange = node.original_file_range(db);
66 let mut res = NavigationTarget::from_syntax( 67 let mut res = NavigationTarget::from_syntax(
67 frange.file_id, 68 frange.file_id,
68 name, 69 name,
@@ -104,8 +105,8 @@ impl NavigationTarget {
104 let name = 105 let name =
105 node.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_")); 106 node.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_"));
106 let focus_range = 107 let focus_range =
107 node.value.name().map(|it| original_range(db, node.with_value(it.syntax())).range); 108 node.value.name().map(|it| node.with_value(it.syntax()).original_file_range(db).range);
108 let frange = original_range(db, node.map(|it| it.syntax())); 109 let frange = node.map(|it| it.syntax()).original_file_range(db);
109 110
110 NavigationTarget::from_syntax( 111 NavigationTarget::from_syntax(
111 frange.file_id, 112 frange.file_id,
@@ -124,7 +125,7 @@ impl NavigationTarget {
124 ) -> NavigationTarget { 125 ) -> NavigationTarget {
125 let name = 126 let name =
126 named.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_")); 127 named.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_"));
127 let frange = original_range(db, node.map(|it| it.syntax())); 128 let frange = node.map(|it| it.syntax()).original_file_range(db);
128 129
129 NavigationTarget::from_syntax( 130 NavigationTarget::from_syntax(
130 frange.file_id, 131 frange.file_id,
@@ -236,7 +237,7 @@ impl ToNav for hir::Module {
236 (node.syntax(), node.name().map(|it| it.syntax().text_range())) 237 (node.syntax(), node.name().map(|it| it.syntax().text_range()))
237 } 238 }
238 }; 239 };
239 let frange = original_range(db, src.with_value(syntax)); 240 let frange = src.with_value(syntax).original_file_range(db);
240 NavigationTarget::from_syntax(frange.file_id, name, focus, frange.range, syntax.kind()) 241 NavigationTarget::from_syntax(frange.file_id, name, focus, frange.range, syntax.kind())
241 } 242 }
242} 243}
@@ -246,14 +247,14 @@ impl ToNav for hir::ImplDef {
246 let src = self.source(db); 247 let src = self.source(db);
247 let derive_attr = self.is_builtin_derive(db); 248 let derive_attr = self.is_builtin_derive(db);
248 let frange = if let Some(item) = &derive_attr { 249 let frange = if let Some(item) = &derive_attr {
249 original_range(db, item.syntax()) 250 item.syntax().original_file_range(db)
250 } else { 251 } else {
251 original_range(db, src.as_ref().map(|it| it.syntax())) 252 src.as_ref().map(|it| it.syntax()).original_file_range(db)
252 }; 253 };
253 let focus_range = if derive_attr.is_some() { 254 let focus_range = if derive_attr.is_some() {
254 None 255 None
255 } else { 256 } else {
256 src.value.self_ty().map(|ty| original_range(db, src.with_value(ty.syntax())).range) 257 src.value.self_ty().map(|ty| src.with_value(ty.syntax()).original_file_range(db).range)
257 }; 258 };
258 259
259 NavigationTarget::from_syntax( 260 NavigationTarget::from_syntax(
@@ -278,7 +279,7 @@ impl ToNav for hir::Field {
278 res 279 res
279 } 280 }
280 FieldSource::Pos(it) => { 281 FieldSource::Pos(it) => {
281 let frange = original_range(db, src.with_value(it.syntax())); 282 let frange = src.with_value(it.syntax()).original_file_range(db);
282 NavigationTarget::from_syntax( 283 NavigationTarget::from_syntax(
283 frange.file_id, 284 frange.file_id,
284 "".into(), 285 "".into(),
@@ -331,7 +332,7 @@ impl ToNav for hir::Local {
331 } 332 }
332 Either::Right(it) => it.syntax().clone(), 333 Either::Right(it) => it.syntax().clone(),
333 }; 334 };
334 let full_range = original_range(db, src.with_value(&node)); 335 let full_range = src.with_value(&node).original_file_range(db);
335 let name = match self.name(db) { 336 let name = match self.name(db) {
336 Some(it) => it.to_string().into(), 337 Some(it) => it.to_string().into(),
337 None => "".into(), 338 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::{
8 8
9use hir::{ 9use hir::{
10 db::{AstDatabase, DefDatabase, HirDatabase}, 10 db::{AstDatabase, DefDatabase, HirDatabase},
11 original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef, 11 AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
12}; 12};
13use hir_def::FunctionId; 13use hir_def::FunctionId;
14use hir_ty::{Ty, TypeWalk}; 14use hir_ty::{Ty, TypeWalk};
@@ -232,7 +232,7 @@ impl AnalysisStatsCmd {
232 // But also, we should just turn the type mismatches into diagnostics and provide these 232 // But also, we should just turn the type mismatches into diagnostics and provide these
233 let root = db.parse_or_expand(src.file_id).unwrap(); 233 let root = db.parse_or_expand(src.file_id).unwrap();
234 let node = src.map(|e| e.to_node(&root).syntax().clone()); 234 let node = src.map(|e| e.to_node(&root).syntax().clone());
235 let original_range = original_range(db, node.as_ref()); 235 let original_range = node.as_ref().original_file_range(db);
236 let path = vfs.file_path(original_range.file_id); 236 let path = vfs.file_path(original_range.file_id);
237 let line_index = 237 let line_index =
238 host.analysis().file_line_index(original_range.file_id).unwrap(); 238 host.analysis().file_line_index(original_range.file_id).unwrap();