Merge #813

813: Add support for container_name in workspace/symbol query r=matklad a=vipentti

Currently this does not fill in the container_info if a type is defined on the top level in a file. 

e.g. `foo.rs`
```rust
enum Foo { }
```
`Foo` will have None as the container_name, however

```rust
mod foo_mod {
    enum Foo { } 
}
```
`Foo` has `foo_mod` as the container_name. 

This closes #559 

Co-authored-by: Ville Penttinen <villem.penttinen@gmail.com>

Author: bors[bot]

crates/ra_ide_api/src/navigation_target.rs

@@ -19,13 +19,18 @@ pub struct NavigationTarget {
     kind: SyntaxKind,
     full_range: TextRange,
     focus_range: Option<TextRange>,
+    container_name: Option<SmolStr>,
 }
 
 impl NavigationTarget {
     pub fn name(&self) -> &SmolStr {
         &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.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,
         }
     }
 }

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<SymbolIndex> {
     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::<Vec<_>>();
+
+    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<Item = (FileId, TreeArc<SourceFile>)>,
     ) -> 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::<Vec<_>>()
-            })
+            .flat_map(|(file_id, file)| source_file_to_file_symbols(&file, file_id))
             .collect::<Vec<_>>();
         SymbolIndex::new(symbols)
     }
@@ -215,12 +207,40 @@ pub(crate) struct FileSymbol {
     pub(crate) file_id: FileId,
     pub(crate) name: SmolStr,
     pub(crate) ptr: SyntaxNodePtr,
+    pub(crate) container_name: Option<SmolStr>,
+}
+
+fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec<FileSymbol> {
+    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().cloned();
+
+                    stack.push(symbol.name.clone());
+                    symbols.push(symbol);
+                }
+            }
+
+            WalkEvent::Leave(node) => {
+                if to_symbol(node).is_some() {
+                    stack.pop();
+                }
+            }
+        }
+    }
+
+    symbols
 }
 
 fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> {
     fn decl<N: NameOwner>(node: &N) -> Option<(SmolStr, SyntaxNodePtr)> {
         let name = node.name()?.text().clone();
         let ptr = SyntaxNodePtr::new(node.syntax());
+
         Some((name, ptr))
     }
     visitor()
@@ -234,3 +254,7 @@ fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> {
         .visit(decl::<ast::StaticDef>)
         .accept(node)?
 }
+
+fn to_file_symbol(node: &SyntaxNode, file_id: FileId) -> Option<FileSymbol> {
+    to_symbol(node).map(move |(name, ptr)| FileSymbol { name, ptr, file_id, container_name: None })
+}

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<NavigationTarget> {
+    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,49 @@ 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() {

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);