Option begone part 2

crates/hir-def/src/attr.rs

@@ -498,7 +498,7 @@ impl AttrsWithOwner {
             AttrDefId::FieldId(id) => {
                 let map = db.fields_attrs_source_map(id.parent);
                 let file_id = id.parent.file_id(db);
-                let root = db.parse_or_expand(file_id).unwrap();
+                let root = db.parse_or_expand(file_id);
                 let owner = match &map[id.local_id] {
                     Either::Left(it) => ast::AnyHasAttrs::new(it.to_node(&root)),
                     Either::Right(it) => ast::AnyHasAttrs::new(it.to_node(&root)),
@@ -514,7 +514,7 @@ impl AttrsWithOwner {
             AttrDefId::EnumVariantId(id) => {
                 let map = db.variants_attrs_source_map(id.parent);
                 let file_id = id.parent.lookup(db).id.file_id();
-                let root = db.parse_or_expand(file_id).unwrap();
+                let root = db.parse_or_expand(file_id);
                 InFile::new(file_id, ast::AnyHasAttrs::new(map[id.local_id].to_node(&root)))
             }
             AttrDefId::StaticId(id) => id.lookup(db).source(db).map(ast::AnyHasAttrs::new),

crates/hir-def/src/body.rs

@@ -174,30 +174,12 @@ impl Expander {
     fn enter_expand_inner(
         db: &dyn DefDatabase,
         call_id: MacroCallId,
-        mut error: Option<ExpandError>,
+        error: Option<ExpandError>,
     ) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
         let file_id = call_id.as_file();
         let ExpandResult { value, err } = db.parse_or_expand_with_err(file_id);
 
-        if error.is_none() {
-            error = err;
-        }
-
-        let parse = match value {
-            Some(it) => it,
-            None => {
-                // Only `None` if the macro expansion produced no usable AST.
-                if error.is_none() {
-                    tracing::warn!("no error despite `parse_or_expand` failing");
-                }
-
-                return ExpandResult::only_err(error.unwrap_or_else(|| {
-                    ExpandError::Other("failed to parse macro invocation".into())
-                }));
-            }
-        };
-
-        ExpandResult { value: Some(InFile::new(file_id, parse)), err: error }
+        ExpandResult { value: Some(InFile::new(file_id, value)), err: error.or(err) }
     }
 
     pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {

crates/hir-def/src/data.rs

@@ -640,22 +640,20 @@ impl<'a> AssocItemCollector<'a> {
                 AssocItem::MacroCall(call) => {
                     let file_id = self.expander.current_file_id();
                     let root = self.db.parse_or_expand(file_id);
-                    if let Some(root) = root {
-                        let call = &item_tree[call];
-
-                        let ast_id_map = self.db.ast_id_map(file_id);
-                        let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
-                        let _cx = stdx::panic_context::enter(format!(
-                            "collect_items MacroCall: {macro_call}"
-                        ));
-                        if let Ok(res) =
-                            self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
-                        {
-                            self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
-                                ast_id: InFile::new(file_id, call.ast_id),
-                                expand_to: hir_expand::ExpandTo::Items,
-                            });
-                        }
+                    let call = &item_tree[call];
+
+                    let ast_id_map = self.db.ast_id_map(file_id);
+                    let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
+                    let _cx = stdx::panic_context::enter(format!(
+                        "collect_items MacroCall: {macro_call}"
+                    ));
+                    if let Ok(res) =
+                        self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
+                    {
+                        self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
+                            ast_id: InFile::new(file_id, call.ast_id),
+                            expand_to: hir_expand::ExpandTo::Items,
+                        });
                     }
                 }
             }

crates/hir-def/src/item_tree.rs

@@ -108,10 +108,7 @@ pub struct ItemTree {
 impl ItemTree {
     pub(crate) fn file_item_tree_query(db: &dyn DefDatabase, file_id: HirFileId) -> Arc<ItemTree> {
         let _p = profile::span("file_item_tree_query").detail(|| format!("{file_id:?}"));
-        let syntax = match db.parse_or_expand(file_id) {
-            Some(node) => node,
-            None => return Default::default(),
-        };
+        let syntax = db.parse_or_expand(file_id);
         if never!(syntax.kind() == SyntaxKind::ERROR, "{:?} from {:?} {}", file_id, syntax, syntax)
         {
             // FIXME: not 100% sure why these crop up, but return an empty tree to avoid a panic

crates/hir-def/src/src.rs

@@ -20,7 +20,7 @@ impl<N: ItemTreeNode> HasSource for AssocItemLoc<N> {
     fn source(&self, db: &dyn DefDatabase) -> InFile<N::Source> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -33,7 +33,7 @@ impl<N: ItemTreeNode> HasSource for ItemLoc<N> {
     fn source(&self, db: &dyn DefDatabase) -> InFile<N::Source> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -46,7 +46,7 @@ impl HasSource for Macro2Loc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -59,7 +59,7 @@ impl HasSource for MacroRulesLoc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -72,7 +72,7 @@ impl HasSource for ProcMacroLoc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))

crates/hir-expand/src/builtin_derive_macro.rs

@@ -198,7 +198,7 @@ fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, ExpandError> {
 fn expand_simple_derive(tt: &tt::Subtree, trait_path: tt::Subtree) -> ExpandResult<tt::Subtree> {
     let info = match parse_adt(tt) {
         Ok(info) => info,
-        Err(e) => return ExpandResult::with_err(tt::Subtree::empty(), e),
+        Err(e) => return ExpandResult::new(tt::Subtree::empty(), e),
     };
     let mut where_block = vec![];
     let (params, args): (Vec<_>, Vec<_>) = info

crates/hir-expand/src/builtin_fn_macro.rs

@@ -249,10 +249,7 @@ fn format_args_expand(
     let mut args = parse_exprs_with_sep(tt, ',');
 
     if args.is_empty() {
-        return ExpandResult::with_err(
-            tt::Subtree::empty(),
-            mbe::ExpandError::NoMatchingRule.into(),
-        );
+        return ExpandResult::new(tt::Subtree::empty(), mbe::ExpandError::NoMatchingRule.into());
     }
     for arg in &mut args {
         // Remove `key =`.
@@ -575,7 +572,7 @@ fn include_expand(
         Ok((subtree, map, file_id)) => {
             ExpandResult::ok(ExpandedEager { subtree, included_file: Some((file_id, map)) })
         }
-        Err(e) => ExpandResult::with_err(
+        Err(e) => ExpandResult::new(
             ExpandedEager { subtree: tt::Subtree::empty(), included_file: None },
             e,
         ),
@@ -588,7 +585,7 @@ fn include_bytes_expand(
     tt: &tt::Subtree,
 ) -> ExpandResult<ExpandedEager> {
     if let Err(e) = parse_string(tt) {
-        return ExpandResult::with_err(
+        return ExpandResult::new(
             ExpandedEager { subtree: tt::Subtree::empty(), included_file: None },
             e,
         );
@@ -613,7 +610,7 @@ fn include_str_expand(
     let path = match parse_string(tt) {
         Ok(it) => it,
         Err(e) => {
-            return ExpandResult::with_err(
+            return ExpandResult::new(
                 ExpandedEager { subtree: tt::Subtree::empty(), included_file: None },
                 e,
             )
@@ -650,7 +647,7 @@ fn env_expand(
     let key = match parse_string(tt) {
         Ok(it) => it,
         Err(e) => {
-            return ExpandResult::with_err(
+            return ExpandResult::new(
                 ExpandedEager { subtree: tt::Subtree::empty(), included_file: None },
                 e,
             )
@@ -686,7 +683,7 @@ fn option_env_expand(
     let key = match parse_string(tt) {
         Ok(it) => it,
         Err(e) => {
-            return ExpandResult::with_err(
+            return ExpandResult::new(
                 ExpandedEager { subtree: tt::Subtree::empty(), included_file: None },
                 e,
             )

crates/hir-expand/src/db.rs

@@ -98,12 +98,9 @@ pub trait ExpandDatabase: SourceDatabase {
     /// Main public API -- parses a hir file, not caring whether it's a real
     /// file or a macro expansion.
     #[salsa::transparent]
-    fn parse_or_expand(&self, file_id: HirFileId) -> Option<SyntaxNode>;
+    fn parse_or_expand(&self, file_id: HirFileId) -> SyntaxNode;
     #[salsa::transparent]
-    fn parse_or_expand_with_err(
-        &self,
-        file_id: HirFileId,
-    ) -> ExpandResult<Option<Parse<SyntaxNode>>>;
+    fn parse_or_expand_with_err(&self, file_id: HirFileId) -> ExpandResult<Parse<SyntaxNode>>;
     /// Implementation for the macro case.
     fn parse_macro_expansion(
         &self,
@@ -252,27 +249,26 @@ pub fn expand_speculative(
 }
 
 fn ast_id_map(db: &dyn ExpandDatabase, file_id: HirFileId) -> Arc<AstIdMap> {
-    let map = db.parse_or_expand(file_id).map(|it| AstIdMap::from_source(&it)).unwrap_or_default();
-    Arc::new(map)
+    Arc::new(AstIdMap::from_source(&db.parse_or_expand(file_id)))
 }
 
-fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> Option<SyntaxNode> {
-    Some(match file_id.repr() {
+fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> SyntaxNode {
+    match file_id.repr() {
         HirFileIdRepr::FileId(file_id) => db.parse(file_id).tree().syntax().clone(),
         HirFileIdRepr::MacroFile(macro_file) => {
             db.parse_macro_expansion(macro_file).value.0.syntax_node()
         }
-    })
+    }
 }
 
 fn parse_or_expand_with_err(
     db: &dyn ExpandDatabase,
     file_id: HirFileId,
-) -> ExpandResult<Option<Parse<SyntaxNode>>> {
+) -> ExpandResult<Parse<SyntaxNode>> {
     match file_id.repr() {
-        HirFileIdRepr::FileId(file_id) => ExpandResult::ok(Some(db.parse(file_id).to_syntax())),
+        HirFileIdRepr::FileId(file_id) => ExpandResult::ok(db.parse(file_id).to_syntax()),
         HirFileIdRepr::MacroFile(macro_file) => {
-            db.parse_macro_expansion(macro_file).map(|it| Some(it.0))
+            db.parse_macro_expansion(macro_file).map(|(it, _)| it)
         }
     }
 }

crates/hir-expand/src/eager.rs

@@ -111,7 +111,7 @@ fn lazy_expand(
     def: &MacroDefId,
     macro_call: InFile<ast::MacroCall>,
     krate: CrateId,
-) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
+) -> ExpandResult<InFile<Parse<SyntaxNode>>> {
     let ast_id = db.ast_id_map(macro_call.file_id).ast_id(&macro_call.value);
 
     let expand_to = ExpandTo::from_call_site(&macro_call.value);
@@ -121,8 +121,7 @@ fn lazy_expand(
         MacroCallKind::FnLike { ast_id: macro_call.with_value(ast_id), expand_to },
     );
 
-    db.parse_or_expand_with_err(id.as_file())
-        .map(|parse| parse.map(|parse| InFile::new(id.as_file(), parse)))
+    db.parse_or_expand_with_err(id.as_file()).map(|parse| InFile::new(id.as_file(), parse))
 }
 
 fn eager_macro_recur(
@@ -162,8 +161,7 @@ fn eager_macro_recur(
                     Err(err) => return Err(err),
                 };
                 id.map(|call| {
-                    call.and_then(|call| db.parse_or_expand(call.as_file()))
-                        .map(|it| it.clone_for_update())
+                    call.map(|call| db.parse_or_expand(call.as_file()).clone_for_update())
                 })
             }
             MacroDefKind::Declarative(_)
@@ -174,23 +172,18 @@ fn eager_macro_recur(
                 let ExpandResult { value, err } =
                     lazy_expand(db, &def, curr.with_value(child.clone()), krate);
 
-                match value {
-                    Some(val) => {
-                        // replace macro inside
-                        let hygiene = Hygiene::new(db, val.file_id);
-                        let ExpandResult { value, err: error } = eager_macro_recur(
-                            db,
-                            &hygiene,
-                            // FIXME: We discard parse errors here
-                            val.map(|it| it.syntax_node()),
-                            krate,
-                            macro_resolver,
-                        )?;
-                        let err = if err.is_none() { error } else { err };
-                        ExpandResult { value, err }
-                    }
-                    None => ExpandResult { value: None, err },
-                }
+                // replace macro inside
+                let hygiene = Hygiene::new(db, value.file_id);
+                let ExpandResult { value, err: error } = eager_macro_recur(
+                    db,
+                    &hygiene,
+                    // FIXME: We discard parse errors here
+                    value.map(|it| it.syntax_node()),
+                    krate,
+                    macro_resolver,
+                )?;
+                let err = if err.is_none() { error } else { err };
+                ExpandResult { value, err }
             }
         };
         if err.is_some() {

crates/hir-expand/src/lib.rs

@@ -730,7 +730,7 @@ pub type AstId<N> = InFile<FileAstId<N>>;
 
 impl<N: AstNode> AstId<N> {
     pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> N {
-        let root = db.parse_or_expand(self.file_id).unwrap();
+        let root = db.parse_or_expand(self.file_id);
         db.ast_id_map(self.file_id).get(self.value).to_node(&root)
     }
 }
@@ -766,7 +766,7 @@ impl<T> InFile<T> {
     }
 
     pub fn file_syntax(&self, db: &dyn db::ExpandDatabase) -> SyntaxNode {
-        db.parse_or_expand(self.file_id).expect("source created from invalid file")
+        db.parse_or_expand(self.file_id)
     }
 }
 

crates/hir-expand/src/proc_macro.rs

@@ -38,7 +38,7 @@ impl ProcMacroExpander {
                     Some(Ok(proc_macros)) => proc_macros,
                     Some(Err(_)) | None => {
                         never!("Non-dummy expander even though there are no proc macros");
-                        return ExpandResult::with_err(
+                        return ExpandResult::new(
                             tt::Subtree::empty(),
                             ExpandError::Other("Internal error".into()),
                         );
@@ -52,7 +52,7 @@ impl ProcMacroExpander {
                             proc_macros.len(),
                             id.0
                         );
-                        return ExpandResult::with_err(
+                        return ExpandResult::new(
                             tt::Subtree::empty(),
                             ExpandError::Other("Internal error".into()),
                         );
@@ -75,17 +75,15 @@ impl ProcMacroExpander {
                             }
                         }
                         ProcMacroExpansionError::System(text)
-                        | ProcMacroExpansionError::Panic(text) => ExpandResult::with_err(
-                            tt::Subtree::empty(),
-                            ExpandError::Other(text.into()),
-                        ),
+                        | ProcMacroExpansionError::Panic(text) => {
+                            ExpandResult::new(tt::Subtree::empty(), ExpandError::Other(text.into()))
+                        }
                     },
                 }
             }
-            None => ExpandResult::with_err(
-                tt::Subtree::empty(),
-                ExpandError::UnresolvedProcMacro(def_crate),
-            ),
+            None => {
+                ExpandResult::new(tt::Subtree::empty(), ExpandError::UnresolvedProcMacro(def_crate))
+            }
         }
     }
 }