fix(rust): EqCapture accepted cases where number of captured nodes differed by one#4532
Merged
clason merged 3 commits intotree-sitter:masterfrom Aug 27, 2025
Merged
Conversation
the provided example should not match
If either side yielded None we could not know the value of the other. Thus, returning true even if the other yielded a node.
quentinLeDilavrec
added a commit
to quentinLeDilavrec/HyperAST
that referenced
this pull request
Jun 25, 2025
differed by one also see tree-sitter/tree-sitter#4532
quentinLeDilavrec
added a commit
to HyperAST/HyperAST
that referenced
this pull request
Jul 17, 2025
Many improvements and bug fixes * Introduce lattice graphs and pattern refinement tools to query synthesis * fix a bug in tsquery related to EqCapture where it incorrectly matched instances differing number of captured nodes, also see fix(rust): EqCapture accepted cases where number of captured nodes differed by one tree-sitter/tree-sitter#4532 * In gen/tree_sitter/cpp, The generation was made more resilient to ts errors that were not properly wrapping non-space characters * fix error in vcs/git where multiple configs would crash the server, for now let's consider that handling pom.xml only require a single config
WillLillis
requested changes
Aug 19, 2025
Member
WillLillis
left a comment
There was a problem hiding this comment.
Very nice catch! I think we can stay a little closer to the original iteration logic for the sake of readability.
diff --git a/lib/binding_rust/lib.rs b/lib/binding_rust/lib.rs
index a866acb8..99ba5557 100644
--- a/lib/binding_rust/lib.rs
+++ b/lib/binding_rust/lib.rs
@@ -3356,12 +3356,11 @@ impl<'tree> QueryMatch<'_, 'tree> {
.iter()
.all(|predicate| match predicate {
TextPredicateCapture::EqCapture(i, j, is_positive, match_all_nodes) => {
- let nodes_1 = self.nodes_for_capture_index(*i);
- let mut nodes_2 = self.nodes_for_capture_index(*j);
- for node1 in nodes_1 {
- let Some(node2) = nodes_2.next() else {
- return false;
- };
+ let mut nodes_1 = self.nodes_for_capture_index(*i).peekable();
+ let mut nodes_2 = self.nodes_for_capture_index(*j).peekable();
+ while nodes_1.peek().is_some() && nodes_2.peek().is_some() {
+ let node1 = nodes_1.next().unwrap();
+ let node2 = nodes_2.next().unwrap();
let mut text1 = text_provider.text(node1);
let mut text2 = text_provider.text(node2);
let text1 = node_text1.get_text(&mut text1);
@@ -3374,7 +3373,7 @@ impl<'tree> QueryMatch<'_, 'tree> {
return true;
}
}
- nodes_2.next().is_none()
+ nodes_1.next().is_none() && nodes_2.next().is_none()
}
TextPredicateCapture::EqString(i, s, is_positive, match_all_nodes) => {
let nodes = self.nodes_for_capture_index(*i);apply suggestion in tree-sitter#4532 (review)
WillLillis
approved these changes
Aug 27, 2025
Member
|
@quentinLeDilavrec Changes look good to me, thanks again for the fix! You've opened up the PR from your fork's master branch, so I'm not able to push to it. Could you squash those three commits into ccd5dc3? Happy to merge after that! |
|
Successfully created backport PR for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed a bug with the implementation of the
eqpredicate in Rust, when using alternations.The bug becomes apparent when comparing values of captures in different alternations not always using the same capture name.
The fix is very straight forward, I just had to iterate the left and right captured nodes more independently.
Hope it helps :)
Thank you a lot for improving and maintaining Tree-Sitter !