From ab2647769c611dc7f7696d940c9e09c1cc966f83 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jul 2021 16:16:16 +0200 Subject: [PATCH] Return both field and local references for shorthands in goto_def --- crates/ide/src/fixture.rs | 8 --- crates/ide/src/goto_declaration.rs | 36 ++++++---- crates/ide/src/goto_definition.rs | 104 +++++++++++++++++++---------- 3 files changed, 92 insertions(+), 56 deletions(-) diff --git a/crates/ide/src/fixture.rs b/crates/ide/src/fixture.rs index cf679edd3b3..7c737510451 100644 --- a/crates/ide/src/fixture.rs +++ b/crates/ide/src/fixture.rs @@ -51,11 +51,3 @@ pub(crate) fn annotations(ra_fixture: &str) -> (Analysis, FilePosition, Vec<(Fil .collect(); (host.analysis(), FilePosition { file_id, offset }, annotations) } - -pub(crate) fn nav_target_annotation(ra_fixture: &str) -> (Analysis, FilePosition, FileRange) { - let (analysis, position, mut annotations) = annotations(ra_fixture); - let (expected, data) = annotations.pop().unwrap(); - assert!(annotations.is_empty()); - assert_eq!(data, ""); - (analysis, position, expected) -} diff --git a/crates/ide/src/goto_declaration.rs b/crates/ide/src/goto_declaration.rs index 9cc67b13e91..2e4132b6a0c 100644 --- a/crates/ide/src/goto_declaration.rs +++ b/crates/ide/src/goto_declaration.rs @@ -24,36 +24,35 @@ pub(crate) fn goto_declaration( let def = match_ast! { match parent { ast::NameRef(name_ref) => match NameRefClass::classify(&sema, &name_ref)? { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { - Definition::Local(local_ref) - } + NameRefClass::Definition(it) => Some(it), + _ => None }, ast::Name(name) => match NameClass::classify(&sema, &name)? { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def, field_ref: _ } => Definition::Local(local_def), + NameClass::Definition(it) => Some(it), + _ => None }, - _ => return None, + _ => None, } }; - match def { + match def? { Definition::ModuleDef(hir::ModuleDef::Module(module)) => Some(RangeInfo::new( original_token.text_range(), vec![NavigationTarget::from_module_to_decl(db, module)], )), - _ => return None, + _ => None, } } #[cfg(test)] mod tests { use ide_db::base_db::FileRange; + use itertools::Itertools; use crate::fixture; fn check(ra_fixture: &str) { - let (analysis, position, expected) = fixture::nav_target_annotation(ra_fixture); - let mut navs = analysis + let (analysis, position, expected) = fixture::annotations(ra_fixture); + let navs = analysis .goto_declaration(position) .unwrap() .expect("no declaration or definition found") @@ -61,10 +60,19 @@ mod tests { if navs.len() == 0 { panic!("unresolved reference") } - assert_eq!(navs.len(), 1); - let nav = navs.pop().unwrap(); - assert_eq!(expected, FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }); + let cmp = |&FileRange { file_id, range }: &_| (file_id, range.start()); + let navs = navs + .into_iter() + .map(|nav| FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }) + .sorted_by_key(cmp) + .collect::>(); + let expected = expected + .into_iter() + .map(|(FileRange { file_id, range }, _)| FileRange { file_id, range }) + .sorted_by_key(cmp) + .collect::>(); + assert_eq!(expected, navs); } #[test] diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 55177d77e94..595c0ec05c4 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -1,4 +1,4 @@ -use std::convert::TryInto; +use std::{convert::TryInto, iter}; use either::Either; use hir::{AsAssocItem, InFile, ModuleDef, Semantics}; @@ -11,7 +11,7 @@ use ide_db::{ use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TextRange, T}; use crate::{ - display::TryToNav, + display::{ToNav, TryToNav}, doc_links::{doc_attributes, extract_definitions_from_markdown, resolve_doc_path_for_def}, FilePosition, NavigationTarget, RangeInfo, }; @@ -54,33 +54,36 @@ pub(crate) fn goto_definition( let nav = resolve_doc_path_for_def(db, def, &link, ns)?.try_to_nav(db)?; return Some(RangeInfo::new(original_token.text_range(), vec![nav])); } - let nav = match_ast! { + + let navs = match_ast! { match parent { ast::NameRef(name_ref) => { reference_definition(&sema, Either::Right(&name_ref)) }, ast::Name(name) => { - let def = match NameClass::classify(&sema, &name)? { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def, field_ref: _ } => Definition::Local(local_def), - }; - try_find_trait_item_definition(sema.db, &def).or_else(|| def.try_to_nav(sema.db)) + match NameClass::classify(&sema, &name)? { + NameClass::Definition(def) | NameClass::ConstReference(def) => { + try_find_trait_item_definition(sema.db, &def).unwrap_or_else(|| def_to_nav(sema.db, def)) + } + NameClass::PatFieldShorthand { local_def, field_ref } => { + local_and_field_to_nav(sema.db, local_def, field_ref) + }, + } }, ast::Lifetime(lt) => if let Some(name_class) = NameClass::classify_lifetime(&sema, <) { - let def = match name_class { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def, field_ref: _ } => Definition::Local(local_def), - }; - def.try_to_nav(sema.db) + match name_class { + NameClass::Definition(def) => def_to_nav(sema.db, def), + _ => return None, + } } else { reference_definition(&sema, Either::Left(<)) }, - ast::TokenTree(tt) => try_lookup_include_path(sema.db, tt, token, position.file_id), + ast::TokenTree(tt) => try_lookup_include_path(sema.db, tt, token, position.file_id)?, _ => return None, } }; - Some(RangeInfo::new(original_token.text_range(), nav.into_iter().collect())) + Some(RangeInfo::new(original_token.text_range(), navs)) } fn try_lookup_include_path( @@ -88,7 +91,7 @@ fn try_lookup_include_path( tt: ast::TokenTree, token: SyntaxToken, file_id: FileId, -) -> Option { +) -> Option> { let path = ast::String::cast(token)?.value()?.into_owned(); let macro_call = tt.syntax().parent().and_then(ast::MacroCall::cast)?; let name = macro_call.path()?.segment()?.name_ref()?; @@ -97,7 +100,7 @@ fn try_lookup_include_path( } let file_id = db.resolve_path(AnchoredPath { anchor: file_id, path: &path })?; let size = db.file_text(file_id).len().try_into().ok()?; - Some(NavigationTarget { + Some(vec![NavigationTarget { file_id, full_range: TextRange::new(0.into(), size), name: path.into(), @@ -106,7 +109,7 @@ fn try_lookup_include_path( container_name: None, description: None, docs: None, - }) + }]) } /// finds the trait definition of an impl'd item @@ -116,7 +119,10 @@ fn try_lookup_include_path( /// struct S; /// impl A for S { fn a(); } // <-- on this function, will get the location of a() in the trait /// ``` -fn try_find_trait_item_definition(db: &RootDatabase, def: &Definition) -> Option { +fn try_find_trait_item_definition( + db: &RootDatabase, + def: &Definition, +) -> Option> { let name = def.name(db)?; let assoc = match def { Definition::ModuleDef(ModuleDef::Function(f)) => f.as_assoc_item(db), @@ -135,40 +141,66 @@ fn try_find_trait_item_definition(db: &RootDatabase, def: &Definition) -> Option .items(db) .iter() .find_map(|itm| (itm.name(db)? == name).then(|| itm.try_to_nav(db)).flatten()) + .map(|it| vec![it]) } pub(crate) fn reference_definition( sema: &Semantics, name_ref: Either<&ast::Lifetime, &ast::NameRef>, -) -> Option { - let name_kind = name_ref.either( +) -> Vec { + let name_kind = match name_ref.either( |lifetime| NameRefClass::classify_lifetime(sema, lifetime), |name_ref| NameRefClass::classify(sema, name_ref), - )?; - let def = match name_kind { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref, field_ref: _ } => Definition::Local(local_ref), + ) { + Some(class) => class, + None => return Vec::new(), }; - def.try_to_nav(sema.db) + match name_kind { + NameRefClass::Definition(def) => def_to_nav(sema.db, def), + NameRefClass::FieldShorthand { local_ref, field_ref } => { + local_and_field_to_nav(sema.db, local_ref, field_ref) + } + } +} + +fn def_to_nav(db: &RootDatabase, def: Definition) -> Vec { + def.try_to_nav(db).map(|it| vec![it]).unwrap_or_default() +} + +fn local_and_field_to_nav( + db: &RootDatabase, + local: hir::Local, + field: hir::Field, +) -> Vec { + iter::once(local.to_nav(db)).chain(field.try_to_nav(db)).collect() } #[cfg(test)] mod tests { use ide_db::base_db::FileRange; + use itertools::Itertools; use crate::fixture; fn check(ra_fixture: &str) { - let (analysis, position, expected) = fixture::nav_target_annotation(ra_fixture); - let mut navs = - analysis.goto_definition(position).unwrap().expect("no definition found").info; + let (analysis, position, expected) = fixture::annotations(ra_fixture); + let navs = analysis.goto_definition(position).unwrap().expect("no definition found").info; if navs.len() == 0 { panic!("unresolved reference") } - assert_eq!(navs.len(), 1); - let nav = navs.pop().unwrap(); - assert_eq!(expected, FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }); + let cmp = |&FileRange { file_id, range }: &_| (file_id, range.start()); + let navs = navs + .into_iter() + .map(|nav| FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }) + .sorted_by_key(cmp) + .collect::>(); + let expected = expected + .into_iter() + .map(|(FileRange { file_id, range }, _)| FileRange { file_id, range }) + .sorted_by_key(cmp) + .collect::>(); + assert_eq!(expected, navs); } fn check_unresolved(ra_fixture: &str) { @@ -871,6 +903,7 @@ fn bar() { check( r#" struct Foo { x: i32 } + //^ fn main() { let x = 92; //^ @@ -886,6 +919,7 @@ fn main() { r#" enum Foo { Bar { x: i32 } + //^ } fn baz(foo: Foo) { match foo { @@ -1135,13 +1169,15 @@ fn foo<'foobar>(_: &'foobar ()) { fn goto_lifetime_hrtb() { // FIXME: requires the HIR to somehow track these hrtb lifetimes check_unresolved( - r#"trait Foo {} + r#" +trait Foo {} fn foo() where for<'a> T: Foo<&'a$0 (u8, u16)>, {} //^^ "#, ); check_unresolved( - r#"trait Foo {} + r#" +trait Foo {} fn foo() where for<'a$0> T: Foo<&'a (u8, u16)>, {} //^^ "#,