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]>

4 files changed, 17 insertions, 24 deletions

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> {