From b66f181bc07c5d3b22755dc997e4c1fc2d6b84c2 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 7 Dec 2021 15:02:22 +0100 Subject: [PATCH 1/3] Fix some TryToNav impls not upmapping ranges out of macros --- crates/ide/src/navigation_target.rs | 157 ++++++++++++++++------------ 1 file changed, 89 insertions(+), 68 deletions(-) diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index d97e52200d..68bd476a4a 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -91,18 +91,17 @@ impl NavigationTarget { pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget { let name = module.name(db).map(|it| it.to_smol_str()).unwrap_or_default(); - if let Some(src) = module.declaration_source(db) { - let node = src.syntax(); - let full_range = node.original_file_range(db); - let focus_range = src - .value + if let Some(src @ InFile { value, .. }) = &module.declaration_source(db) { + let FileRange { file_id, range: full_range } = src.syntax().original_file_range(db); + let focus_range = value .name() - .map(|name| src.with_value(name.syntax()).original_file_range(db).range); + .and_then(|name| src.with_value(name.syntax()).original_file_range_opt(db)) + .map(|it| it.range); let mut res = NavigationTarget::from_syntax( - full_range.file_id, + file_id, name, focus_range, - full_range.range, + full_range, SymbolKind::Module, ); res.docs = module.attrs(db).docs(); @@ -142,9 +141,9 @@ impl NavigationTarget { .name() .and_then(|it| node.with_value(it.syntax()).original_file_range_opt(db)) .map(|it| it.range); - let frange = node.map(|it| it.syntax()).original_file_range(db); + let FileRange { file_id, range } = node.map(|it| it.syntax()).original_file_range(db); - NavigationTarget::from_syntax(frange.file_id, name, focus_range, frange.range, kind) + NavigationTarget::from_syntax(file_id, name, focus_range, range, kind) } fn from_syntax( @@ -287,39 +286,49 @@ where impl ToNav for hir::Module { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.definition_source(db); + let InFile { file_id, value } = self.definition_source(db); + let name = self.name(db).map(|it| it.to_smol_str()).unwrap_or_default(); - let (syntax, focus) = match &src.value { + let (syntax, focus) = match &value { ModuleSource::SourceFile(node) => (node.syntax(), None), - ModuleSource::Module(node) => { - (node.syntax(), node.name().map(|it| it.syntax().text_range())) - } + ModuleSource::Module(node) => ( + node.syntax(), + node.name() + .and_then(|it| InFile::new(file_id, it.syntax()).original_file_range_opt(db)) + .map(|it| it.range), + ), ModuleSource::BlockExpr(node) => (node.syntax(), None), }; - let frange = src.with_value(syntax).original_file_range(db); - NavigationTarget::from_syntax(frange.file_id, name, focus, frange.range, SymbolKind::Module) + let FileRange { file_id, range: full_range } = + InFile::new(file_id, syntax).original_file_range(db); + NavigationTarget::from_syntax(file_id, name, focus, full_range, SymbolKind::Module) } } impl TryToNav for hir::Impl { fn try_to_nav(&self, db: &RootDatabase) -> Option { - let src = self.source(db)?; + let InFile { file_id, value } = self.source(db)?; let derive_attr = self.is_builtin_derive(db); - let frange = match &derive_attr { - Some(item) => item.syntax().original_file_range(db), - None => src.syntax().original_file_range(db), - }; + + let range = |syntax: &_| InFile::new(file_id, syntax).original_file_range(db); let focus_range = if derive_attr.is_some() { None } else { - src.value.self_ty().map(|ty| src.with_value(ty.syntax()).original_file_range(db).range) + value + .self_ty() + .and_then(|ty| InFile::new(file_id, ty.syntax()).original_file_range_opt(db)) + .map(|it| it.range) + }; + let FileRange { file_id, range: full_range } = match &derive_attr { + Some(InFile { value, .. }) => range(value.syntax()), + None => range(value.syntax()), }; Some(NavigationTarget::from_syntax( - frange.file_id, + file_id, "impl".into(), focus_range, - frange.range, + full_range, SymbolKind::Impl, )) } @@ -338,14 +347,9 @@ impl TryToNav for hir::Field { res } FieldSource::Pos(it) => { - let frange = src.with_value(it.syntax()).original_file_range(db); - NavigationTarget::from_syntax( - frange.file_id, - "".into(), - None, - frange.range, - SymbolKind::Field, - ) + let FileRange { file_id, range } = + src.with_value(it.syntax()).original_file_range(db); + NavigationTarget::from_syntax(file_id, "".into(), None, range, SymbolKind::Field) } }; Some(field_source) @@ -408,15 +412,17 @@ impl TryToNav for hir::GenericParam { impl ToNav for hir::Local { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); - let (node, name) = match &src.value { - Either::Left(bind_pat) => (bind_pat.syntax().clone(), bind_pat.name()), - Either::Right(it) => (it.syntax().clone(), it.name()), + let InFile { file_id, value } = self.source(db); + let (node, name) = match &value { + Either::Left(bind_pat) => (bind_pat.syntax(), bind_pat.name()), + Either::Right(it) => (it.syntax(), it.name()), }; - let focus_range = - name.map(|it| src.with_value(&it.syntax().clone()).original_file_range(db).range); + let focus_range = name + .and_then(|it| InFile::new(file_id, it.syntax()).original_file_range_opt(db)) + .map(|it| it.range); + let FileRange { file_id, range: full_range } = + InFile::new(file_id, node).original_file_range(db); - let full_range = src.with_value(&node).original_file_range(db); let name = match self.name(db) { Some(it) => it.to_smol_str(), None => "".into(), @@ -429,10 +435,10 @@ impl ToNav for hir::Local { SymbolKind::Local }; NavigationTarget { - file_id: full_range.file_id, + file_id, name, kind: Some(kind), - full_range: full_range.range, + full_range, focus_range, container_name: None, description: None, @@ -443,17 +449,18 @@ impl ToNav for hir::Local { impl ToNav for hir::Label { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); - let node = src.value.syntax(); - let FileRange { file_id, range } = src.with_value(node).original_file_range(db); - let focus_range = - src.value.lifetime().and_then(|lt| lt.lifetime_ident_token()).map(|lt| lt.text_range()); + let InFile { file_id, value } = self.source(db); let name = self.name(db).to_smol_str(); + + let range = |syntax: &_| InFile::new(file_id, syntax).original_file_range(db); + let FileRange { file_id, range: full_range } = range(value.syntax()); + let focus_range = value.lifetime().map(|lt| range(lt.syntax()).range); + NavigationTarget { file_id, name, kind: Some(SymbolKind::Label), - full_range: range, + full_range, focus_range, container_name: None, description: None, @@ -464,21 +471,25 @@ impl ToNav for hir::Label { impl TryToNav for hir::TypeParam { fn try_to_nav(&self, db: &RootDatabase) -> Option { - let src = self.source(db)?; - let full_range = match &src.value { - Either::Left(type_param) => type_param.syntax().text_range(), + let InFile { file_id, value } = self.source(db)?; + let name = self.name(db).to_smol_str(); + + let range = |syntax: &_| InFile::new(file_id, syntax).original_file_range(db); + let focus_range = |syntax: &_| InFile::new(file_id, syntax).original_file_range_opt(db); + let FileRange { file_id, range: full_range } = match &value { + Either::Left(type_param) => range(type_param.syntax()), Either::Right(trait_) => trait_ .name() - .map_or_else(|| trait_.syntax().text_range(), |name| name.syntax().text_range()), + .and_then(|name| focus_range(name.syntax())) + .unwrap_or_else(|| range(trait_.syntax())), }; - let focus_range = match &src.value { - Either::Left(it) => it.name(), - Either::Right(it) => it.name(), - } - .map(|it| it.syntax().text_range()); + let focus_range = value + .either(|it| it.name(), |it| it.name()) + .and_then(|it| focus_range(it.syntax())) + .map(|it| it.range); Some(NavigationTarget { - file_id: src.file_id.original_file(db), - name: self.name(db).to_smol_str(), + file_id, + name, kind: Some(SymbolKind::TypeParam), full_range, focus_range, @@ -491,11 +502,14 @@ impl TryToNav for hir::TypeParam { impl TryToNav for hir::LifetimeParam { fn try_to_nav(&self, db: &RootDatabase) -> Option { - let src = self.source(db)?; - let full_range = src.value.syntax().text_range(); + let InFile { file_id, value } = self.source(db)?; + let name = self.name(db).to_smol_str(); + + let FileRange { file_id, range: full_range } = + InFile::new(file_id, value.syntax()).original_file_range(db); Some(NavigationTarget { - file_id: src.file_id.original_file(db), - name: self.name(db).to_smol_str(), + file_id, + name, kind: Some(SymbolKind::LifetimeParam), full_range, focus_range: Some(full_range), @@ -508,14 +522,21 @@ impl TryToNav for hir::LifetimeParam { impl TryToNav for hir::ConstParam { fn try_to_nav(&self, db: &RootDatabase) -> Option { - let src = self.source(db)?; - let full_range = src.value.syntax().text_range(); + let InFile { file_id, value } = self.source(db)?; + let name = self.name(db).to_smol_str(); + + let focus_range = value + .name() + .and_then(|it| InFile::new(file_id, it.syntax()).original_file_range_opt(db)) + .map(|it| it.range); + let FileRange { file_id, range: full_range } = + InFile::new(file_id, value.syntax()).original_file_range(db); Some(NavigationTarget { - file_id: src.file_id.original_file(db), - name: self.name(db).to_smol_str(), + file_id, + name, kind: Some(SymbolKind::ConstParam), full_range, - focus_range: src.value.name().map(|n| n.syntax().text_range()), + focus_range, container_name: None, description: None, docs: None, From e09d410dcd927ab93e8c169160f196a34d317946 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 7 Dec 2021 15:06:56 +0100 Subject: [PATCH 2/3] Simplify --- crates/ide/src/navigation_target.rs | 22 ++----------------- .../ide/src/syntax_highlighting/highlight.rs | 8 +------ crates/ide_completion/src/render/macro_.rs | 10 ++------- crates/ide_db/src/lib.rs | 12 ++++++++++ crates/ide_db/src/symbol_index.rs | 19 +++++++++++++++- 5 files changed, 35 insertions(+), 36 deletions(-) diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index 68bd476a4a..50799738d1 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -9,7 +9,6 @@ use hir::{ }; use ide_db::{ base_db::{FileId, FileRange}, - symbol_index::FileSymbolKind, SymbolKind, }; use ide_db::{defs::Definition, RootDatabase}; @@ -174,18 +173,7 @@ impl TryToNav for FileSymbol { Some(NavigationTarget { file_id: full_range.file_id, name: self.name.clone(), - kind: Some(match self.kind { - FileSymbolKind::Function => SymbolKind::Function, - FileSymbolKind::Struct => SymbolKind::Struct, - FileSymbolKind::Enum => SymbolKind::Enum, - FileSymbolKind::Trait => SymbolKind::Trait, - FileSymbolKind::Module => SymbolKind::Module, - FileSymbolKind::TypeAlias => SymbolKind::TypeAlias, - FileSymbolKind::Const => SymbolKind::Const, - FileSymbolKind::Static => SymbolKind::Static, - FileSymbolKind::Macro => SymbolKind::Macro, - FileSymbolKind::Union => SymbolKind::Union, - }), + kind: Some(self.kind.into()), full_range: full_range.range, focus_range: Some(name_range.range), container_name: self.container_name.clone(), @@ -367,13 +355,7 @@ impl TryToNav for hir::MacroDef { let mut res = NavigationTarget::from_named( db, src.as_ref().with_value(name_owner), - match self.kind() { - hir::MacroKind::Declarative - | hir::MacroKind::BuiltIn - | hir::MacroKind::ProcMacro => SymbolKind::Macro, - hir::MacroKind::Derive => SymbolKind::Derive, - hir::MacroKind::Attr => SymbolKind::Attribute, - }, + self.kind().into(), ); res.docs = self.docs(db); Some(res) diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index a52b2247c5..05d2732251 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -375,13 +375,7 @@ fn highlight_def( ) -> Highlight { let db = sema.db; let mut h = match def { - Definition::Macro(m) => Highlight::new(HlTag::Symbol(match m.kind() { - hir::MacroKind::Declarative | hir::MacroKind::BuiltIn | hir::MacroKind::ProcMacro => { - SymbolKind::Macro - } - hir::MacroKind::Derive => SymbolKind::Derive, - hir::MacroKind::Attr => SymbolKind::Attribute, - })), + Definition::Macro(m) => Highlight::new(HlTag::Symbol(m.kind().into())), Definition::Field(_) => Highlight::new(HlTag::Symbol(SymbolKind::Field)), Definition::Module(module) => { let mut h = Highlight::new(HlTag::Symbol(SymbolKind::Module)); diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs index 22fb1f4825..4b505b3f5a 100644 --- a/crates/ide_completion/src/render/macro_.rs +++ b/crates/ide_completion/src/render/macro_.rs @@ -52,14 +52,8 @@ impl<'a> MacroRender<'a> { } else { Some(self.ctx.source_range()) }?; - let kind = match self.macro_.kind() { - hir::MacroKind::Derive => SymbolKind::Derive, - hir::MacroKind::Attr => SymbolKind::Attribute, - hir::MacroKind::BuiltIn | hir::MacroKind::Declarative | hir::MacroKind::ProcMacro => { - SymbolKind::Macro - } - }; - let mut item = CompletionItem::new(kind, source_range, self.label()); + let mut item = + CompletionItem::new(SymbolKind::from(self.macro_.kind()), source_range, self.label()); item.set_deprecated(self.ctx.is_deprecated(self.macro_)).set_detail(self.detail()); if let Some(import_to_add) = import_to_add { diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 0d14c176b5..bcbb09e31b 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -171,6 +171,18 @@ pub enum SymbolKind { Variant, } +impl From for SymbolKind { + fn from(it: hir::MacroKind) -> Self { + match it { + hir::MacroKind::Declarative | hir::MacroKind::BuiltIn | hir::MacroKind::ProcMacro => { + SymbolKind::Macro + } + hir::MacroKind::Derive => SymbolKind::Derive, + hir::MacroKind::Attr => SymbolKind::Attribute, + } + } +} + #[cfg(test)] mod tests { mod sourcegen_lints; diff --git a/crates/ide_db/src/symbol_index.rs b/crates/ide_db/src/symbol_index.rs index cc2b2bbb7b..e8c6ec3e0e 100644 --- a/crates/ide_db/src/symbol_index.rs +++ b/crates/ide_db/src/symbol_index.rs @@ -44,7 +44,7 @@ use rayon::prelude::*; use rustc_hash::FxHashSet; use syntax::{ast::HasName, AstNode, SmolStr, SyntaxNode, SyntaxNodePtr}; -use crate::RootDatabase; +use crate::{RootDatabase, SymbolKind}; #[derive(Debug)] pub struct Query { @@ -430,6 +430,23 @@ impl FileSymbolKind { } } +impl From for SymbolKind { + fn from(it: FileSymbolKind) -> Self { + match it { + FileSymbolKind::Const => SymbolKind::Const, + FileSymbolKind::Enum => SymbolKind::Enum, + FileSymbolKind::Function => SymbolKind::Function, + FileSymbolKind::Macro => SymbolKind::Macro, + FileSymbolKind::Module => SymbolKind::Module, + FileSymbolKind::Static => SymbolKind::Static, + FileSymbolKind::Struct => SymbolKind::Struct, + FileSymbolKind::Trait => SymbolKind::Trait, + FileSymbolKind::TypeAlias => SymbolKind::TypeAlias, + FileSymbolKind::Union => SymbolKind::Union, + } + } +} + /// Represents an outstanding module that the symbol collector must collect symbols from. struct SymbolCollectorWork { module_id: ModuleId, From f781e599cc6376a3706498dcc9ce5e7ad59e16e1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 7 Dec 2021 15:42:54 +0100 Subject: [PATCH 3/3] Adjust incorrect runnable tests --- crates/ide/src/navigation_target.rs | 6 +++--- crates/ide/src/runnables.rs | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index 50799738d1..52e099513a 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -298,7 +298,6 @@ impl TryToNav for hir::Impl { let InFile { file_id, value } = self.source(db)?; let derive_attr = self.is_builtin_derive(db); - let range = |syntax: &_| InFile::new(file_id, syntax).original_file_range(db); let focus_range = if derive_attr.is_some() { None } else { @@ -307,9 +306,10 @@ impl TryToNav for hir::Impl { .and_then(|ty| InFile::new(file_id, ty.syntax()).original_file_range_opt(db)) .map(|it| it.range) }; + let FileRange { file_id, range: full_range } = match &derive_attr { - Some(InFile { value, .. }) => range(value.syntax()), - None => range(value.syntax()), + Some(attr) => attr.syntax().original_file_range(db), + None => InFile::new(file_id, value.syntax()).original_file_range(db), }; Some(NavigationTarget::from_syntax( diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 3078789d12..67062654d6 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -1437,7 +1437,6 @@ gen2!(); 0, ), full_range: 228..236, - focus_range: 228..236, name: "tests2", kind: Module, description: "mod tests2", @@ -1522,7 +1521,6 @@ foo!(); 0, ), full_range: 210..217, - focus_range: 210..217, name: "foo_tests", kind: Module, description: "mod foo_tests",