From c65bb7f8b99f16f9d0ebec6276797cb21cfd7e47 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jul 2021 14:03:35 +0200 Subject: [PATCH 1/3] Explicitly check for reference locals or fields in Name classification --- crates/ide/src/doc_links.rs | 4 +- crates/ide/src/goto_declaration.rs | 4 +- crates/ide/src/goto_definition.rs | 12 ++--- crates/ide/src/goto_implementation.rs | 4 +- crates/ide/src/hover.rs | 10 ++-- crates/ide/src/references.rs | 14 ++++-- crates/ide/src/rename.rs | 9 ++-- .../ide/src/syntax_highlighting/highlight.rs | 6 +-- .../src/handlers/extract_function.rs | 2 +- crates/ide_db/src/defs.rs | 49 +++++++++++++------ crates/ide_db/src/search.rs | 3 +- 11 files changed, 67 insertions(+), 50 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 1c0380943e..00b6f7a5c3 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -112,8 +112,8 @@ pub(crate) fn external_docs( let node = token.parent()?; let definition = match_ast! { match node { - ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced())?, - ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.referenced_or_defined())?, + ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field())?, + ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.defined_or_referenced_field())?, _ => return None, } }; diff --git a/crates/ide/src/goto_declaration.rs b/crates/ide/src/goto_declaration.rs index 4f6b5e6186..59306ac065 100644 --- a/crates/ide/src/goto_declaration.rs +++ b/crates/ide/src/goto_declaration.rs @@ -25,10 +25,10 @@ pub(crate) fn goto_declaration( match parent { ast::NameRef(name_ref) => { let name_kind = NameRefClass::classify(&sema, &name_ref)?; - name_kind.referenced() + name_kind.referenced_local() }, ast::Name(name) => { - NameClass::classify(&sema, &name)?.referenced_or_defined() + NameClass::classify(&sema, &name)?.defined_or_referenced_local() }, _ => return None, } diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index d1ad6db2fd..e9ca233631 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -60,12 +60,11 @@ pub(crate) fn goto_definition( reference_definition(&sema, Either::Right(&name_ref)) }, ast::Name(name) => { - let def = NameClass::classify(&sema, &name)?.referenced_or_defined(); - try_find_trait_item_definition(sema.db, &def) - .or_else(|| def.try_to_nav(sema.db)) + let def = NameClass::classify(&sema, &name)?.defined_or_referenced_local(); + try_find_trait_item_definition(sema.db, &def).or_else(|| def.try_to_nav(sema.db)) }, ast::Lifetime(lt) => if let Some(name_class) = NameClass::classify_lifetime(&sema, <) { - let def = name_class.referenced_or_defined(); + let def = name_class.defined_or_referenced_local(); def.try_to_nav(sema.db) } else { reference_definition(&sema, Either::Left(<)) @@ -140,7 +139,7 @@ pub(crate) fn reference_definition( |lifetime| NameRefClass::classify_lifetime(sema, lifetime), |name_ref| NameRefClass::classify(sema, name_ref), )?; - let def = name_kind.referenced(); + let def = name_kind.referenced_local(); def.try_to_nav(sema.db) } @@ -878,10 +877,11 @@ fn main() { r#" enum Foo { Bar { x: i32 } -} //^ +} fn baz(foo: Foo) { match foo { Foo::Bar { x$0 } => x + //^ }; } "#, diff --git a/crates/ide/src/goto_implementation.rs b/crates/ide/src/goto_implementation.rs index 636642cfe7..d110123045 100644 --- a/crates/ide/src/goto_implementation.rs +++ b/crates/ide/src/goto_implementation.rs @@ -29,10 +29,10 @@ pub(crate) fn goto_implementation( let node = sema.find_node_at_offset_with_descend(&syntax, position.offset)?; let def = match &node { ast::NameLike::Name(name) => { - NameClass::classify(&sema, name).map(|class| class.referenced_or_defined()) + NameClass::classify(&sema, name).map(|class| class.defined_or_referenced_local()) } ast::NameLike::NameRef(name_ref) => { - NameRefClass::classify(&sema, name_ref).map(|class| class.referenced()) + NameRefClass::classify(&sema, name_ref).map(|class| class.referenced_local()) } ast::NameLike::Lifetime(_) => None, }?; diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 743aa183f7..8c0bbda536 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -96,18 +96,14 @@ pub(crate) fn hover( match node { // we don't use NameClass::referenced_or_defined here as we do not want to resolve // field pattern shorthands to their definition - ast::Name(name) => NameClass::classify(&sema, &name).and_then(|class| match class { - NameClass::ConstReference(def) => Some(def), - def => def.defined(), - }), + ast::Name(name) => NameClass::classify(&sema, &name).map(|class| class.defined_or_referenced_local()), ast::NameRef(name_ref) => { - NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced()) + NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field()) }, ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime).map_or_else( - || NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced()), + || NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced_local()), |d| d.defined(), ), - _ => { if ast::Comment::cast(token.clone()).is_some() { cov_mark::hit!(no_highlight_on_comment_hover); diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index fbe79741b3..9eeb2cf5af 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -58,7 +58,7 @@ pub(crate) fn find_all_refs( let (def, is_literal_search) = if let Some(name) = get_name_of_item_declaration(&syntax, position) { - (NameClass::classify(sema, &name)?.referenced_or_defined(), true) + (NameClass::classify(sema, &name)?.defined_or_referenced_field(), true) } else { (find_def(sema, &syntax, position.offset)?, false) }; @@ -116,13 +116,17 @@ pub(crate) fn find_def( offset: TextSize, ) -> Option { let def = match sema.find_node_at_offset_with_descend(syntax, offset)? { - ast::NameLike::NameRef(name_ref) => NameRefClass::classify(sema, &name_ref)?.referenced(), - ast::NameLike::Name(name) => NameClass::classify(sema, &name)?.referenced_or_defined(), + ast::NameLike::NameRef(name_ref) => { + NameRefClass::classify(sema, &name_ref)?.referenced_local() + } + ast::NameLike::Name(name) => { + NameClass::classify(sema, &name)?.defined_or_referenced_local() + } ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime) - .map(|class| class.referenced()) + .map(|class| class.referenced_local()) .or_else(|| { NameClass::classify_lifetime(sema, &lifetime) - .map(|class| class.referenced_or_defined()) + .map(|class| class.defined_or_referenced_local()) })?, }; Some(def) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 9047d0fb32..f19dcaeb80 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -108,11 +108,11 @@ fn find_definition( bail!("Renaming aliases is currently unsupported") } ast::NameLike::Name(name) => { - NameClass::classify(sema, &name).map(|class| class.referenced_or_defined()) + NameClass::classify(sema, &name).map(|class| class.defined_or_referenced_local()) } ast::NameLike::NameRef(name_ref) => { if let Some(def) = - NameRefClass::classify(sema, &name_ref).map(|class| class.referenced()) + NameRefClass::classify(sema, &name_ref).map(|class| class.referenced_local()) { // if the name differs from the definitions name it has to be an alias if def.name(sema.db).map_or(false, |it| it.to_string() != name_ref.text()) { @@ -124,9 +124,10 @@ fn find_definition( } } ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime) - .map(|class| class.referenced()) + .map(|class| class.referenced_local()) .or_else(|| { - NameClass::classify_lifetime(sema, &lifetime).map(|it| it.referenced_or_defined()) + NameClass::classify_lifetime(sema, &lifetime) + .map(|it| it.defined_or_referenced_field()) }), } .ok_or_else(|| format_err!("No references found at position"))?; diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index e16fe644d5..197a32da06 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -58,10 +58,8 @@ pub(super) fn element( Some(NameClass::ConstReference(def)) => highlight_def(db, krate, def), Some(NameClass::PatFieldShorthand { field_ref, .. }) => { let mut h = HlTag::Symbol(SymbolKind::Field).into(); - if let Definition::Field(field) = field_ref { - if let hir::VariantDef::Union(_) = field.parent_def(db) { - h |= HlMod::Unsafe; - } + if let hir::VariantDef::Union(_) = field_ref.parent_def(db) { + h |= HlMod::Unsafe; } h } diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index eb6f7a7e6c..ea2db0b4eb 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -638,7 +638,7 @@ fn vars_used_in_body(ctx: &AssistContext, body: &FunctionBody) -> Vec { body.descendants() .filter_map(ast::NameRef::cast) .filter_map(|name_ref| NameRefClass::classify(&ctx.sema, &name_ref)) - .map(|name_kind| name_kind.referenced()) + .map(|name_kind| name_kind.referenced_local()) .filter_map(|definition| match definition { Definition::Local(local) => Some(local), _ => None, diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs index eeae055349..a72345564d 100644 --- a/crates/ide_db/src/defs.rs +++ b/crates/ide_db/src/defs.rs @@ -112,12 +112,11 @@ pub enum NameClass { /// `None` in `if let None = Some(82) {}`. /// Syntactically, it is a name, but semantically it is a reference. ConstReference(Definition), - /// `field` in `if let Foo { field } = foo`. Here, `ast::Name` both Here the - /// name both introduces a definition into a local scope, and refers to an - /// existing definition. + /// `field` in `if let Foo { field } = foo`. Here, `ast::Name` both introduces + /// a definition into a local scope, and refers to an existing definition. PatFieldShorthand { local_def: Local, - field_ref: Definition, + field_ref: Field, }, } @@ -134,11 +133,23 @@ impl NameClass { Some(res) } - /// `Definition` referenced or defined by this name. - pub fn referenced_or_defined(self) -> Definition { + /// `Definition` referenced or defined by this name, in case of a shorthand this will yield the field reference. + pub fn defined_or_referenced_field(self) -> Definition { match self { NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def: _, field_ref } => field_ref, + NameClass::PatFieldShorthand { local_def: _, field_ref } => { + Definition::Field(field_ref) + } + } + } + + /// `Definition` referenced or defined by this name, in case of a shorthand this will yield the local definition. + pub fn defined_or_referenced_local(self) -> Definition { + match self { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => { + Definition::Local(local_def) + } } } @@ -183,7 +194,7 @@ impl NameClass { }) .and_then(|name_ref| NameRefClass::classify(sema, &name_ref))?; - Some(NameClass::Definition(name_ref_class.referenced())) + Some(NameClass::Definition(name_ref_class.referenced_field())) } else { let extern_crate = it.syntax().parent().and_then(ast::ExternCrate::cast)?; let krate = sema.resolve_extern_crate(&extern_crate)?; @@ -197,7 +208,6 @@ impl NameClass { if let Some(record_pat_field) = it.syntax().parent().and_then(ast::RecordPatField::cast) { if record_pat_field.name_ref().is_none() { if let Some(field) = sema.resolve_record_pat_field(&record_pat_field) { - let field = Definition::Field(field); return Some(NameClass::PatFieldShorthand { local_def: local, field_ref: field }); } } @@ -302,17 +312,25 @@ impl NameClass { #[derive(Debug)] pub enum NameRefClass { Definition(Definition), - FieldShorthand { local_ref: Local, field_ref: Definition }, + FieldShorthand { local_ref: Local, field_ref: Field }, } impl NameRefClass { - /// `Definition`, which this name refers to. - pub fn referenced(self) -> Definition { + /// `Definition`, which this name refers to with a preference for the field reference in case of a field shorthand. + pub fn referenced_field(self) -> Definition { + match self { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref: _, field_ref } => { + Definition::Field(field_ref) + } + } + } + + /// `Definition`, which this name refers to with a preference for the local reference in case of a field shorthand. + pub fn referenced_local(self) -> Definition { match self { NameRefClass::Definition(def) => def, NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { - // FIXME: this is inherently ambiguous -- this name refers to - // two different defs.... Definition::Local(local_ref) } } @@ -342,9 +360,8 @@ impl NameRefClass { if let Some(record_field) = ast::RecordExprField::for_field_name(name_ref) { if let Some((field, local, _)) = sema.resolve_record_field(&record_field) { - let field = Definition::Field(field); let res = match local { - None => NameRefClass::Definition(field), + None => NameRefClass::Definition(Definition::Field(field)), Some(local) => { NameRefClass::FieldShorthand { field_ref: field, local_ref: local } } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 7be76dca10..41254b7847 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -550,6 +550,7 @@ impl<'a> FindUsages<'a> { } } Some(NameRefClass::FieldShorthand { local_ref: local, field_ref: field }) => { + let field = Definition::Field(field); let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); let access = match self.def { Definition::Field(_) if field == self.def => reference_access(&field, name_ref), @@ -574,7 +575,7 @@ impl<'a> FindUsages<'a> { match NameClass::classify(self.sema, name) { Some(NameClass::PatFieldShorthand { local_def: _, field_ref }) if matches!( - self.def, Definition::Field(_) if field_ref == self.def + self.def, Definition::Field(_) if Definition::Field(field_ref) == self.def ) => { let FileRange { file_id, range } = self.sema.original_range(name.syntax()); From c41f1279a39e641918014e77e52de18061d07ba0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jul 2021 15:39:09 +0200 Subject: [PATCH 2/3] Inline name classification reference calls --- crates/ide/src/doc_links.rs | 12 ++++- crates/ide/src/goto_declaration.rs | 13 +++-- crates/ide/src/goto_definition.rs | 15 ++++-- crates/ide/src/goto_implementation.rs | 16 +++++-- crates/ide/src/hover.rs | 19 ++++++-- crates/ide/src/references.rs | 39 +++++++++++---- crates/ide/src/rename.rs | 29 ++++++++---- .../src/handlers/extract_function.rs | 7 ++- crates/ide_db/src/defs.rs | 47 +++---------------- 9 files changed, 117 insertions(+), 80 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 00b6f7a5c3..480adb8419 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -112,8 +112,16 @@ pub(crate) fn external_docs( let node = token.parent()?; let definition = match_ast! { match node { - ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field())?, - ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.defined_or_referenced_field())?, + ast::NameRef(name_ref) => match NameRefClass::classify(&sema, &name_ref)? { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref: _, field_ref } => { + Definition::Field(field_ref) + } + }, + ast::Name(name) => match NameClass::classify(&sema, &name)? { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def: _, field_ref } => Definition::Field(field_ref), + }, _ => return None, } }; diff --git a/crates/ide/src/goto_declaration.rs b/crates/ide/src/goto_declaration.rs index 59306ac065..9cc67b13e9 100644 --- a/crates/ide/src/goto_declaration.rs +++ b/crates/ide/src/goto_declaration.rs @@ -23,12 +23,15 @@ pub(crate) fn goto_declaration( let parent = token.parent()?; let def = match_ast! { match parent { - ast::NameRef(name_ref) => { - let name_kind = NameRefClass::classify(&sema, &name_ref)?; - name_kind.referenced_local() + ast::NameRef(name_ref) => match NameRefClass::classify(&sema, &name_ref)? { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } }, - ast::Name(name) => { - NameClass::classify(&sema, &name)?.defined_or_referenced_local() + 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), }, _ => return None, } diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index e9ca233631..55177d77e9 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -60,11 +60,17 @@ pub(crate) fn goto_definition( reference_definition(&sema, Either::Right(&name_ref)) }, ast::Name(name) => { - let def = NameClass::classify(&sema, &name)?.defined_or_referenced_local(); + 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)) }, ast::Lifetime(lt) => if let Some(name_class) = NameClass::classify_lifetime(&sema, <) { - let def = name_class.defined_or_referenced_local(); + 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) } else { reference_definition(&sema, Either::Left(<)) @@ -139,7 +145,10 @@ pub(crate) fn reference_definition( |lifetime| NameRefClass::classify_lifetime(sema, lifetime), |name_ref| NameRefClass::classify(sema, name_ref), )?; - let def = name_kind.referenced_local(); + let def = match name_kind { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => Definition::Local(local_ref), + }; def.try_to_nav(sema.db) } diff --git a/crates/ide/src/goto_implementation.rs b/crates/ide/src/goto_implementation.rs index d110123045..bde0fbd176 100644 --- a/crates/ide/src/goto_implementation.rs +++ b/crates/ide/src/goto_implementation.rs @@ -28,11 +28,19 @@ pub(crate) fn goto_implementation( let node = sema.find_node_at_offset_with_descend(&syntax, position.offset)?; let def = match &node { - ast::NameLike::Name(name) => { - NameClass::classify(&sema, name).map(|class| class.defined_or_referenced_local()) - } + ast::NameLike::Name(name) => NameClass::classify(&sema, name).map(|class| match class { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => { + Definition::Local(local_def) + } + }), ast::NameLike::NameRef(name_ref) => { - NameRefClass::classify(&sema, name_ref).map(|class| class.referenced_local()) + NameRefClass::classify(&sema, name_ref).map(|class| match class { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } + }) } ast::NameLike::Lifetime(_) => None, }?; diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 8c0bbda536..49a06a2851 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -96,12 +96,21 @@ pub(crate) fn hover( match node { // we don't use NameClass::referenced_or_defined here as we do not want to resolve // field pattern shorthands to their definition - ast::Name(name) => NameClass::classify(&sema, &name).map(|class| class.defined_or_referenced_local()), - ast::NameRef(name_ref) => { - NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field()) - }, + ast::Name(name) => NameClass::classify(&sema, &name).map(|class| match class { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => Definition::Local(local_def), + }), + ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|class| match class { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref: _, field_ref } => { + Definition::Field(field_ref) + } + }), ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime).map_or_else( - || NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced_local()), + || NameRefClass::classify_lifetime(&sema, &lifetime).and_then(|class| match class { + NameRefClass::Definition(it) => Some(it), + _ => None, + }), |d| d.defined(), ), _ => { diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index 9eeb2cf5af..6b08212c75 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -58,7 +58,15 @@ pub(crate) fn find_all_refs( let (def, is_literal_search) = if let Some(name) = get_name_of_item_declaration(&syntax, position) { - (NameClass::classify(sema, &name)?.defined_or_referenced_field(), true) + ( + match NameClass::classify(sema, &name)? { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def: _, field_ref } => { + Definition::Field(field_ref) + } + }, + true, + ) } else { (find_def(sema, &syntax, position.offset)?, false) }; @@ -116,17 +124,28 @@ pub(crate) fn find_def( offset: TextSize, ) -> Option { let def = match sema.find_node_at_offset_with_descend(syntax, offset)? { - ast::NameLike::NameRef(name_ref) => { - NameRefClass::classify(sema, &name_ref)?.referenced_local() - } - ast::NameLike::Name(name) => { - NameClass::classify(sema, &name)?.defined_or_referenced_local() - } + ast::NameLike::NameRef(name_ref) => match NameRefClass::classify(sema, &name_ref)? { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } + }, + ast::NameLike::Name(name) => match NameClass::classify(sema, &name)? { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => { + Definition::Local(local_def) + } + }, ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime) - .map(|class| class.referenced_local()) + .and_then(|class| match class { + NameRefClass::Definition(it) => Some(it), + _ => None, + }) .or_else(|| { - NameClass::classify_lifetime(sema, &lifetime) - .map(|class| class.defined_or_referenced_local()) + NameClass::classify_lifetime(sema, &lifetime).and_then(|class| match class { + NameClass::Definition(it) => Some(it), + _ => None, + }) })?, }; Some(def) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index f19dcaeb80..f58be6d3fd 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -107,13 +107,19 @@ fn find_definition( { bail!("Renaming aliases is currently unsupported") } - ast::NameLike::Name(name) => { - NameClass::classify(sema, &name).map(|class| class.defined_or_referenced_local()) - } + ast::NameLike::Name(name) => NameClass::classify(sema, &name).map(|class| match class { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => { + Definition::Local(local_def) + } + }), ast::NameLike::NameRef(name_ref) => { - if let Some(def) = - NameRefClass::classify(sema, &name_ref).map(|class| class.referenced_local()) - { + if let Some(def) = NameRefClass::classify(sema, &name_ref).map(|class| match class { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } + }) { // if the name differs from the definitions name it has to be an alias if def.name(sema.db).map_or(false, |it| it.to_string() != name_ref.text()) { bail!("Renaming aliases is currently unsupported"); @@ -124,10 +130,15 @@ fn find_definition( } } ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime) - .map(|class| class.referenced_local()) + .and_then(|class| match class { + NameRefClass::Definition(def) => Some(def), + _ => None, + }) .or_else(|| { - NameClass::classify_lifetime(sema, &lifetime) - .map(|it| it.defined_or_referenced_field()) + NameClass::classify_lifetime(sema, &lifetime).and_then(|it| match it { + NameClass::Definition(it) => Some(it), + _ => None, + }) }), } .ok_or_else(|| format_err!("No references found at position"))?; diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index ea2db0b4eb..4f0b1f5b79 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -638,7 +638,12 @@ fn vars_used_in_body(ctx: &AssistContext, body: &FunctionBody) -> Vec { body.descendants() .filter_map(ast::NameRef::cast) .filter_map(|name_ref| NameRefClass::classify(&ctx.sema, &name_ref)) - .map(|name_kind| name_kind.referenced_local()) + .map(|name_kind| match name_kind { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } + }) .filter_map(|definition| match definition { Definition::Local(local) => Some(local), _ => None, diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs index a72345564d..198b568ad8 100644 --- a/crates/ide_db/src/defs.rs +++ b/crates/ide_db/src/defs.rs @@ -133,26 +133,6 @@ impl NameClass { Some(res) } - /// `Definition` referenced or defined by this name, in case of a shorthand this will yield the field reference. - pub fn defined_or_referenced_field(self) -> Definition { - match self { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def: _, field_ref } => { - Definition::Field(field_ref) - } - } - } - - /// `Definition` referenced or defined by this name, in case of a shorthand this will yield the local definition. - pub fn defined_or_referenced_local(self) -> Definition { - match self { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def, field_ref: _ } => { - Definition::Local(local_def) - } - } - } - pub fn classify(sema: &Semantics, name: &ast::Name) -> Option { let _p = profile::span("classify_name"); @@ -194,7 +174,12 @@ impl NameClass { }) .and_then(|name_ref| NameRefClass::classify(sema, &name_ref))?; - Some(NameClass::Definition(name_ref_class.referenced_field())) + Some(NameClass::Definition(match name_ref_class { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref: _, field_ref } => { + Definition::Field(field_ref) + } + })) } else { let extern_crate = it.syntax().parent().and_then(ast::ExternCrate::cast)?; let krate = sema.resolve_extern_crate(&extern_crate)?; @@ -316,26 +301,6 @@ pub enum NameRefClass { } impl NameRefClass { - /// `Definition`, which this name refers to with a preference for the field reference in case of a field shorthand. - pub fn referenced_field(self) -> Definition { - match self { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref: _, field_ref } => { - Definition::Field(field_ref) - } - } - } - - /// `Definition`, which this name refers to with a preference for the local reference in case of a field shorthand. - pub fn referenced_local(self) -> Definition { - match self { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { - Definition::Local(local_ref) - } - } - } - // Note: we don't have unit-tests for this rather important function. // It is primarily exercised via goto definition tests in `ide`. pub fn classify( From ab2647769c611dc7f7696d940c9e09c1cc966f83 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jul 2021 16:16:16 +0200 Subject: [PATCH 3/3] 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 cf679edd3b..7c73751045 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 9cc67b13e9..2e4132b6a0 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 55177d77e9..595c0ec05c 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)>, {} //^^ "#,