2 files changed, 137 insertions, 84 deletions

diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs
index 5083d5410..b2faa86d2 100644
--- a/crates/ra_mbe/src/mbe_expander.rs
+++ b/crates/ra_mbe/src/mbe_expander.rs
@@ -8,33 +8,44 @@ mod transcriber;
 use ra_syntax::SmolStr;
 use rustc_hash::FxHashMap;
 
-use crate::{ExpandResult, ExpandError};
-
-pub(crate) fn expand(
-    rules: &crate::MacroRules,
-    input: &tt::Subtree,
-) -> ExpandResult<tt::Subtree> {
-    let (mut result, mut left_over, mut err) = (tt::Subtree::default(), usize::max_value(), Some(ExpandError::NoMatchingRule));
+use crate::{ExpandError, ExpandResult};
+
+pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandResult<tt::Subtree> {
+    let (mut result, mut unmatched_tokens, mut unmatched_patterns, mut err) = (
+        tt::Subtree::default(),
+        usize::max_value(),
+        usize::max_value(),
+        Some(ExpandError::NoMatchingRule),
+    );
     for rule in &rules.rules {
-        let ((res, left), e) = expand_rule(rule, input);
+        let ((res, tokens, patterns), e) = expand_rule(rule, input);
         if e.is_none() {
             // if we find a rule that applies without errors, we're done
             return (res, None);
         }
-        // use the rule if we matched more tokens
-        if left < left_over {
+        // use the rule if we matched more tokens, or had fewer patterns left
+        if tokens < unmatched_tokens || tokens == unmatched_tokens && patterns < unmatched_patterns
+        {
             result = res;
             err = e;
-            left_over = left;
+            unmatched_tokens = tokens;
+            unmatched_patterns = patterns;
         }
     }
     (result, err)
 }
 
-fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> ExpandResult<(tt::Subtree, usize)> {
-    let ((bindings, left_over), bindings_err) = dbg!(matcher::match_(&rule.lhs, input));
-    let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &bindings));
-    ((res, left_over), bindings_err.or(transcribe_err))
+fn expand_rule(
+    rule: &crate::Rule,
+    input: &tt::Subtree,
+) -> ExpandResult<(tt::Subtree, usize, usize)> {
+    dbg!(&rule.lhs);
+    let (match_result, bindings_err) = dbg!(matcher::match_(&rule.lhs, input));
+    let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &match_result.bindings));
+    (
+        (res, match_result.unmatched_tokens, match_result.unmatched_patterns),
+        bindings_err.or(transcribe_err),
+    )
 }
 
 /// The actual algorithm for expansion is not too hard, but is pretty tricky.
@@ -149,10 +160,7 @@ mod tests {
         crate::MacroRules::parse(&definition_tt).unwrap()
     }
 
-    fn expand_first(
-        rules: &crate::MacroRules,
-        invocation: &str,
-    ) -> ExpandResult<tt::Subtree> {
+    fn expand_first(rules: &crate::MacroRules, invocation: &str) -> ExpandResult<tt::Subtree> {
         let source_file = ast::SourceFile::parse(invocation).ok().unwrap();
         let macro_invocation =
             source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
@@ -160,6 +168,7 @@ mod tests {
         let (invocation_tt, _) =
             ast_to_token_tree(&macro_invocation.token_tree().unwrap()).unwrap();
 
-        expand_rule(&rules.rules[0], &invocation_tt)
+        let expanded = expand_rule(&rules.rules[0], &invocation_tt);
+        ((expanded.0).0, expanded.1)
     }
 }
diff --git a/crates/ra_mbe/src/mbe_expander/matcher.rs b/crates/ra_mbe/src/mbe_expander/matcher.rs
index 52f1ac252..ae65fb69a 100644
--- a/crates/ra_mbe/src/mbe_expander/matcher.rs
+++ b/crates/ra_mbe/src/mbe_expander/matcher.rs
@@ -8,10 +8,10 @@ use crate::{
     ExpandError,
 };
 
+use super::ExpandResult;
 use ra_parser::{FragmentKind::*, TreeSink};
 use ra_syntax::{SmolStr, SyntaxKind};
 use tt::buffer::{Cursor, TokenBuffer};
-use super::ExpandResult;
 
 impl Bindings {
     fn push_optional(&mut self, name: &SmolStr) {
@@ -59,36 +59,50 @@ macro_rules! err {
     };
 }
 
-macro_rules! bail {
-    ($($tt:tt)*) => {
-        return Err(err!($($tt)*))
-    };
+#[derive(Debug, Default)]
+pub(super) struct Match {
+    pub bindings: Bindings,
+    pub unmatched_tokens: usize,
+    pub unmatched_patterns: usize,
 }
 
-pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult<(Bindings, usize)> {
+pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult<Match> {
     assert!(pattern.delimiter == None);
 
-    let mut res = Bindings::default();
+    let mut res = Match::default();
     let mut src = TtIter::new(src);
 
     let mut err = match_subtree(&mut res, pattern, &mut src).err();
 
+    res.unmatched_tokens += src.len();
     if src.len() > 0 && err.is_none() {
         err = Some(err!("leftover tokens"));
     }
 
-    ((res, src.len()), err)
+    (res, err)
 }
 
 fn match_subtree(
-    bindings: &mut Bindings,
+    res: &mut Match,
     pattern: &tt::Subtree,
     src: &mut TtIter,
 ) -> Result<(), ExpandError> {
+    let mut result = Ok(());
     for op in parse_pattern(pattern) {
+        if result.is_err() {
+            // We're just going through the patterns to count how many we missed
+            res.unmatched_patterns += 1;
+            continue;
+        }
         match op? {
             Op::TokenTree(tt::TokenTree::Leaf(lhs)) => {
-                let rhs = src.expect_leaf().map_err(|()| err!("expected leaf: `{}`", lhs))?;
+                let rhs = match src.expect_leaf() {
+                    Ok(l) => l,
+                    Err(()) => {
+                        result = Err(err!("expected leaf: `{}`", lhs));
+                        continue;
+                    }
+                };
                 match (lhs, rhs) {
                     (
                         tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
@@ -102,35 +116,54 @@ fn match_subtree(
                         tt::Leaf::Literal(tt::Literal { text: lhs, .. }),
                         tt::Leaf::Literal(tt::Literal { text: rhs, .. }),
                     ) if lhs == rhs => (),
-                    _ => return Err(ExpandError::UnexpectedToken),
+                    _ => {
+                        result = Err(ExpandError::UnexpectedToken);
+                    }
                 }
             }
             Op::TokenTree(tt::TokenTree::Subtree(lhs)) => {
-                let rhs = src.expect_subtree().map_err(|()| err!("expected subtree"))?;
+                let rhs = match src.expect_subtree() {
+                    Ok(s) => s,
+                    Err(()) => {
+                        result = Err(err!("expected subtree"));
+                        continue;
+                    }
+                };
                 if lhs.delimiter_kind() != rhs.delimiter_kind() {
-                    bail!("mismatched delimiter")
+                    result = Err(err!("mismatched delimiter"));
+                    continue;
                 }
                 let mut src = TtIter::new(rhs);
-                match_subtree(bindings, lhs, &mut src)?;
-                if src.len() > 0 {
-                    bail!("leftover tokens");
+                result = match_subtree(res, lhs, &mut src);
+                res.unmatched_tokens += src.len();
+                if src.len() > 0 && result.is_ok() {
+                    result = Err(err!("leftover tokens"));
                 }
             }
             Op::Var { name, kind } => {
-                let kind = kind.as_ref().ok_or(ExpandError::UnexpectedToken)?;
-                match match_meta_var(kind.as_str(), src)? {
+                let kind = match kind {
+                    Some(k) => k,
+                    None => {
+                        result = Err(ExpandError::UnexpectedToken);
+                        continue;
+                    }
+                };
+                let (matched, match_err) = match_meta_var(kind.as_str(), src);
+                match matched {
                     Some(fragment) => {
-                        bindings.inner.insert(name.clone(), Binding::Fragment(fragment));
+                        res.bindings.inner.insert(name.clone(), Binding::Fragment(fragment));
                     }
-                    None => bindings.push_optional(name),
+                    None if match_err.is_none() => res.bindings.push_optional(name),
+                    _ => {}
                 }
+                result = match_err.map_or(Ok(()), Err);
             }
             Op::Repeat { subtree, kind, separator } => {
-                match_repeat(bindings, subtree, kind, separator, src)?
+                result = match_repeat(res, subtree, kind, separator, src);
             }
         }
     }
-    Ok(())
+    result
 }
 
 impl<'a> TtIter<'a> {
@@ -222,7 +255,7 @@ impl<'a> TtIter<'a> {
     pub(crate) fn expect_fragment(
         &mut self,
         fragment_kind: ra_parser::FragmentKind,
-    ) -> Result<tt::TokenTree, ()> {
+    ) -> ExpandResult<tt::TokenTree> {
         pub(crate) struct OffsetTokenSink<'a> {
             pub(crate) cursor: Cursor<'a>,
             pub(crate) error: bool,
@@ -247,45 +280,47 @@ impl<'a> TtIter<'a> {
 
         ra_parser::parse_fragment(&mut src, &mut sink, fragment_kind);
 
+        let mut err = None;
         if !sink.cursor.is_root() || sink.error {
-            // FIXME better recovery in this case would help completion inside macros immensely
-            return Err(());
+            err = Some(err!("expected {:?}", fragment_kind));
         }
 
         let mut curr = buffer.begin();
         let mut res = vec![];
 
-        while curr != sink.cursor {
-            if let Some(token) = curr.token_tree() {
-                res.push(token);
+        if sink.cursor.is_root() {
+            while curr != sink.cursor {
+                if let Some(token) = curr.token_tree() {
+                    res.push(token);
+                }
+                curr = curr.bump();
             }
-            curr = curr.bump();
         }
         self.inner = self.inner.as_slice()[res.len()..].iter();
-        match res.len() {
-            0 => Err(()),
-            1 => Ok(res[0].clone()),
-            _ => Ok(tt::TokenTree::Subtree(tt::Subtree {
+        let res = match res.len() {
+            1 => res[0].clone(),
+            _ => tt::TokenTree::Subtree(tt::Subtree {
                 delimiter: None,
                 token_trees: res.into_iter().cloned().collect(),
-            })),
-        }
+            }),
+        };
+        (res, err)
     }
 
     pub(crate) fn eat_vis(&mut self) -> Option<tt::TokenTree> {
         let mut fork = self.clone();
         match fork.expect_fragment(Visibility) {
-            Ok(tt) => {
+            (tt, None) => {
                 *self = fork;
                 Some(tt)
             }
-            Err(()) => None,
+            (_, Some(_)) => None,
         }
     }
 }
 
 pub(super) fn match_repeat(
-    bindings: &mut Bindings,
+    res: &mut Match,
     pattern: &tt::Subtree,
     kind: RepeatKind,
     separator: Option<Separator>,
@@ -305,17 +340,23 @@ pub(super) fn match_repeat(
             }
         }
 
-        let mut nested = Bindings::default();
+        let mut nested = Match::default();
         match match_subtree(&mut nested, pattern, &mut fork) {
             Ok(()) => {
                 limit -= 1;
                 if limit == 0 {
-                    log::warn!("match_lhs excced in repeat pattern exceed limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", pattern, src, kind, separator);
+                    log::warn!(
+                        "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}",
+                        pattern,
+                        src,
+                        kind,
+                        separator
+                    );
                     break;
                 }
                 *src = fork;
 
-                bindings.push_nested(counter, nested)?;
+                res.bindings.push_nested(counter, nested.bindings)?;
                 counter += 1;
                 if counter == 1 {
                     if let RepeatKind::ZeroOrOne = kind {
@@ -334,7 +375,7 @@ pub(super) fn match_repeat(
             let mut vars = Vec::new();
             collect_vars(&mut vars, pattern)?;
             for var in vars {
-                bindings.push_empty(&var)
+                res.bindings.push_empty(&var)
             }
         }
         _ => (),
@@ -342,7 +383,7 @@ pub(super) fn match_repeat(
     Ok(())
 }
 
-fn match_meta_var(kind: &str, input: &mut TtIter) -> Result<Option<Fragment>, ExpandError> {
+fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragment>> {
     let fragment = match kind {
         "path" => Path,
         "expr" => Expr,
@@ -353,34 +394,33 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> Result<Option<Fragment>, Ex
         "meta" => MetaItem,
         "item" => Item,
         _ => {
-            let tt = match kind {
-                "ident" => {
-                    let ident = input.expect_ident().map_err(|()| err!("expected ident"))?.clone();
-                    tt::Leaf::from(ident).into()
-                }
-                "tt" => input.expect_tt().map_err(|()| err!())?.clone(),
-                "lifetime" => {
-                    let ident = input.expect_lifetime().map_err(|()| err!())?;
-                    tt::Leaf::Ident(ident.clone()).into()
-                }
-                "literal" => {
-                    let literal = input.expect_literal().map_err(|()| err!())?.clone();
-                    tt::Leaf::from(literal).into()
-                }
+            let tt_result = match kind {
+                "ident" => input
+                    .expect_ident()
+                    .map(|ident| Some(tt::Leaf::from(ident.clone()).into()))
+                    .map_err(|()| err!("expected ident")),
+                "tt" => input.expect_tt().map(Some).map_err(|()| err!()),
+                "lifetime" => input
+                    .expect_lifetime()
+                    .map(|ident| Some(tt::Leaf::Ident(ident.clone()).into()))
+                    .map_err(|()| err!("expected lifetime")),
+                "literal" => input
+                    .expect_literal()
+                    .map(|literal| Some(tt::Leaf::from(literal.clone()).into()))
+                    .map_err(|()| err!()),
                 // `vis` is optional
                 "vis" => match input.eat_vis() {
-                    Some(vis) => vis,
-                    None => return Ok(None),
+                    Some(vis) => Ok(Some(vis)),
+                    None => Ok(None),
                 },
-                _ => return Err(ExpandError::UnexpectedToken),
+                _ => Err(ExpandError::UnexpectedToken),
             };
-            return Ok(Some(Fragment::Tokens(tt)));
+            return to_expand_result(tt_result.map(|it| it.map(Fragment::Tokens)));
         }
     };
-    let tt =
-        input.expect_fragment(fragment).map_err(|()| err!("fragment did not parse as {}", kind))?;
+    let (tt, err) = input.expect_fragment(fragment);
     let fragment = if kind == "expr" { Fragment::Ast(tt) } else { Fragment::Tokens(tt) };
-    Ok(Some(fragment))
+    (Some(fragment), err)
 }
 
 fn collect_vars(buf: &mut Vec<SmolStr>, pattern: &tt::Subtree) -> Result<(), ExpandError> {
@@ -394,3 +434,7 @@ fn collect_vars(buf: &mut Vec<SmolStr>, pattern: &tt::Subtree) -> Result<(), Exp
     }
     Ok(())
 }
+
+fn to_expand_result<T: Default>(result: Result<T, ExpandError>) -> ExpandResult<T> {
+    result.map_or_else(|e| (Default::default(), Some(e)), |it| (it, None))
+}