From 27cadcd531c017aa7c78c6f7a36f2b7f2ce8a196 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 13:05:28 +1100 Subject: [PATCH 01/17] HasSource::source -> HasSource::source_old To start migrating HasSource::source to return an Option. --- .../assists/src/handlers/fill_match_arms.rs | 2 +- crates/assists/src/handlers/fix_visibility.rs | 4 +-- crates/assists/src/utils.rs | 6 ++-- .../completion/src/completions/trait_impl.rs | 4 +-- crates/completion/src/render/const_.rs | 2 +- crates/completion/src/render/function.rs | 2 +- crates/completion/src/render/macro_.rs | 2 +- crates/completion/src/render/type_alias.rs | 2 +- crates/hir/src/code_model.rs | 4 +-- crates/hir/src/has_source.rs | 30 +++++++++---------- crates/ide/src/diagnostics/fixes.rs | 6 ++-- crates/ide/src/display/navigation_target.rs | 12 ++++---- crates/ide/src/hover.rs | 8 ++--- crates/ide_db/src/search.rs | 24 +++++++-------- .../rust-analyzer/src/cli/analysis_stats.rs | 2 +- 15 files changed, 55 insertions(+), 55 deletions(-) diff --git a/crates/assists/src/handlers/fill_match_arms.rs b/crates/assists/src/handlers/fill_match_arms.rs index cb60a31282..a8efad6d63 100644 --- a/crates/assists/src/handlers/fill_match_arms.rs +++ b/crates/assists/src/handlers/fill_match_arms.rs @@ -196,7 +196,7 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::Variant) -> Optio let path = mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var))?); // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though - let pat: ast::Pat = match var.source(db).value.kind() { + let pat: ast::Pat = match var.source_old(db).value.kind() { ast::StructKind::Tuple(field_list) => { let pats = iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count()); make::tuple_struct_pat(path, pats).into() diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index 8558a8ff01..d8150abd96 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -97,7 +97,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> let parent_name = parent.name(ctx.db()); let target_module = parent.module(ctx.db()); - let in_file_source = record_field_def.source(ctx.db()); + let in_file_source = record_field_def.source_old(ctx.db()); let (offset, current_visibility, target) = match in_file_source.value { hir::FieldSource::Named(it) => { let s = it.syntax(); @@ -150,7 +150,7 @@ fn target_data_for_def( S: HasSource, Ast: AstNode + ast::VisibilityOwner, { - let source = x.source(db); + let source = x.source_old(db); let in_file_syntax = source.syntax(); let file_id = in_file_syntax.file_id; let syntax = in_file_syntax.value; diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index d41084b599..7ee7111aee 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -99,9 +99,9 @@ pub fn filter_assoc_items( items .iter() .map(|i| match i { - hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db).value), - hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db).value), - hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db).value), + hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source_old(db).value), + hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source_old(db).value), + hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source_old(db).value), }) .filter(has_def_name) .filter(|it| match it { diff --git a/crates/completion/src/completions/trait_impl.rs b/crates/completion/src/completions/trait_impl.rs index c4e0d06698..759253c53d 100644 --- a/crates/completion/src/completions/trait_impl.rs +++ b/crates/completion/src/completions/trait_impl.rs @@ -156,7 +156,7 @@ fn add_function_impl( }; let range = TextRange::new(fn_def_node.text_range().start(), ctx.source_range().end()); - let function_decl = function_declaration(&func.source(ctx.db).value); + let function_decl = function_declaration(&func.source_old(ctx.db).value); match ctx.config.snippet_cap { Some(cap) => { let snippet = format!("{} {{\n $0\n}}", function_decl); @@ -200,7 +200,7 @@ fn add_const_impl( let const_name = const_.name(ctx.db).map(|n| n.to_string()); if let Some(const_name) = const_name { - let snippet = make_const_compl_syntax(&const_.source(ctx.db).value); + let snippet = make_const_compl_syntax(&const_.source_old(ctx.db).value); let range = TextRange::new(const_def_node.text_range().start(), ctx.source_range().end()); diff --git a/crates/completion/src/render/const_.rs b/crates/completion/src/render/const_.rs index 039bdabc05..a8820a4fe1 100644 --- a/crates/completion/src/render/const_.rs +++ b/crates/completion/src/render/const_.rs @@ -27,7 +27,7 @@ struct ConstRender<'a> { impl<'a> ConstRender<'a> { fn new(ctx: RenderContext<'a>, const_: hir::Const) -> ConstRender<'a> { - let ast_node = const_.source(ctx.db()).value; + let ast_node = const_.source_old(ctx.db()).value; ConstRender { ctx, const_, ast_node } } diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index 316e05b529..d9ea425a0a 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -34,7 +34,7 @@ impl<'a> FunctionRender<'a> { fn_: hir::Function, ) -> FunctionRender<'a> { let name = local_name.unwrap_or_else(|| fn_.name(ctx.db()).to_string()); - let ast_node = fn_.source(ctx.db()).value; + let ast_node = fn_.source_old(ctx.db()).value; FunctionRender { ctx, name, func: fn_, ast_node } } diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs index dac79592f7..3d13fd9e2f 100644 --- a/crates/completion/src/render/macro_.rs +++ b/crates/completion/src/render/macro_.rs @@ -96,7 +96,7 @@ impl<'a> MacroRender<'a> { } fn detail(&self) -> String { - let ast_node = self.macro_.source(self.ctx.db()).value; + let ast_node = self.macro_.source_old(self.ctx.db()).value; macro_label(&ast_node) } } diff --git a/crates/completion/src/render/type_alias.rs b/crates/completion/src/render/type_alias.rs index 9605c7fa94..4099a5d0e1 100644 --- a/crates/completion/src/render/type_alias.rs +++ b/crates/completion/src/render/type_alias.rs @@ -27,7 +27,7 @@ struct TypeAliasRender<'a> { impl<'a> TypeAliasRender<'a> { fn new(ctx: RenderContext<'a>, type_alias: hir::TypeAlias) -> TypeAliasRender<'a> { - let ast_node = type_alias.source(ctx.db()).value; + let ast_node = type_alias.source_old(ctx.db()).value; TypeAliasRender { ctx, type_alias, ast_node } } diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 97b7a8b5f5..5020aa196d 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -989,7 +989,7 @@ impl MacroDef { if self.is_proc_macro() { return None; } - self.source(db).value.name().map(|it| it.as_name()) + self.source_old(db).value.name().map(|it| it.as_name()) } /// Indicate it is a proc-macro @@ -1378,7 +1378,7 @@ impl Impl { } pub fn is_builtin_derive(self, db: &dyn HirDatabase) -> Option> { - let src = self.source(db); + let src = self.source_old(db); let item = src.file_id.is_builtin_derive(db.upcast())?; let hygenic = hir_expand::hygiene::Hygiene::new(db.upcast(), item.file_id); diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index dd7c0c5706..a8256c1812 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -16,7 +16,7 @@ use crate::{ pub trait HasSource { type Ast; - fn source(self, db: &dyn HirDatabase) -> InFile; + fn source_old(self, db: &dyn HirDatabase) -> InFile; } /// NB: Module is !HasSource, because it has two source nodes at the same time: @@ -46,7 +46,7 @@ impl Module { impl HasSource for Field { type Ast = FieldSource; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { let var = VariantId::from(self.parent); let src = var.child_source(db.upcast()); src.map(|it| match it[self.id].clone() { @@ -57,61 +57,61 @@ impl HasSource for Field { } impl HasSource for Struct { type Ast = ast::Struct; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Union { type Ast = ast::Union; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Enum { type Ast = ast::Enum; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Variant { type Ast = ast::Variant; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.parent.id.child_source(db.upcast()).map(|map| map[self.id].clone()) } } impl HasSource for Function { type Ast = ast::Fn; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Const { type Ast = ast::Const; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Static { type Ast = ast::Static; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for Trait { type Ast = ast::Trait; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for TypeAlias { type Ast = ast::TypeAlias; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for MacroDef { type Ast = ast::Macro; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { InFile { file_id: self.id.ast_id.expect("MacroDef without ast_id").file_id, value: self.id.ast_id.expect("MacroDef without ast_id").to_node(db.upcast()), @@ -120,14 +120,14 @@ impl HasSource for MacroDef { } impl HasSource for Impl { type Ast = ast::Impl; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } } impl HasSource for TypeParam { type Ast = Either; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { let child_source = self.id.parent.child_source(db.upcast()); child_source.map(|it| it[self.id.local_id].clone()) } @@ -135,7 +135,7 @@ impl HasSource for TypeParam { impl HasSource for LifetimeParam { type Ast = ast::LifetimeParam; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source_old(self, db: &dyn HirDatabase) -> InFile { let child_source = self.id.parent.child_source(db.upcast()); child_source.map(|it| it[self.id.local_id].clone()) } diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index d79f5c1700..702e8239d1 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -156,20 +156,20 @@ fn missing_record_expr_field_fix( let record_fields = match VariantDef::from(def_id) { VariantDef::Struct(s) => { module = s.module(sema.db); - let source = s.source(sema.db); + let source = s.source_old(sema.db); def_file_id = source.file_id; let fields = source.value.field_list()?; record_field_list(fields)? } VariantDef::Union(u) => { module = u.module(sema.db); - let source = u.source(sema.db); + let source = u.source_old(sema.db); def_file_id = source.file_id; source.value.record_field_list()? } VariantDef::Variant(e) => { module = e.module(sema.db); - let source = e.source(sema.db); + let source = e.source_old(sema.db); def_file_id = source.file_id; let fields = source.value.field_list()?; record_field_list(fields)? diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index bcde2b6f12..de4c0fa12d 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -285,7 +285,7 @@ where D::Ast: ast::NameOwner + ShortLabel, { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); let mut res = NavigationTarget::from_named( db, src.as_ref().map(|it| it as &dyn ast::NameOwner), @@ -314,7 +314,7 @@ impl ToNav for hir::Module { impl ToNav for hir::Impl { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); let derive_attr = self.is_builtin_derive(db); let frange = if let Some(item) = &derive_attr { item.syntax().original_file_range(db) @@ -339,7 +339,7 @@ impl ToNav for hir::Impl { impl ToNav for hir::Field { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); match &src.value { FieldSource::Named(it) => { @@ -365,7 +365,7 @@ impl ToNav for hir::Field { impl ToNav for hir::MacroDef { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); log::debug!("nav target {:#?}", src.value.syntax()); let mut res = NavigationTarget::from_named( db, @@ -448,7 +448,7 @@ impl ToNav for hir::Label { impl ToNav for hir::TypeParam { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); let full_range = match &src.value { Either::Left(it) => it.syntax().text_range(), Either::Right(it) => it.syntax().text_range(), @@ -472,7 +472,7 @@ impl ToNav for hir::TypeParam { impl ToNav for hir::LifetimeParam { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); + let src = self.source_old(db); let full_range = src.value.syntax().text_range(); NavigationTarget { file_id: src.file_id.original_file(db), diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 98c7bfbe51..90781ea342 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -206,7 +206,7 @@ fn runnable_action( _ => None, }, ModuleDef::Function(it) => { - let src = it.source(sema.db); + let src = it.source_old(sema.db); if src.file_id != file_id.into() { mark::hit!(hover_macro_generated_struct_fn_doc_comment); mark::hit!(hover_macro_generated_struct_fn_doc_attr); @@ -332,11 +332,11 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { if it.is_proc_macro() { return None; } - let label = macro_label(&it.source(db).value); + let label = macro_label(&it.source_old(db).value); from_def_source_labeled(db, it, Some(label), mod_path) } Definition::Field(def) => { - let src = def.source(db).value; + let src = def.source_old(db).value; if let FieldSource::Named(it) = src { from_def_source_labeled(db, def, it.short_label(), mod_path) } else { @@ -385,7 +385,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { D: HasSource + HasAttrs + Copy, A: ShortLabel, { - let short_label = def.source(db).value.short_label(); + let short_label = def.source_old(db).value.short_label(); from_def_source_labeled(db, def, short_label, mod_path) } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index ff10f71c35..2df4894a1e 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -121,9 +121,9 @@ impl Definition { if let Definition::Local(var) = self { let range = match var.parent(db) { - DefWithBody::Function(f) => f.source(db).value.syntax().text_range(), - DefWithBody::Const(c) => c.source(db).value.syntax().text_range(), - DefWithBody::Static(s) => s.source(db).value.syntax().text_range(), + DefWithBody::Function(f) => f.source_old(db).value.syntax().text_range(), + DefWithBody::Const(c) => c.source_old(db).value.syntax().text_range(), + DefWithBody::Static(s) => s.source_old(db).value.syntax().text_range(), }; let mut res = FxHashMap::default(); res.insert(file_id, Some(range)); @@ -132,17 +132,17 @@ impl Definition { if let Definition::LifetimeParam(param) = self { let range = match param.parent(db) { - hir::GenericDef::Function(it) => it.source(db).value.syntax().text_range(), + hir::GenericDef::Function(it) => it.source_old(db).value.syntax().text_range(), hir::GenericDef::Adt(it) => match it { - hir::Adt::Struct(it) => it.source(db).value.syntax().text_range(), - hir::Adt::Union(it) => it.source(db).value.syntax().text_range(), - hir::Adt::Enum(it) => it.source(db).value.syntax().text_range(), + hir::Adt::Struct(it) => it.source_old(db).value.syntax().text_range(), + hir::Adt::Union(it) => it.source_old(db).value.syntax().text_range(), + hir::Adt::Enum(it) => it.source_old(db).value.syntax().text_range(), }, - hir::GenericDef::Trait(it) => it.source(db).value.syntax().text_range(), - hir::GenericDef::TypeAlias(it) => it.source(db).value.syntax().text_range(), - hir::GenericDef::Impl(it) => it.source(db).value.syntax().text_range(), - hir::GenericDef::Variant(it) => it.source(db).value.syntax().text_range(), - hir::GenericDef::Const(it) => it.source(db).value.syntax().text_range(), + hir::GenericDef::Trait(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::TypeAlias(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::Impl(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::Variant(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::Const(it) => it.source_old(db).value.syntax().text_range(), }; let mut res = FxHashMap::default(); res.insert(file_id, Some(range)); diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index a23fb7a33a..3ee11a8f07 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -161,7 +161,7 @@ impl AnalysisStatsCmd { } let mut msg = format!("processing: {}", full_name); if verbosity.is_verbose() { - let src = f.source(db); + let src = f.source_old(db); let original_file = src.file_id.original_file(db); let path = vfs.file_path(original_file); let syntax_range = src.value.syntax().text_range(); From 2de2b1eca3c3a3a74c0374f4de0b0c3ff25e66a9 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 13:27:38 +1100 Subject: [PATCH 02/17] Implement new HasSource::source for all implementors of HasSource --- crates/hir/src/has_source.rs | 66 ++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index a8256c1812..84fbeca751 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -17,6 +17,7 @@ use crate::{ pub trait HasSource { type Ast; fn source_old(self, db: &dyn HirDatabase) -> InFile; + fn source(self, db: &dyn HirDatabase) -> Option>; } /// NB: Module is !HasSource, because it has two source nodes at the same time: @@ -54,60 +55,106 @@ impl HasSource for Field { Either::Right(it) => FieldSource::Named(it), }) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + let var = VariantId::from(self.parent); + let src = var.child_source(db.upcast()); + let field_source = src.map(|it| match it[self.id].clone() { + Either::Left(it) => FieldSource::Pos(it), + Either::Right(it) => FieldSource::Named(it), + }); + Some(field_source) + } } impl HasSource for Struct { type Ast = ast::Struct; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Union { type Ast = ast::Union; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Enum { type Ast = ast::Enum; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Variant { type Ast = ast::Variant; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.parent.id.child_source(db.upcast()).map(|map| map[self.id].clone()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.parent.id.child_source(db.upcast()).map(|map| map[self.id].clone())) + } } impl HasSource for Function { type Ast = ast::Fn; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Const { type Ast = ast::Const; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Static { type Ast = ast::Static; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for Trait { type Ast = ast::Trait; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for TypeAlias { type Ast = ast::TypeAlias; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for MacroDef { type Ast = ast::Macro; @@ -117,12 +164,21 @@ impl HasSource for MacroDef { value: self.id.ast_id.expect("MacroDef without ast_id").to_node(db.upcast()), } } + + fn source(self, db: &dyn HirDatabase) -> Option> { + let ast_id = self.id.ast_id?; + Some(InFile { file_id: ast_id.file_id, value: ast_id.to_node(db.upcast()) }) + } } impl HasSource for Impl { type Ast = ast::Impl; fn source_old(self, db: &dyn HirDatabase) -> InFile { self.id.lookup(db.upcast()).source(db.upcast()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + Some(self.id.lookup(db.upcast()).source(db.upcast())) + } } impl HasSource for TypeParam { @@ -131,6 +187,11 @@ impl HasSource for TypeParam { let child_source = self.id.parent.child_source(db.upcast()); child_source.map(|it| it[self.id.local_id].clone()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + let child_source = self.id.parent.child_source(db.upcast()); + Some(child_source.map(|it| it[self.id.local_id].clone())) + } } impl HasSource for LifetimeParam { @@ -139,6 +200,11 @@ impl HasSource for LifetimeParam { let child_source = self.id.parent.child_source(db.upcast()); child_source.map(|it| it[self.id.local_id].clone()) } + + fn source(self, db: &dyn HirDatabase) -> Option> { + let child_source = self.id.parent.child_source(db.upcast()); + Some(child_source.map(|it| it[self.id.local_id].clone())) + } } impl HasSource for ConstParam { From ea4708c444509449b86c50b7b1b23f9ff5af4e97 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 13:50:50 +1100 Subject: [PATCH 03/17] Mark HasSource::source_old as deprecated but allow at all call sites --- crates/assists/src/handlers/fill_match_arms.rs | 1 + crates/assists/src/handlers/fix_visibility.rs | 2 ++ crates/assists/src/utils.rs | 11 +++++++---- crates/completion/src/completions/trait_impl.rs | 2 ++ crates/completion/src/render/const_.rs | 1 + crates/completion/src/render/function.rs | 1 + crates/completion/src/render/macro_.rs | 1 + crates/completion/src/render/type_alias.rs | 1 + crates/hir/src/code_model.rs | 2 ++ crates/hir/src/has_source.rs | 1 + crates/ide/src/diagnostics/fixes.rs | 3 +++ crates/ide/src/display/navigation_target.rs | 6 ++++++ crates/ide/src/hover.rs | 4 ++++ crates/ide_db/src/search.rs | 2 ++ crates/rust-analyzer/src/cli/analysis_stats.rs | 1 + 15 files changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/assists/src/handlers/fill_match_arms.rs b/crates/assists/src/handlers/fill_match_arms.rs index a8efad6d63..d17c82e18a 100644 --- a/crates/assists/src/handlers/fill_match_arms.rs +++ b/crates/assists/src/handlers/fill_match_arms.rs @@ -196,6 +196,7 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::Variant) -> Optio let path = mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var))?); // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though + #[allow(deprecated)] let pat: ast::Pat = match var.source_old(db).value.kind() { ast::StructKind::Tuple(field_list) => { let pats = iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count()); diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index d8150abd96..7d440d4206 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -97,6 +97,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> let parent_name = parent.name(ctx.db()); let target_module = parent.module(ctx.db()); + #[allow(deprecated)] let in_file_source = record_field_def.source_old(ctx.db()); let (offset, current_visibility, target) = match in_file_source.value { hir::FieldSource::Named(it) => { @@ -150,6 +151,7 @@ fn target_data_for_def( S: HasSource, Ast: AstNode + ast::VisibilityOwner, { + #[allow(deprecated)] let source = x.source_old(db); let in_file_syntax = source.syntax(); let file_id = in_file_syntax.file_id; diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index 7ee7111aee..d15e5a24be 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -98,10 +98,13 @@ pub fn filter_assoc_items( items .iter() - .map(|i| match i { - hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source_old(db).value), - hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source_old(db).value), - hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source_old(db).value), + .map(|i| { + #[allow(deprecated)] + match i { + hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source_old(db).value), + hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source_old(db).value), + hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source_old(db).value), + } }) .filter(has_def_name) .filter(|it| match it { diff --git a/crates/completion/src/completions/trait_impl.rs b/crates/completion/src/completions/trait_impl.rs index 759253c53d..43b3d939f3 100644 --- a/crates/completion/src/completions/trait_impl.rs +++ b/crates/completion/src/completions/trait_impl.rs @@ -156,6 +156,7 @@ fn add_function_impl( }; let range = TextRange::new(fn_def_node.text_range().start(), ctx.source_range().end()); + #[allow(deprecated)] let function_decl = function_declaration(&func.source_old(ctx.db).value); match ctx.config.snippet_cap { Some(cap) => { @@ -200,6 +201,7 @@ fn add_const_impl( let const_name = const_.name(ctx.db).map(|n| n.to_string()); if let Some(const_name) = const_name { + #[allow(deprecated)] let snippet = make_const_compl_syntax(&const_.source_old(ctx.db).value); let range = TextRange::new(const_def_node.text_range().start(), ctx.source_range().end()); diff --git a/crates/completion/src/render/const_.rs b/crates/completion/src/render/const_.rs index a8820a4fe1..648a1afc51 100644 --- a/crates/completion/src/render/const_.rs +++ b/crates/completion/src/render/const_.rs @@ -27,6 +27,7 @@ struct ConstRender<'a> { impl<'a> ConstRender<'a> { fn new(ctx: RenderContext<'a>, const_: hir::Const) -> ConstRender<'a> { + #[allow(deprecated)] let ast_node = const_.source_old(ctx.db()).value; ConstRender { ctx, const_, ast_node } } diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index d9ea425a0a..4c89962040 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -34,6 +34,7 @@ impl<'a> FunctionRender<'a> { fn_: hir::Function, ) -> FunctionRender<'a> { let name = local_name.unwrap_or_else(|| fn_.name(ctx.db()).to_string()); + #[allow(deprecated)] let ast_node = fn_.source_old(ctx.db()).value; FunctionRender { ctx, name, func: fn_, ast_node } diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs index 3d13fd9e2f..95408ff9af 100644 --- a/crates/completion/src/render/macro_.rs +++ b/crates/completion/src/render/macro_.rs @@ -96,6 +96,7 @@ impl<'a> MacroRender<'a> { } fn detail(&self) -> String { + #[allow(deprecated)] let ast_node = self.macro_.source_old(self.ctx.db()).value; macro_label(&ast_node) } diff --git a/crates/completion/src/render/type_alias.rs b/crates/completion/src/render/type_alias.rs index 4099a5d0e1..276090015e 100644 --- a/crates/completion/src/render/type_alias.rs +++ b/crates/completion/src/render/type_alias.rs @@ -27,6 +27,7 @@ struct TypeAliasRender<'a> { impl<'a> TypeAliasRender<'a> { fn new(ctx: RenderContext<'a>, type_alias: hir::TypeAlias) -> TypeAliasRender<'a> { + #[allow(deprecated)] let ast_node = type_alias.source_old(ctx.db()).value; TypeAliasRender { ctx, type_alias, ast_node } } diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 5020aa196d..285905e96c 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -989,6 +989,7 @@ impl MacroDef { if self.is_proc_macro() { return None; } + #[allow(deprecated)] self.source_old(db).value.name().map(|it| it.as_name()) } @@ -1378,6 +1379,7 @@ impl Impl { } pub fn is_builtin_derive(self, db: &dyn HirDatabase) -> Option> { + #[allow(deprecated)] let src = self.source_old(db); let item = src.file_id.is_builtin_derive(db.upcast())?; let hygenic = hir_expand::hygiene::Hygiene::new(db.upcast(), item.file_id); diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index 84fbeca751..8a7306def8 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -16,6 +16,7 @@ use crate::{ pub trait HasSource { type Ast; + #[deprecated = "migrating to source() method that returns an Option"] fn source_old(self, db: &dyn HirDatabase) -> InFile; fn source(self, db: &dyn HirDatabase) -> Option>; } diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 702e8239d1..0b5e0a4c17 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -156,6 +156,7 @@ fn missing_record_expr_field_fix( let record_fields = match VariantDef::from(def_id) { VariantDef::Struct(s) => { module = s.module(sema.db); + #[allow(deprecated)] let source = s.source_old(sema.db); def_file_id = source.file_id; let fields = source.value.field_list()?; @@ -163,12 +164,14 @@ fn missing_record_expr_field_fix( } VariantDef::Union(u) => { module = u.module(sema.db); + #[allow(deprecated)] let source = u.source_old(sema.db); def_file_id = source.file_id; source.value.record_field_list()? } VariantDef::Variant(e) => { module = e.module(sema.db); + #[allow(deprecated)] let source = e.source_old(sema.db); def_file_id = source.file_id; let fields = source.value.field_list()?; diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index de4c0fa12d..efa0418ada 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -285,6 +285,7 @@ where D::Ast: ast::NameOwner + ShortLabel, { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); let mut res = NavigationTarget::from_named( db, @@ -314,6 +315,7 @@ impl ToNav for hir::Module { impl ToNav for hir::Impl { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); let derive_attr = self.is_builtin_derive(db); let frange = if let Some(item) = &derive_attr { @@ -339,6 +341,7 @@ impl ToNav for hir::Impl { impl ToNav for hir::Field { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); match &src.value { @@ -365,6 +368,7 @@ impl ToNav for hir::Field { impl ToNav for hir::MacroDef { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); log::debug!("nav target {:#?}", src.value.syntax()); let mut res = NavigationTarget::from_named( @@ -448,6 +452,7 @@ impl ToNav for hir::Label { impl ToNav for hir::TypeParam { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); let full_range = match &src.value { Either::Left(it) => it.syntax().text_range(), @@ -472,6 +477,7 @@ impl ToNav for hir::TypeParam { impl ToNav for hir::LifetimeParam { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { + #[allow(deprecated)] let src = self.source_old(db); let full_range = src.value.syntax().text_range(); NavigationTarget { diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 90781ea342..c192e3ed7f 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -206,6 +206,7 @@ fn runnable_action( _ => None, }, ModuleDef::Function(it) => { + #[allow(deprecated)] let src = it.source_old(sema.db); if src.file_id != file_id.into() { mark::hit!(hover_macro_generated_struct_fn_doc_comment); @@ -332,10 +333,12 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { if it.is_proc_macro() { return None; } + #[allow(deprecated)] let label = macro_label(&it.source_old(db).value); from_def_source_labeled(db, it, Some(label), mod_path) } Definition::Field(def) => { + #[allow(deprecated)] let src = def.source_old(db).value; if let FieldSource::Named(it) = src { from_def_source_labeled(db, def, it.short_label(), mod_path) @@ -385,6 +388,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { D: HasSource + HasAttrs + Copy, A: ShortLabel, { + #[allow(deprecated)] let short_label = def.source_old(db).value.short_label(); from_def_source_labeled(db, def, short_label, mod_path) } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 2df4894a1e..e69f9d1410 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -120,6 +120,7 @@ impl Definition { let file_id = module_src.file_id.original_file(db); if let Definition::Local(var) = self { + #[allow(deprecated)] let range = match var.parent(db) { DefWithBody::Function(f) => f.source_old(db).value.syntax().text_range(), DefWithBody::Const(c) => c.source_old(db).value.syntax().text_range(), @@ -131,6 +132,7 @@ impl Definition { } if let Definition::LifetimeParam(param) = self { + #[allow(deprecated)] let range = match param.parent(db) { hir::GenericDef::Function(it) => it.source_old(db).value.syntax().text_range(), hir::GenericDef::Adt(it) => match it { diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index 3ee11a8f07..bfc7d7b5a6 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -161,6 +161,7 @@ impl AnalysisStatsCmd { } let mut msg = format!("processing: {}", full_name); if verbosity.is_verbose() { + #[allow(deprecated)] let src = f.source_old(db); let original_file = src.file_id.original_file(db); let path = vfs.file_path(original_file); From 14d0db0759c5b8e1d085ebab03a8b944a8921f2e Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 14:14:09 +1100 Subject: [PATCH 04/17] HasSource::source_old -> HasSource::source for places where proc-macros were special cased In #6901 some special case handling for proc-macros was introduced to prevent panicing as they have no AST. Now the new HasSource::source method is used that returns an option. Generally this was a pretty trivial change, the only thing of much interest is that `hir::MacroDef` now implements `TryToNav` not `ToNav` as this allows us to handle `HasSource::source` now returning an option. --- crates/completion/src/render/macro_.rs | 16 ++++------------ crates/hir/src/code_model.rs | 9 +-------- crates/ide/src/display/navigation_target.rs | 19 +++++-------------- crates/ide/src/hover.rs | 9 +-------- 4 files changed, 11 insertions(+), 42 deletions(-) diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs index 95408ff9af..0612591fd2 100644 --- a/crates/completion/src/render/macro_.rs +++ b/crates/completion/src/render/macro_.rs @@ -39,20 +39,13 @@ impl<'a> MacroRender<'a> { } fn render(&self, import_to_add: Option) -> Option { - // FIXME: Currently proc-macro do not have ast-node, - // such that it does not have source - // more discussion: https://github.com/rust-analyzer/rust-analyzer/issues/6913 - if self.macro_.is_proc_macro() { - return None; - } - let mut builder = CompletionItem::new(CompletionKind::Reference, self.ctx.source_range(), &self.label()) .kind(CompletionItemKind::Macro) .set_documentation(self.docs.clone()) .set_deprecated(self.ctx.is_deprecated(self.macro_)) .add_import(import_to_add) - .detail(self.detail()); + .detail(self.detail()?); let needs_bang = self.needs_bang(); builder = match self.ctx.snippet_cap() { @@ -95,10 +88,9 @@ impl<'a> MacroRender<'a> { format!("{}!", self.name) } - fn detail(&self) -> String { - #[allow(deprecated)] - let ast_node = self.macro_.source_old(self.ctx.db()).value; - macro_label(&ast_node) + fn detail(&self) -> Option { + let ast_node = self.macro_.source(self.ctx.db())?.value; + Some(macro_label(&ast_node)) } } diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 285905e96c..62237f4817 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -983,14 +983,7 @@ impl MacroDef { /// XXX: this parses the file pub fn name(self, db: &dyn HirDatabase) -> Option { - // FIXME: Currently proc-macro do not have ast-node, - // such that it does not have source - // more discussion: https://github.com/rust-analyzer/rust-analyzer/issues/6913 - if self.is_proc_macro() { - return None; - } - #[allow(deprecated)] - self.source_old(db).value.name().map(|it| it.as_name()) + self.source(db)?.value.name().map(|it| it.as_name()) } /// Indicate it is a proc-macro diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index efa0418ada..5dc3f41287 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -210,15 +210,7 @@ impl ToNav for FileSymbol { impl TryToNav for Definition { fn try_to_nav(&self, db: &RootDatabase) -> Option { match self { - Definition::Macro(it) => { - // FIXME: Currently proc-macro do not have ast-node, - // such that it does not have source - // more discussion: https://github.com/rust-analyzer/rust-analyzer/issues/6913 - if it.is_proc_macro() { - return None; - } - Some(it.to_nav(db)) - } + Definition::Macro(it) => it.try_to_nav(db), Definition::Field(it) => Some(it.to_nav(db)), Definition::ModuleDef(it) => it.try_to_nav(db), Definition::SelfType(it) => Some(it.to_nav(db)), @@ -366,10 +358,9 @@ impl ToNav for hir::Field { } } -impl ToNav for hir::MacroDef { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); +impl TryToNav for hir::MacroDef { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; log::debug!("nav target {:#?}", src.value.syntax()); let mut res = NavigationTarget::from_named( db, @@ -377,7 +368,7 @@ impl ToNav for hir::MacroDef { SymbolKind::Macro, ); res.docs = self.docs(db); - res + Some(res) } } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index c192e3ed7f..a18dcdd8e2 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -327,14 +327,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { let mod_path = definition_mod_path(db, &def); return match def { Definition::Macro(it) => { - // FIXME: Currently proc-macro do not have ast-node, - // such that it does not have source - // more discussion: https://github.com/rust-analyzer/rust-analyzer/issues/6913 - if it.is_proc_macro() { - return None; - } - #[allow(deprecated)] - let label = macro_label(&it.source_old(db).value); + let label = macro_label(&it.source(db)?.value); from_def_source_labeled(db, it, Some(label), mod_path) } Definition::Field(def) => { From 562e2ee28a4397878accfde014d68ab17d1b504a Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 14:38:23 +1100 Subject: [PATCH 05/17] Only log path and syntax range when processing function if source exists --- crates/rust-analyzer/src/cli/analysis_stats.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index bfc7d7b5a6..9445aec074 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -161,12 +161,12 @@ impl AnalysisStatsCmd { } let mut msg = format!("processing: {}", full_name); if verbosity.is_verbose() { - #[allow(deprecated)] - let src = f.source_old(db); - let original_file = src.file_id.original_file(db); - let path = vfs.file_path(original_file); - let syntax_range = src.value.syntax().text_range(); - format_to!(msg, " ({} {:?})", path, syntax_range); + if let Some(src) = f.source(db) { + let original_file = src.file_id.original_file(db); + let path = vfs.file_path(original_file); + let syntax_range = src.value.syntax().text_range(); + format_to!(msg, " ({} {:?})", path, syntax_range); + } } if verbosity.is_spammy() { bar.println(msg.to_string()); From c936e4b86fd5de8e9709cd01547a69054cdec91b Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 15:02:39 +1100 Subject: [PATCH 06/17] source_old -> source for cases that can be handled by simple bubbling --- crates/assists/src/handlers/fill_match_arms.rs | 3 +-- crates/assists/src/handlers/fix_visibility.rs | 5 ++--- crates/hir/src/code_model.rs | 3 +-- crates/ide/src/diagnostics/fixes.rs | 6 +++--- crates/ide/src/hover.rs | 6 +++--- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/assists/src/handlers/fill_match_arms.rs b/crates/assists/src/handlers/fill_match_arms.rs index d17c82e18a..f9a62b9fac 100644 --- a/crates/assists/src/handlers/fill_match_arms.rs +++ b/crates/assists/src/handlers/fill_match_arms.rs @@ -196,8 +196,7 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::Variant) -> Optio let path = mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var))?); // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though - #[allow(deprecated)] - let pat: ast::Pat = match var.source_old(db).value.kind() { + let pat: ast::Pat = match var.source(db)?.value.kind() { ast::StructKind::Tuple(field_list) => { let pats = iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count()); make::tuple_struct_pat(path, pats).into() diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index 7d440d4206..aa6aed9c24 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -98,7 +98,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> let target_module = parent.module(ctx.db()); #[allow(deprecated)] - let in_file_source = record_field_def.source_old(ctx.db()); + let in_file_source = record_field_def.source(ctx.db())?; let (offset, current_visibility, target) = match in_file_source.value { hir::FieldSource::Named(it) => { let s = it.syntax(); @@ -151,8 +151,7 @@ fn target_data_for_def( S: HasSource, Ast: AstNode + ast::VisibilityOwner, { - #[allow(deprecated)] - let source = x.source_old(db); + let source = x.source(db)?; let in_file_syntax = source.syntax(); let file_id = in_file_syntax.file_id; let syntax = in_file_syntax.value; diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 62237f4817..3c83231cf1 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -1372,8 +1372,7 @@ impl Impl { } pub fn is_builtin_derive(self, db: &dyn HirDatabase) -> Option> { - #[allow(deprecated)] - let src = self.source_old(db); + let src = self.source(db)?; let item = src.file_id.is_builtin_derive(db.upcast())?; let hygenic = hir_expand::hygiene::Hygiene::new(db.upcast(), item.file_id); diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 0b5e0a4c17..ec0f840e9d 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -157,7 +157,7 @@ fn missing_record_expr_field_fix( VariantDef::Struct(s) => { module = s.module(sema.db); #[allow(deprecated)] - let source = s.source_old(sema.db); + let source = s.source(sema.db)?; def_file_id = source.file_id; let fields = source.value.field_list()?; record_field_list(fields)? @@ -165,14 +165,14 @@ fn missing_record_expr_field_fix( VariantDef::Union(u) => { module = u.module(sema.db); #[allow(deprecated)] - let source = u.source_old(sema.db); + let source = u.source(sema.db)?; def_file_id = source.file_id; source.value.record_field_list()? } VariantDef::Variant(e) => { module = e.module(sema.db); #[allow(deprecated)] - let source = e.source_old(sema.db); + let source = e.source(sema.db)?; def_file_id = source.file_id; let fields = source.value.field_list()?; record_field_list(fields)? diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index a18dcdd8e2..d2a0cfcd40 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -207,7 +207,7 @@ fn runnable_action( }, ModuleDef::Function(it) => { #[allow(deprecated)] - let src = it.source_old(sema.db); + let src = it.source(sema.db)?; if src.file_id != file_id.into() { mark::hit!(hover_macro_generated_struct_fn_doc_comment); mark::hit!(hover_macro_generated_struct_fn_doc_attr); @@ -332,7 +332,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { } Definition::Field(def) => { #[allow(deprecated)] - let src = def.source_old(db).value; + let src = def.source(db)?.value; if let FieldSource::Named(it) = src { from_def_source_labeled(db, def, it.short_label(), mod_path) } else { @@ -382,7 +382,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option { A: ShortLabel, { #[allow(deprecated)] - let short_label = def.source_old(db).value.short_label(); + let short_label = def.source(db)?.value.short_label(); from_def_source_labeled(db, def, short_label, mod_path) } From 68b4efd53601fd67fbba4a2aa25ac94ecff58e5e Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 15:33:06 +1100 Subject: [PATCH 07/17] Handle not finding range in Definition::search_scope The `LifetimeParam` and `Local` variants use `source()` to find their range. Now that `source()` returns an `Option` we need to handle the `None` case. --- crates/ide_db/src/search.rs | 48 +++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index e69f9d1410..c4aff39325 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -122,9 +122,15 @@ impl Definition { if let Definition::Local(var) = self { #[allow(deprecated)] let range = match var.parent(db) { - DefWithBody::Function(f) => f.source_old(db).value.syntax().text_range(), - DefWithBody::Const(c) => c.source_old(db).value.syntax().text_range(), - DefWithBody::Static(s) => s.source_old(db).value.syntax().text_range(), + DefWithBody::Function(f) => { + f.source(db).and_then(|src| src.value.syntax().text_range()) + } + DefWithBody::Const(c) => { + c.source(db).and_then(|src| src.value.syntax().text_range()) + } + DefWithBody::Static(s) => { + s.source(db).and_then(|src| src.value.syntax().text_range()) + } }; let mut res = FxHashMap::default(); res.insert(file_id, Some(range)); @@ -134,17 +140,35 @@ impl Definition { if let Definition::LifetimeParam(param) = self { #[allow(deprecated)] let range = match param.parent(db) { - hir::GenericDef::Function(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::Function(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } hir::GenericDef::Adt(it) => match it { - hir::Adt::Struct(it) => it.source_old(db).value.syntax().text_range(), - hir::Adt::Union(it) => it.source_old(db).value.syntax().text_range(), - hir::Adt::Enum(it) => it.source_old(db).value.syntax().text_range(), + hir::Adt::Struct(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::Adt::Union(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::Adt::Enum(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } }, - hir::GenericDef::Trait(it) => it.source_old(db).value.syntax().text_range(), - hir::GenericDef::TypeAlias(it) => it.source_old(db).value.syntax().text_range(), - hir::GenericDef::Impl(it) => it.source_old(db).value.syntax().text_range(), - hir::GenericDef::Variant(it) => it.source_old(db).value.syntax().text_range(), - hir::GenericDef::Const(it) => it.source_old(db).value.syntax().text_range(), + hir::GenericDef::Trait(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::GenericDef::TypeAlias(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::GenericDef::Impl(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::GenericDef::Variant(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } + hir::GenericDef::Const(it) => { + it.source(db).and_then(|src| src.value.syntax().text_range()) + } }; let mut res = FxHashMap::default(); res.insert(file_id, Some(range)); From 71c9a884d1d9b27bf9da8ce7d328d74349db54a2 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 15:41:04 +1100 Subject: [PATCH 08/17] Fix type error with .and_then --- crates/ide_db/src/search.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index c4aff39325..436c59d2c7 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -120,20 +120,19 @@ impl Definition { let file_id = module_src.file_id.original_file(db); if let Definition::Local(var) = self { - #[allow(deprecated)] let range = match var.parent(db) { DefWithBody::Function(f) => { - f.source(db).and_then(|src| src.value.syntax().text_range()) + f.source(db).and_then(|src| Some(src.value.syntax().text_range())) } DefWithBody::Const(c) => { - c.source(db).and_then(|src| src.value.syntax().text_range()) + c.source(db).and_then(|src| Some(src.value.syntax().text_range())) } DefWithBody::Static(s) => { - s.source(db).and_then(|src| src.value.syntax().text_range()) + s.source(db).and_then(|src| Some(src.value.syntax().text_range())) } }; let mut res = FxHashMap::default(); - res.insert(file_id, Some(range)); + res.insert(file_id, range); return SearchScope::new(res); } @@ -141,37 +140,37 @@ impl Definition { #[allow(deprecated)] let range = match param.parent(db) { hir::GenericDef::Function(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::GenericDef::Adt(it) => match it { hir::Adt::Struct(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::Adt::Union(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::Adt::Enum(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } }, hir::GenericDef::Trait(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::GenericDef::TypeAlias(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::GenericDef::Impl(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::GenericDef::Variant(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } hir::GenericDef::Const(it) => { - it.source(db).and_then(|src| src.value.syntax().text_range()) + it.source(db).and_then(|src| Some(src.value.syntax().text_range())) } }; let mut res = FxHashMap::default(); - res.insert(file_id, Some(range)); + res.insert(file_id, range); return SearchScope::new(res); } From 1c009c9f1243d81c5caee082ce638d60c955a1bb Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 15:41:41 +1100 Subject: [PATCH 09/17] Go back to use of source_old() in offset_target_and_file_id as it's not as simple as I thought --- crates/assists/src/handlers/fix_visibility.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index aa6aed9c24..f5cd5f9da4 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -151,7 +151,8 @@ fn target_data_for_def( S: HasSource, Ast: AstNode + ast::VisibilityOwner, { - let source = x.source(db)?; + #[allow(deprecated)] + let source = x.source_old(db); let in_file_syntax = source.syntax(); let file_id = in_file_syntax.file_id; let syntax = in_file_syntax.value; From 0a9b73524059cf29a19f9333c7753c11e268ef51 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 16:49:44 +1100 Subject: [PATCH 10/17] Handle missing source in filter_assoc_items --- crates/assists/src/utils.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index d15e5a24be..7c159b5ba0 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -98,13 +98,14 @@ pub fn filter_assoc_items( items .iter() - .map(|i| { - #[allow(deprecated)] - match i { - hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source_old(db).value), - hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source_old(db).value), - hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source_old(db).value), - } + // Note: This throws away items with no source. + .filter_map(|i| { + let item = match i { + hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db)?.value), + hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db)?.value), + hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db)?.value), + }; + Some(item) }) .filter(has_def_name) .filter(|it| match it { From 6800c606ec7be5f19c1728a246eb2e2ffa4110f6 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 16:52:59 +1100 Subject: [PATCH 11/17] Handle missing source in target_data_for_def --- crates/assists/src/handlers/fix_visibility.rs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index f5cd5f9da4..de1e8f0bfa 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -146,54 +146,53 @@ fn target_data_for_def( fn offset_target_and_file_id( db: &dyn HirDatabase, x: S, - ) -> (TextSize, Option, TextRange, FileId) + ) -> Option<(TextSize, Option, TextRange, FileId)> where S: HasSource, Ast: AstNode + ast::VisibilityOwner, { - #[allow(deprecated)] - let source = x.source_old(db); + let source = x.source(db)?; let in_file_syntax = source.syntax(); let file_id = in_file_syntax.file_id; let syntax = in_file_syntax.value; let current_visibility = source.value.visibility(); - ( + Some(( vis_offset(syntax), current_visibility, syntax.text_range(), file_id.original_file(db.upcast()), - ) + )) } let target_name; let (offset, current_visibility, target, target_file) = match def { hir::ModuleDef::Function(f) => { target_name = Some(f.name(db)); - offset_target_and_file_id(db, f) + offset_target_and_file_id(db, f)? } hir::ModuleDef::Adt(adt) => { target_name = Some(adt.name(db)); match adt { - hir::Adt::Struct(s) => offset_target_and_file_id(db, s), - hir::Adt::Union(u) => offset_target_and_file_id(db, u), - hir::Adt::Enum(e) => offset_target_and_file_id(db, e), + hir::Adt::Struct(s) => offset_target_and_file_id(db, s)?, + hir::Adt::Union(u) => offset_target_and_file_id(db, u)?, + hir::Adt::Enum(e) => offset_target_and_file_id(db, e)?, } } hir::ModuleDef::Const(c) => { target_name = c.name(db); - offset_target_and_file_id(db, c) + offset_target_and_file_id(db, c)? } hir::ModuleDef::Static(s) => { target_name = s.name(db); - offset_target_and_file_id(db, s) + offset_target_and_file_id(db, s)? } hir::ModuleDef::Trait(t) => { target_name = Some(t.name(db)); - offset_target_and_file_id(db, t) + offset_target_and_file_id(db, t)? } hir::ModuleDef::TypeAlias(t) => { target_name = Some(t.name(db)); - offset_target_and_file_id(db, t) + offset_target_and_file_id(db, t)? } hir::ModuleDef::Module(m) => { target_name = m.name(db); From 3f1b3df65bee923e5de0652ea4b676530da29127 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 17:13:15 +1100 Subject: [PATCH 12/17] Move impls of ToNav that use source() to TryToNav --- crates/ide/src/call_hierarchy.rs | 8 +- crates/ide/src/display/navigation_target.rs | 105 ++++++++++---------- crates/ide/src/goto_implementation.rs | 6 +- crates/ide/src/goto_type_definition.rs | 4 +- crates/ide/src/hover.rs | 10 +- 5 files changed, 64 insertions(+), 69 deletions(-) diff --git a/crates/ide/src/call_hierarchy.rs b/crates/ide/src/call_hierarchy.rs index 60e0cd4add..3c2d39f5df 100644 --- a/crates/ide/src/call_hierarchy.rs +++ b/crates/ide/src/call_hierarchy.rs @@ -8,7 +8,7 @@ use ide_db::RootDatabase; use syntax::{ast, match_ast, AstNode, TextRange}; use crate::{ - display::ToNav, goto_definition, references, FilePosition, NavigationTarget, RangeInfo, + display::TryToNav, goto_definition, references, FilePosition, NavigationTarget, RangeInfo, }; #[derive(Debug, Clone)] @@ -61,7 +61,7 @@ pub(crate) fn incoming_calls(db: &RootDatabase, position: FilePosition) -> Optio match node { ast::Fn(it) => { let def = sema.to_def(&it)?; - Some(def.to_nav(sema.db)) + def.try_to_nav(sema.db) }, _ => None, } @@ -99,7 +99,7 @@ pub(crate) fn outgoing_calls(db: &RootDatabase, position: FilePosition) -> Optio match callable.kind() { hir::CallableKind::Function(it) => { let fn_def: hir::Function = it.into(); - let nav = fn_def.to_nav(db); + let nav = fn_def.try_to_nav(db)?; Some(nav) } _ => None, @@ -107,7 +107,7 @@ pub(crate) fn outgoing_calls(db: &RootDatabase, position: FilePosition) -> Optio } FnCallNode::MethodCallExpr(expr) => { let function = sema.resolve_method_call(&expr)?; - Some(function.to_nav(db)) + function.try_to_nav(db) } } { Some((func_target, name_ref.syntax().text_range())) diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index 5dc3f41287..1fb26c2266 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -211,12 +211,12 @@ impl TryToNav for Definition { fn try_to_nav(&self, db: &RootDatabase) -> Option { match self { Definition::Macro(it) => it.try_to_nav(db), - Definition::Field(it) => Some(it.to_nav(db)), + Definition::Field(it) => it.try_to_nav(db), Definition::ModuleDef(it) => it.try_to_nav(db), - Definition::SelfType(it) => Some(it.to_nav(db)), + Definition::SelfType(it) => it.try_to_nav(db), Definition::Local(it) => Some(it.to_nav(db)), - Definition::TypeParam(it) => Some(it.to_nav(db)), - Definition::LifetimeParam(it) => Some(it.to_nav(db)), + Definition::TypeParam(it) => it.try_to_nav(db), + Definition::LifetimeParam(it) => it.try_to_nav(db), Definition::Label(it) => Some(it.to_nav(db)), Definition::ConstParam(it) => Some(it.to_nav(db)), } @@ -225,18 +225,17 @@ impl TryToNav for Definition { impl TryToNav for hir::ModuleDef { fn try_to_nav(&self, db: &RootDatabase) -> Option { - let res = match self { - hir::ModuleDef::Module(it) => it.to_nav(db), - hir::ModuleDef::Function(it) => it.to_nav(db), - hir::ModuleDef::Adt(it) => it.to_nav(db), - hir::ModuleDef::Variant(it) => it.to_nav(db), - hir::ModuleDef::Const(it) => it.to_nav(db), - hir::ModuleDef::Static(it) => it.to_nav(db), - hir::ModuleDef::Trait(it) => it.to_nav(db), - hir::ModuleDef::TypeAlias(it) => it.to_nav(db), - hir::ModuleDef::BuiltinType(_) => return None, - }; - Some(res) + match self { + hir::ModuleDef::Module(it) => Some(it.to_nav(db)), + hir::ModuleDef::Function(it) => it.try_to_nav(db), + hir::ModuleDef::Adt(it) => it.try_to_nav(db), + hir::ModuleDef::Variant(it) => it.try_to_nav(db), + hir::ModuleDef::Const(it) => it.try_to_nav(db), + hir::ModuleDef::Static(it) => it.try_to_nav(db), + hir::ModuleDef::Trait(it) => it.try_to_nav(db), + hir::ModuleDef::TypeAlias(it) => it.try_to_nav(db), + hir::ModuleDef::BuiltinType(_) => None, + } } } @@ -271,14 +270,13 @@ impl ToNavFromAst for hir::Trait { const KIND: SymbolKind = SymbolKind::Trait; } -impl ToNav for D +impl TryToNav for D where D: HasSource + ToNavFromAst + Copy + HasAttrs, D::Ast: ast::NameOwner + ShortLabel, { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; let mut res = NavigationTarget::from_named( db, src.as_ref().map(|it| it as &dyn ast::NameOwner), @@ -286,7 +284,7 @@ where ); res.docs = self.docs(db); res.description = src.value.short_label(); - res + Some(res) } } @@ -305,10 +303,9 @@ impl ToNav for hir::Module { } } -impl ToNav for hir::Impl { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); +impl TryToNav for hir::Impl { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; let derive_attr = self.is_builtin_derive(db); let frange = if let Some(item) = &derive_attr { item.syntax().original_file_range(db) @@ -321,22 +318,21 @@ impl ToNav for hir::Impl { src.value.self_ty().map(|ty| src.with_value(ty.syntax()).original_file_range(db).range) }; - NavigationTarget::from_syntax( + Some(NavigationTarget::from_syntax( frange.file_id, "impl".into(), focus_range, frange.range, SymbolKind::Impl, - ) + )) } } -impl ToNav for hir::Field { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); +impl TryToNav for hir::Field { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; - match &src.value { + let field_source = match &src.value { FieldSource::Named(it) => { let mut res = NavigationTarget::from_named(db, src.with_value(it), SymbolKind::Field); @@ -354,7 +350,8 @@ impl ToNav for hir::Field { SymbolKind::Field, ) } - } + }; + Some(field_source) } } @@ -372,22 +369,22 @@ impl TryToNav for hir::MacroDef { } } -impl ToNav for hir::Adt { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { +impl TryToNav for hir::Adt { + fn try_to_nav(&self, db: &RootDatabase) -> Option { match self { - hir::Adt::Struct(it) => it.to_nav(db), - hir::Adt::Union(it) => it.to_nav(db), - hir::Adt::Enum(it) => it.to_nav(db), + hir::Adt::Struct(it) => it.try_to_nav(db), + hir::Adt::Union(it) => it.try_to_nav(db), + hir::Adt::Enum(it) => it.try_to_nav(db), } } } -impl ToNav for hir::AssocItem { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { +impl TryToNav for hir::AssocItem { + fn try_to_nav(&self, db: &RootDatabase) -> Option { match self { - AssocItem::Function(it) => it.to_nav(db), - AssocItem::Const(it) => it.to_nav(db), - AssocItem::TypeAlias(it) => it.to_nav(db), + AssocItem::Function(it) => it.try_to_nav(db), + AssocItem::Const(it) => it.try_to_nav(db), + AssocItem::TypeAlias(it) => it.try_to_nav(db), } } } @@ -441,10 +438,9 @@ impl ToNav for hir::Label { } } -impl ToNav for hir::TypeParam { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); +impl TryToNav for hir::TypeParam { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; let full_range = match &src.value { Either::Left(it) => it.syntax().text_range(), Either::Right(it) => it.syntax().text_range(), @@ -453,7 +449,7 @@ impl ToNav for hir::TypeParam { Either::Left(_) => None, Either::Right(it) => it.name().map(|it| it.syntax().text_range()), }; - NavigationTarget { + Some(NavigationTarget { file_id: src.file_id.original_file(db), name: self.name(db).to_string().into(), kind: Some(SymbolKind::TypeParam), @@ -462,16 +458,15 @@ impl ToNav for hir::TypeParam { container_name: None, description: None, docs: None, - } + }) } } -impl ToNav for hir::LifetimeParam { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - #[allow(deprecated)] - let src = self.source_old(db); +impl TryToNav for hir::LifetimeParam { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; let full_range = src.value.syntax().text_range(); - NavigationTarget { + Some(NavigationTarget { file_id: src.file_id.original_file(db), name: self.name(db).to_string().into(), kind: Some(SymbolKind::LifetimeParam), @@ -480,7 +475,7 @@ impl ToNav for hir::LifetimeParam { container_name: None, description: None, docs: None, - } + }) } } diff --git a/crates/ide/src/goto_implementation.rs b/crates/ide/src/goto_implementation.rs index 6eac396392..da9378a97e 100644 --- a/crates/ide/src/goto_implementation.rs +++ b/crates/ide/src/goto_implementation.rs @@ -2,7 +2,7 @@ use hir::{Crate, Impl, Semantics}; use ide_db::RootDatabase; use syntax::{algo::find_node_at_offset, ast, AstNode}; -use crate::{display::ToNav, FilePosition, NavigationTarget, RangeInfo}; +use crate::{display::TryToNav, FilePosition, NavigationTarget, RangeInfo}; // Feature: Go to Implementation // @@ -55,7 +55,7 @@ fn impls_for_def( impls .into_iter() .filter(|impl_def| ty.is_equal_for_find_impls(&impl_def.target_ty(sema.db))) - .map(|imp| imp.to_nav(sema.db)) + .filter_map(|imp| imp.try_to_nav(sema.db)) .collect(), ) } @@ -69,7 +69,7 @@ fn impls_for_trait( let impls = Impl::for_trait(sema.db, krate, tr); - Some(impls.into_iter().map(|imp| imp.to_nav(sema.db)).collect()) + Some(impls.into_iter().filter_map(|imp| imp.try_to_nav(sema.db)).collect()) } #[cfg(test)] diff --git a/crates/ide/src/goto_type_definition.rs b/crates/ide/src/goto_type_definition.rs index aba6bf5dc2..7e84e06bea 100644 --- a/crates/ide/src/goto_type_definition.rs +++ b/crates/ide/src/goto_type_definition.rs @@ -1,7 +1,7 @@ use ide_db::RootDatabase; use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; -use crate::{display::ToNav, FilePosition, NavigationTarget, RangeInfo}; +use crate::{display::TryToNav, FilePosition, NavigationTarget, RangeInfo}; // Feature: Go to Type Definition // @@ -37,7 +37,7 @@ pub(crate) fn goto_type_definition( let adt_def = ty.autoderef(db).filter_map(|ty| ty.as_adt()).last()?; - let nav = adt_def.to_nav(db); + let nav = adt_def.try_to_nav(db)?; Some(RangeInfo::new(node.text_range(), vec![nav])) } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index d2a0cfcd40..2737c900f4 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -13,7 +13,7 @@ use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, use test_utils::mark; use crate::{ - display::{macro_label, ShortLabel, ToNav, TryToNav}, + display::{macro_label, ShortLabel, TryToNav}, doc_links::{remove_links, rewrite_links}, markdown_remove::remove_markdown, markup::Markup, @@ -183,10 +183,10 @@ fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option match it { - ModuleDef::Adt(Adt::Struct(it)) => Some(to_action(it.to_nav(db))), - ModuleDef::Adt(Adt::Union(it)) => Some(to_action(it.to_nav(db))), - ModuleDef::Adt(Adt::Enum(it)) => Some(to_action(it.to_nav(db))), - ModuleDef::Trait(it) => Some(to_action(it.to_nav(db))), + ModuleDef::Adt(Adt::Struct(it)) => Some(to_action(it.try_to_nav(db)?)), + ModuleDef::Adt(Adt::Union(it)) => Some(to_action(it.try_to_nav(db)?)), + ModuleDef::Adt(Adt::Enum(it)) => Some(to_action(it.try_to_nav(db)?)), + ModuleDef::Trait(it) => Some(to_action(it.try_to_nav(db)?)), _ => None, }, _ => None, From 7bfec89cf969aa630f184f7d4a66e8e12a423d2f Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 17:26:39 +1100 Subject: [PATCH 13/17] Make the result of Const, FunctionRender and TypeAliasRender constructors optional They use source() which now returns an Option so they need to too. --- crates/completion/src/completions.rs | 5 +++-- crates/completion/src/render.rs | 3 +-- crates/completion/src/render/const_.rs | 9 ++++----- crates/completion/src/render/function.rs | 11 +++++------ crates/completion/src/render/type_alias.rs | 9 ++++----- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/crates/completion/src/completions.rs b/crates/completion/src/completions.rs index d9fe134855..00c9e76f03 100644 --- a/crates/completion/src/completions.rs +++ b/crates/completion/src/completions.rs @@ -106,8 +106,9 @@ impl Completions { func: hir::Function, local_name: Option, ) { - let item = render_fn(RenderContext::new(ctx), None, local_name, func); - self.add(item) + if let Some(item) = render_fn(RenderContext::new(ctx), None, local_name, func) { + self.add(item) + } } pub(crate) fn add_variant_pat( diff --git a/crates/completion/src/render.rs b/crates/completion/src/render.rs index 1ba7201a13..ac0b2a5139 100644 --- a/crates/completion/src/render.rs +++ b/crates/completion/src/render.rs @@ -157,8 +157,7 @@ impl<'a> Render<'a> { let kind = match resolution { ScopeDef::ModuleDef(Function(func)) => { - let item = render_fn(self.ctx, import_to_add, Some(local_name), *func); - return Some(item); + return render_fn(self.ctx, import_to_add, Some(local_name), *func); } ScopeDef::ModuleDef(Variant(_)) if self.ctx.completion.is_pat_binding_or_const diff --git a/crates/completion/src/render/const_.rs b/crates/completion/src/render/const_.rs index 648a1afc51..ce924f3095 100644 --- a/crates/completion/src/render/const_.rs +++ b/crates/completion/src/render/const_.rs @@ -15,7 +15,7 @@ pub(crate) fn render_const<'a>( ctx: RenderContext<'a>, const_: hir::Const, ) -> Option { - ConstRender::new(ctx, const_).render() + ConstRender::new(ctx, const_)?.render() } #[derive(Debug)] @@ -26,10 +26,9 @@ struct ConstRender<'a> { } impl<'a> ConstRender<'a> { - fn new(ctx: RenderContext<'a>, const_: hir::Const) -> ConstRender<'a> { - #[allow(deprecated)] - let ast_node = const_.source_old(ctx.db()).value; - ConstRender { ctx, const_, ast_node } + fn new(ctx: RenderContext<'a>, const_: hir::Const) -> Option> { + let ast_node = const_.source(ctx.db())?.value; + Some(ConstRender { ctx, const_, ast_node }) } fn render(self) -> Option { diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index 4c89962040..081be14f4e 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -14,9 +14,9 @@ pub(crate) fn render_fn<'a>( import_to_add: Option, local_name: Option, fn_: hir::Function, -) -> CompletionItem { +) -> Option { let _p = profile::span("render_fn"); - FunctionRender::new(ctx, local_name, fn_).render(import_to_add) + Some(FunctionRender::new(ctx, local_name, fn_)?.render(import_to_add)) } #[derive(Debug)] @@ -32,12 +32,11 @@ impl<'a> FunctionRender<'a> { ctx: RenderContext<'a>, local_name: Option, fn_: hir::Function, - ) -> FunctionRender<'a> { + ) -> Option> { let name = local_name.unwrap_or_else(|| fn_.name(ctx.db()).to_string()); - #[allow(deprecated)] - let ast_node = fn_.source_old(ctx.db()).value; + let ast_node = fn_.source(ctx.db())?.value; - FunctionRender { ctx, name, func: fn_, ast_node } + Some(FunctionRender { ctx, name, func: fn_, ast_node }) } fn render(self, import_to_add: Option) -> CompletionItem { diff --git a/crates/completion/src/render/type_alias.rs b/crates/completion/src/render/type_alias.rs index 276090015e..69b445b9c6 100644 --- a/crates/completion/src/render/type_alias.rs +++ b/crates/completion/src/render/type_alias.rs @@ -15,7 +15,7 @@ pub(crate) fn render_type_alias<'a>( ctx: RenderContext<'a>, type_alias: hir::TypeAlias, ) -> Option { - TypeAliasRender::new(ctx, type_alias).render() + TypeAliasRender::new(ctx, type_alias)?.render() } #[derive(Debug)] @@ -26,10 +26,9 @@ struct TypeAliasRender<'a> { } impl<'a> TypeAliasRender<'a> { - fn new(ctx: RenderContext<'a>, type_alias: hir::TypeAlias) -> TypeAliasRender<'a> { - #[allow(deprecated)] - let ast_node = type_alias.source_old(ctx.db()).value; - TypeAliasRender { ctx, type_alias, ast_node } + fn new(ctx: RenderContext<'a>, type_alias: hir::TypeAlias) -> Option> { + let ast_node = type_alias.source(ctx.db())?.value; + Some(TypeAliasRender { ctx, type_alias, ast_node }) } fn render(self) -> Option { From 3a1f8e897bc2af56cd7a6c08bad1bf88cc97b257 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Fri, 1 Jan 2021 17:30:13 +1100 Subject: [PATCH 14/17] Remove source_old from adding const and function impls --- .../completion/src/completions/trait_impl.rs | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/crates/completion/src/completions/trait_impl.rs b/crates/completion/src/completions/trait_impl.rs index 43b3d939f3..54bb897e9b 100644 --- a/crates/completion/src/completions/trait_impl.rs +++ b/crates/completion/src/completions/trait_impl.rs @@ -156,20 +156,21 @@ fn add_function_impl( }; let range = TextRange::new(fn_def_node.text_range().start(), ctx.source_range().end()); - #[allow(deprecated)] - let function_decl = function_declaration(&func.source_old(ctx.db).value); - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = format!("{} {{\n $0\n}}", function_decl); - builder.snippet_edit(cap, TextEdit::replace(range, snippet)) - } - None => { - let header = format!("{} {{", function_decl); - builder.text_edit(TextEdit::replace(range, header)) + if let Some(src) = func.source(ctx.db) { + let function_decl = function_declaration(&src.value); + match ctx.config.snippet_cap { + Some(cap) => { + let snippet = format!("{} {{\n $0\n}}", function_decl); + builder.snippet_edit(cap, TextEdit::replace(range, snippet)) + } + None => { + let header = format!("{} {{", function_decl); + builder.text_edit(TextEdit::replace(range, header)) + } } + .kind(completion_kind) + .add_to(acc); } - .kind(completion_kind) - .add_to(acc); } fn add_type_alias_impl( @@ -201,17 +202,19 @@ fn add_const_impl( let const_name = const_.name(ctx.db).map(|n| n.to_string()); if let Some(const_name) = const_name { - #[allow(deprecated)] - let snippet = make_const_compl_syntax(&const_.source_old(ctx.db).value); + if let Some(source) = const_.source(ctx.db) { + let snippet = make_const_compl_syntax(&source.value); - let range = TextRange::new(const_def_node.text_range().start(), ctx.source_range().end()); + let range = + TextRange::new(const_def_node.text_range().start(), ctx.source_range().end()); - CompletionItem::new(CompletionKind::Magic, ctx.source_range(), snippet.clone()) - .text_edit(TextEdit::replace(range, snippet)) - .lookup_by(const_name) - .kind(CompletionItemKind::Const) - .set_documentation(const_.docs(ctx.db)) - .add_to(acc); + CompletionItem::new(CompletionKind::Magic, ctx.source_range(), snippet.clone()) + .text_edit(TextEdit::replace(range, snippet)) + .lookup_by(const_name) + .kind(CompletionItemKind::Const) + .set_documentation(const_.docs(ctx.db)) + .add_to(acc); + } } } From 5c659c21dda26311be584d0bb89b80f8ae3d7bae Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Sat, 2 Jan 2021 09:48:51 +1100 Subject: [PATCH 15/17] Handle case where detail doesn't exist without giving up on completion Co-authored-by: Aleksey Kladov --- crates/completion/src/render/macro_.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs index 0612591fd2..6f4f9945c1 100644 --- a/crates/completion/src/render/macro_.rs +++ b/crates/completion/src/render/macro_.rs @@ -45,7 +45,7 @@ impl<'a> MacroRender<'a> { .set_documentation(self.docs.clone()) .set_deprecated(self.ctx.is_deprecated(self.macro_)) .add_import(import_to_add) - .detail(self.detail()?); + .set_detail(self.detail()); let needs_bang = self.needs_bang(); builder = match self.ctx.snippet_cap() { From 887028fcf52bf7f3af55114f112123c902989bed Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Sat, 2 Jan 2021 16:25:36 +1100 Subject: [PATCH 16/17] Remove old_source now we've fully migrated Fixes #6913 --- crates/hir/src/has_source.rs | 68 ------------------------------------ 1 file changed, 68 deletions(-) diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index 8a7306def8..57baeb3cf3 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -16,8 +16,6 @@ use crate::{ pub trait HasSource { type Ast; - #[deprecated = "migrating to source() method that returns an Option"] - fn source_old(self, db: &dyn HirDatabase) -> InFile; fn source(self, db: &dyn HirDatabase) -> Option>; } @@ -48,15 +46,6 @@ impl Module { impl HasSource for Field { type Ast = FieldSource; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - let var = VariantId::from(self.parent); - let src = var.child_source(db.upcast()); - src.map(|it| match it[self.id].clone() { - Either::Left(it) => FieldSource::Pos(it), - Either::Right(it) => FieldSource::Named(it), - }) - } - fn source(self, db: &dyn HirDatabase) -> Option> { let var = VariantId::from(self.parent); let src = var.child_source(db.upcast()); @@ -69,103 +58,60 @@ impl HasSource for Field { } impl HasSource for Struct { type Ast = ast::Struct; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Union { type Ast = ast::Union; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Enum { type Ast = ast::Enum; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Variant { type Ast = ast::Variant; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.parent.id.child_source(db.upcast()).map(|map| map[self.id].clone()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.parent.id.child_source(db.upcast()).map(|map| map[self.id].clone())) } } impl HasSource for Function { type Ast = ast::Fn; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Const { type Ast = ast::Const; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Static { type Ast = ast::Static; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for Trait { type Ast = ast::Trait; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for TypeAlias { type Ast = ast::TypeAlias; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } } impl HasSource for MacroDef { type Ast = ast::Macro; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - InFile { - file_id: self.id.ast_id.expect("MacroDef without ast_id").file_id, - value: self.id.ast_id.expect("MacroDef without ast_id").to_node(db.upcast()), - } - } - fn source(self, db: &dyn HirDatabase) -> Option> { let ast_id = self.id.ast_id?; Some(InFile { file_id: ast_id.file_id, value: ast_id.to_node(db.upcast()) }) @@ -173,10 +119,6 @@ impl HasSource for MacroDef { } impl HasSource for Impl { type Ast = ast::Impl; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - self.id.lookup(db.upcast()).source(db.upcast()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { Some(self.id.lookup(db.upcast()).source(db.upcast())) } @@ -184,11 +126,6 @@ impl HasSource for Impl { impl HasSource for TypeParam { type Ast = Either; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - let child_source = self.id.parent.child_source(db.upcast()); - child_source.map(|it| it[self.id.local_id].clone()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { let child_source = self.id.parent.child_source(db.upcast()); Some(child_source.map(|it| it[self.id.local_id].clone())) @@ -197,11 +134,6 @@ impl HasSource for TypeParam { impl HasSource for LifetimeParam { type Ast = ast::LifetimeParam; - fn source_old(self, db: &dyn HirDatabase) -> InFile { - let child_source = self.id.parent.child_source(db.upcast()); - child_source.map(|it| it[self.id.local_id].clone()) - } - fn source(self, db: &dyn HirDatabase) -> Option> { let child_source = self.id.parent.child_source(db.upcast()); Some(child_source.map(|it| it[self.id.local_id].clone())) From 40cd6cdf67dcfad89a80ff3a662bec2dfd983d67 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Sat, 2 Jan 2021 22:11:25 +1100 Subject: [PATCH 17/17] Fix ConstParam HasSource impl and implement TryToNav not Nav --- crates/hir/src/has_source.rs | 4 ++-- crates/ide/src/display/navigation_target.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index 57baeb3cf3..7c57d8378c 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -142,8 +142,8 @@ impl HasSource for LifetimeParam { impl HasSource for ConstParam { type Ast = ast::ConstParam; - fn source(self, db: &dyn HirDatabase) -> InFile { + fn source(self, db: &dyn HirDatabase) -> Option> { let child_source = self.id.parent.child_source(db.upcast()); - child_source.map(|it| it[self.id.local_id].clone()) + Some(child_source.map(|it| it[self.id.local_id].clone())) } } diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index 1fb26c2266..e24c783017 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -218,7 +218,7 @@ impl TryToNav for Definition { Definition::TypeParam(it) => it.try_to_nav(db), Definition::LifetimeParam(it) => it.try_to_nav(db), Definition::Label(it) => Some(it.to_nav(db)), - Definition::ConstParam(it) => Some(it.to_nav(db)), + Definition::ConstParam(it) => it.try_to_nav(db), } } } @@ -479,11 +479,11 @@ impl TryToNav for hir::LifetimeParam { } } -impl ToNav for hir::ConstParam { - fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { - let src = self.source(db); +impl TryToNav for hir::ConstParam { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let src = self.source(db)?; let full_range = src.value.syntax().text_range(); - NavigationTarget { + Some(NavigationTarget { file_id: src.file_id.original_file(db), name: self.name(db).to_string().into(), kind: Some(SymbolKind::ConstParam), @@ -492,7 +492,7 @@ impl ToNav for hir::ConstParam { container_name: None, description: None, docs: None, - } + }) } }