aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <bors[bot]@users.noreply.github.com>2019-02-23 12:17:53 +0000
committerbors[bot] <bors[bot]@users.noreply.github.com>2019-02-23 12:17:53 +0000
commite5fb33a94618d45051192d99e859bdd44c3daa36 (patch)
treeb77da2812c973e2270d7abf808634cb9655e2c68
parent38add103c3f30493e336a37827a76f48a6e7c584 (diff)
parent40e6cb196b3e4fdb580812a418edfb8df08cf423 (diff)
Merge #879
879: Fixes to goto definition r=vipentti a=vipentti Previously goto definition would fail when the cursor was over the name of the definition. Now we should properly resolve to a `NavigationTarget` when on top of the name of a definition. In addition this adds `name_range` field to `FileSymbol`, this further fixes goto_definition and symbol based navigation by allowing the `NavigationTarget` to actually have a `focus_range`, meaning instead of focusing on the start of the `full_range`, we can have the cursor focus on the name. e.g. goto definition ```rust fn bar() { fn foo() { } foo<|>(); } ``` Previously this would put the cursor at the start of the FN_DEF: ```rust fn bar() { <|>fn foo() { } foo(); } ``` Now when using the symbol based resolving, we'll have a proper focus range and instead put the cursor at the start of the name. ```rust fn bar() { fn <|>foo() { } foo(); } ``` This fixes #877 but doesn't contain the refactoring of the return type for `goto_definition` Co-authored-by: Ville Penttinen <[email protected]>
-rw-r--r--crates/ra_ide_api/src/goto_definition.rs121
-rw-r--r--crates/ra_ide_api/src/navigation_target.rs7
-rw-r--r--crates/ra_ide_api/src/symbol_index.rs22
3 files changed, 139 insertions, 11 deletions
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index 96ed8c8e9..4dac96bfe 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -1,7 +1,8 @@
1use ra_db::{FileId, SourceDatabase}; 1use ra_db::{FileId, SourceDatabase};
2use ra_syntax::{ 2use ra_syntax::{
3 AstNode, ast, 3 AstNode, ast,
4 algo::find_node_at_offset, 4 algo::{find_node_at_offset, visit::{visitor, Visitor}},
5 SyntaxNode,
5}; 6};
6use test_utils::tested_by; 7use test_utils::tested_by;
7use hir::Resolution; 8use hir::Resolution;
@@ -114,7 +115,9 @@ fn name_definition(
114 file_id: FileId, 115 file_id: FileId,
115 name: &ast::Name, 116 name: &ast::Name,
116) -> Option<Vec<NavigationTarget>> { 117) -> Option<Vec<NavigationTarget>> {
117 if let Some(module) = name.syntax().parent().and_then(ast::Module::cast) { 118 let parent = name.syntax().parent()?;
119
120 if let Some(module) = ast::Module::cast(&parent) {
118 if module.has_semi() { 121 if module.has_semi() {
119 if let Some(child_module) = 122 if let Some(child_module) =
120 hir::source_binder::module_from_declaration(db, file_id, module) 123 hir::source_binder::module_from_declaration(db, file_id, module)
@@ -124,9 +127,29 @@ fn name_definition(
124 } 127 }
125 } 128 }
126 } 129 }
130
131 if let Some(nav) = named_target(file_id, &parent) {
132 return Some(vec![nav]);
133 }
134
127 None 135 None
128} 136}
129 137
138fn named_target(file_id: FileId, node: &SyntaxNode) -> Option<NavigationTarget> {
139 visitor()
140 .visit(|node: &ast::StructDef| NavigationTarget::from_named(file_id, node))
141 .visit(|node: &ast::EnumDef| NavigationTarget::from_named(file_id, node))
142 .visit(|node: &ast::EnumVariant| NavigationTarget::from_named(file_id, node))
143 .visit(|node: &ast::FnDef| NavigationTarget::from_named(file_id, node))
144 .visit(|node: &ast::TypeDef| NavigationTarget::from_named(file_id, node))
145 .visit(|node: &ast::ConstDef| NavigationTarget::from_named(file_id, node))
146 .visit(|node: &ast::StaticDef| NavigationTarget::from_named(file_id, node))
147 .visit(|node: &ast::TraitDef| NavigationTarget::from_named(file_id, node))
148 .visit(|node: &ast::NamedFieldDef| NavigationTarget::from_named(file_id, node))
149 .visit(|node: &ast::Module| NavigationTarget::from_named(file_id, node))
150 .accept(node)
151}
152
130#[cfg(test)] 153#[cfg(test)]
131mod tests { 154mod tests {
132 use test_utils::covers; 155 use test_utils::covers;
@@ -231,4 +254,98 @@ mod tests {
231 "spam NAMED_FIELD_DEF FileId(1) [17; 26) [17; 21)", 254 "spam NAMED_FIELD_DEF FileId(1) [17; 26) [17; 21)",
232 ); 255 );
233 } 256 }
257
258 #[test]
259 fn goto_definition_works_when_used_on_definition_name_itself() {
260 check_goto(
261 "
262 //- /lib.rs
263 struct Foo<|> { value: u32 }
264 ",
265 "Foo STRUCT_DEF FileId(1) [0; 25) [7; 10)",
266 );
267
268 check_goto(
269 r#"
270 //- /lib.rs
271 struct Foo {
272 field<|>: string,
273 }
274 "#,
275 "field NAMED_FIELD_DEF FileId(1) [17; 30) [17; 22)",
276 );
277
278 check_goto(
279 "
280 //- /lib.rs
281 fn foo_test<|>() {
282 }
283 ",
284 "foo_test FN_DEF FileId(1) [0; 17) [3; 11)",
285 );
286
287 check_goto(
288 "
289 //- /lib.rs
290 enum Foo<|> {
291 Variant,
292 }
293 ",
294 "Foo ENUM_DEF FileId(1) [0; 25) [5; 8)",
295 );
296
297 check_goto(
298 "
299 //- /lib.rs
300 enum Foo {
301 Variant1,
302 Variant2<|>,
303 Variant3,
304 }
305 ",
306 "Variant2 ENUM_VARIANT FileId(1) [29; 37) [29; 37)",
307 );
308
309 check_goto(
310 r#"
311 //- /lib.rs
312 static inner<|>: &str = "";
313 "#,
314 "inner STATIC_DEF FileId(1) [0; 24) [7; 12)",
315 );
316
317 check_goto(
318 r#"
319 //- /lib.rs
320 const inner<|>: &str = "";
321 "#,
322 "inner CONST_DEF FileId(1) [0; 23) [6; 11)",
323 );
324
325 check_goto(
326 r#"
327 //- /lib.rs
328 type Thing<|> = Option<()>;
329 "#,
330 "Thing TYPE_DEF FileId(1) [0; 24) [5; 10)",
331 );
332
333 check_goto(
334 r#"
335 //- /lib.rs
336 trait Foo<|> {
337 }
338 "#,
339 "Foo TRAIT_DEF FileId(1) [0; 13) [6; 9)",
340 );
341
342 check_goto(
343 r#"
344 //- /lib.rs
345 mod bar<|> {
346 }
347 "#,
348 "bar MODULE FileId(1) [0; 11) [4; 7)",
349 );
350 }
234} 351}
diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs
index fd001179a..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 {
67 name: symbol.name.clone(), 67 name: symbol.name.clone(),
68 kind: symbol.ptr.kind(), 68 kind: symbol.ptr.kind(),
69 full_range: symbol.ptr.range(), 69 full_range: symbol.ptr.range(),
70 focus_range: None, 70 focus_range: symbol.name_range,
71 container_name: symbol.container_name.clone(), 71 container_name: symbol.container_name.clone(),
72 } 72 }
73 } 73 }
@@ -193,12 +193,13 @@ impl NavigationTarget {
193 buf.push_str(&format!(" {:?}", focus_range)) 193 buf.push_str(&format!(" {:?}", focus_range))
194 } 194 }
195 if let Some(container_name) = self.container_name() { 195 if let Some(container_name) = self.container_name() {
196 buf.push_str(&format!(" {:?}", container_name)) 196 buf.push_str(&format!(" {}", container_name))
197 } 197 }
198 buf 198 buf
199 } 199 }
200 200
201 fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget { 201 /// Allows `NavigationTarget` to be created from a `NameOwner`
202 pub(crate) fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget {
202 let name = node.name().map(|it| it.text().clone()).unwrap_or_default(); 203 let name = node.name().map(|it| it.text().clone()).unwrap_or_default();
203 let focus_range = node.name().map(|it| it.syntax().range()); 204 let focus_range = node.name().map(|it| it.syntax().range());
204 NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax()) 205 NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax())
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::{
33 SyntaxKind::{self, *}, 33 SyntaxKind::{self, *},
34 ast::{self, NameOwner}, 34 ast::{self, NameOwner},
35 WalkEvent, 35 WalkEvent,
36 TextRange,
36}; 37};
37use ra_db::{ 38use ra_db::{
38 SourceRootId, SourceDatabase, 39 SourceRootId, SourceDatabase,
@@ -70,7 +71,7 @@ fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc<SymbolIndex>
70 let node = find_covering_node(source_file.syntax(), text_range); 71 let node = find_covering_node(source_file.syntax(), text_range);
71 let ptr = SyntaxNodePtr::new(node); 72 let ptr = SyntaxNodePtr::new(node);
72 // TODO: Should we get container name for macro symbols? 73 // TODO: Should we get container name for macro symbols?
73 symbols.push(FileSymbol { file_id, name, ptr, container_name: None }) 74 symbols.push(FileSymbol { file_id, name, ptr, name_range: None, container_name: None })
74 } 75 }
75 76
76 Arc::new(SymbolIndex::new(symbols)) 77 Arc::new(SymbolIndex::new(symbols))
@@ -207,6 +208,7 @@ pub(crate) struct FileSymbol {
207 pub(crate) file_id: FileId, 208 pub(crate) file_id: FileId,
208 pub(crate) name: SmolStr, 209 pub(crate) name: SmolStr,
209 pub(crate) ptr: SyntaxNodePtr, 210 pub(crate) ptr: SyntaxNodePtr,
211 pub(crate) name_range: Option<TextRange>,
210 pub(crate) container_name: Option<SmolStr>, 212 pub(crate) container_name: Option<SmolStr>,
211} 213}
212 214
@@ -236,12 +238,14 @@ fn source_file_to_file_symbols(source_file: &SourceFile, file_id: FileId) -> Vec
236 symbols 238 symbols
237} 239}
238 240
239fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> { 241fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr, TextRange)> {
240 fn decl<N: NameOwner>(node: &N) -> Option<(SmolStr, SyntaxNodePtr)> { 242 fn decl<N: NameOwner>(node: &N) -> Option<(SmolStr, SyntaxNodePtr, TextRange)> {
241 let name = node.name()?.text().clone(); 243 let name = node.name()?;
244 let name_range = name.syntax().range();
245 let name = name.text().clone();
242 let ptr = SyntaxNodePtr::new(node.syntax()); 246 let ptr = SyntaxNodePtr::new(node.syntax());
243 247
244 Some((name, ptr)) 248 Some((name, ptr, name_range))
245 } 249 }
246 visitor() 250 visitor()
247 .visit(decl::<ast::FnDef>) 251 .visit(decl::<ast::FnDef>)
@@ -256,5 +260,11 @@ fn to_symbol(node: &SyntaxNode) -> Option<(SmolStr, SyntaxNodePtr)> {
256} 260}
257 261
258fn to_file_symbol(node: &SyntaxNode, file_id: FileId) -> Option<FileSymbol> { 262fn to_file_symbol(node: &SyntaxNode, file_id: FileId) -> Option<FileSymbol> {
259 to_symbol(node).map(move |(name, ptr)| FileSymbol { name, ptr, file_id, container_name: None }) 263 to_symbol(node).map(move |(name, ptr, name_range)| FileSymbol {
264 name,
265 ptr,
266 file_id,
267 name_range: Some(name_range),
268 container_name: None,
269 })
260} 270}