Skip to content

Commit 58b47f9

Browse files
VeykrilVeykril
committed
Explicitly check for reference locals or fields in Name classification
1 parent 0fbeacc commit 58b47f9

File tree

11 files changed

+63
-47
lines changed

11 files changed

+63
-47
lines changed

‎crates/ide/src/doc_links.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ pub(crate) fn external_docs(
112112
let node = token.parent()?;
113113
let definition = match_ast! {
114114
match node {
115-
ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(sema.db))?,
116-
ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.referenced_or_defined(sema.db))?,
115+
ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field(sema.db))?,
116+
ast::Name(name) => NameClass::classify(&sema, &name).map(|d| d.defined_or_referenced_field(sema.db))?,
117117
_ => return None,
118118
}
119119
};

‎crates/ide/src/goto_declaration.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ pub(crate) fn goto_declaration(
2525
match parent {
2626
ast::NameRef(name_ref) => {
2727
let name_kind = NameRefClass::classify(&sema, &name_ref)?;
28-
name_kind.referenced(sema.db)
28+
name_kind.referenced_local(sema.db)
2929
},
3030
ast::Name(name) => {
31-
NameClass::classify(&sema, &name)?.referenced_or_defined(sema.db)
31+
NameClass::classify(&sema, &name)?.defined_or_referenced_local(sema.db)
3232
},
3333
_ => return None,
3434
}

‎crates/ide/src/goto_definition.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ pub(crate) fn goto_definition(
6060
reference_definition(&sema, Either::Right(&name_ref))
6161
},
6262
ast::Name(name) => {
63-
let def = NameClass::classify(&sema, &name)?.referenced_or_defined(sema.db);
63+
let def = NameClass::classify(&sema, &name)?.defined_or_referenced_local(sema.db);
6464
try_find_trait_item_definition(sema.db, &def)
6565
.or_else(|| def.try_to_nav(sema.db))
6666
},
6767
ast::Lifetime(lt) => if let Some(name_class) = NameClass::classify_lifetime(&sema, &lt) {
68-
let def = name_class.referenced_or_defined(sema.db);
68+
let def = name_class.defined_or_referenced_local(sema.db);
6969
def.try_to_nav(sema.db)
7070
} else {
7171
reference_definition(&sema, Either::Left(&lt))
@@ -140,7 +140,7 @@ pub(crate) fn reference_definition(
140140
|lifetime| NameRefClass::classify_lifetime(sema, lifetime),
141141
|name_ref| NameRefClass::classify(sema, name_ref),
142142
)?;
143-
let def = name_kind.referenced(sema.db);
143+
let def = name_kind.referenced_local(sema.db);
144144
def.try_to_nav(sema.db)
145145
}
146146

@@ -878,10 +878,11 @@ fn main() {
878878
r#"
879879
enum Foo {
880880
Bar { x: i32 }
881-
} //^
881+
}
882882
fn baz(foo: Foo) {
883883
match foo {
884884
Foo::Bar { x$0 } => x
885+
//^
885886
};
886887
}
887888
"#,

‎crates/ide/src/goto_implementation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ pub(crate) fn goto_implementation(
2929
let node = sema.find_node_at_offset_with_descend(&syntax, position.offset)?;
3030
let def = match &node {
3131
ast::NameLike::Name(name) => {
32-
NameClass::classify(&sema, name).map(|class| class.referenced_or_defined(sema.db))
32+
NameClass::classify(&sema, name).map(|class| class.defined_or_referenced_local(sema.db))
3333
}
3434
ast::NameLike::NameRef(name_ref) => {
35-
NameRefClass::classify(&sema, name_ref).map(|class| class.referenced(sema.db))
35+
NameRefClass::classify(&sema, name_ref).map(|class| class.referenced_local(sema.db))
3636
}
3737
ast::NameLike::Lifetime(_) => None,
3838
}?;

‎crates/ide/src/hover.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,12 @@ pub(crate) fn hover(
9696
match node {
9797
// we don't use NameClass::referenced_or_defined here as we do not want to resolve
9898
// field pattern shorthands to their definition
99-
ast::Name(name) => NameClass::classify(&sema, &name).and_then(|class| match class {
100-
NameClass::ConstReference(def) => Some(def),
101-
def => def.defined(db),
102-
}),
99+
ast::Name(name) => NameClass::classify(&sema, &name).map(|class| class.defined_or_referenced_local(db)),
103100
ast::NameRef(name_ref) => {
104-
NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(db))
101+
NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced_field(db))
105102
},
106103
ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime).map_or_else(
107-
|| NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(db)),
104+
|| NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced_local(db)),
108105
|d| d.defined(db),
109106
),
110107

‎crates/ide/src/references.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub(crate) fn find_all_refs(
5858

5959
let (def, is_literal_search) =
6060
if let Some(name) = get_name_of_item_declaration(&syntax, position) {
61-
(NameClass::classify(sema, &name)?.referenced_or_defined(sema.db), true)
61+
(NameClass::classify(sema, &name)?.defined_or_referenced_field(sema.db), true)
6262
} else {
6363
(find_def(sema, &syntax, position.offset)?, false)
6464
};
@@ -117,16 +117,16 @@ pub(crate) fn find_def(
117117
) -> Option<Definition> {
118118
let def = match sema.find_node_at_offset_with_descend(syntax, offset)? {
119119
ast::NameLike::NameRef(name_ref) => {
120-
NameRefClass::classify(sema, &name_ref)?.referenced(sema.db)
120+
NameRefClass::classify(sema, &name_ref)?.referenced_local(sema.db)
121121
}
122122
ast::NameLike::Name(name) => {
123-
NameClass::classify(sema, &name)?.referenced_or_defined(sema.db)
123+
NameClass::classify(sema, &name)?.defined_or_referenced_local(sema.db)
124124
}
125125
ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime)
126-
.map(|class| class.referenced(sema.db))
126+
.map(|class| class.referenced_local(sema.db))
127127
.or_else(|| {
128128
NameClass::classify_lifetime(sema, &lifetime)
129-
.map(|class| class.referenced_or_defined(sema.db))
129+
.map(|class| class.defined_or_referenced_local(sema.db))
130130
})?,
131131
};
132132
Some(def)

‎crates/ide/src/rename.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ fn find_definition(
108108
bail!("Renaming aliases is currently unsupported")
109109
}
110110
ast::NameLike::Name(name) => {
111-
NameClass::classify(sema, &name).map(|class| class.referenced_or_defined(sema.db))
111+
NameClass::classify(sema, &name).map(|class| class.defined_or_referenced_local(sema.db))
112112
}
113113
ast::NameLike::NameRef(name_ref) => {
114114
if let Some(def) =
115-
NameRefClass::classify(sema, &name_ref).map(|class| class.referenced(sema.db))
115+
NameRefClass::classify(sema, &name_ref).map(|class| class.referenced_local(sema.db))
116116
{
117117
// if the name differs from the definitions name it has to be an alias
118118
if def.name(sema.db).map_or(false, |it| it.to_string() != name_ref.text()) {
@@ -124,10 +124,10 @@ fn find_definition(
124124
}
125125
}
126126
ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime)
127-
.map(|class| NameRefClass::referenced(class, sema.db))
127+
.map(|class| NameRefClass::referenced_local(class, sema.db))
128128
.or_else(|| {
129129
NameClass::classify_lifetime(sema, &lifetime)
130-
.map(|it| it.referenced_or_defined(sema.db))
130+
.map(|it| it.defined_or_referenced_field(sema.db))
131131
}),
132132
}
133133
.ok_or_else(|| format_err!("No references found at position"))?;

‎crates/ide/src/syntax_highlighting/highlight.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,8 @@ pub(super) fn element(
5959
Some(NameClass::ConstReference(def)) => highlight_def(db, krate, def),
6060
Some(NameClass::PatFieldShorthand { field_ref, .. }) => {
6161
let mut h = HlTag::Symbol(SymbolKind::Field).into();
62-
if let Definition::Field(field) = field_ref {
63-
if let hir::VariantDef::Union(_) = field.parent_def(db) {
64-
h |= HlMod::Unsafe;
65-
}
62+
if let hir::VariantDef::Union(_) = field_ref.parent_def(db) {
63+
h |= HlMod::Unsafe;
6664
}
6765
h
6866
}

‎crates/ide_assists/src/handlers/extract_function.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ fn vars_used_in_body(ctx: &AssistContext, body: &FunctionBody) -> Vec<Local> {
638638
body.descendants()
639639
.filter_map(ast::NameRef::cast)
640640
.filter_map(|name_ref| NameRefClass::classify(&ctx.sema, &name_ref))
641-
.map(|name_kind| name_kind.referenced(ctx.db()))
641+
.map(|name_kind| name_kind.referenced_local(ctx.db()))
642642
.filter_map(|definition| match definition {
643643
Definition::Local(local) => Some(local),
644644
_ => None,

‎crates/ide_db/src/defs.rs

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,11 @@ pub enum NameClass {
113113
/// `None` in `if let None = Some(82) {}`.
114114
/// Syntactically, it is a name, but semantically it is a reference.
115115
ConstReference(Definition),
116-
/// `field` in `if let Foo { field } = foo`. Here, `ast::Name` both Here the
117-
/// name both introduces a definition into a local scope, and refers to an
118-
/// existing definition.
116+
/// `field` in `if let Foo { field } = foo`. Here, `ast::Name` both introduces
117+
/// a definition into a local scope, and refers to an existing definition.
119118
PatFieldShorthand {
120119
local_def: Local,
121-
field_ref: Definition,
120+
field_ref: Field,
122121
},
123122
}
124123

@@ -136,12 +135,25 @@ impl NameClass {
136135
Some(res)
137136
}
138137

139-
/// `Definition` referenced or defined by this name.
140-
pub fn referenced_or_defined(self, db: &dyn HirDatabase) -> Definition {
138+
/// `Definition` referenced or defined by this name, in case of a shorthand this will yield the field reference.
139+
pub fn defined_or_referenced_field(self, db: &dyn HirDatabase) -> Definition {
141140
match self {
142141
NameClass::ExternCrate(krate) => Definition::ModuleDef(krate.root_module(db).into()),
143142
NameClass::Definition(it) | NameClass::ConstReference(it) => it,
144-
NameClass::PatFieldShorthand { local_def: _, field_ref } => field_ref,
143+
NameClass::PatFieldShorthand { local_def: _, field_ref } => {
144+
Definition::Field(field_ref)
145+
}
146+
}
147+
}
148+
149+
/// `Definition` referenced or defined by this name, in case of a shorthand this will yield the local definition.
150+
pub fn defined_or_referenced_local(self, db: &dyn HirDatabase) -> Definition {
151+
match self {
152+
NameClass::ExternCrate(krate) => Definition::ModuleDef(krate.root_module(db).into()),
153+
NameClass::Definition(it) | NameClass::ConstReference(it) => it,
154+
NameClass::PatFieldShorthand { local_def, field_ref: _ } => {
155+
Definition::Local(local_def)
156+
}
145157
}
146158
}
147159

@@ -186,7 +198,7 @@ impl NameClass {
186198
})
187199
.and_then(|name_ref| NameRefClass::classify(sema, &name_ref))?;
188200

189-
Some(NameClass::Definition(name_ref_class.referenced(sema.db)))
201+
Some(NameClass::Definition(name_ref_class.referenced_field(sema.db)))
190202
} else {
191203
let extern_crate = it.syntax().parent().and_then(ast::ExternCrate::cast)?;
192204
let resolved = sema.resolve_extern_crate(&extern_crate)?;
@@ -199,7 +211,6 @@ impl NameClass {
199211
if let Some(record_pat_field) = it.syntax().parent().and_then(ast::RecordPatField::cast) {
200212
if record_pat_field.name_ref().is_none() {
201213
if let Some(field) = sema.resolve_record_pat_field(&record_pat_field) {
202-
let field = Definition::Field(field);
203214
return Some(NameClass::PatFieldShorthand { local_def: local, field_ref: field });
204215
}
205216
}
@@ -305,18 +316,27 @@ impl NameClass {
305316
pub enum NameRefClass {
306317
ExternCrate(Crate),
307318
Definition(Definition),
308-
FieldShorthand { local_ref: Local, field_ref: Definition },
319+
FieldShorthand { local_ref: Local, field_ref: Field },
309320
}
310321

311322
impl NameRefClass {
312-
/// `Definition`, which this name refers to.
313-
pub fn referenced(self, db: &dyn HirDatabase) -> Definition {
323+
/// `Definition`, which this name refers to with a preference for the field reference in case of a field shorthand.
324+
pub fn referenced_field(self, db: &dyn HirDatabase) -> Definition {
325+
match self {
326+
NameRefClass::ExternCrate(krate) => Definition::ModuleDef(krate.root_module(db).into()),
327+
NameRefClass::Definition(def) => def,
328+
NameRefClass::FieldShorthand { local_ref: _, field_ref } => {
329+
Definition::Field(field_ref)
330+
}
331+
}
332+
}
333+
334+
/// `Definition`, which this name refers to with a preference for the local reference in case of a field shorthand.
335+
pub fn referenced_local(self, db: &dyn HirDatabase) -> Definition {
314336
match self {
315337
NameRefClass::ExternCrate(krate) => Definition::ModuleDef(krate.root_module(db).into()),
316338
NameRefClass::Definition(def) => def,
317339
NameRefClass::FieldShorthand { local_ref, field_ref: _ } => {
318-
// FIXME: this is inherently ambiguous -- this name refers to
319-
// two different defs....
320340
Definition::Local(local_ref)
321341
}
322342
}
@@ -346,9 +366,8 @@ impl NameRefClass {
346366

347367
if let Some(record_field) = ast::RecordExprField::for_field_name(name_ref) {
348368
if let Some((field, local, _)) = sema.resolve_record_field(&record_field) {
349-
let field = Definition::Field(field);
350369
let res = match local {
351-
None => NameRefClass::Definition(field),
370+
None => NameRefClass::Definition(Definition::Field(field)),
352371
Some(local) => {
353372
NameRefClass::FieldShorthand { field_ref: field, local_ref: local }
354373
}

‎crates/ide_db/src/search.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ impl<'a> FindUsages<'a> {
550550
}
551551
}
552552
Some(NameRefClass::FieldShorthand { local_ref: local, field_ref: field }) => {
553+
let field = Definition::Field(field);
553554
let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax());
554555
let access = match self.def {
555556
Definition::Field(_) if field == self.def => reference_access(&field, name_ref),
@@ -574,7 +575,7 @@ impl<'a> FindUsages<'a> {
574575
match NameClass::classify(self.sema, name) {
575576
Some(NameClass::PatFieldShorthand { local_def: _, field_ref })
576577
if matches!(
577-
self.def, Definition::Field(_) if field_ref == self.def
578+
self.def, Definition::Field(_) if Definition::Field(field_ref) == self.def
578579
) =>
579580
{
580581
let FileRange { file_id, range } = self.sema.original_range(name.syntax());

0 commit comments

Comments
 (0)