9569: internal: Explicitly check for reference locals or fields in Name classification r=Veykril a=Veykril

Closes #7524
Inlines all the calls to reference related name classification as well as emits both goto definition targets for field shorthands.

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2021-07-11 14:28:08 +00:00 committed by GitHub
commit fe00358888
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 202 additions and 112 deletions

View file

@ -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())?,
ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.referenced_or_defined())?,
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,
}
};

View file

@ -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)
}

View file

@ -23,34 +23,36 @@ 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()
ast::NameRef(name_ref) => match NameRefClass::classify(&sema, &name_ref)? {
NameRefClass::Definition(it) => Some(it),
_ => None
},
ast::Name(name) => {
NameClass::classify(&sema, &name)?.referenced_or_defined()
ast::Name(name) => match NameClass::classify(&sema, &name)? {
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")
@ -58,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::<Vec<_>>();
let expected = expected
.into_iter()
.map(|(FileRange { file_id, range }, _)| FileRange { file_id, range })
.sorted_by_key(cmp)
.collect::<Vec<_>>();
assert_eq!(expected, navs);
}
#[test]

View file

@ -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,28 +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 = NameClass::classify(&sema, &name)?.referenced_or_defined();
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, &lt) {
let def = name_class.referenced_or_defined();
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(&lt))
},
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(
@ -83,7 +91,7 @@ fn try_lookup_include_path(
tt: ast::TokenTree,
token: SyntaxToken,
file_id: FileId,
) -> Option<NavigationTarget> {
) -> Option<Vec<NavigationTarget>> {
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()?;
@ -92,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(),
@ -101,7 +109,7 @@ fn try_lookup_include_path(
container_name: None,
description: None,
docs: None,
})
}])
}
/// finds the trait definition of an impl'd item
@ -111,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<NavigationTarget> {
fn try_find_trait_item_definition(
db: &RootDatabase,
def: &Definition,
) -> Option<Vec<NavigationTarget>> {
let name = def.name(db)?;
let assoc = match def {
Definition::ModuleDef(ModuleDef::Function(f)) => f.as_assoc_item(db),
@ -130,37 +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<RootDatabase>,
name_ref: Either<&ast::Lifetime, &ast::NameRef>,
) -> Option<NavigationTarget> {
let name_kind = name_ref.either(
) -> Vec<NavigationTarget> {
let name_kind = match name_ref.either(
|lifetime| NameRefClass::classify_lifetime(sema, lifetime),
|name_ref| NameRefClass::classify(sema, name_ref),
)?;
let def = name_kind.referenced();
def.try_to_nav(sema.db)
) {
Some(class) => class,
None => return Vec::new(),
};
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<NavigationTarget> {
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<NavigationTarget> {
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::<Vec<_>>();
let expected = expected
.into_iter()
.map(|(FileRange { file_id, range }, _)| FileRange { file_id, range })
.sorted_by_key(cmp)
.collect::<Vec<_>>();
assert_eq!(expected, navs);
}
fn check_unresolved(ra_fixture: &str) {
@ -863,6 +903,7 @@ fn bar() {
check(
r#"
struct Foo { x: i32 }
//^
fn main() {
let x = 92;
//^
@ -878,10 +919,12 @@ fn main() {
r#"
enum Foo {
Bar { x: i32 }
} //^
//^
}
fn baz(foo: Foo) {
match foo {
Foo::Bar { x$0 } => x
//^
};
}
"#,
@ -1126,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<T> {}
r#"
trait Foo<T> {}
fn foo<T>() where for<'a> T: Foo<&'a$0 (u8, u16)>, {}
//^^
"#,
);
check_unresolved(
r#"trait Foo<T> {}
r#"
trait Foo<T> {}
fn foo<T>() where for<'a$0> T: Foo<&'a (u8, u16)>, {}
//^^
"#,

View file

@ -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.referenced_or_defined())
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())
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,
}?;

View file

@ -96,18 +96,23 @@ 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| 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::NameRef(name_ref) => {
NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced())
},
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).and_then(|class| match class {
NameRefClass::Definition(it) => Some(it),
_ => None,
}),
|d| d.defined(),
),
_ => {
if ast::Comment::cast(token.clone()).is_some() {
cov_mark::hit!(no_highlight_on_comment_hover);

View file

@ -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)?.referenced_or_defined(), 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,13 +124,28 @@ pub(crate) fn find_def(
offset: TextSize,
) -> Option<Definition> {
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) => 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())
.and_then(|class| match class {
NameRefClass::Definition(it) => Some(it),
_ => None,
})
.or_else(|| {
NameClass::classify_lifetime(sema, &lifetime)
.map(|class| class.referenced_or_defined())
NameClass::classify_lifetime(sema, &lifetime).and_then(|class| match class {
NameClass::Definition(it) => Some(it),
_ => None,
})
})?,
};
Some(def)

View file

@ -107,13 +107,19 @@ fn find_definition(
{
bail!("Renaming aliases is currently unsupported")
}
ast::NameLike::Name(name) => {
NameClass::classify(sema, &name).map(|class| class.referenced_or_defined())
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())
{
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,9 +130,15 @@ fn find_definition(
}
}
ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime)
.map(|class| class.referenced())
.and_then(|class| match class {
NameRefClass::Definition(def) => Some(def),
_ => None,
})
.or_else(|| {
NameClass::classify_lifetime(sema, &lifetime).map(|it| it.referenced_or_defined())
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"))?;

View file

@ -58,11 +58,9 @@ 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) {
if let hir::VariantDef::Union(_) = field_ref.parent_def(db) {
h |= HlMod::Unsafe;
}
}
h
}
None => highlight_name_by_syntax(name) | HlMod::Definition,

View file

@ -638,7 +638,12 @@ fn vars_used_in_body(ctx: &AssistContext, body: &FunctionBody) -> Vec<Local> {
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| 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,

View file

@ -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,14 +133,6 @@ impl NameClass {
Some(res)
}
/// `Definition` referenced or defined by this name.
pub fn referenced_or_defined(self) -> Definition {
match self {
NameClass::Definition(it) | NameClass::ConstReference(it) => it,
NameClass::PatFieldShorthand { local_def: _, field_ref } => field_ref,
}
}
pub fn classify(sema: &Semantics<RootDatabase>, name: &ast::Name) -> Option<NameClass> {
let _p = profile::span("classify_name");
@ -183,7 +174,12 @@ impl NameClass {
})
.and_then(|name_ref| NameRefClass::classify(sema, &name_ref))?;
Some(NameClass::Definition(name_ref_class.referenced()))
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)?;
@ -197,7 +193,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,22 +297,10 @@ 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 {
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)
}
}
}
// 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(
@ -342,9 +325,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 }
}

View file

@ -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());