yeast: Tighten up parsing of parenthesised items

The previous parser was pretty lax in what it allowed to occur inside a
parenthesised group. Thus, things like
```
(foo @bar)
```
or
```
(foo . bar)
```
would (because neither is valid syntax) simply ignore everything after
the `foo` and behave as if the entire pattern was `(foo)`.

We now expect a parenthesised item to consume its entire contents.
Moreover, we now distinguish between sequences and named nodes as
follows: if the first item is an identifier, then it's a named node,
otherwise the parenthesised item is a sequence.

In particular, this means something like `(_ ",")` is interpreted as
"wildcard followed by comma". (Another valid reading would be "wildcard
named node with comma inside, but this is a convoluted construction that
we would likely want to express in a different way.)
Hopefully this makes it clearer when a parenthesised item is one thing
or the other.

Author: tausbn

shared/yeast-macros/src/lib.rs

@@ -30,15 +30,16 @@ mod template_parse;
 ///
 /// Named fields and bare child patterns may be intermixed in any order.
 ///
-/// Parentheses do not provide general-purpose grouping. A parenthesized query
-/// group contains at least two sibling patterns and may be repeated, but the
-/// group itself cannot be captured because it does not represent one node:
+/// A parenthesized query whose first element is a node kind is always parsed as
+/// that node pattern. Otherwise, two or more sibling patterns form a sequence.
+/// A sequence may be repeated, but cannot itself be captured because it does
+/// not represent one node:
 ///
 /// ```text
 /// ((identifier) @items (integer) @items)*  // valid: explicit node captures
 /// ((identifier) (integer))* @items         // invalid: sequence capture
-/// ("+")*                                   // invalid: redundant grouping
-/// "+"*                                     // valid: repeated unnamed token
+/// ("+" ",")*                               // valid: repeated token sequence
+/// (_ ",")*                                 // valid: repeated wildcard/token sequence
 /// ```
 #[proc_macro]
 pub fn query(input: TokenStream) -> TokenStream {

shared/yeast-macros/src/rule_parse.rs

@@ -42,14 +42,7 @@ fn parse_pattern_atom(tokens: &mut Tokens) -> Result<Pattern> {
         Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::Parenthesis => {
             let group = expect_group(tokens, Delimiter::Parenthesis)?;
             let mut inner = group.stream().into_iter().peekable();
-            let pattern = parse_parenthesized_pattern(&mut inner)?;
-            if let Some(token) = inner.next() {
-                return Err(syn::Error::new_spanned(
-                    token,
-                    "unexpected token in query node",
-                ));
-            }
-            Ok(pattern)
+            parse_parenthesized_pattern(&mut inner, group.span())
         }
         Some(TokenTree::Ident(identifier)) if *identifier == "_" => {
             tokens.next();
@@ -65,25 +58,51 @@ fn parse_pattern_atom(tokens: &mut Tokens) -> Result<Pattern> {
     }
 }
 
-fn parse_parenthesized_pattern(tokens: &mut Tokens) -> Result<Pattern> {
+fn parse_parenthesized_pattern(tokens: &mut Tokens, span: Span) -> Result<Pattern> {
     match tokens.peek() {
-        None => Err(syn::Error::new(
-            Span::call_site(),
-            "empty parenthesized group in query",
-        )),
+        None => Err(syn::Error::new(span, "empty query groups are not allowed")),
         Some(TokenTree::Ident(identifier)) if *identifier == "_" => {
             tokens.next();
-            Ok(Pattern::Any {
-                match_unnamed: false,
-            })
+            if tokens.peek().is_none() {
+                Ok(Pattern::Any {
+                    match_unnamed: false,
+                })
+            } else {
+                let mut patterns = vec![Pattern::Any {
+                    match_unnamed: true,
+                }];
+                patterns.extend(parse_pattern_list(tokens)?);
+                finish_sequence(tokens, patterns, span)
+            }
+        }
+        Some(TokenTree::Literal(_)) => {
+            let first = Pattern::Unnamed(expect_string_literal(tokens)?);
+            if tokens.peek().is_none() {
+                Ok(first)
+            } else {
+                let mut patterns = vec![first];
+                patterns.extend(parse_pattern_list(tokens)?);
+                finish_sequence(tokens, patterns, span)
+            }
         }
-        Some(TokenTree::Literal(_)) => Ok(Pattern::Unnamed(expect_string_literal(tokens)?)),
         Some(TokenTree::Ident(_)) => {
             let kind = expect_ident(tokens, "expected node kind")?.to_string();
-            Ok(Pattern::Node {
+            let pattern = Pattern::Node {
                 kind,
                 fields: parse_pattern_fields(tokens)?,
-            })
+            };
+            if let Some(token) = tokens.next() {
+                Err(syn::Error::new_spanned(
+                    token,
+                    "unexpected token in query node",
+                ))
+            } else {
+                Ok(pattern)
+            }
+        }
+        Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::Parenthesis => {
+            let patterns = parse_pattern_list(tokens)?;
+            finish_sequence(tokens, patterns, span)
         }
         Some(token) => Err(syn::Error::new_spanned(
             token.clone(),
@@ -92,6 +111,23 @@ fn parse_parenthesized_pattern(tokens: &mut Tokens) -> Result<Pattern> {
     }
 }
 
+fn finish_sequence(tokens: &mut Tokens, patterns: Vec<Pattern>, span: Span) -> Result<Pattern> {
+    if let Some(token) = tokens.next() {
+        return Err(syn::Error::new_spanned(
+            token,
+            "unexpected token in query sequence",
+        ));
+    }
+    match patterns.len() {
+        0 => Err(syn::Error::new(span, "empty query groups are not allowed")),
+        1 => Err(syn::Error::new(
+            span,
+            "single-pattern query groups are not allowed; remove the redundant parentheses",
+        )),
+        _ => Ok(Pattern::Sequence(patterns)),
+    }
+}
+
 fn parse_pattern_fields(tokens: &mut Tokens) -> Result<Vec<(String, Pattern)>> {
     let mut fields = Vec::new();
     let mut bare_children = Vec::new();
@@ -152,33 +188,14 @@ fn parse_pattern_list(tokens: &mut Tokens) -> Result<Vec<Pattern>> {
             let mut inner = group.stream().into_iter().peekable();
             if peek_is_repetition(tokens) {
                 let cardinality = expect_cardinality(tokens)?;
-                let is_single_pattern = matches!(inner.peek(), Some(TokenTree::Ident(_)));
-                let repeated = if is_single_pattern {
-                    parse_parenthesized_pattern(&mut inner)?
-                } else {
-                    let patterns = parse_pattern_list(&mut inner)?;
-                    if patterns.is_empty() {
-                        return Err(syn::Error::new(
-                            group.span(),
-                            "empty query groups are not allowed",
-                        ));
-                    }
-                    if patterns.len() == 1 {
-                        return Err(syn::Error::new(
-                            group.span(),
-                            "single-pattern query groups are not allowed; \
-                             remove the redundant parentheses",
-                        ));
-                    }
-                    Pattern::Sequence(patterns)
-                };
+                let repeated = parse_parenthesized_pattern(&mut inner, group.span())?;
                 let repeated = Pattern::Repeated {
                     pattern: Box::new(repeated),
                     cardinality,
                 };
                 patterns.push(maybe_capture(tokens, repeated)?);
             } else {
-                let pattern = parse_parenthesized_pattern(&mut inner)?;
+                let pattern = parse_parenthesized_pattern(&mut inner, group.span())?;
                 patterns.push(maybe_capture(tokens, pattern)?);
             }
             continue;
@@ -593,4 +610,71 @@ mod tests {
             "cannot capture a query sequence; capture the desired nodes explicitly"
         );
     }
+
+    #[test]
+    fn parses_literal_led_repeated_sequence() {
+        let pattern = parse_pattern(quote!((root ("+" ",")*))).unwrap();
+        let Pattern::Node { fields, .. } = pattern else {
+            panic!("expected root node");
+        };
+        let Pattern::Sequence(children) = &fields[0].1 else {
+            panic!("expected child sequence");
+        };
+        let Pattern::Repeated {
+            pattern: repeated, ..
+        } = &children[0]
+        else {
+            panic!("expected repetition");
+        };
+        assert!(matches!(
+            repeated.as_ref(),
+            Pattern::Sequence(patterns)
+                if matches!(
+                    patterns.as_slice(),
+                    [Pattern::Unnamed(left), Pattern::Unnamed(right)]
+                        if left == "+" && right == ","
+                )
+        ));
+    }
+
+    #[test]
+    fn parses_wildcard_led_repeated_sequence() {
+        let pattern = parse_pattern(quote!((root (_ ",")*))).unwrap();
+        let Pattern::Node { fields, .. } = pattern else {
+            panic!("expected root node");
+        };
+        let Pattern::Sequence(children) = &fields[0].1 else {
+            panic!("expected child sequence");
+        };
+        let Pattern::Repeated {
+            pattern: repeated, ..
+        } = &children[0]
+        else {
+            panic!("expected repetition");
+        };
+        assert!(matches!(
+            repeated.as_ref(),
+            Pattern::Sequence(patterns)
+                if matches!(
+                    patterns.as_slice(),
+                    [
+                        Pattern::Any {
+                            match_unnamed: true
+                        },
+                        Pattern::Unnamed(comma)
+                    ] if comma == ","
+                )
+        ));
+    }
+
+    #[test]
+    fn rejects_unconsumed_node_pattern_tokens() {
+        for query in [quote!((root (foo . (bar) @x))), quote!((root(bar @ x)))] {
+            let result = parse_pattern(query);
+            let Err(error) = result else {
+                panic!("expected malformed node pattern to be rejected");
+            };
+            assert!(error.to_string().starts_with("unexpected token"));
+        }
+    }
 }

shared/yeast/doc/yeast.md

@@ -155,27 +155,28 @@ queries, are not supported.
 "+"* @operators        // repeat and capture an unnamed token
 ```
 
-Parentheses are not general-purpose grouping syntax. A parenthesized query
-group represents a sequence of at least two sibling patterns. It may be
-repeated, but it cannot itself be captured because the sequence does not
-correspond to one AST node:
+When a parenthesized query starts with a node-kind identifier, it is always
+parsed as that node pattern, and every remaining token must belong to that
+node's fields or children. Otherwise, two or more sibling patterns form a
+sequence. A sequence may be repeated, but it cannot itself be captured because
+the sequence does not correspond to one AST node:
 
 ```rust
 ((identifier) @items (integer) @items)*  // explicit flattened captures
 ((identifier) (integer))* @items         // error: sequence capture
 ```
 
-Use the bare literal form when quantifying an unnamed token. The parenthesized
-form is valid as a standalone token pattern, but not as a redundant
-single-pattern group:
+Literal- and wildcard-led sequences are valid and unambiguous because neither
+can introduce node fields:
 
 ```rust
-"+"*       // valid
-("+")*     // error: remove the redundant parentheses
+("+" ",")*  // repeat a two-token sequence
+(_ ",")*    // repeat a wildcard followed by a comma
 ```
 
-Empty query groups are also rejected. This does not affect the empty output
-template `()`, which intentionally means “emit no replacement nodes.”
+Single-pattern and empty query groups are rejected as redundant or meaningless.
+This does not affect the empty output template `()`, which intentionally means
+“emit no replacement nodes.”
 
 ## Template language