From 2716a1fa3f8a7410248724a6ce5d4c28837db682 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 2 Mar 2020 19:00:38 +0100 Subject: More principled approach for gotodef for field shorhand Callers can now decide for themselves if they should prefer field or local definition. By default, it's the local. --- crates/ra_hir/src/semantics.rs | 7 +++++-- crates/ra_hir/src/source_analyzer.rs | 27 ++++++++++++++++-------- crates/ra_ide/src/goto_definition.rs | 6 ++++-- crates/ra_ide/src/hover.rs | 2 +- crates/ra_ide/src/references.rs | 8 ++++++-- crates/ra_ide/src/references/classify.rs | 35 ++++++++++++++++++++++++-------- crates/ra_ide/src/syntax_highlighting.rs | 30 +++++++++++++++------------ 7 files changed, 79 insertions(+), 36 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index a0853957c..afc7f7ee7 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -107,8 +107,11 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.analyze(field.syntax()).resolve_field(field) } - pub fn resolve_record_field(&self, field: &ast::RecordField) -> Option { - self.analyze(field.syntax()).resolve_record_field(field) + pub fn resolve_record_field( + &self, + field: &ast::RecordField, + ) -> Option<(StructField, Option)> { + self.analyze(field.syntax()).resolve_record_field(self.db, field) } pub fn resolve_record_literal(&self, record_lit: &ast::RecordLit) -> Option { diff --git a/crates/ra_hir/src/source_analyzer.rs b/crates/ra_hir/src/source_analyzer.rs index 4c121eb73..015389fb0 100644 --- a/crates/ra_hir/src/source_analyzer.rs +++ b/crates/ra_hir/src/source_analyzer.rs @@ -5,7 +5,7 @@ //! //! So, this modules should not be used during hir construction, it exists //! purely for "IDE needs". -use std::sync::Arc; +use std::{iter::once, sync::Arc}; use either::Either; use hir_def::{ @@ -25,8 +25,8 @@ use ra_syntax::{ }; use crate::{ - db::HirDatabase, Adt, Const, EnumVariant, Function, Local, MacroDef, ModuleDef, Path, Static, - Struct, Trait, Type, TypeAlias, TypeParam, + db::HirDatabase, Adt, Const, EnumVariant, Function, Local, MacroDef, ModPath, ModuleDef, Path, + PathKind, Static, Struct, Trait, Type, TypeAlias, TypeParam, }; /// `SourceAnalyzer` is a convenience wrapper which exposes HIR API in terms of @@ -162,16 +162,27 @@ impl SourceAnalyzer { pub(crate) fn resolve_record_field( &self, + db: &impl HirDatabase, field: &ast::RecordField, - ) -> Option { - let expr_id = match field.expr() { - Some(it) => self.expr_id(&it)?, + ) -> Option<(crate::StructField, Option)> { + let (expr_id, local) = match field.expr() { + Some(it) => (self.expr_id(&it)?, None), None => { let src = InFile { file_id: self.file_id, value: field }; - self.body_source_map.as_ref()?.field_init_shorthand_expr(src)? + let expr_id = self.body_source_map.as_ref()?.field_init_shorthand_expr(src)?; + let local_name = field.name_ref()?.as_name(); + let path = ModPath::from_segments(PathKind::Plain, once(local_name)); + let local = match self.resolver.resolve_path_in_value_ns_fully(db, &path) { + Some(ValueNs::LocalBinding(pat_id)) => { + Some(Local { pat_id, parent: self.resolver.body_owner()? }) + } + _ => None, + }; + (expr_id, local) } }; - self.infer.as_ref()?.record_field_resolution(expr_id).map(|it| it.into()) + let struct_field = self.infer.as_ref()?.record_field_resolution(expr_id)?; + Some((struct_field.into(), local)) } pub(crate) fn resolve_record_literal( diff --git a/crates/ra_ide/src/goto_definition.rs b/crates/ra_ide/src/goto_definition.rs index e67585203..76ee232a3 100644 --- a/crates/ra_ide/src/goto_definition.rs +++ b/crates/ra_ide/src/goto_definition.rs @@ -76,6 +76,8 @@ pub(crate) fn reference_definition( let name_kind = classify_name_ref(sema, name_ref); if let Some(def) = name_kind { + let def = def.definition(); + return match def.try_to_nav(sema.db) { Some(nav) => ReferenceResult::Exact(nav), None => ReferenceResult::Approximate(Vec::new()), @@ -795,8 +797,8 @@ mod tests { Foo { x<|> }; } ", - "x RECORD_FIELD_DEF FileId(1) [13; 19) [13; 14)", - "x: i32|x", + "x BIND_PAT FileId(1) [42; 43)", + "x", ) } } diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index 5073bb1cf..b31956626 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -153,7 +153,7 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option { - classify_name_ref(&sema, &name_ref).map(|d| (name_ref.syntax().clone(), d)) + classify_name_ref(&sema, &name_ref).map(|d| (name_ref.syntax().clone(), d.definition())) }, ast::Name(name) => { classify_name(&sema, &name).map(|d| (name.syntax().clone(), d.definition())) diff --git a/crates/ra_ide/src/references.rs b/crates/ra_ide/src/references.rs index f763013ae..2eb047c96 100644 --- a/crates/ra_ide/src/references.rs +++ b/crates/ra_ide/src/references.rs @@ -27,7 +27,10 @@ use test_utils::tested_by; use crate::{display::TryToNav, FilePosition, FileRange, NavigationTarget, RangeInfo}; -pub(crate) use self::{classify::classify_name_ref, rename::rename}; +pub(crate) use self::{ + classify::{classify_name_ref, NameRefClass}, + rename::rename, +}; pub(crate) use ra_ide_db::defs::{classify_name, NameDefinition}; pub use self::search_scope::SearchScope; @@ -160,7 +163,7 @@ fn find_name( return Some(RangeInfo::new(range, (name.text().to_string(), def))); } let name_ref = find_node_at_offset::(&syntax, position.offset)?; - let def = classify_name_ref(sema, &name_ref)?; + let def = classify_name_ref(sema, &name_ref)?.definition(); let range = name_ref.syntax().text_range(); Some(RangeInfo::new(range, (name_ref.text().to_string(), def))) } @@ -212,6 +215,7 @@ fn process_definition( // See https://github.com/rust-lang/rust/pull/68198#issuecomment-574269098 if let Some(d) = classify_name_ref(&sema, &name_ref) { + let d = d.definition(); if d == def { let kind = if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) { diff --git a/crates/ra_ide/src/references/classify.rs b/crates/ra_ide/src/references/classify.rs index fdd07d8d1..e837ca8a3 100644 --- a/crates/ra_ide/src/references/classify.rs +++ b/crates/ra_ide/src/references/classify.rs @@ -1,6 +1,6 @@ //! Functions that are used to classify an element from its definition or reference. -use hir::{PathResolution, Semantics}; +use hir::{Local, PathResolution, Semantics}; use ra_ide_db::defs::NameDefinition; use ra_ide_db::RootDatabase; use ra_prof::profile; @@ -9,10 +9,24 @@ use test_utils::tested_by; pub use ra_ide_db::defs::{from_module_def, from_struct_field}; +pub enum NameRefClass { + NameDefinition(NameDefinition), + FieldShorthand { local: Local, field: NameDefinition }, +} + +impl NameRefClass { + pub fn definition(self) -> NameDefinition { + match self { + NameRefClass::NameDefinition(def) => def, + NameRefClass::FieldShorthand { local, field: _ } => NameDefinition::Local(local), + } + } +} + pub(crate) fn classify_name_ref( sema: &Semantics, name_ref: &ast::NameRef, -) -> Option { +) -> Option { let _p = profile("classify_name_ref"); let parent = name_ref.syntax().parent()?; @@ -20,29 +34,34 @@ pub(crate) fn classify_name_ref( if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) { tested_by!(goto_def_for_methods); if let Some(func) = sema.resolve_method_call(&method_call) { - return Some(from_module_def(func.into())); + return Some(NameRefClass::NameDefinition(NameDefinition::ModuleDef(func.into()))); } } if let Some(field_expr) = ast::FieldExpr::cast(parent.clone()) { tested_by!(goto_def_for_fields); if let Some(field) = sema.resolve_field(&field_expr) { - return Some(from_struct_field(field)); + return Some(NameRefClass::NameDefinition(NameDefinition::StructField(field))); } } if let Some(record_field) = ast::RecordField::cast(parent.clone()) { tested_by!(goto_def_for_record_fields); tested_by!(goto_def_for_field_init_shorthand); - if let Some(field_def) = sema.resolve_record_field(&record_field) { - return Some(from_struct_field(field_def)); + if let Some((field, local)) = sema.resolve_record_field(&record_field) { + let field = NameDefinition::StructField(field); + let res = match local { + None => NameRefClass::NameDefinition(field), + Some(local) => NameRefClass::FieldShorthand { field, local }, + }; + return Some(res); } } if let Some(macro_call) = parent.ancestors().find_map(ast::MacroCall::cast) { tested_by!(goto_def_for_macros); if let Some(macro_def) = sema.resolve_macro_call(¯o_call) { - return Some(NameDefinition::Macro(macro_def)); + return Some(NameRefClass::NameDefinition(NameDefinition::Macro(macro_def))); } } @@ -63,5 +82,5 @@ pub(crate) fn classify_name_ref( PathResolution::Macro(def) => NameDefinition::Macro(def), PathResolution::SelfType(impl_def) => NameDefinition::SelfType(impl_def), }; - Some(res) + Some(NameRefClass::NameDefinition(res)) } diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs index 28117b4d8..b89501aff 100644 --- a/crates/ra_ide/src/syntax_highlighting.rs +++ b/crates/ra_ide/src/syntax_highlighting.rs @@ -19,7 +19,11 @@ use ra_syntax::{ }; use rustc_hash::FxHashMap; -use crate::{call_info::call_info_for_token, references::classify_name_ref, Analysis, FileId}; +use crate::{ + call_info::call_info_for_token, + references::{classify_name_ref, NameRefClass}, + Analysis, FileId, +}; pub(crate) use html::highlight_as_html; pub use tags::{Highlight, HighlightModifier, HighlightModifiers, HighlightTag}; @@ -186,24 +190,24 @@ fn highlight_element( } // Highlight references like the definitions they resolve to - - // Special-case field init shorthand - NAME_REF if element.parent().and_then(ast::RecordField::cast).is_some() => { - HighlightTag::Field.into() - } NAME_REF if element.ancestors().any(|it| it.kind() == ATTR) => return None, NAME_REF => { let name_ref = element.into_node().and_then(ast::NameRef::cast).unwrap(); let name_kind = classify_name_ref(sema, &name_ref)?; - if let NameDefinition::Local(local) = &name_kind { - if let Some(name) = local.name(db) { - let shadow_count = bindings_shadow_count.entry(name.clone()).or_default(); - binding_hash = Some(calc_binding_hash(&name, *shadow_count)) + match name_kind { + NameRefClass::NameDefinition(def) => { + if let NameDefinition::Local(local) = &def { + if let Some(name) = local.name(db) { + let shadow_count = + bindings_shadow_count.entry(name.clone()).or_default(); + binding_hash = Some(calc_binding_hash(&name, *shadow_count)) + } + }; + highlight_name(db, def) } - }; - - highlight_name(db, name_kind) + NameRefClass::FieldShorthand { .. } => HighlightTag::Field.into(), + } } // Simple token-based highlighting -- cgit v1.2.3