diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-01-10 20:44:21 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-01-10 20:44:21 +0000 |
commit | 9bee2937b4289315714b778ba38ab0e7c2a623fe (patch) | |
tree | ec9a223b23f7675a8e0eb1d12c454adc9551baf2 | |
parent | dc2a8d5acc53054c86ad17260b69d4bf4f14dbc6 (diff) | |
parent | a6590ce2318676210b6b5a197b76b5861a3407c9 (diff) |
Merge #463
463: Use name resolution for goto definition r=matklad a=flodiebold
This tries proper name resolution before falling back on the index.
@matklad There was currently no way of getting the location of a `DefId` from outside `ra_hir`. I added something, but it's probably not the best API, maybe you have a better idea?
Co-authored-by: Florian Diebold <[email protected]>
-rw-r--r-- | crates/ra_hir/src/code_model_api.rs | 49 | ||||
-rw-r--r-- | crates/ra_hir/src/code_model_impl/function.rs | 23 | ||||
-rw-r--r-- | crates/ra_hir/src/expr.rs | 2 | ||||
-rw-r--r-- | crates/ra_hir/src/ids.rs | 8 | ||||
-rw-r--r-- | crates/ra_ide_api/src/completion/complete_path.rs | 6 | ||||
-rw-r--r-- | crates/ra_ide_api/src/goto_definition.rs | 43 | ||||
-rw-r--r-- | crates/ra_ide_api/src/lib.rs | 64 |
7 files changed, 179 insertions, 16 deletions
diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index 725bc7d80..e69f546ff 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs | |||
@@ -2,10 +2,10 @@ use std::sync::Arc; | |||
2 | 2 | ||
3 | use relative_path::RelativePathBuf; | 3 | use relative_path::RelativePathBuf; |
4 | use ra_db::{CrateId, Cancelable, FileId}; | 4 | use ra_db::{CrateId, Cancelable, FileId}; |
5 | use ra_syntax::{ast, TreePtr, SyntaxNode}; | 5 | use ra_syntax::{ast, TreePtr, SyntaxNode, AstNode}; |
6 | 6 | ||
7 | use crate::{ | 7 | use crate::{ |
8 | Name, DefId, Path, PerNs, ScopesWithSyntaxMapping, Ty, | 8 | Name, DefId, Path, PerNs, ScopesWithSyntaxMapping, Ty, HirFileId, |
9 | type_ref::TypeRef, | 9 | type_ref::TypeRef, |
10 | nameres::ModuleScope, | 10 | nameres::ModuleScope, |
11 | db::HirDatabase, | 11 | db::HirDatabase, |
@@ -181,6 +181,19 @@ impl Struct { | |||
181 | .collect(); | 181 | .collect(); |
182 | Ok(res) | 182 | Ok(res) |
183 | } | 183 | } |
184 | |||
185 | pub fn source( | ||
186 | &self, | ||
187 | db: &impl HirDatabase, | ||
188 | ) -> Cancelable<(HirFileId, TreePtr<ast::StructDef>)> { | ||
189 | let (file_id, syntax) = self.def_id.source(db); | ||
190 | Ok(( | ||
191 | file_id, | ||
192 | ast::StructDef::cast(&syntax) | ||
193 | .expect("struct def should point to StructDef node") | ||
194 | .to_owned(), | ||
195 | )) | ||
196 | } | ||
184 | } | 197 | } |
185 | 198 | ||
186 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] | 199 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] |
@@ -204,6 +217,16 @@ impl Enum { | |||
204 | pub fn variants(&self, db: &impl HirDatabase) -> Cancelable<Vec<(Name, EnumVariant)>> { | 217 | pub fn variants(&self, db: &impl HirDatabase) -> Cancelable<Vec<(Name, EnumVariant)>> { |
205 | Ok(db.enum_data(self.def_id)?.variants.clone()) | 218 | Ok(db.enum_data(self.def_id)?.variants.clone()) |
206 | } | 219 | } |
220 | |||
221 | pub fn source(&self, db: &impl HirDatabase) -> Cancelable<(HirFileId, TreePtr<ast::EnumDef>)> { | ||
222 | let (file_id, syntax) = self.def_id.source(db); | ||
223 | Ok(( | ||
224 | file_id, | ||
225 | ast::EnumDef::cast(&syntax) | ||
226 | .expect("enum def should point to EnumDef node") | ||
227 | .to_owned(), | ||
228 | )) | ||
229 | } | ||
207 | } | 230 | } |
208 | 231 | ||
209 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] | 232 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] |
@@ -231,6 +254,19 @@ impl EnumVariant { | |||
231 | pub fn variant_data(&self, db: &impl HirDatabase) -> Cancelable<Arc<VariantData>> { | 254 | pub fn variant_data(&self, db: &impl HirDatabase) -> Cancelable<Arc<VariantData>> { |
232 | Ok(db.enum_variant_data(self.def_id)?.variant_data.clone()) | 255 | Ok(db.enum_variant_data(self.def_id)?.variant_data.clone()) |
233 | } | 256 | } |
257 | |||
258 | pub fn source( | ||
259 | &self, | ||
260 | db: &impl HirDatabase, | ||
261 | ) -> Cancelable<(HirFileId, TreePtr<ast::EnumVariant>)> { | ||
262 | let (file_id, syntax) = self.def_id.source(db); | ||
263 | Ok(( | ||
264 | file_id, | ||
265 | ast::EnumVariant::cast(&syntax) | ||
266 | .expect("variant def should point to EnumVariant node") | ||
267 | .to_owned(), | ||
268 | )) | ||
269 | } | ||
234 | } | 270 | } |
235 | 271 | ||
236 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] | 272 | #[derive(Debug, Clone, PartialEq, Eq, Hash)] |
@@ -241,11 +277,16 @@ pub struct Function { | |||
241 | /// The declared signature of a function. | 277 | /// The declared signature of a function. |
242 | #[derive(Debug, Clone, PartialEq, Eq)] | 278 | #[derive(Debug, Clone, PartialEq, Eq)] |
243 | pub struct FnSignature { | 279 | pub struct FnSignature { |
280 | pub(crate) name: Name, | ||
244 | pub(crate) args: Vec<TypeRef>, | 281 | pub(crate) args: Vec<TypeRef>, |
245 | pub(crate) ret_type: TypeRef, | 282 | pub(crate) ret_type: TypeRef, |
246 | } | 283 | } |
247 | 284 | ||
248 | impl FnSignature { | 285 | impl FnSignature { |
286 | pub fn name(&self) -> &Name { | ||
287 | &self.name | ||
288 | } | ||
289 | |||
249 | pub fn args(&self) -> &[TypeRef] { | 290 | pub fn args(&self) -> &[TypeRef] { |
250 | &self.args | 291 | &self.args |
251 | } | 292 | } |
@@ -260,8 +301,8 @@ impl Function { | |||
260 | self.def_id | 301 | self.def_id |
261 | } | 302 | } |
262 | 303 | ||
263 | pub fn source(&self, db: &impl HirDatabase) -> TreePtr<ast::FnDef> { | 304 | pub fn source(&self, db: &impl HirDatabase) -> Cancelable<(HirFileId, TreePtr<ast::FnDef>)> { |
264 | self.source_impl(db) | 305 | Ok(self.source_impl(db)) |
265 | } | 306 | } |
266 | 307 | ||
267 | pub fn body_syntax_mapping(&self, db: &impl HirDatabase) -> Cancelable<Arc<BodySyntaxMapping>> { | 308 | pub fn body_syntax_mapping(&self, db: &impl HirDatabase) -> Cancelable<Arc<BodySyntaxMapping>> { |
diff --git a/crates/ra_hir/src/code_model_impl/function.rs b/crates/ra_hir/src/code_model_impl/function.rs index 13c57ed21..1bd4cc802 100644 --- a/crates/ra_hir/src/code_model_impl/function.rs +++ b/crates/ra_hir/src/code_model_impl/function.rs | |||
@@ -5,11 +5,11 @@ use std::sync::Arc; | |||
5 | use ra_db::Cancelable; | 5 | use ra_db::Cancelable; |
6 | use ra_syntax::{ | 6 | use ra_syntax::{ |
7 | TreePtr, | 7 | TreePtr, |
8 | ast::{self, AstNode}, | 8 | ast::{self, AstNode, NameOwner}, |
9 | }; | 9 | }; |
10 | 10 | ||
11 | use crate::{ | 11 | use crate::{ |
12 | DefId, DefKind, HirDatabase, Name, Function, FnSignature, Module, | 12 | DefId, DefKind, HirDatabase, Name, AsName, Function, FnSignature, Module, HirFileId, |
13 | type_ref::{TypeRef, Mutability}, | 13 | type_ref::{TypeRef, Mutability}, |
14 | expr::Body, | 14 | expr::Body, |
15 | impl_block::ImplBlock, | 15 | impl_block::ImplBlock, |
@@ -22,11 +22,14 @@ impl Function { | |||
22 | Function { def_id } | 22 | Function { def_id } |
23 | } | 23 | } |
24 | 24 | ||
25 | pub(crate) fn source_impl(&self, db: &impl HirDatabase) -> TreePtr<ast::FnDef> { | 25 | pub(crate) fn source_impl(&self, db: &impl HirDatabase) -> (HirFileId, TreePtr<ast::FnDef>) { |
26 | let def_loc = self.def_id.loc(db); | 26 | let def_loc = self.def_id.loc(db); |
27 | assert!(def_loc.kind == DefKind::Function); | 27 | assert!(def_loc.kind == DefKind::Function); |
28 | let syntax = db.file_item(def_loc.source_item_id); | 28 | let syntax = db.file_item(def_loc.source_item_id); |
29 | ast::FnDef::cast(&syntax).unwrap().to_owned() | 29 | ( |
30 | def_loc.source_item_id.file_id, | ||
31 | ast::FnDef::cast(&syntax).unwrap().to_owned(), | ||
32 | ) | ||
30 | } | 33 | } |
31 | 34 | ||
32 | pub(crate) fn body(&self, db: &impl HirDatabase) -> Cancelable<Arc<Body>> { | 35 | pub(crate) fn body(&self, db: &impl HirDatabase) -> Cancelable<Arc<Body>> { |
@@ -46,7 +49,11 @@ impl Function { | |||
46 | impl FnSignature { | 49 | impl FnSignature { |
47 | pub(crate) fn fn_signature_query(db: &impl HirDatabase, def_id: DefId) -> Arc<FnSignature> { | 50 | pub(crate) fn fn_signature_query(db: &impl HirDatabase, def_id: DefId) -> Arc<FnSignature> { |
48 | let func = Function::new(def_id); | 51 | let func = Function::new(def_id); |
49 | let node = func.source(db); | 52 | let node = func.source_impl(db).1; // TODO we're using source_impl here to avoid returning Cancelable... this is a bit hacky |
53 | let name = node | ||
54 | .name() | ||
55 | .map(|n| n.as_name()) | ||
56 | .unwrap_or_else(Name::missing); | ||
50 | let mut args = Vec::new(); | 57 | let mut args = Vec::new(); |
51 | if let Some(param_list) = node.param_list() { | 58 | if let Some(param_list) = node.param_list() { |
52 | if let Some(self_param) = param_list.self_param() { | 59 | if let Some(self_param) = param_list.self_param() { |
@@ -76,7 +83,11 @@ impl FnSignature { | |||
76 | } else { | 83 | } else { |
77 | TypeRef::unit() | 84 | TypeRef::unit() |
78 | }; | 85 | }; |
79 | let sig = FnSignature { args, ret_type }; | 86 | let sig = FnSignature { |
87 | name, | ||
88 | args, | ||
89 | ret_type, | ||
90 | }; | ||
80 | Arc::new(sig) | 91 | Arc::new(sig) |
81 | } | 92 | } |
82 | } | 93 | } |
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index ebb83d084..e5596cbaa 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs | |||
@@ -762,7 +762,7 @@ pub(crate) fn body_syntax_mapping( | |||
762 | let def = def_id.resolve(db)?; | 762 | let def = def_id.resolve(db)?; |
763 | 763 | ||
764 | let body_syntax_mapping = match def { | 764 | let body_syntax_mapping = match def { |
765 | Def::Function(f) => collect_fn_body_syntax(&f.source(db)), | 765 | Def::Function(f) => collect_fn_body_syntax(&f.source(db)?.1), |
766 | // TODO: consts, etc. | 766 | // TODO: consts, etc. |
767 | _ => panic!("Trying to get body for item type without body"), | 767 | _ => panic!("Trying to get body for item type without body"), |
768 | }; | 768 | }; |
diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs index db0107e53..c75ef4ae7 100644 --- a/crates/ra_hir/src/ids.rs +++ b/crates/ra_hir/src/ids.rs | |||
@@ -34,7 +34,7 @@ pub struct HirFileId(HirFileIdRepr); | |||
34 | impl HirFileId { | 34 | impl HirFileId { |
35 | /// For macro-expansion files, returns the file original source file the | 35 | /// For macro-expansion files, returns the file original source file the |
36 | /// expansionoriginated from. | 36 | /// expansionoriginated from. |
37 | pub(crate) fn original_file(self, db: &impl HirDatabase) -> FileId { | 37 | pub fn original_file(self, db: &impl HirDatabase) -> FileId { |
38 | match self.0 { | 38 | match self.0 { |
39 | HirFileIdRepr::File(file_id) => file_id, | 39 | HirFileIdRepr::File(file_id) => file_id, |
40 | HirFileIdRepr::Macro(macro_call_id) => { | 40 | HirFileIdRepr::Macro(macro_call_id) => { |
@@ -179,6 +179,12 @@ impl DefId { | |||
179 | Ok(res) | 179 | Ok(res) |
180 | } | 180 | } |
181 | 181 | ||
182 | pub(crate) fn source(self, db: &impl HirDatabase) -> (HirFileId, TreePtr<SyntaxNode>) { | ||
183 | let loc = self.loc(db); | ||
184 | let syntax = db.file_item(loc.source_item_id); | ||
185 | (loc.source_item_id.file_id, syntax) | ||
186 | } | ||
187 | |||
182 | /// For a module, returns that module; for any other def, returns the containing module. | 188 | /// For a module, returns that module; for any other def, returns the containing module. |
183 | pub fn module(self, db: &impl HirDatabase) -> Cancelable<Module> { | 189 | pub fn module(self, db: &impl HirDatabase) -> Cancelable<Module> { |
184 | let loc = self.loc(db); | 190 | let loc = self.loc(db); |
diff --git a/crates/ra_ide_api/src/completion/complete_path.rs b/crates/ra_ide_api/src/completion/complete_path.rs index 4860db629..a25ad3f13 100644 --- a/crates/ra_ide_api/src/completion/complete_path.rs +++ b/crates/ra_ide_api/src/completion/complete_path.rs | |||
@@ -15,11 +15,11 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) -> C | |||
15 | match def_id.resolve(ctx.db)? { | 15 | match def_id.resolve(ctx.db)? { |
16 | hir::Def::Module(module) => { | 16 | hir::Def::Module(module) => { |
17 | let module_scope = module.scope(ctx.db)?; | 17 | let module_scope = module.scope(ctx.db)?; |
18 | module_scope.entries().for_each(|(name, res)| { | 18 | for (name, res) in module_scope.entries() { |
19 | CompletionItem::new(CompletionKind::Reference, name.to_string()) | 19 | CompletionItem::new(CompletionKind::Reference, name.to_string()) |
20 | .from_resolution(ctx, res) | 20 | .from_resolution(ctx, res) |
21 | .add_to(acc) | 21 | .add_to(acc); |
22 | }); | 22 | } |
23 | } | 23 | } |
24 | hir::Def::Enum(e) => { | 24 | hir::Def::Enum(e) => { |
25 | e.variants(ctx.db)? | 25 | e.variants(ctx.db)? |
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index 0d524b6f1..eaddd5083 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs | |||
@@ -42,6 +42,24 @@ pub(crate) fn reference_definition( | |||
42 | return Ok(vec![nav]); | 42 | return Ok(vec![nav]); |
43 | }; | 43 | }; |
44 | } | 44 | } |
45 | // Then try module name resolution | ||
46 | if let Some(module) = | ||
47 | hir::source_binder::module_from_child_node(db, file_id, name_ref.syntax())? | ||
48 | { | ||
49 | if let Some(path) = name_ref | ||
50 | .syntax() | ||
51 | .ancestors() | ||
52 | .find_map(ast::Path::cast) | ||
53 | .and_then(hir::Path::from_ast) | ||
54 | { | ||
55 | let resolved = module.resolve_path(db, &path)?; | ||
56 | if let Some(def_id) = resolved.take_types().or(resolved.take_values()) { | ||
57 | if let Some(target) = NavigationTarget::from_def(db, def_id.resolve(db)?)? { | ||
58 | return Ok(vec![target]); | ||
59 | } | ||
60 | } | ||
61 | } | ||
62 | } | ||
45 | // If that fails try the index based approach. | 63 | // If that fails try the index based approach. |
46 | let navs = db | 64 | let navs = db |
47 | .index_resolve(name_ref)? | 65 | .index_resolve(name_ref)? |
@@ -105,6 +123,31 @@ mod tests { | |||
105 | } | 123 | } |
106 | 124 | ||
107 | #[test] | 125 | #[test] |
126 | fn goto_definition_resolves_correct_name() { | ||
127 | let (analysis, pos) = analysis_and_position( | ||
128 | " | ||
129 | //- /lib.rs | ||
130 | use a::Foo; | ||
131 | mod a; | ||
132 | mod b; | ||
133 | enum E { X(Foo<|>) } | ||
134 | //- /a.rs | ||
135 | struct Foo; | ||
136 | //- /b.rs | ||
137 | struct Foo; | ||
138 | ", | ||
139 | ); | ||
140 | |||
141 | let symbols = analysis.goto_definition(pos).unwrap().unwrap(); | ||
142 | assert_eq_dbg( | ||
143 | r#"[NavigationTarget { file_id: FileId(2), name: "Foo", | ||
144 | kind: STRUCT_DEF, range: [0; 11), | ||
145 | ptr: Some(LocalSyntaxPtr { range: [0; 11), kind: STRUCT_DEF }) }]"#, | ||
146 | &symbols, | ||
147 | ); | ||
148 | } | ||
149 | |||
150 | #[test] | ||
108 | fn goto_definition_works_for_module_declaration() { | 151 | fn goto_definition_works_for_module_declaration() { |
109 | let (analysis, pos) = analysis_and_position( | 152 | let (analysis, pos) = analysis_and_position( |
110 | " | 153 | " |
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs index f505959ce..65d21d899 100644 --- a/crates/ra_ide_api/src/lib.rs +++ b/crates/ra_ide_api/src/lib.rs | |||
@@ -33,7 +33,8 @@ mod syntax_highlighting; | |||
33 | 33 | ||
34 | use std::{fmt, sync::Arc}; | 34 | use std::{fmt, sync::Arc}; |
35 | 35 | ||
36 | use ra_syntax::{SmolStr, SourceFile, TreePtr, SyntaxKind, TextRange, TextUnit}; | 36 | use hir::{Def, ModuleSource, Name}; |
37 | use ra_syntax::{SmolStr, SourceFile, TreePtr, SyntaxKind, SyntaxNode, TextRange, TextUnit, AstNode}; | ||
37 | use ra_text_edit::TextEdit; | 38 | use ra_text_edit::TextEdit; |
38 | use ra_db::{SyntaxDatabase, FilesDatabase, LocalSyntaxPtr, BaseDatabase}; | 39 | use ra_db::{SyntaxDatabase, FilesDatabase, LocalSyntaxPtr, BaseDatabase}; |
39 | use rayon::prelude::*; | 40 | use rayon::prelude::*; |
@@ -268,6 +269,67 @@ impl NavigationTarget { | |||
268 | } | 269 | } |
269 | } | 270 | } |
270 | 271 | ||
272 | fn from_syntax(name: Option<Name>, file_id: FileId, node: &SyntaxNode) -> NavigationTarget { | ||
273 | NavigationTarget { | ||
274 | file_id, | ||
275 | name: name.map(|n| n.to_string().into()).unwrap_or("".into()), | ||
276 | kind: node.kind(), | ||
277 | range: node.range(), | ||
278 | ptr: Some(LocalSyntaxPtr::new(node)), | ||
279 | } | ||
280 | } | ||
281 | // TODO once Def::Item is gone, this should be able to always return a NavigationTarget | ||
282 | fn from_def(db: &db::RootDatabase, def: Def) -> Cancelable<Option<NavigationTarget>> { | ||
283 | Ok(match def { | ||
284 | Def::Struct(s) => { | ||
285 | let (file_id, node) = s.source(db)?; | ||
286 | Some(NavigationTarget::from_syntax( | ||
287 | s.name(db)?, | ||
288 | file_id.original_file(db), | ||
289 | node.syntax(), | ||
290 | )) | ||
291 | } | ||
292 | Def::Enum(e) => { | ||
293 | let (file_id, node) = e.source(db)?; | ||
294 | Some(NavigationTarget::from_syntax( | ||
295 | e.name(db)?, | ||
296 | file_id.original_file(db), | ||
297 | node.syntax(), | ||
298 | )) | ||
299 | } | ||
300 | Def::EnumVariant(ev) => { | ||
301 | let (file_id, node) = ev.source(db)?; | ||
302 | Some(NavigationTarget::from_syntax( | ||
303 | ev.name(db)?, | ||
304 | file_id.original_file(db), | ||
305 | node.syntax(), | ||
306 | )) | ||
307 | } | ||
308 | Def::Function(f) => { | ||
309 | let (file_id, node) = f.source(db)?; | ||
310 | let name = f.signature(db).name().clone(); | ||
311 | Some(NavigationTarget::from_syntax( | ||
312 | Some(name), | ||
313 | file_id.original_file(db), | ||
314 | node.syntax(), | ||
315 | )) | ||
316 | } | ||
317 | Def::Module(m) => { | ||
318 | let (file_id, source) = m.definition_source(db)?; | ||
319 | let name = m.name(db)?; | ||
320 | match source { | ||
321 | ModuleSource::SourceFile(node) => { | ||
322 | Some(NavigationTarget::from_syntax(name, file_id, node.syntax())) | ||
323 | } | ||
324 | ModuleSource::Module(node) => { | ||
325 | Some(NavigationTarget::from_syntax(name, file_id, node.syntax())) | ||
326 | } | ||
327 | } | ||
328 | } | ||
329 | Def::Item => None, | ||
330 | }) | ||
331 | } | ||
332 | |||
271 | pub fn name(&self) -> &SmolStr { | 333 | pub fn name(&self) -> &SmolStr { |
272 | &self.name | 334 | &self.name |
273 | } | 335 | } |