highlighter: Maintain highlight ordering when captures emit out-of-order

This fixes a case that pops up with some specific CSS highlights which
causes the `active_highlights` stack to become out of order. The odd
highlight is this:

    "#" @punctuation
    ((color_value) "#") @string.special
    (color_value) @string.special

The `#` node is fought over by the first two patterns. Requiring that
the pattern matches `#` in the second pattern causes tree-sitter to
finish its capture after the first pattern, so we capture the child node
`{Node # 9..10}` first and then `{Node color_value 9..13}` - reversed of
the normal ordering.

In this case we need to maintain the ordering of `active_highlights` by
`Vec::insert`ing into the correct position.

Author: the-mikedavis

fixtures/highlighter/parent_child_highlight_precedence.css

@@ -0,0 +1,6 @@
+   --color: #fff;
+// ┡━━━━━┛╿ ╿┡━┛╰─ punctuation.delimiter
+// │      │ │╰─ string.special
+// │      │ ╰─ string.special punctuation
+// │      ╰─ punctuation.delimiter
+// ╰─ variable

highlighter/src/highlighter.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use std::cmp;
 use std::fmt;
 use std::mem::replace;
 use std::num::NonZeroU32;
@@ -386,21 +387,45 @@ impl<'a, 'tree: 'a, Loader: LanguageLoader> Highlighter<'a, 'tree, Loader> {
             config.highlight_query.highlight_indices.load()[node.capture.idx()]
         };
 
+        let highlight = highlight.map(|highlight| HighlightedNode {
+            end: range.end,
+            highlight,
+        });
+
         // If multiple patterns match this exact node, prefer the last one which matched.
         // This matches the precedence of Neovim, Zed, and tree-sitter-cli.
-        if !*first_highlight
-            && self
+        if !*first_highlight {
+            // NOTE: `!*first_highlight` implies that the start positions are the same.
+            let insert_position = self
                 .active_highlights
-                .last()
-                .is_some_and(|prev_node| prev_node.end == range.end)
-        {
-            self.active_highlights.pop();
-        }
-        if let Some(highlight) = highlight {
-            self.active_highlights.push(HighlightedNode {
-                end: range.end,
-                highlight,
-            });
+                .iter()
+                .rposition(|h| h.end <= range.end);
+            if let Some(idx) = insert_position {
+                match self.active_highlights[idx].end.cmp(&range.end) {
+                    // If there is a prior highlight for this start..end range, replace it.
+                    cmp::Ordering::Equal => {
+                        if let Some(highlight) = highlight {
+                            self.active_highlights[idx] = highlight;
+                        } else {
+                            self.active_highlights.remove(idx);
+                        }
+                    }
+                    // Captures are emitted in the order that they are finished. Insert any
+                    // highlights which start at the same position into the active highlights so
+                    // that the ordering invariant remains satisfied.
+                    cmp::Ordering::Less => {
+                        if let Some(highlight) = highlight {
+                            self.active_highlights.insert(idx, highlight)
+                        }
+                    }
+                    // By definition of our `rposition` predicate:
+                    cmp::Ordering::Greater => unreachable!(),
+                }
+            } else {
+                self.active_highlights.extend(highlight);
+            }
+        } else if let Some(highlight) = highlight {
+            self.active_highlights.push(highlight);
             *first_highlight = false;
         }
 
@@ -420,7 +445,7 @@ impl<'a, 'tree: 'a, Loader: LanguageLoader> Highlighter<'a, 'tree, Loader> {
                     .map(|layer| layer.parent_highlights)
                     .unwrap_or_default();
 
-                self.active_highlights[layer_start..].is_sorted_by_key(|h| std::cmp::Reverse(h.end))
+                self.active_highlights[layer_start..].is_sorted_by_key(|h| cmp::Reverse(h.end))
             },
             "unsorted highlights on layer {:?}: {:?}\nall active highlights must be sorted by `end` descending",
             self.current_layer,

highlighter/src/tests.rs

@@ -123,8 +123,6 @@ impl TestLanguageLoader {
         self.lang_config[lang.idx()] = OnceCell::new();
     }
 
-    // TODO: remove on first use.
-    #[allow(dead_code)]
     fn overwrite_highlights(&mut self, lang: &str, content: String) {
         let lang = self.get(lang);
         self.overwrites[lang.idx()].highlights = Some(content);
@@ -144,8 +142,6 @@ impl TestLanguageLoader {
         self.lang_config[lang.idx()] = OnceCell::new();
     }
 
-    // TODO: remove on first use.
-    #[allow(dead_code)]
     fn shadow_highlights(&mut self, lang: &str, content: &str) {
         let lang = self.get(lang);
         let skidder_config = skidder_config();
@@ -195,9 +191,10 @@ fn lang_for_path(path: &Path, loader: &TestLanguageLoader) -> Language {
     {
         "rs" => loader.get("rust"),
         "html" => loader.get("html"),
+        "css" => loader.get("css"),
         "erl" => loader.get("erlang"),
         "md" => loader.get("markdown"),
-        extension => unreachable!("unknown file type .{extension}"),
+        extension => panic!("unknown file type .{extension}"),
     }
 }
 
@@ -509,3 +506,35 @@ fn markdown_bold_highlight() {
     // into one span.
     highlight_fixture(&loader, "highlighter/markdown_bold.md");
 }
+
+#[test]
+fn css_parent_child_highlight_precedence() {
+    let mut loader = TestLanguageLoader::new();
+    // NOTE: the pattern being tested here `((color_value) "#") @string.special` is odd and should
+    // be rewritten to `(color_value "#" @string.special)` - that was probably the original intent
+    // of the pattern. We overwrite the highlights to take the parts we need for this case so that
+    // if/when we fix that pattern in the future it does not break this test case.
+    loader.overwrite_highlights(
+        "css",
+        r##"
+((property_name) @variable
+ (#match? @variable "^--"))
+
+"#" @punctuation
+((color_value) "#") @string.special
+(color_value) @string.special
+
+[";" ":"] @punctuation.delimiter
+"##
+        .to_string(),
+    );
+
+    // In this case two patterns fight over the `#` character and both should actually highlight
+    // it. Because of the odd way that the pattern `((color_value) "#") @string.special` is
+    // written, the QueryIter yields the captures in the opposite order it would normally:
+    // first the child pattern `{Node # 9..10}` and then `{Node color_value 9..13}`.
+    //
+    // This case checks the invariant that "`active_highlights` ends are sorted descending" is
+    // preserved.
+    highlight_fixture(&loader, "highlighter/parent_child_highlight_precedence.css");
+}