From aeea47323a94bcc69d686632d97f74e7ccb8c1bb Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Wed, 23 Nov 2022 13:37:22 -0800
Subject: [PATCH] Fix enclosing-bracket bug that appeared in JS for loops

Previously, we were relying on the tree-sitter query's range restriction to
avoid returning brackets that did not contain the given range. But the
query's range restriction only guarantees that we don't descend into parent
nodes unless they intersect the range.
---
 crates/language/src/buffer.rs       | 31 ++++++++-------
 crates/language/src/buffer_tests.rs | 62 ++++++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs
index 2a0b158a7a412695c1d2022448068d93702ed7b0..e8bc2bf314c45784de7653c1d63d803379166f2c 100644
--- a/crates/language/src/buffer.rs
+++ b/crates/language/src/buffer.rs
@@ -2225,11 +2225,12 @@ impl BufferSnapshot {
         range: Range<T>,
     ) -> Option<(Range<usize>, Range<usize>)> {
         // Find bracket pairs that *inclusively* contain the given range.
-        let range = range.start.to_offset(self).saturating_sub(1)
-            ..self.len().min(range.end.to_offset(self) + 1);
-        let mut matches = self.syntax.matches(range, &self.text, |grammar| {
-            grammar.brackets_config.as_ref().map(|c| &c.query)
-        });
+        let range = range.start.to_offset(self)..range.end.to_offset(self);
+        let mut matches = self.syntax.matches(
+            range.start.saturating_sub(1)..self.len().min(range.end + 1),
+            &self.text,
+            |grammar| grammar.brackets_config.as_ref().map(|c| &c.query),
+        );
         let configs = matches
             .grammars()
             .iter()
@@ -2252,18 +2253,20 @@ impl BufferSnapshot {
 
             matches.advance();
 
-            if let Some((open, close)) = open.zip(close) {
-                let len = close.end - open.start;
+            let Some((open, close)) = open.zip(close) else { continue };
+            if open.start > range.start || close.end < range.end {
+                continue;
+            }
+            let len = close.end - open.start;
 
-                if let Some((existing_open, existing_close)) = &result {
-                    let existing_len = existing_close.end - existing_open.start;
-                    if len > existing_len {
-                        continue;
-                    }
+            if let Some((existing_open, existing_close)) = &result {
+                let existing_len = existing_close.end - existing_open.start;
+                if len > existing_len {
+                    continue;
                 }
-
-                result = Some((open, close));
             }
+
+            result = Some((open, close));
         }
 
         result
diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs
index 6043127dd526c2d44d0bb66050c83169c73efbe5..116bf175f1e6c480886c61afc94d22fbc94d85b4 100644
--- a/crates/language/src/buffer_tests.rs
+++ b/crates/language/src/buffer_tests.rs
@@ -573,14 +573,72 @@ fn test_enclosing_bracket_ranges(cx: &mut MutableAppContext) {
         ))
     );
 
-    // Regression test: avoid crash when querying at the end of the buffer.
     assert_eq!(
-        buffer.enclosing_bracket_point_ranges(buffer.len() - 1..buffer.len()),
+        buffer.enclosing_bracket_point_ranges(Point::new(4, 1)..Point::new(4, 1)),
         Some((
             Point::new(0, 6)..Point::new(0, 7),
             Point::new(4, 0)..Point::new(4, 1)
         ))
     );
+
+    // Regression test: avoid crash when querying at the end of the buffer.
+    assert_eq!(
+        buffer.enclosing_bracket_point_ranges(Point::new(4, 1)..Point::new(5, 0)),
+        None
+    );
+}
+
+#[gpui::test]
+fn test_enclosing_bracket_ranges_where_brackets_are_not_outermost_children(
+    cx: &mut MutableAppContext,
+) {
+    let javascript_language = Arc::new(
+        Language::new(
+            LanguageConfig {
+                name: "JavaScript".into(),
+                ..Default::default()
+            },
+            Some(tree_sitter_javascript::language()),
+        )
+        .with_brackets_query(
+            r#"
+            ("{" @open "}" @close)
+            ("(" @open ")" @close)
+            "#,
+        )
+        .unwrap(),
+    );
+
+    cx.set_global(Settings::test(cx));
+    let buffer = cx.add_model(|cx| {
+        let text = "
+            for (const a in b) {
+                // a comment that's longer than the for-loop header
+            }
+        "
+        .unindent();
+        Buffer::new(0, text, cx).with_language(javascript_language, cx)
+    });
+
+    let buffer = buffer.read(cx);
+    assert_eq!(
+        buffer.enclosing_bracket_point_ranges(Point::new(0, 18)..Point::new(0, 18)),
+        Some((
+            Point::new(0, 4)..Point::new(0, 5),
+            Point::new(0, 17)..Point::new(0, 18)
+        ))
+    );
+
+    // Regression test: even though the parent node of the parentheses (the for loop) does
+    // intersect the given range, the parentheses themselves do not contain the range, so
+    // they should not be returned. Only the curly braces contain the range.
+    assert_eq!(
+        buffer.enclosing_bracket_point_ranges(Point::new(0, 20)..Point::new(0, 20)),
+        Some((
+            Point::new(0, 19)..Point::new(0, 20),
+            Point::new(2, 0)..Point::new(2, 1)
+        ))
+    );
 }
 
 #[gpui::test]