From dd6307ddc4535bef09e2b14caff5acfaeb88891e Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Tue, 12 Feb 2019 21:47:51 +0200 Subject: Add support for container_name in workspace/symbol query --- crates/ra_ide_api/src/navigation_target.rs | 11 +++++ crates/ra_ide_api/src/symbol_index.rs | 62 ++++++++++++++++++++------ crates/ra_ide_api/tests/test/main.rs | 58 +++++++++++++++++++++++- crates/ra_lsp_server/src/main_loop/handlers.rs | 2 +- 4 files changed, 116 insertions(+), 17 deletions(-) diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs index a2e4b6506..bcacbe6fc 100644 --- a/crates/ra_ide_api/src/navigation_target.rs +++ b/crates/ra_ide_api/src/navigation_target.rs @@ -19,6 +19,7 @@ pub struct NavigationTarget { kind: SyntaxKind, full_range: TextRange, focus_range: Option, + container_name: Option, } impl NavigationTarget { @@ -26,6 +27,10 @@ impl NavigationTarget { &self.name } + pub fn container_name(&self) -> Option<&SmolStr> { + self.container_name.as_ref() + } + pub fn kind(&self) -> SyntaxKind { self.kind } @@ -53,6 +58,7 @@ impl NavigationTarget { kind: symbol.ptr.kind(), full_range: symbol.ptr.range(), focus_range: None, + container_name: symbol.container_name.map(|v| v.clone()), } } @@ -67,6 +73,7 @@ impl NavigationTarget { full_range: ptr.range(), focus_range: None, kind: NAME, + container_name: None, } } @@ -170,6 +177,9 @@ impl NavigationTarget { if let Some(focus_range) = self.focus_range() { buf.push_str(&format!(" {:?}", focus_range)) } + if let Some(container_name) = self.container_name() { + buf.push_str(&format!(" {:?}", container_name)) + } buf } @@ -192,6 +202,7 @@ impl NavigationTarget { full_range: node.range(), focus_range, // ptr: Some(LocalSyntaxPtr::new(node)), + container_name: None, } } } diff --git a/crates/ra_ide_api/src/symbol_index.rs b/crates/ra_ide_api/src/symbol_index.rs index 15348124b..62d0979fe 100644 --- a/crates/ra_ide_api/src/symbol_index.rs +++ b/crates/ra_ide_api/src/symbol_index.rs @@ -32,6 +32,7 @@ use ra_syntax::{ algo::{visit::{visitor, Visitor}, find_covering_node}, SyntaxKind::{self, *}, ast::{self, NameOwner}, + WalkEvent, }; use ra_db::{ SourceRootId, SourceDatabase, @@ -62,17 +63,14 @@ pub(crate) trait SymbolsDatabase: hir::db::HirDatabase { fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc { db.check_canceled(); let source_file = db.parse(file_id); - let mut symbols = source_file - .syntax() - .descendants() - .filter_map(to_symbol) - .map(move |(name, ptr)| FileSymbol { name, ptr, file_id }) - .collect::>(); + + let mut symbols = source_file_to_file_symbols(&source_file, file_id); for (name, text_range) in hir::source_binder::macro_symbols(db, file_id) { let node = find_covering_node(source_file.syntax(), text_range); let ptr = SyntaxNodePtr::new(node); - symbols.push(FileSymbol { file_id, name, ptr }) + // TODO: Should we get container name for macro symbols? + symbols.push(FileSymbol { file_id, name, ptr, container_name: None }) } Arc::new(SymbolIndex::new(symbols)) @@ -158,13 +156,7 @@ impl SymbolIndex { files: impl ParallelIterator)>, ) -> SymbolIndex { let symbols = files - .flat_map(|(file_id, file)| { - file.syntax() - .descendants() - .filter_map(to_symbol) - .map(move |(name, ptr)| FileSymbol { name, ptr, file_id }) - .collect::>() - }) + .flat_map(|(file_id, file)| source_file_to_file_symbols(&file, file_id)) .collect::>(); SymbolIndex::new(symbols) } @@ -208,6 +200,16 @@ fn is_type(kind: SyntaxKind) -> bool { } } +fn is_symbol_def(kind: SyntaxKind) -> bool { + match kind { + FN_DEF | STRUCT_DEF | ENUM_DEF | TRAIT_DEF | MODULE | TYPE_DEF | CONST_DEF | STATIC_DEF => { + true + } + + _ => false, + } +} + /// The actual data that is stored in the index. It should be as compact as /// possible. #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -215,12 +217,40 @@ pub(crate) struct FileSymbol { pub(crate) file_id: FileId, pub(crate) name: SmolStr, pub(crate) ptr: SyntaxNodePtr, + pub(crate) container_name: Option, +} + +fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec { + let mut symbols = Vec::new(); + let mut stack = Vec::new(); + + for event in source_file.syntax().preorder() { + match event { + WalkEvent::Enter(node) => { + if let Some(mut symbol) = to_file_symbol(node, file_id) { + symbol.container_name = stack.last().map(|v: &SmolStr| v.clone()); + + stack.push(symbol.name.clone()); + symbols.push(symbol); + } + } + + WalkEvent::Leave(node) => { + if is_symbol_def(node.kind()) { + stack.pop(); + } + } + } + } + + symbols } fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> { fn decl(node: &N) -> Option<(SmolStr, SyntaxNodePtr)> { let name = node.name()?.text().clone(); let ptr = SyntaxNodePtr::new(node.syntax()); + Some((name, ptr)) } visitor() @@ -234,3 +264,7 @@ fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> { .visit(decl::) .accept(node)? } + +fn to_file_symbol(node: &SyntaxNode, file_id: FileId) -> Option { + to_symbol(node).map(move |(name, ptr)| FileSymbol { name, ptr, file_id, container_name: None }) +} diff --git a/crates/ra_ide_api/tests/test/main.rs b/crates/ra_ide_api/tests/test/main.rs index 7d1695cfd..4126bd160 100644 --- a/crates/ra_ide_api/tests/test/main.rs +++ b/crates/ra_ide_api/tests/test/main.rs @@ -1,9 +1,9 @@ use insta::assert_debug_snapshot_matches; use ra_ide_api::{ mock_analysis::{single_file, single_file_with_position, MockAnalysis}, - AnalysisChange, CrateGraph, FileId, Query, + AnalysisChange, CrateGraph, FileId, Query, NavigationTarget, }; -use ra_syntax::TextRange; +use ra_syntax::{TextRange, SmolStr}; #[test] fn test_unresolved_module_diagnostic() { @@ -49,6 +49,11 @@ fn get_all_refs(text: &str) -> Vec<(FileId, TextRange)> { analysis.find_all_refs(position).unwrap() } +fn get_symbols_matching(text: &str, query: &str) -> Vec { + let (analysis, _) = single_file(text); + analysis.symbol_search(Query::new(query.into())).unwrap() +} + #[test] fn test_find_all_refs_for_local() { let code = r#" @@ -90,6 +95,55 @@ fn test_find_all_refs_for_fn_param() { assert_eq!(refs.len(), 2); } +#[test] +fn test_world_symbols_with_no_container() { + { + let code = r#" + enum FooInner { } + "#; + + let mut symbols = get_symbols_matching(code, "FooInner"); + + let s = symbols.pop().unwrap(); + + assert_eq!(s.name(), "FooInner"); + assert!(s.container_name().is_none()); + } +} + +#[test] +fn test_world_symbols_include_container_name() { + { + let code = r#" + fn foo() { + enum FooInner { } + } + "#; + + let mut symbols = get_symbols_matching(code, "FooInner"); + + let s = symbols.pop().unwrap(); + + assert_eq!(s.name(), "FooInner"); + assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); + } + + { + let code = r#" + mod foo { + struct FooInner; + } + "#; + + let mut symbols = get_symbols_matching(code, "FooInner"); + + let s = symbols.pop().unwrap(); + + assert_eq!(s.name(), "FooInner"); + assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); + } +} + #[test] #[ignore] fn world_symbols_include_stuff_from_macros() { diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 0cdb39c32..09d896c40 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -190,7 +190,7 @@ pub fn handle_workspace_symbol( name: nav.name().to_string(), kind: nav.kind().conv(), location: nav.try_conv_with(world)?, - container_name: None, + container_name: nav.container_name().map(|v| v.to_string()), deprecated: None, }; res.push(info); -- cgit v1.2.3 From 2ef6c469ef15fd951a2a18ac6c0767f1a3024ae5 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Wed, 13 Feb 2019 11:08:25 +0200 Subject: Remove unnecessary braces --- crates/ra_ide_api/tests/test/main.rs | 56 ++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/crates/ra_ide_api/tests/test/main.rs b/crates/ra_ide_api/tests/test/main.rs index 4126bd160..4cf842452 100644 --- a/crates/ra_ide_api/tests/test/main.rs +++ b/crates/ra_ide_api/tests/test/main.rs @@ -97,51 +97,45 @@ fn test_find_all_refs_for_fn_param() { #[test] fn test_world_symbols_with_no_container() { - { - let code = r#" - enum FooInner { } - "#; + let code = r#" + enum FooInner { } + "#; - let mut symbols = get_symbols_matching(code, "FooInner"); + let mut symbols = get_symbols_matching(code, "FooInner"); - let s = symbols.pop().unwrap(); + let s = symbols.pop().unwrap(); - assert_eq!(s.name(), "FooInner"); - assert!(s.container_name().is_none()); - } + assert_eq!(s.name(), "FooInner"); + assert!(s.container_name().is_none()); } #[test] fn test_world_symbols_include_container_name() { - { - let code = r#" - fn foo() { - enum FooInner { } - } - "#; + let code = r#" +fn foo() { + enum FooInner { } +} + "#; - let mut symbols = get_symbols_matching(code, "FooInner"); + let mut symbols = get_symbols_matching(code, "FooInner"); - let s = symbols.pop().unwrap(); + let s = symbols.pop().unwrap(); - assert_eq!(s.name(), "FooInner"); - assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); - } + assert_eq!(s.name(), "FooInner"); + assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); - { - let code = r#" - mod foo { - struct FooInner; - } - "#; + let code = r#" +mod foo { + struct FooInner; +} + "#; - let mut symbols = get_symbols_matching(code, "FooInner"); + let mut symbols = get_symbols_matching(code, "FooInner"); - let s = symbols.pop().unwrap(); + let s = symbols.pop().unwrap(); - assert_eq!(s.name(), "FooInner"); - assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); - } + assert_eq!(s.name(), "FooInner"); + assert_eq!(s.container_name(), Some(&SmolStr::new("foo"))); } #[test] -- cgit v1.2.3 From d9905f7be52c17335fb529a63b2afbefee7466b4 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Wed, 13 Feb 2019 17:28:15 +0200 Subject: Use clone directly rather than map + clone --- crates/ra_ide_api/src/navigation_target.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs index bcacbe6fc..c559dca11 100644 --- a/crates/ra_ide_api/src/navigation_target.rs +++ b/crates/ra_ide_api/src/navigation_target.rs @@ -58,7 +58,7 @@ impl NavigationTarget { kind: symbol.ptr.kind(), full_range: symbol.ptr.range(), focus_range: None, - container_name: symbol.container_name.map(|v| v.clone()), + container_name: symbol.container_name.clone(), } } -- cgit v1.2.3 From 0c37a9cc28a38e87a136e0cad9dcc5512c64029c Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Wed, 13 Feb 2019 17:42:15 +0200 Subject: Use cloned over map + clone --- crates/ra_ide_api/src/symbol_index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_ide_api/src/symbol_index.rs b/crates/ra_ide_api/src/symbol_index.rs index 62d0979fe..100df8279 100644 --- a/crates/ra_ide_api/src/symbol_index.rs +++ b/crates/ra_ide_api/src/symbol_index.rs @@ -228,7 +228,7 @@ fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec match event { WalkEvent::Enter(node) => { if let Some(mut symbol) = to_file_symbol(node, file_id) { - symbol.container_name = stack.last().map(|v: &SmolStr| v.clone()); + symbol.container_name = stack.last().cloned(); stack.push(symbol.name.clone()); symbols.push(symbol); -- cgit v1.2.3 From 3973974de133867c46727ed516b0445d7f1cb63f Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Wed, 13 Feb 2019 18:02:18 +0200 Subject: Fix possible issue where unnamed is_symbol_def would pop stack wrongly This removes is_symbol_def as unnecessary. --- crates/ra_ide_api/src/symbol_index.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/crates/ra_ide_api/src/symbol_index.rs b/crates/ra_ide_api/src/symbol_index.rs index 100df8279..afb10fa92 100644 --- a/crates/ra_ide_api/src/symbol_index.rs +++ b/crates/ra_ide_api/src/symbol_index.rs @@ -200,16 +200,6 @@ fn is_type(kind: SyntaxKind) -> bool { } } -fn is_symbol_def(kind: SyntaxKind) -> bool { - match kind { - FN_DEF | STRUCT_DEF | ENUM_DEF | TRAIT_DEF | MODULE | TYPE_DEF | CONST_DEF | STATIC_DEF => { - true - } - - _ => false, - } -} - /// The actual data that is stored in the index. It should be as compact as /// possible. #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -236,7 +226,7 @@ fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec } WalkEvent::Leave(node) => { - if is_symbol_def(node.kind()) { + if to_symbol(node).is_some() { stack.pop(); } } -- cgit v1.2.3