From d9f6cee100fde49d3a76b3fac84b9b13cedf1898 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 6 Apr 2022 13:58:40 +0200 Subject: [PATCH] fix: Fix SearchScope using incorrect text ranges for macro-emitted inline modules --- crates/hir_expand/src/lib.rs | 40 ++++++++++--- crates/ide_db/src/search.rs | 108 +++++++++++++++++++---------------- 2 files changed, 89 insertions(+), 59 deletions(-) diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 537ea087f9e..e698726638a 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -175,15 +175,17 @@ impl HirFileId { /// For macro-expansion files, returns the file original source file the /// expansion originated from. pub fn original_file(self, db: &dyn db::AstDatabase) -> FileId { - match self.0 { - HirFileIdRepr::FileId(file_id) => file_id, - HirFileIdRepr::MacroFile(macro_file) => { - let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); - let file_id = match loc.eager { - Some(EagerCallInfo { included_file: Some(file), .. }) => file.into(), - _ => loc.kind.file_id(), - }; - file_id.original_file(db) + let mut file_id = self; + loop { + match file_id.0 { + HirFileIdRepr::FileId(id) => break id, + HirFileIdRepr::MacroFile(MacroFile { macro_call_id }) => { + let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_call_id); + file_id = match loc.eager { + Some(EagerCallInfo { included_file: Some(file), .. }) => file.into(), + _ => loc.kind.file_id(), + }; + } } } } @@ -211,6 +213,24 @@ pub fn call_node(self, db: &dyn db::AstDatabase) -> Option> { } } + /// If this is a macro call, returns the syntax node of the very first macro call this file resides in. + pub fn original_call_node(self, db: &dyn db::AstDatabase) -> Option<(FileId, SyntaxNode)> { + let mut call = match self.0 { + HirFileIdRepr::FileId(_) => return None, + HirFileIdRepr::MacroFile(MacroFile { macro_call_id }) => { + db.lookup_intern_macro_call(macro_call_id).kind.to_node(db) + } + }; + loop { + match call.file_id.0 { + HirFileIdRepr::FileId(file_id) => break Some((file_id, call.value)), + HirFileIdRepr::MacroFile(MacroFile { macro_call_id }) => { + call = db.lookup_intern_macro_call(macro_call_id).kind.to_node(db); + } + } + } + } + /// Return expansion information if it is a macro-expansion file pub fn expansion_info(self, db: &dyn db::AstDatabase) -> Option { match self.0 { @@ -675,9 +695,11 @@ pub fn with_value(&self, value: U) -> InFile { pub fn map U, U>(self, f: F) -> InFile { InFile::new(self.file_id, f(self.value)) } + pub fn as_ref(&self) -> InFile<&T> { self.with_value(&self.value) } + pub fn file_syntax(&self, db: &dyn db::AstDatabase) -> SyntaxNode { db.parse_or_expand(self.file_id).expect("source created from invalid file") } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index fe99c2e2ce0..4a11fb73cd6 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -85,6 +85,7 @@ fn new(entries: FxHashMap>) -> SearchScope { SearchScope { entries } } + /// Build a search scope spanning the entire crate graph of files. fn crate_graph(db: &RootDatabase) -> SearchScope { let mut entries = FxHashMap::default(); @@ -98,6 +99,7 @@ fn crate_graph(db: &RootDatabase) -> SearchScope { SearchScope { entries } } + /// Build a search scope spanning all the reverse dependencies of the given crate. fn reverse_dependencies(db: &RootDatabase, of: hir::Crate) -> SearchScope { let mut entries = FxHashMap::default(); for rev_dep in of.transitive_reverse_dependencies(db) { @@ -109,6 +111,7 @@ fn reverse_dependencies(db: &RootDatabase, of: hir::Crate) -> SearchScope { SearchScope { entries } } + /// Build a search scope spanning the given crate. fn krate(db: &RootDatabase, of: hir::Crate) -> SearchScope { let root_file = of.root_file(db); let source_root_id = db.file_source_root(root_file); @@ -118,55 +121,55 @@ fn krate(db: &RootDatabase, of: hir::Crate) -> SearchScope { } } - fn module(db: &RootDatabase, module: hir::Module) -> SearchScope { + /// Build a search scope spanning the given module and all its submodules. + fn module_and_children(db: &RootDatabase, module: hir::Module) -> SearchScope { let mut entries = FxHashMap::default(); - let mut to_visit = vec![module]; - let mut is_first = true; + let (file_id, range) = { + let InFile { file_id, value } = module.definition_source(db); + if let Some((file_id, call_source)) = file_id.original_call_node(db) { + (file_id, Some(call_source.text_range())) + } else { + ( + file_id.original_file(db), + match value { + ModuleSource::SourceFile(_) => None, + ModuleSource::Module(it) => Some(it.syntax().text_range()), + ModuleSource::BlockExpr(it) => Some(it.syntax().text_range()), + }, + ) + } + }; + entries.insert(file_id, range); + + let mut to_visit: Vec<_> = module.children(db).collect(); while let Some(module) = to_visit.pop() { - let src = module.definition_source(db); - let file_id = src.file_id.original_file(db); - match src.value { - ModuleSource::Module(m) => { - if is_first { - let range = Some(m.syntax().text_range()); - entries.insert(file_id, range); - } else { - // We have already added the enclosing file to the search scope, - // so do nothing. - } - } - ModuleSource::BlockExpr(b) => { - if is_first { - let range = Some(b.syntax().text_range()); - entries.insert(file_id, range); - } else { - // We have already added the enclosing file to the search scope, - // so do nothing. - } - } - ModuleSource::SourceFile(_) => { - entries.insert(file_id, None); - } - }; - is_first = false; + if let InFile { file_id, value: ModuleSource::SourceFile(_) } = + module.definition_source(db) + { + entries.insert(file_id.original_file(db), None); + } to_visit.extend(module.children(db)); } SearchScope { entries } } + /// Build an empty search scope. pub fn empty() -> SearchScope { SearchScope::new(FxHashMap::default()) } + /// Build a empty search scope spanning the given file. pub fn single_file(file: FileId) -> SearchScope { SearchScope::new(std::iter::once((file, None)).collect()) } + /// Build a empty search scope spanning the text range of the given file. pub fn file_range(range: FileRange) -> SearchScope { SearchScope::new(std::iter::once((range.file_id, Some(range.range))).collect()) } + /// Build a empty search scope spanning the given files. pub fn files(files: &[FileId]) -> SearchScope { SearchScope::new(files.iter().map(|f| (*f, None)).collect()) } @@ -177,29 +180,23 @@ pub fn intersection(&self, other: &SearchScope) -> SearchScope { mem::swap(&mut small, &mut large) } + let intersect_ranges = + |r1: Option, r2: Option| -> Option> { + match (r1, r2) { + (None, r) | (r, None) => Some(r), + (Some(r1), Some(r2)) => r1.intersect(r2).map(Some), + } + }; let res = small .iter() - .filter_map(|(file_id, r1)| { - let r2 = large.get(file_id)?; - let r = intersect_ranges(*r1, *r2)?; - Some((*file_id, r)) + .filter_map(|(&file_id, &r1)| { + let &r2 = large.get(&file_id)?; + let r = intersect_ranges(r1, r2)?; + Some((file_id, r)) }) .collect(); - return SearchScope::new(res); - - fn intersect_ranges( - r1: Option, - r2: Option, - ) -> Option> { - match (r1, r2) { - (None, r) | (r, None) => Some(r), - (Some(r1), Some(r2)) => { - let r = r1.intersect(r2)?; - Some(Some(r)) - } - } - } + SearchScope::new(res) } } @@ -282,7 +279,8 @@ fn search_scope(&self, db: &RootDatabase) -> SearchScope { hir::MacroKind::BuiltIn => SearchScope::crate_graph(db), // FIXME: We don't actually see derives in derive attributes as these do not // expand to something that references the derive macro in the output. - // We could get around this by emitting dummy `use DeriveMacroPathHere as _;` items maybe? + // We could get around this by doing pseudo expansions for proc_macro_derive like we + // do for the derive attribute hir::MacroKind::Derive | hir::MacroKind::Attr | hir::MacroKind::ProcMacro => { SearchScope::reverse_dependencies(db, module.krate()) } @@ -294,7 +292,7 @@ fn search_scope(&self, db: &RootDatabase) -> SearchScope { return SearchScope::reverse_dependencies(db, module.krate()); } if let Some(Visibility::Module(module)) = vis { - return SearchScope::module(db, module.into()); + return SearchScope::module_and_children(db, module.into()); } let range = match module_source { @@ -341,10 +339,12 @@ pub fn include_self_refs(mut self) -> FindUsages<'a> { self } + /// Limit the search to a given [`SearchScope`]. pub fn in_scope(self, scope: SearchScope) -> FindUsages<'a> { self.set_scope(Some(scope)) } + /// Limit the search to a given [`SearchScope`]. pub fn set_scope(mut self, scope: Option) -> FindUsages<'a> { assert!(self.scope.is_none()); self.scope = scope; @@ -420,6 +420,7 @@ fn match_indices<'a>( Some(offset) }) } + fn scope_files<'a>( sema: &'a Semantics, scope: &'a SearchScope, @@ -433,6 +434,12 @@ fn scope_files<'a>( }) } + // FIXME: There should be optimization potential here + // Currently we try to descend everything we find which + // means we call `Semantics::descend_into_macros` on + // every textual hit. That function is notoriously + // expensive even for things that do not get down mapped + // into macros. for (text, file_id, search_range) in scope_files(sema, &search_scope) { let tree = Lazy::new(move || sema.parse(file_id).syntax().clone()); @@ -463,7 +470,8 @@ fn scope_files<'a>( // Search for `super` and `crate` resolving to our module match self.def { Definition::Module(module) => { - let scope = search_scope.intersection(&SearchScope::module(self.sema.db, module)); + let scope = search_scope + .intersection(&SearchScope::module_and_children(self.sema.db, module)); let is_crate_root = module.is_crate_root(self.sema.db); -- 2.44.0