Merge #3826

3826: Flatten nested highlight ranges during DFS traversal r=matklad a=ltentrup

Implements the flattening of nested highlights from #3447.


There is a caveat: I needed to add `Clone` to `HighlightedRange` to split highlight ranges  ~and the nesting does not appear in the syntax highlighting test (it does appear in the accidental-quadratic test but there it is not checked against a ground-truth)~.

I have added a test case for the example mentioned in #3447.

Co-authored-by: Leander Tentrup <[email protected]>

5 files changed, 148 insertions, 51 deletions

diff --git a/crates/ra_ide/src/snapshots/highlight_injection.html b/crates/ra_ide/src/snapshots/highlight_injection.html
new file mode 100644
index 000000000..6ec13bd80
--- /dev/null
+++ b/crates/ra_ide/src/snapshots/highlight_injection.html
@@ -0,0 +1,39 @@
+
+<style>
+body                { margin: 0; }
+pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padding: 0.4em; }
+
+.lifetime           { color: #DFAF8F; font-style: italic; }
+.comment            { color: #7F9F7F; }
+.struct, .enum      { color: #7CB8BB; }
+.enum_variant       { color: #BDE0F3; }
+.string_literal     { color: #CC9393; }
+.field              { color: #94BFF3; }
+.function           { color: #93E0E3; }
+.parameter          { color: #94BFF3; }
+.text               { color: #DCDCCC; }
+.type               { color: #7CB8BB; }
+.builtin_type       { color: #8CD0D3; }
+.type_param         { color: #DFAF8F; }
+.attribute          { color: #94BFF3; }
+.numeric_literal    { color: #BFEBBF; }
+.macro              { color: #94BFF3; }
+.module             { color: #AFD8AF; }
+.variable           { color: #DCDCCC; }
+.mutable            { text-decoration: underline; }
+
+.keyword            { color: #F0DFAF; font-weight: bold; }
+.keyword.unsafe     { color: #BC8383; font-weight: bold; }
+.control            { font-style: italic; }
+</style>
+<pre><code><span class="keyword">fn</span> <span class="function declaration">fixture</span>(<span class="variable declaration">ra_fixture</span>: &<span class="builtin_type">str</span>) {}
+
+<span class="keyword">fn</span> <span class="function declaration">main</span>() {
+    <span class="function">fixture</span>(<span class="string_literal">r#"</span>
+        <span class="keyword">trait</span> <span class="trait declaration">Foo</span> {
+            <span class="keyword">fn</span> <span class="function declaration">foo</span>() {
+                <span class="macro">println!</span>(<span class="string_literal">"2 + 2 = {}"</span>, <span class="numeric_literal">4</span>);
+            }
+        }<span class="string_literal">"#</span>
+    );
+}</code></pre>
\ No newline at end of file
diff --git a/crates/ra_ide/src/snapshots/highlighting.html b/crates/ra_ide/src/snapshots/highlighting.html
index 495b07f69..214dcbb62 100644
--- a/crates/ra_ide/src/snapshots/highlighting.html
+++ b/crates/ra_ide/src/snapshots/highlighting.html
@@ -26,7 +26,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 .keyword.unsafe     { color: #BC8383; font-weight: bold; }
 .control            { font-style: italic; }
 </style>
-<pre><code><span class="attribute">#</span><span class="attribute">[</span><span class="attribute">derive</span><span class="attribute">(</span><span class="attribute">Clone</span><span class="attribute">,</span><span class="attribute"> </span><span class="attribute">Debug</span><span class="attribute">)</span><span class="attribute">]</span>
+<pre><code><span class="attribute">#[derive(Clone, Debug)]</span>
 <span class="keyword">struct</span> <span class="struct declaration">Foo</span> {
     <span class="keyword">pub</span> <span class="field declaration">x</span>: <span class="builtin_type">i32</span>,
     <span class="keyword">pub</span> <span class="field declaration">y</span>: <span class="builtin_type">i32</span>,
@@ -36,11 +36,11 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
     <span class="function">foo</span>::&lt;<span class="lifetime">'a</span>, <span class="builtin_type">i32</span>&gt;()
 }
 
-<span class="macro">macro_rules</span><span class="macro">!</span> def_fn {
+<span class="macro">macro_rules!</span> def_fn {
     ($($tt:tt)*) =&gt; {$($tt)*}
 }
 
-<span class="macro">def_fn</span><span class="macro">!</span> {
+<span class="macro">def_fn!</span> {
     <span class="keyword">fn</span> <span class="function declaration">bar</span>() -&gt; <span class="builtin_type">u32</span> {
         <span class="numeric_literal">100</span>
     }
@@ -48,7 +48,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 
 <span class="comment">// comment</span>
 <span class="keyword">fn</span> <span class="function declaration">main</span>() {
-    <span class="macro">println</span><span class="macro">!</span>(<span class="string_literal">"Hello, {}!"</span>, <span class="numeric_literal">92</span>);
+    <span class="macro">println!</span>(<span class="string_literal">"Hello, {}!"</span>, <span class="numeric_literal">92</span>);
 
     <span class="keyword">let</span> <span class="keyword">mut</span> <span class="variable declaration mutable">vec</span> = Vec::new();
     <span class="keyword control">if</span> <span class="keyword">true</span> {
@@ -73,7 +73,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 <span class="keyword">impl</span>&lt;<span class="type_param declaration">T</span>&gt; <span class="enum">Option</span>&lt;<span class="type_param">T</span>&gt; {
     <span class="keyword">fn</span> <span class="function declaration">and</span>&lt;<span class="type_param declaration">U</span>&gt;(<span class="keyword">self</span>, <span class="variable declaration">other</span>: <span class="enum">Option</span>&lt;<span class="type_param">U</span>&gt;) -&gt; <span class="enum">Option</span>&lt;(<span class="type_param">T</span>, <span class="type_param">U</span>)&gt; {
         <span class="keyword control">match</span> <span class="variable">other</span> {
-            <span class="enum_variant">None</span> =&gt; <span class="macro">unimplemented</span><span class="macro">!</span>(),
+            <span class="enum_variant">None</span> =&gt; <span class="macro">unimplemented!</span>(),
             <span class="variable declaration">Nope</span> =&gt; <span class="variable">Nope</span>,
         }
     }
diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs
index d833a816b..83d161f45 100644
--- a/crates/ra_ide/src/syntax_highlighting.rs
+++ b/crates/ra_ide/src/syntax_highlighting.rs
@@ -24,7 +24,7 @@ use crate::{call_info::call_info_for_token, Analysis, FileId};
 pub(crate) use html::highlight_as_html;
 pub use tags::{Highlight, HighlightModifier, HighlightModifiers, HighlightTag};
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct HighlightedRange {
     pub range: TextRange,
     pub highlight: Highlight,
@@ -55,13 +55,55 @@ pub(crate) fn highlight(
     };
 
     let mut bindings_shadow_count: FxHashMap<Name, u32> = FxHashMap::default();
-    let mut res = Vec::new();
+    // We use a stack for the DFS traversal below.
+    // When we leave a node, the we use it to flatten the highlighted ranges.
+    let mut res: Vec<Vec<HighlightedRange>> = vec![Vec::new()];
 
     let mut current_macro_call: Option<ast::MacroCall> = None;
 
     // Walk all nodes, keeping track of whether we are inside a macro or not.
     // If in macro, expand it first and highlight the expanded code.
     for event in root.preorder_with_tokens() {
+        match &event {
+            WalkEvent::Enter(_) => res.push(Vec::new()),
+            WalkEvent::Leave(_) => {
+                /* Flattens the highlighted ranges.
+                 *
+                 * For example `#[cfg(feature = "foo")]` contains the nested ranges:
+                 * 1) parent-range: Attribute [0, 23)
+                 * 2) child-range: String [16, 21)
+                 *
+                 * The following code implements the flattening, for our example this results to:
+                 * `[Attribute [0, 16), String [16, 21), Attribute [21, 23)]`
+                 */
+                let children = res.pop().unwrap();
+                let prev = res.last_mut().unwrap();
+                let needs_flattening = !children.is_empty()
+                    && !prev.is_empty()
+                    && children.first().unwrap().range.is_subrange(&prev.last().unwrap().range);
+                if !needs_flattening {
+                    prev.extend(children);
+                } else {
+                    let mut parent = prev.pop().unwrap();
+                    for ele in children {
+                        assert!(ele.range.is_subrange(&parent.range));
+                        let mut cloned = parent.clone();
+                        parent.range = TextRange::from_to(parent.range.start(), ele.range.start());
+                        cloned.range = TextRange::from_to(ele.range.end(), cloned.range.end());
+                        if !parent.range.is_empty() {
+                            prev.push(parent);
+                        }
+                        prev.push(ele);
+                        parent = cloned;
+                    }
+                    if !parent.range.is_empty() {
+                        prev.push(parent);
+                    }
+                }
+            }
+        };
+        let current = res.last_mut().expect("during DFS traversal, the stack must not be empty");
+
         let event_range = match &event {
             WalkEvent::Enter(it) => it.text_range(),
             WalkEvent::Leave(it) => it.text_range(),
@@ -77,7 +119,7 @@ pub(crate) fn highlight(
             WalkEvent::Enter(Some(mc)) => {
                 current_macro_call = Some(mc.clone());
                 if let Some(range) = macro_call_range(&mc) {
-                    res.push(HighlightedRange {
+                    current.push(HighlightedRange {
                         range,
                         highlight: HighlightTag::Macro.into(),
                         binding_hash: None,
@@ -119,7 +161,7 @@ pub(crate) fn highlight(
 
         if let Some(token) = element.as_token().cloned().and_then(ast::RawString::cast) {
             let expanded = element_to_highlight.as_token().unwrap().clone();
-            if highlight_injection(&mut res, &sema, token, expanded).is_some() {
+            if highlight_injection(current, &sema, token, expanded).is_some() {
                 continue;
             }
         }
@@ -127,10 +169,17 @@ pub(crate) fn highlight(
         if let Some((highlight, binding_hash)) =
             highlight_element(&sema, &mut bindings_shadow_count, element_to_highlight)
         {
-            res.push(HighlightedRange { range, highlight, binding_hash });
+            current.push(HighlightedRange { range, highlight, binding_hash });
         }
     }
 
+    assert_eq!(res.len(), 1, "after DFS traversal, the stack should only contain a single element");
+    let res = res.pop().unwrap();
+    // Check that ranges are sorted and disjoint
+    assert!(res
+        .iter()
+        .zip(res.iter().skip(1))
+        .all(|(left, right)| left.range.end() <= right.range.start()));
     res
 }
 
diff --git a/crates/ra_ide/src/syntax_highlighting/html.rs b/crates/ra_ide/src/syntax_highlighting/html.rs
index e13766c9d..4496529a1 100644
--- a/crates/ra_ide/src/syntax_highlighting/html.rs
+++ b/crates/ra_ide/src/syntax_highlighting/html.rs
@@ -1,9 +1,9 @@
 //! Renders a bit of code as HTML.
 
 use ra_db::SourceDatabase;
-use ra_syntax::AstNode;
+use ra_syntax::{AstNode, TextUnit};
 
-use crate::{FileId, HighlightedRange, RootDatabase};
+use crate::{FileId, RootDatabase};
 
 use super::highlight;
 
@@ -21,51 +21,35 @@ pub(crate) fn highlight_as_html(db: &RootDatabase, file_id: FileId, rainbow: boo
         )
     }
 
-    let mut ranges = highlight(db, file_id, None);
-    ranges.sort_by_key(|it| it.range.start());
-    // quick non-optimal heuristic to intersect token ranges and highlighted ranges
-    let mut frontier = 0;
-    let mut could_intersect: Vec<&HighlightedRange> = Vec::new();
-
+    let ranges = highlight(db, file_id, None);
+    let text = parse.tree().syntax().to_string();
+    let mut prev_pos = TextUnit::from(0);
     let mut buf = String::new();
     buf.push_str(&STYLE);
     buf.push_str("<pre><code>");
-    let tokens = parse.tree().syntax().descendants_with_tokens().filter_map(|it| it.into_token());
-    for token in tokens {
-        could_intersect.retain(|it| token.text_range().start() <= it.range.end());
-        while let Some(r) = ranges.get(frontier) {
-            if r.range.start() <= token.text_range().end() {
-                could_intersect.push(r);
-                frontier += 1;
-            } else {
-                break;
-            }
-        }
-        let text = html_escape(&token.text());
-        let ranges = could_intersect
-            .iter()
-            .filter(|it| token.text_range().is_subrange(&it.range))
-            .collect::<Vec<_>>();
-        if ranges.is_empty() {
+    for range in &ranges {
+        if range.range.start() > prev_pos {
+            let curr = &text[prev_pos.to_usize()..range.range.start().to_usize()];
+            let text = html_escape(curr);
             buf.push_str(&text);
-        } else {
-            let classes = ranges
-                .iter()
-                .map(|it| it.highlight.to_string().replace('.', " "))
-                .collect::<Vec<_>>()
-                .join(" ");
-            let binding_hash = ranges.first().and_then(|x| x.binding_hash);
-            let color = match (rainbow, binding_hash) {
-                (true, Some(hash)) => format!(
-                    " data-binding-hash=\"{}\" style=\"color: {};\"",
-                    hash,
-                    rainbowify(hash)
-                ),
-                _ => "".into(),
-            };
-            buf.push_str(&format!("<span class=\"{}\"{}>{}</span>", classes, color, text));
         }
+        let curr = &text[range.range.start().to_usize()..range.range.end().to_usize()];
+
+        let class = range.highlight.to_string().replace('.', " ");
+        let color = match (rainbow, range.binding_hash) {
+            (true, Some(hash)) => {
+                format!(" data-binding-hash=\"{}\" style=\"color: {};\"", hash, rainbowify(hash))
+            }
+            _ => "".into(),
+        };
+        buf.push_str(&format!("<span class=\"{}\"{}>{}</span>", class, color, html_escape(curr)));
+
+        prev_pos = range.range.end();
     }
+    // Add the remaining (non-highlighted) text
+    let curr = &text[prev_pos.to_usize()..];
+    let text = html_escape(curr);
+    buf.push_str(&text);
     buf.push_str("</code></pre>");
     buf
 }
diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs
index 98c030791..110887c2a 100644
--- a/crates/ra_ide/src/syntax_highlighting/tests.rs
+++ b/crates/ra_ide/src/syntax_highlighting/tests.rs
@@ -131,3 +131,28 @@ fn test_ranges() {
 
     assert_eq!(&highlights[0].highlight.to_string(), "field.declaration");
 }
+
+#[test]
+fn test_flattening() {
+    let (analysis, file_id) = single_file(
+        r##"
+fn fixture(ra_fixture: &str) {}
+
+fn main() {
+    fixture(r#"
+        trait Foo {
+            fn foo() {
+                println!("2 + 2 = {}", 4);
+            }
+        }"#
+    );
+}"##
+        .trim(),
+    );
+
+    let dst_file = project_dir().join("crates/ra_ide/src/snapshots/highlight_injection.html");
+    let actual_html = &analysis.highlight_as_html(file_id, false).unwrap();
+    let expected_html = &read_text(&dst_file);
+    fs::write(dst_file, &actual_html).unwrap();
+    assert_eq_text!(expected_html, actual_html);
+}