Merge #7115

7115: Migrate HasSource::source to return Option r=matklad a=nick96

I've made a start on fixing #6913 based on the provided work plan, migrating `HasSource::source` to return an `Option`. The simple cases are migrated but there are a few that I'm unsure exactly how they should be handled:

- Logging the processing of functions in `AnalysisStatsCmd::run`: In verbose mode it includes the path to the module containing the function and the syntax range. I've handled this with an if-let but would it be better to blow up here with `expect`? I'm not 100% on the code paths but if we're processing a function definition then the source should exist.

I've handled `source()` in all code paths as `None` being a valid return value but are there some cases where we should just blow up? Also, all I've done is bubble up the returned `None`s, there may be some places where we can recover and still provide something.

Co-authored-by: Nick Spain <[email protected]>
Co-authored-by: Nick Spain <[email protected]>

7 files changed, 47 insertions, 49 deletions

diff --git a/crates/completion/src/completions.rs b/crates/completion/src/completions.rs
index d9fe13485..00c9e76f0 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<String>,
     ) {
-        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/completions/trait_impl.rs b/crates/completion/src/completions/trait_impl.rs
index c4e0d0669..54bb897e9 100644
--- a/crates/completion/src/completions/trait_impl.rs
+++ b/crates/completion/src/completions/trait_impl.rs
@@ -156,19 +156,21 @@ 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);
-    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(
@@ -200,16 +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 {
-        let snippet = make_const_compl_syntax(&const_.source(ctx.db).value);
-
-        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);
+        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());
+
+            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);
+        }
     }
 }
 
diff --git a/crates/completion/src/render.rs b/crates/completion/src/render.rs
index 1ba7201a1..ac0b2a513 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 039bdabc0..ce924f309 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<CompletionItem> {
-    ConstRender::new(ctx, const_).render()
+    ConstRender::new(ctx, const_)?.render()
 }
 
 #[derive(Debug)]
@@ -26,9 +26,9 @@ 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;
-        ConstRender { ctx, const_, ast_node }
+    fn new(ctx: RenderContext<'a>, const_: hir::Const) -> Option<ConstRender<'a>> {
+        let ast_node = const_.source(ctx.db())?.value;
+        Some(ConstRender { ctx, const_, ast_node })
     }
 
     fn render(self) -> Option<CompletionItem> {
diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs
index 316e05b52..081be14f4 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<ImportEdit>,
     local_name: Option<String>,
     fn_: hir::Function,
-) -> CompletionItem {
+) -> Option<CompletionItem> {
     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,11 +32,11 @@ impl<'a> FunctionRender<'a> {
         ctx: RenderContext<'a>,
         local_name: Option<String>,
         fn_: hir::Function,
-    ) -> FunctionRender<'a> {
+    ) -> Option<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(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<ImportEdit>) -> CompletionItem {
diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs
index dac79592f..6f4f9945c 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<ImportEdit>) -> Option<CompletionItem> {
-        // 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());
+                .set_detail(self.detail());
 
         let needs_bang = self.needs_bang();
         builder = match self.ctx.snippet_cap() {
@@ -95,9 +88,9 @@ impl<'a> MacroRender<'a> {
         format!("{}!", self.name)
     }
 
-    fn detail(&self) -> String {
-        let ast_node = self.macro_.source(self.ctx.db()).value;
-        macro_label(&ast_node)
+    fn detail(&self) -> Option<String> {
+        let ast_node = self.macro_.source(self.ctx.db())?.value;
+        Some(macro_label(&ast_node))
     }
 }
 
diff --git a/crates/completion/src/render/type_alias.rs b/crates/completion/src/render/type_alias.rs
index 9605c7fa9..69b445b9c 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<CompletionItem> {
-    TypeAliasRender::new(ctx, type_alias).render()
+    TypeAliasRender::new(ctx, type_alias)?.render()
 }
 
 #[derive(Debug)]
@@ -26,9 +26,9 @@ 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;
-        TypeAliasRender { ctx, type_alias, ast_node }
+    fn new(ctx: RenderContext<'a>, type_alias: hir::TypeAlias) -> Option<TypeAliasRender<'a>> {
+        let ast_node = type_alias.source(ctx.db())?.value;
+        Some(TypeAliasRender { ctx, type_alias, ast_node })
     }
 
     fn render(self) -> Option<CompletionItem> {