Merge #916

916: Error handling for macros r=matklad a=detrumi

Part of #720 

Co-authored-by: Wilco Kusee <[email protected]>

4 files changed, 220 insertions, 72 deletions

diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs
index 907402f5f..2c75b7b4f 100644
--- a/crates/ra_mbe/src/lib.rs
+++ b/crates/ra_mbe/src/lib.rs
@@ -24,6 +24,18 @@ use ra_syntax::SmolStr;
 
 pub use tt::{Delimiter, Punct};
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum ParseError {
+    Expected(String),
+}
+
+#[derive(Debug, PartialEq, Eq)]
+pub enum ExpandError {
+    NoMatchingRule,
+    UnexpectedToken,
+    BindingError(String),
+}
+
 pub use crate::syntax_bridge::{ast_to_token_tree, token_tree_to_ast_item_list};
 
 /// This struct contains AST for a single `macro_rules` definition. What might
@@ -36,11 +48,11 @@ pub struct MacroRules {
 }
 
 impl MacroRules {
-    pub fn parse(tt: &tt::Subtree) -> Option<MacroRules> {
+    pub fn parse(tt: &tt::Subtree) -> Result<MacroRules, ParseError> {
         mbe_parser::parse(tt)
     }
-    pub fn expand(&self, tt: &tt::Subtree) -> Option<tt::Subtree> {
-        mbe_expander::exapnd(self, tt)
+    pub fn expand(&self, tt: &tt::Subtree) -> Result<tt::Subtree, ExpandError> {
+        mbe_expander::expand(self, tt)
     }
 }
 
diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs
index 1acba86ea..2dd97b665 100644
--- a/crates/ra_mbe/src/mbe_expander.rs
+++ b/crates/ra_mbe/src/mbe_expander.rs
@@ -5,17 +5,21 @@ use rustc_hash::FxHashMap;
 use ra_syntax::SmolStr;
 use tt::TokenId;
 
+use crate::ExpandError;
 use crate::tt_cursor::TtCursor;
 
-pub(crate) fn exapnd(rules: &crate::MacroRules, input: &tt::Subtree) -> Option<tt::Subtree> {
-    rules.rules.iter().find_map(|it| expand_rule(it, input))
+pub(crate) fn expand(
+    rules: &crate::MacroRules,
+    input: &tt::Subtree,
+) -> Result<tt::Subtree, ExpandError> {
+    rules.rules.iter().find_map(|it| expand_rule(it, input).ok()).ok_or(ExpandError::NoMatchingRule)
 }
 
-fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> Option<tt::Subtree> {
+fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> Result<tt::Subtree, ExpandError> {
     let mut input = TtCursor::new(input);
     let bindings = match_lhs(&rule.lhs, &mut input)?;
     if !input.is_eof() {
-        return None;
+        return Err(ExpandError::UnexpectedToken);
     }
     expand_subtree(&rule.rhs, &bindings, &mut Vec::new())
 }
@@ -77,70 +81,86 @@ enum Binding {
 }
 
 impl Bindings {
-    fn get(&self, name: &SmolStr, nesting: &[usize]) -> Option<&tt::TokenTree> {
-        let mut b = self.inner.get(name)?;
+    fn get(&self, name: &SmolStr, nesting: &[usize]) -> Result<&tt::TokenTree, ExpandError> {
+        let mut b = self
+            .inner
+            .get(name)
+            .ok_or(ExpandError::BindingError(format!("could not find binding `{}`", name)))?;
         for &idx in nesting.iter() {
             b = match b {
                 Binding::Simple(_) => break,
-                Binding::Nested(bs) => bs.get(idx)?,
+                Binding::Nested(bs) => bs.get(idx).ok_or(ExpandError::BindingError(format!(
+                    "could not find nested binding `{}`",
+                    name
+                )))?,
             };
         }
         match b {
-            Binding::Simple(it) => Some(it),
-            Binding::Nested(_) => None,
+            Binding::Simple(it) => Ok(it),
+            Binding::Nested(_) => Err(ExpandError::BindingError(format!(
+                "expected simple binding, found nested binding `{}`",
+                name
+            ))),
         }
     }
-    fn push_nested(&mut self, nested: Bindings) -> Option<()> {
+
+    fn push_nested(&mut self, nested: Bindings) -> Result<(), ExpandError> {
         for (key, value) in nested.inner {
             if !self.inner.contains_key(&key) {
                 self.inner.insert(key.clone(), Binding::Nested(Vec::new()));
             }
             match self.inner.get_mut(&key) {
                 Some(Binding::Nested(it)) => it.push(value),
-                _ => return None,
+                _ => {
+                    return Err(ExpandError::BindingError(format!(
+                        "could not find binding `{}`",
+                        key
+                    )));
+                }
             }
         }
-        Some(())
+        Ok(())
     }
 }
 
-fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Option<Bindings> {
+fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings, ExpandError> {
     let mut res = Bindings::default();
     for pat in pattern.token_trees.iter() {
         match pat {
             crate::TokenTree::Leaf(leaf) => match leaf {
                 crate::Leaf::Var(crate::Var { text, kind }) => {
-                    let kind = kind.clone()?;
+                    let kind = kind.clone().ok_or(ExpandError::UnexpectedToken)?;
                     match kind.as_str() {
                         "ident" => {
-                            let ident = input.eat_ident()?.clone();
+                            let ident =
+                                input.eat_ident().ok_or(ExpandError::UnexpectedToken)?.clone();
                             res.inner.insert(
                                 text.clone(),
                                 Binding::Simple(tt::Leaf::from(ident).into()),
                             );
                         }
-                        _ => return None,
+                        _ => return Err(ExpandError::UnexpectedToken),
                     }
                 }
                 crate::Leaf::Punct(punct) => {
-                    if input.eat_punct()? != punct {
-                        return None;
+                    if input.eat_punct() != Some(punct) {
+                        return Err(ExpandError::UnexpectedToken);
                     }
                 }
                 crate::Leaf::Ident(ident) => {
-                    if input.eat_ident()?.text != ident.text {
-                        return None;
+                    if input.eat_ident().map(|i| &i.text) != Some(&ident.text) {
+                        return Err(ExpandError::UnexpectedToken);
                     }
                 }
-                _ => return None,
+                _ => return Err(ExpandError::UnexpectedToken),
             },
             crate::TokenTree::Repeat(crate::Repeat { subtree, kind: _, separator }) => {
-                while let Some(nested) = match_lhs(subtree, input) {
+                while let Ok(nested) = match_lhs(subtree, input) {
                     res.push_nested(nested)?;
                     if let Some(separator) = *separator {
                         if !input.is_eof() {
-                            if input.eat_punct()?.char != separator {
-                                return None;
+                            if input.eat_punct().map(|p| p.char) != Some(separator) {
+                                return Err(ExpandError::UnexpectedToken);
                             }
                         }
                     }
@@ -149,34 +169,34 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Option<Bindings>
             _ => {}
         }
     }
-    Some(res)
+    Ok(res)
 }
 
 fn expand_subtree(
     template: &crate::Subtree,
     bindings: &Bindings,
     nesting: &mut Vec<usize>,
-) -> Option<tt::Subtree> {
+) -> Result<tt::Subtree, ExpandError> {
     let token_trees = template
         .token_trees
         .iter()
         .map(|it| expand_tt(it, bindings, nesting))
-        .collect::<Option<Vec<_>>>()?;
+        .collect::<Result<Vec<_>, ExpandError>>()?;
 
-    Some(tt::Subtree { token_trees, delimiter: template.delimiter })
+    Ok(tt::Subtree { token_trees, delimiter: template.delimiter })
 }
 
 fn expand_tt(
     template: &crate::TokenTree,
     bindings: &Bindings,
     nesting: &mut Vec<usize>,
-) -> Option<tt::TokenTree> {
+) -> Result<tt::TokenTree, ExpandError> {
     let res: tt::TokenTree = match template {
         crate::TokenTree::Subtree(subtree) => expand_subtree(subtree, bindings, nesting)?.into(),
         crate::TokenTree::Repeat(repeat) => {
             let mut token_trees = Vec::new();
             nesting.push(0);
-            while let Some(t) = expand_subtree(&repeat.subtree, bindings, nesting) {
+            while let Ok(t) = expand_subtree(&repeat.subtree, bindings, nesting) {
                 let idx = nesting.pop().unwrap();
                 nesting.push(idx + 1);
                 token_trees.push(t.into())
@@ -194,5 +214,70 @@ fn expand_tt(
             crate::Leaf::Literal(l) => tt::Leaf::from(tt::Literal { text: l.text.clone() }).into(),
         },
     };
-    Some(res)
+    Ok(res)
+}
+
+#[cfg(test)]
+mod tests {
+    use ra_syntax::{ast, AstNode};
+
+    use super::*;
+    use crate::ast_to_token_tree;
+
+    #[test]
+    fn test_expand_rule() {
+        assert_err(
+            "($i:ident) => ($j)",
+            "foo!{a}",
+            ExpandError::BindingError(String::from("could not find binding `j`")),
+        );
+
+        assert_err(
+            "($($i:ident);*) => ($i)",
+            "foo!{a}",
+            ExpandError::BindingError(String::from(
+                "expected simple binding, found nested binding `i`",
+            )),
+        );
+
+        assert_err("($i) => ($i)", "foo!{a}", ExpandError::UnexpectedToken);
+        assert_err("($i:) => ($i)", "foo!{a}", ExpandError::UnexpectedToken);
+    }
+
+    fn assert_err(macro_body: &str, invocation: &str, err: ExpandError) {
+        assert_eq!(expand_first(&create_rules(&format_macro(macro_body)), invocation), Err(err));
+    }
+
+    fn format_macro(macro_body: &str) -> String {
+        format!(
+            "
+        macro_rules! foo {{
+            {}
+        }}
+",
+            macro_body
+        )
+    }
+
+    fn create_rules(macro_definition: &str) -> crate::MacroRules {
+        let source_file = ast::SourceFile::parse(macro_definition);
+        let macro_definition =
+            source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
+
+        let (definition_tt, _) = ast_to_token_tree(macro_definition.token_tree().unwrap()).unwrap();
+        crate::MacroRules::parse(&definition_tt).unwrap()
+    }
+
+    fn expand_first(
+        rules: &crate::MacroRules,
+        invocation: &str,
+    ) -> Result<tt::Subtree, ExpandError> {
+        let source_file = ast::SourceFile::parse(invocation);
+        let macro_invocation =
+            source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
+
+        let (invocation_tt, _) = ast_to_token_tree(macro_invocation.token_tree().unwrap()).unwrap();
+
+        expand_rule(&rules.rules[0], &invocation_tt)
+    }
 }
diff --git a/crates/ra_mbe/src/mbe_parser.rs b/crates/ra_mbe/src/mbe_parser.rs
index 58e2533f1..e3669f66c 100644
--- a/crates/ra_mbe/src/mbe_parser.rs
+++ b/crates/ra_mbe/src/mbe_parser.rs
@@ -1,33 +1,34 @@
 /// This module parses a raw `tt::TokenStream` into macro-by-example token
 /// stream. This is a *mostly* identify function, expect for handling of
 /// `$var:tt_kind` and `$(repeat),*` constructs.
+use crate::ParseError;
 use crate::tt_cursor::TtCursor;
 
-pub(crate) fn parse(tt: &tt::Subtree) -> Option<crate::MacroRules> {
+pub(crate) fn parse(tt: &tt::Subtree) -> Result<crate::MacroRules, ParseError> {
     let mut parser = TtCursor::new(tt);
     let mut rules = Vec::new();
     while !parser.is_eof() {
         rules.push(parse_rule(&mut parser)?);
-        if parser.expect_char(';') == None {
+        if let Err(e) = parser.expect_char(';') {
             if !parser.is_eof() {
-                return None;
+                return Err(e);
             }
             break;
         }
     }
-    Some(crate::MacroRules { rules })
+    Ok(crate::MacroRules { rules })
 }
 
-fn parse_rule(p: &mut TtCursor) -> Option<crate::Rule> {
+fn parse_rule(p: &mut TtCursor) -> Result<crate::Rule, ParseError> {
     let lhs = parse_subtree(p.eat_subtree()?)?;
     p.expect_char('=')?;
     p.expect_char('>')?;
     let mut rhs = parse_subtree(p.eat_subtree()?)?;
     rhs.delimiter = crate::Delimiter::None;
-    Some(crate::Rule { lhs, rhs })
+    Ok(crate::Rule { lhs, rhs })
 }
 
-fn parse_subtree(tt: &tt::Subtree) -> Option<crate::Subtree> {
+fn parse_subtree(tt: &tt::Subtree) -> Result<crate::Subtree, ParseError> {
     let mut token_trees = Vec::new();
     let mut p = TtCursor::new(tt);
     while let Some(tt) = p.eat() {
@@ -52,10 +53,10 @@ fn parse_subtree(tt: &tt::Subtree) -> Option<crate::Subtree> {
         };
         token_trees.push(child);
     }
-    Some(crate::Subtree { token_trees, delimiter: tt.delimiter })
+    Ok(crate::Subtree { token_trees, delimiter: tt.delimiter })
 }
 
-fn parse_var(p: &mut TtCursor) -> Option<crate::Var> {
+fn parse_var(p: &mut TtCursor) -> Result<crate::Var, ParseError> {
     let ident = p.eat_ident().unwrap();
     let text = ident.text.clone();
     let kind = if p.at_char(':') {
@@ -69,25 +70,77 @@ fn parse_var(p: &mut TtCursor) -> Option<crate::Var> {
     } else {
         None
     };
-    Some(crate::Var { text, kind })
+    Ok(crate::Var { text, kind })
 }
 
-fn parse_repeat(p: &mut TtCursor) -> Option<crate::Repeat> {
+fn parse_repeat(p: &mut TtCursor) -> Result<crate::Repeat, ParseError> {
     let subtree = p.eat_subtree().unwrap();
     let mut subtree = parse_subtree(subtree)?;
     subtree.delimiter = crate::Delimiter::None;
-    let sep = p.eat_punct()?;
+    let sep = p.eat_punct().ok_or(ParseError::Expected(String::from("separator")))?;
     let (separator, rep) = match sep.char {
         '*' | '+' | '?' => (None, sep.char),
-        char => (Some(char), p.eat_punct()?.char),
+        char => {
+            (Some(char), p.eat_punct().ok_or(ParseError::Expected(String::from("separator")))?.char)
+        }
     };
 
     let kind = match rep {
         '*' => crate::RepeatKind::ZeroOrMore,
         '+' => crate::RepeatKind::OneOrMore,
         '?' => crate::RepeatKind::ZeroOrOne,
-        _ => return None,
+        _ => return Err(ParseError::Expected(String::from("repeat"))),
     };
     p.bump();
-    Some(crate::Repeat { subtree, kind, separator })
+    Ok(crate::Repeat { subtree, kind, separator })
+}
+
+#[cfg(test)]
+mod tests {
+    use ra_syntax::{ast, AstNode};
+
+    use super::*;
+    use crate::ast_to_token_tree;
+
+    #[test]
+    fn test_invalid_parse() {
+        expect_err("invalid", "subtree");
+
+        is_valid("($i:ident) => ()");
+        expect_err("$i:ident => ()", "subtree");
+        expect_err("($i:ident) ()", "`=`");
+        expect_err("($($i:ident)_) => ()", "separator");
+    }
+
+    fn expect_err(macro_body: &str, expected: &str) {
+        assert_eq!(
+            create_rules(&format_macro(macro_body)),
+            Err(ParseError::Expected(String::from(expected)))
+        );
+    }
+
+    fn is_valid(macro_body: &str) {
+        assert!(create_rules(&format_macro(macro_body)).is_ok());
+    }
+
+    fn format_macro(macro_body: &str) -> String {
+        format!(
+            "
+        macro_rules! foo {{
+            {}
+        }}
+",
+            macro_body
+        )
+    }
+
+    fn create_rules(macro_definition: &str) -> Result<crate::MacroRules, ParseError> {
+        let source_file = ast::SourceFile::parse(macro_definition);
+        let macro_definition =
+            source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
+
+        let (definition_tt, _) = ast_to_token_tree(macro_definition.token_tree().unwrap()).unwrap();
+        parse(&definition_tt)
+    }
+
 }
diff --git a/crates/ra_mbe/src/tt_cursor.rs b/crates/ra_mbe/src/tt_cursor.rs
index 30c8eda67..3128cb9ae 100644
--- a/crates/ra_mbe/src/tt_cursor.rs
+++ b/crates/ra_mbe/src/tt_cursor.rs
@@ -1,3 +1,5 @@
+use crate::ParseError;
+
 #[derive(Clone)]
 pub(crate) struct TtCursor<'a> {
     subtree: &'a tt::Subtree,
@@ -46,46 +48,42 @@ impl<'a> TtCursor<'a> {
     }
 
     pub(crate) fn eat(&mut self) -> Option<&'a tt::TokenTree> {
-        match self.current() {
-            Some(it) => {
-                self.bump();
-                Some(it)
-            }
-            None => None,
-        }
+        self.current().map(|it| {
+            self.bump();
+            it
+        })
     }
 
-    pub(crate) fn eat_subtree(&mut self) -> Option<&'a tt::Subtree> {
-        match self.current()? {
-            tt::TokenTree::Subtree(sub) => {
+    pub(crate) fn eat_subtree(&mut self) -> Result<&'a tt::Subtree, ParseError> {
+        match self.current() {
+            Some(tt::TokenTree::Subtree(sub)) => {
                 self.bump();
-                Some(sub)
+                Ok(sub)
             }
-            _ => return None,
+            _ => Err(ParseError::Expected(String::from("subtree"))),
         }
     }
 
     pub(crate) fn eat_punct(&mut self) -> Option<&'a tt::Punct> {
-        if let Some(it) = self.at_punct() {
+        self.at_punct().map(|it| {
             self.bump();
-            return Some(it);
-        }
-        None
+            it
+        })
     }
 
     pub(crate) fn eat_ident(&mut self) -> Option<&'a tt::Ident> {
-        if let Some(i) = self.at_ident() {
+        self.at_ident().map(|i| {
             self.bump();
-            return Some(i);
-        }
-        None
+            i
+        })
     }
 
-    pub(crate) fn expect_char(&mut self, char: char) -> Option<()> {
+    pub(crate) fn expect_char(&mut self, char: char) -> Result<(), ParseError> {
         if self.at_char(char) {
             self.bump();
-            return Some(());
+            Ok(())
+        } else {
+            Err(ParseError::Expected(format!("`{}`", char)))
         }
-        None
     }
 }