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 <heysmitbarmase@gmail.com>

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>

* Fix lint

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>

---------

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>

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<Loader> = 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")
 }

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();

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();

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();

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];

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))

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: $ => /#.*/
+    }
+});