From 2a3abe2ce3ebaa0411012cc1be6829c9cb6ea16f Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Sat, 23 Feb 2019 11:02:42 +0200 Subject: Fix goto def not working when cursor was over the name of a def We now allow goto_definition to return the named NavigationTarget if the cursor is on the name of a definition. --- crates/ra_ide_api/src/goto_definition.rs | 127 ++++++++++++++++++++++++++++- crates/ra_ide_api/src/navigation_target.rs | 3 +- 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index 96ed8c8e9..a4ee77d94 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs @@ -1,7 +1,8 @@ use ra_db::{FileId, SourceDatabase}; use ra_syntax::{ - AstNode, ast, - algo::find_node_at_offset, + AstNode, ast::{self, NameOwner}, + algo::{find_node_at_offset, visit::{visitor, Visitor}}, + SyntaxNode, }; use test_utils::tested_by; use hir::Resolution; @@ -114,7 +115,9 @@ fn name_definition( file_id: FileId, name: &ast::Name, ) -> Option> { - if let Some(module) = name.syntax().parent().and_then(ast::Module::cast) { + let parent = name.syntax().parent()?; + + if let Some(module) = ast::Module::cast(&parent) { if module.has_semi() { if let Some(child_module) = hir::source_binder::module_from_declaration(db, file_id, module) @@ -124,9 +127,33 @@ fn name_definition( } } } + + if let Some(nav) = named_target(file_id, &parent) { + return Some(vec![nav]); + } + None } +fn named_target(file_id: FileId, node: &SyntaxNode) -> Option { + fn to_nav_target(node: &N, file_id: FileId) -> Option { + Some(NavigationTarget::from_named(file_id, node)) + } + + visitor() + .visit(|n: &ast::StructDef| to_nav_target(n, file_id)) + .visit(|n: &ast::EnumDef| to_nav_target(n, file_id)) + .visit(|n: &ast::EnumVariant| to_nav_target(n, file_id)) + .visit(|n: &ast::FnDef| to_nav_target(n, file_id)) + .visit(|n: &ast::TypeDef| to_nav_target(n, file_id)) + .visit(|n: &ast::ConstDef| to_nav_target(n, file_id)) + .visit(|n: &ast::StaticDef| to_nav_target(n, file_id)) + .visit(|n: &ast::TraitDef| to_nav_target(n, file_id)) + .visit(|n: &ast::NamedFieldDef| to_nav_target(n, file_id)) + .visit(|n: &ast::Module| to_nav_target(n, file_id)) + .accept(node)? +} + #[cfg(test)] mod tests { use test_utils::covers; @@ -231,4 +258,98 @@ mod tests { "spam NAMED_FIELD_DEF FileId(1) [17; 26) [17; 21)", ); } + + #[test] + fn goto_definition_works_when_used_on_definition_name_itself() { + check_goto( + " + //- /lib.rs + struct Foo<|> { value: u32 } + ", + "Foo STRUCT_DEF FileId(1) [0; 25) [7; 10)", + ); + + check_goto( + r#" + //- /lib.rs + struct Foo { + field<|>: string, + } + "#, + "field NAMED_FIELD_DEF FileId(1) [17; 30) [17; 22)", + ); + + check_goto( + " + //- /lib.rs + fn foo_test<|>() { + } + ", + "foo_test FN_DEF FileId(1) [0; 17) [3; 11)", + ); + + check_goto( + " + //- /lib.rs + enum Foo<|> { + Variant, + } + ", + "Foo ENUM_DEF FileId(1) [0; 25) [5; 8)", + ); + + check_goto( + " + //- /lib.rs + enum Foo { + Variant1, + Variant2<|>, + Variant3, + } + ", + "Variant2 ENUM_VARIANT FileId(1) [29; 37) [29; 37)", + ); + + check_goto( + r#" + //- /lib.rs + static inner<|>: &str = ""; + "#, + "inner STATIC_DEF FileId(1) [0; 24) [7; 12)", + ); + + check_goto( + r#" + //- /lib.rs + const inner<|>: &str = ""; + "#, + "inner CONST_DEF FileId(1) [0; 23) [6; 11)", + ); + + check_goto( + r#" + //- /lib.rs + type Thing<|> = Option<()>; + "#, + "Thing TYPE_DEF FileId(1) [0; 24) [5; 10)", + ); + + check_goto( + r#" + //- /lib.rs + trait Foo<|> { + } + "#, + "Foo TRAIT_DEF FileId(1) [0; 13) [6; 9)", + ); + + check_goto( + r#" + //- /lib.rs + mod bar<|> { + } + "#, + "bar MODULE FileId(1) [0; 11) [4; 7)", + ); + } } diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs index fd001179a..e9240283e 100644 --- a/crates/ra_ide_api/src/navigation_target.rs +++ b/crates/ra_ide_api/src/navigation_target.rs @@ -198,7 +198,8 @@ impl NavigationTarget { buf } - fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget { + /// Allows `NavigationTarget` to be created from a `NameOwner` + pub(crate) fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget { let name = node.name().map(|it| it.text().clone()).unwrap_or_default(); let focus_range = node.name().map(|it| it.syntax().range()); NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax()) -- cgit v1.2.3 From 7046b162756b0fa1b6e6e2223ffbfdf6f41ca6bc Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Sat, 23 Feb 2019 12:53:53 +0200 Subject: Fix NavigationTarget debug_render container_name output --- 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 e9240283e..2c9ec970a 100644 --- a/crates/ra_ide_api/src/navigation_target.rs +++ b/crates/ra_ide_api/src/navigation_target.rs @@ -193,7 +193,7 @@ impl NavigationTarget { buf.push_str(&format!(" {:?}", focus_range)) } if let Some(container_name) = self.container_name() { - buf.push_str(&format!(" {:?}", container_name)) + buf.push_str(&format!(" {}", container_name)) } buf } -- cgit v1.2.3 From c565ec2d6e736c90b8c5a6b89795022d1cc1c1a3 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Sat, 23 Feb 2019 13:05:45 +0200 Subject: Add name_range field to FileSymbol This contains the syntax range of the name itself, allowing NavigationTarget to properly set the focus_range. This should make it so that when using symbol based navigation, we should always focus on the name, instead of the full range. --- crates/ra_ide_api/src/navigation_target.rs | 2 +- crates/ra_ide_api/src/symbol_index.rs | 22 ++++++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs index 2c9ec970a..ce5ae0363 100644 --- a/crates/ra_ide_api/src/navigation_target.rs +++ b/crates/ra_ide_api/src/navigation_target.rs @@ -67,7 +67,7 @@ impl NavigationTarget { name: symbol.name.clone(), kind: symbol.ptr.kind(), full_range: symbol.ptr.range(), - focus_range: None, + focus_range: symbol.name_range, container_name: symbol.container_name.clone(), } } diff --git a/crates/ra_ide_api/src/symbol_index.rs b/crates/ra_ide_api/src/symbol_index.rs index afb10fa92..93bdf05d8 100644 --- a/crates/ra_ide_api/src/symbol_index.rs +++ b/crates/ra_ide_api/src/symbol_index.rs @@ -33,6 +33,7 @@ use ra_syntax::{ SyntaxKind::{self, *}, ast::{self, NameOwner}, WalkEvent, + TextRange, }; use ra_db::{ SourceRootId, SourceDatabase, @@ -70,7 +71,7 @@ fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc let node = find_covering_node(source_file.syntax(), text_range); let ptr = SyntaxNodePtr::new(node); // TODO: Should we get container name for macro symbols? - symbols.push(FileSymbol { file_id, name, ptr, container_name: None }) + symbols.push(FileSymbol { file_id, name, ptr, name_range: None, container_name: None }) } Arc::new(SymbolIndex::new(symbols)) @@ -207,6 +208,7 @@ pub(crate) struct FileSymbol { pub(crate) file_id: FileId, pub(crate) name: SmolStr, pub(crate) ptr: SyntaxNodePtr, + pub(crate) name_range: Option, pub(crate) container_name: Option, } @@ -236,12 +238,14 @@ fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec symbols } -fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> { - fn decl(node: &N) -> Option<(SmolStr, SyntaxNodePtr)> { - let name = node.name()?.text().clone(); +fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr, TextRange)> { + fn decl(node: &N) -> Option<(SmolStr, SyntaxNodePtr, TextRange)> { + let name = node.name()?; + let name_range = name.syntax().range(); + let name = name.text().clone(); let ptr = SyntaxNodePtr::new(node.syntax()); - Some((name, ptr)) + Some((name, ptr, name_range)) } visitor() .visit(decl::) @@ -256,5 +260,11 @@ fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> { } 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 }) + to_symbol(node).map(move |(name, ptr, name_range)| FileSymbol { + name, + ptr, + file_id, + name_range: Some(name_range), + container_name: None, + }) } -- cgit v1.2.3 From 40e6cb196b3e4fdb580812a418edfb8df08cf423 Mon Sep 17 00:00:00 2001 From: Ville Penttinen Date: Sat, 23 Feb 2019 14:08:57 +0200 Subject: Remove unnecessary to_nav_target --- crates/ra_ide_api/src/goto_definition.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index a4ee77d94..4dac96bfe 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs @@ -1,6 +1,6 @@ use ra_db::{FileId, SourceDatabase}; use ra_syntax::{ - AstNode, ast::{self, NameOwner}, + AstNode, ast, algo::{find_node_at_offset, visit::{visitor, Visitor}}, SyntaxNode, }; @@ -136,22 +136,18 @@ fn name_definition( } fn named_target(file_id: FileId, node: &SyntaxNode) -> Option { - fn to_nav_target(node: &N, file_id: FileId) -> Option { - Some(NavigationTarget::from_named(file_id, node)) - } - visitor() - .visit(|n: &ast::StructDef| to_nav_target(n, file_id)) - .visit(|n: &ast::EnumDef| to_nav_target(n, file_id)) - .visit(|n: &ast::EnumVariant| to_nav_target(n, file_id)) - .visit(|n: &ast::FnDef| to_nav_target(n, file_id)) - .visit(|n: &ast::TypeDef| to_nav_target(n, file_id)) - .visit(|n: &ast::ConstDef| to_nav_target(n, file_id)) - .visit(|n: &ast::StaticDef| to_nav_target(n, file_id)) - .visit(|n: &ast::TraitDef| to_nav_target(n, file_id)) - .visit(|n: &ast::NamedFieldDef| to_nav_target(n, file_id)) - .visit(|n: &ast::Module| to_nav_target(n, file_id)) - .accept(node)? + .visit(|node: &ast::StructDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::EnumDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::EnumVariant| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::FnDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::TypeDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::ConstDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::StaticDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::TraitDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::NamedFieldDef| NavigationTarget::from_named(file_id, node)) + .visit(|node: &ast::Module| NavigationTarget::from_named(file_id, node)) + .accept(node) } #[cfg(test)] -- cgit v1.2.3