From f91255a201f598aadfbc9f8312bac4c5c2ee4e4d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 May 2025 16:56:33 -0700 Subject: [PATCH] Fix crash w/ goto_previous_sibling when parent node has leading extra child (#4472) * Fix crash w/ goto_previous_sibling when parent node has leading extra child Co-authored-by: Smit Barmase Co-authored-by: Smit Barmase * Fix lint Co-authored-by: Smit Barmase --------- Co-authored-by: Smit Barmase --- cli/src/tests/helpers/fixtures.rs | 11 ++++++- cli/src/tests/node_test.rs | 18 +++-------- cli/src/tests/parser_test.rs | 32 +++---------------- cli/src/tests/tree_test.rs | 31 +++++++++++++++++- lib/src/tree_cursor.c | 12 ++++--- .../test_grammars/aliases_in_root/corpus.txt | 13 ++++++++ .../test_grammars/aliases_in_root/grammar.js | 19 +++++++++++ 7 files changed, 90 insertions(+), 46 deletions(-) create mode 100644 test/fixtures/test_grammars/aliases_in_root/corpus.txt create mode 100644 test/fixtures/test_grammars/aliases_in_root/grammar.js diff --git a/cli/src/tests/helpers/fixtures.rs b/cli/src/tests/helpers/fixtures.rs index 44da9b48..0b046bcc 100644 --- a/cli/src/tests/helpers/fixtures.rs +++ b/cli/src/tests/helpers/fixtures.rs @@ -6,11 +6,13 @@ use std::{ use anyhow::Context; use tree_sitter::Language; -use tree_sitter_generate::{ALLOC_HEADER, ARRAY_HEADER}; +use tree_sitter_generate::{load_grammar_file, ALLOC_HEADER, ARRAY_HEADER}; use tree_sitter_highlight::HighlightConfiguration; use tree_sitter_loader::{CompileConfig, Loader}; use tree_sitter_tags::TagsConfiguration; +use crate::tests::generate_parser; + include!("./dirs.rs"); static TEST_LOADER: LazyLock = LazyLock::new(|| { @@ -40,6 +42,13 @@ pub fn get_language(name: &str) -> Language { TEST_LOADER.load_language_at_path(config).unwrap() } +pub fn get_test_fixture_language(name: &str) -> Language { + let grammar_dir_path = fixtures_dir().join("test_grammars").join(name); + let grammar_json = load_grammar_file(&grammar_dir_path.join("grammar.js"), None).unwrap(); + let (parser_name, parser_code) = generate_parser(&grammar_json).unwrap(); + get_test_language(&parser_name, &parser_code, Some(&grammar_dir_path)) +} + pub fn get_language_queries_path(language_name: &str) -> PathBuf { GRAMMARS_DIR.join(language_name).join("queries") } diff --git a/cli/src/tests/node_test.rs b/cli/src/tests/node_test.rs index 22e920d6..515d73aa 100644 --- a/cli/src/tests/node_test.rs +++ b/cli/src/tests/node_test.rs @@ -6,7 +6,10 @@ use super::{ helpers::fixtures::{fixtures_dir, get_language, get_test_language}, Rand, }; -use crate::{parse::perform_edit, tests::generate_parser}; +use crate::{ + parse::perform_edit, + tests::{generate_parser, helpers::fixtures::get_test_fixture_language}, +}; const JSON_EXAMPLE: &str = r#" @@ -308,19 +311,8 @@ fn test_parent_of_zero_width_node() { #[test] fn test_next_sibling_of_zero_width_node() { - let grammar_json = load_grammar_file( - &fixtures_dir() - .join("test_grammars") - .join("next_sibling_from_zwt") - .join("grammar.js"), - None, - ) - .unwrap(); - - let (parser_name, parser_code) = generate_parser(&grammar_json).unwrap(); - let mut parser = Parser::new(); - let language = get_test_language(&parser_name, &parser_code, None); + let language = get_test_fixture_language("next_sibling_from_zwt"); parser.set_language(&language).unwrap(); let tree = parser.parse("abdef", None).unwrap(); diff --git a/cli/src/tests/parser_test.rs b/cli/src/tests/parser_test.rs index a1f730d8..d8b9767d 100644 --- a/cli/src/tests/parser_test.rs +++ b/cli/src/tests/parser_test.rs @@ -6,7 +6,6 @@ use std::{ use tree_sitter::{ Decode, IncludedRangesError, InputEdit, LogType, ParseOptions, ParseState, Parser, Point, Range, }; -use tree_sitter_generate::load_grammar_file; use tree_sitter_proc_macro::retry; use super::helpers::{ @@ -17,7 +16,7 @@ use super::helpers::{ use crate::{ fuzz::edits::Edit, parse::perform_edit, - tests::{generate_parser, helpers::fixtures::fixtures_dir, invert_edit}, + tests::{generate_parser, helpers::fixtures::get_test_fixture_language, invert_edit}, }; #[test] @@ -482,15 +481,9 @@ fn test_parsing_empty_file_with_reused_tree() { #[test] fn test_parsing_after_editing_tree_that_depends_on_column_values() { - let dir = fixtures_dir() - .join("test_grammars") - .join("uses_current_column"); - let grammar_json = load_grammar_file(&dir.join("grammar.js"), None).unwrap(); - let (grammar_name, parser_code) = generate_parser(&grammar_json).unwrap(); - let mut parser = Parser::new(); parser - .set_language(&get_test_language(&grammar_name, &parser_code, Some(&dir))) + .set_language(&get_test_fixture_language("uses_current_column")) .unwrap(); let mut code = b" @@ -559,16 +552,9 @@ h + i #[test] fn test_parsing_after_editing_tree_that_depends_on_column_position() { - let dir = fixtures_dir() - .join("test_grammars") - .join("depends_on_column"); - - let grammar_json = load_grammar_file(&dir.join("grammar.js"), None).unwrap(); - let (grammar_name, parser_code) = generate_parser(grammar_json.as_str()).unwrap(); - let mut parser = Parser::new(); parser - .set_language(&get_test_language(&grammar_name, &parser_code, Some(&dir))) + .set_language(&get_test_fixture_language("depends_on_column")) .unwrap(); let mut code = b"\n x".to_vec(); @@ -1702,13 +1688,9 @@ if foo && bar || baz {} #[test] fn test_parsing_with_scanner_logging() { - let dir = fixtures_dir().join("test_grammars").join("external_tokens"); - let grammar_json = load_grammar_file(&dir.join("grammar.js"), None).unwrap(); - let (grammar_name, parser_code) = generate_parser(&grammar_json).unwrap(); - let mut parser = Parser::new(); parser - .set_language(&get_test_language(&grammar_name, &parser_code, Some(&dir))) + .set_language(&get_test_fixture_language("external_tokens")) .unwrap(); let mut found = false; @@ -1726,13 +1708,9 @@ fn test_parsing_with_scanner_logging() { #[test] fn test_parsing_get_column_at_eof() { - let dir = fixtures_dir().join("test_grammars").join("get_col_eof"); - let grammar_json = load_grammar_file(&dir.join("grammar.js"), None).unwrap(); - let (grammar_name, parser_code) = generate_parser(&grammar_json).unwrap(); - let mut parser = Parser::new(); parser - .set_language(&get_test_language(&grammar_name, &parser_code, Some(&dir))) + .set_language(&get_test_fixture_language("get_col_eof")) .unwrap(); parser.parse("a", None).unwrap(); diff --git a/cli/src/tests/tree_test.rs b/cli/src/tests/tree_test.rs index 083955b1..a5dca965 100644 --- a/cli/src/tests/tree_test.rs +++ b/cli/src/tests/tree_test.rs @@ -3,7 +3,11 @@ use std::str; use tree_sitter::{InputEdit, Parser, Point, Range, Tree}; use super::helpers::fixtures::get_language; -use crate::{fuzz::edits::Edit, parse::perform_edit, tests::invert_edit}; +use crate::{ + fuzz::edits::Edit, + parse::perform_edit, + tests::{helpers::fixtures::get_test_fixture_language, invert_edit}, +}; #[test] fn test_tree_edit() { @@ -377,6 +381,31 @@ fn test_tree_cursor() { assert_eq!(copy.node().kind(), "struct_item"); } +#[test] +fn test_tree_cursor_previous_sibling_with_aliases() { + let mut parser = Parser::new(); + parser + .set_language(&get_test_fixture_language("aliases_in_root")) + .unwrap(); + + let text = "# comment\nfoo foo"; + let tree = parser.parse(text, None).unwrap(); + let mut cursor = tree.walk(); + assert_eq!(cursor.node().kind(), "document"); + + cursor.goto_first_child(); + assert_eq!(cursor.node().kind(), "comment"); + + assert!(cursor.goto_next_sibling()); + assert_eq!(cursor.node().kind(), "bar"); + + assert!(cursor.goto_previous_sibling()); + assert_eq!(cursor.node().kind(), "comment"); + + assert!(cursor.goto_next_sibling()); + assert_eq!(cursor.node().kind(), "bar"); +} + #[test] fn test_tree_cursor_previous_sibling() { let mut parser = Parser::new(); diff --git a/lib/src/tree_cursor.c b/lib/src/tree_cursor.c index 81e17ca7..9d2ddd3d 100644 --- a/lib/src/tree_cursor.c +++ b/lib/src/tree_cursor.c @@ -129,14 +129,18 @@ static inline bool ts_tree_cursor_child_iterator_previous( }; *visible = ts_subtree_visible(*child); bool extra = ts_subtree_extra(*child); - if (!extra && self->alias_sequence) { - *visible |= self->alias_sequence[self->structural_child_index]; - self->structural_child_index--; - } self->position = length_backtrack(self->position, ts_subtree_padding(*child)); self->child_index--; + if (!extra && self->alias_sequence) { + *visible |= self->alias_sequence[self->structural_child_index]; + if (self->child_index > 0) { + self->structural_child_index--; + } + } + + // unsigned can underflow so compare it to child_count if (self->child_index < self->parent.ptr->child_count) { Subtree previous_child = ts_subtree_children(self->parent)[self->child_index]; diff --git a/test/fixtures/test_grammars/aliases_in_root/corpus.txt b/test/fixtures/test_grammars/aliases_in_root/corpus.txt new file mode 100644 index 00000000..ed78852b --- /dev/null +++ b/test/fixtures/test_grammars/aliases_in_root/corpus.txt @@ -0,0 +1,13 @@ +====================================== +Aliases within the root node +====================================== + +# this is a comment +foo foo + +--- + +(document + (comment) + (bar) + (foo)) diff --git a/test/fixtures/test_grammars/aliases_in_root/grammar.js b/test/fixtures/test_grammars/aliases_in_root/grammar.js new file mode 100644 index 00000000..02d61646 --- /dev/null +++ b/test/fixtures/test_grammars/aliases_in_root/grammar.js @@ -0,0 +1,19 @@ +module.exports = grammar({ + name: 'aliases_in_root', + + extras: $ => [ + /\s/, + $.comment, + ], + + rules: { + document: $ => seq( + alias($.foo, $.bar), + $.foo, + ), + + foo: $ => "foo", + + comment: $ => /#.*/ + } +});