Explicitly check for reference locals or fields in Name classification

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::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field())?,
-            ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.referenced_or_defined())?,
+            ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.defined_or_referenced_field())?,
             _ => return None,
         }
     };

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,
         }

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();
+                let def = NameClass::classify(&sema, &name)?.defined_or_referenced_local();
-                try_find_trait_item_definition(sema.db, &def)
+                try_find_trait_item_definition(sema.db, &def).or_else(|| def.try_to_nav(sema.db))
-                    .or_else(|| def.try_to_nav(sema.db))
             },
             ast::Lifetime(lt) => if let Some(name_class) = NameClass::classify_lifetime(&sema, &lt) {
-                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(&lt))
@@ -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
+                 //^
     };
 }
 "#,

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,
     }?;

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 {
+            ast::Name(name) => NameClass::classify(&sema, &name).map(|class| class.defined_or_referenced_local()),
-                NameClass::ConstReference(def) => Some(def),
-                def => def.defined(),
-            }),
             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);

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<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::NameRef(name_ref) => {
-        ast::NameLike::Name(name) => NameClass::classify(sema, &name)?.referenced_or_defined(),
+            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)

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"))?;

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_ref.parent_def(db) {
-                        if let hir::VariantDef::Union(_) = field.parent_def(db) {
+                        h |= HlMod::Unsafe;
-                            h |= HlMod::Unsafe;
-                        }
                     }
                     h
                 }

crates/ide_assists/src/handlers/extract_function.rs

@@ -638,7 +638,7 @@ 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| name_kind.referenced_local())
         .filter_map(|definition| match definition {
             Definition::Local(local) => Some(local),
             _ => None,

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
+    /// `field` in `if let Foo { field } = foo`. Here, `ast::Name` both introduces
-    /// name both introduces a definition into a local scope, and refers to an
+    /// a definition into a local scope, and refers to an existing definition.
-    /// 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.
+    /// `Definition` referenced or defined by this name, in case of a shorthand this will yield the field reference.
-    pub fn referenced_or_defined(self) -> Definition {
+    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.
+    /// `Definition`, which this name refers to with a preference for the field reference in case of a field shorthand.
-    pub fn referenced(self) -> Definition {
+    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 }
                     }

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