Skip to content

Commit f1b59e3

Browse files
committed
yeast: Reject duplicate capture names
Currently, it's legal to write something like ``` (foo bar: (bar) @item baz: (baz) @item ) ``` with the effect that `item` gets both the `bar` and the `baz` that were matched. This is not behaviour that we actually use, and it also goes somewhat against the "pattern matching" semantics that queries/patterns are intended to have. Since it's not a feature that we're actually using anywhere, I decided the easiest solution would be to simply make multiple occurrences of the same capture variable a compile-time error.
1 parent 8cc5bac commit f1b59e3

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

‎shared/yeast-macros/src/lower.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ impl Pattern {
190190
}
191191
}
192192
}
193+
193194
fn captures(&self) -> Vec<CaptureBinding> {
194195
let mut captures = Vec::new();
195196
self.collect_captures(Cardinality::SINGLE, &mut captures);

‎shared/yeast-macros/src/rule_parse.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use proc_macro2::{Delimiter, Ident, Span, TokenStream, TokenTree};
2+
use std::collections::BTreeSet;
23
use std::iter::Peekable;
34

45
use crate::ast::{Cardinality, Pattern, Replacement, ReturnAnnotation, Rule};
@@ -15,9 +16,46 @@ pub(crate) fn parse_pattern(input: TokenStream) -> Result<Pattern> {
1516
"unexpected token after query",
1617
));
1718
}
19+
validate_unique_capture_names(&pattern, &mut BTreeSet::new())?;
1820
Ok(pattern)
1921
}
2022

23+
fn validate_unique_capture_names(
24+
pattern: &Pattern,
25+
capture_names: &mut BTreeSet<String>,
26+
) -> Result<()> {
27+
match pattern {
28+
Pattern::Any { .. } | Pattern::Unnamed(_) => {}
29+
Pattern::Node { fields, .. } => {
30+
for (_, pattern) in fields {
31+
validate_unique_capture_names(pattern, capture_names)?;
32+
}
33+
}
34+
Pattern::Capture { name, pattern, .. } => {
35+
validate_unique_capture_names(pattern, capture_names)?;
36+
let name_string = name.to_string();
37+
if !capture_names.insert(name_string.clone()) {
38+
return Err(syn::Error::new_spanned(
39+
name,
40+
format!(
41+
"capture `{name_string}` is used more than once; \
42+
use a distinct name for each capture site"
43+
),
44+
));
45+
}
46+
}
47+
Pattern::Sequence(patterns) => {
48+
for pattern in patterns {
49+
validate_unique_capture_names(pattern, capture_names)?;
50+
}
51+
}
52+
Pattern::Repeated { pattern, .. } => {
53+
validate_unique_capture_names(pattern, capture_names)?;
54+
}
55+
}
56+
Ok(())
57+
}
58+
2159
pub(crate) fn parse_rule(input: TokenStream) -> Result<Rule> {
2260
let (pattern_and_guard, replacement) = split_arrow(input)?;
2361
let (pattern, guard) = split_guard(pattern_and_guard)?;
@@ -590,6 +628,22 @@ mod tests {
590628
);
591629
}
592630

631+
#[test]
632+
fn rejects_duplicate_capture_names() {
633+
let result = parse_pattern(quote! {
634+
(assignment
635+
left: (_) @item
636+
right: (_) @item)
637+
});
638+
let Err(error) = result else {
639+
panic!("expected duplicate capture name to be rejected");
640+
};
641+
assert_eq!(
642+
error.to_string(),
643+
"capture `item` is used more than once; use a distinct name for each capture site"
644+
);
645+
}
646+
593647
#[test]
594648
fn parses_literal_led_repeated_sequence() {
595649
let pattern = parse_pattern(quote!((root ("+" ",")*))).unwrap();

0 commit comments

Comments
 (0)