From bb45aca9093ffe61cf444d538b3de737117c9a64 Mon Sep 17 00:00:00 2001 From: Leander Tentrup Date: Thu, 2 Apr 2020 14:34:51 +0200 Subject: Flatten nested highlight ranges during DFS traversal --- crates/ra_ide/src/syntax_highlighting.rs | 55 +++++++++++++++++++++++--- crates/ra_ide/src/syntax_highlighting/tests.rs | 16 ++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) (limited to 'crates/ra_ide/src') diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs index 7fc94d3cd..eb1c54639 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 = 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![Vec::new()]; let mut current_macro_call: Option = 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,11 +169,12 @@ 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 }); } } - res + assert_eq!(res.len(), 1, "after DFS traversal, the stack should only contain a single element"); + res.pop().unwrap() } fn macro_call_range(macro_call: &ast::MacroCall) -> Option { diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index 98c030791..7f442bd0f 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -131,3 +131,19 @@ fn test_ranges() { assert_eq!(&highlights[0].highlight.to_string(), "field.declaration"); } + +#[test] +fn test_flattening() { + let (analysis, file_id) = single_file(r#"#[cfg(feature = "foo")]"#); + + let highlights = analysis.highlight(file_id).unwrap(); + + // The source code snippet contains 2 nested highlights: + // 1) Attribute spanning the whole string + // 2) The string "foo" + // The resulting flattening splits the attribute range: + assert_eq!(highlights.len(), 3); + assert_eq!(&highlights[0].highlight.to_string(), "attribute"); + assert_eq!(&highlights[1].highlight.to_string(), "string_literal"); + assert_eq!(&highlights[2].highlight.to_string(), "attribute"); +} -- cgit v1.2.3 From bf96d46fee1242ad701cb037a03c9575e84221b1 Mon Sep 17 00:00:00 2001 From: Leander Tentrup Date: Mon, 6 Apr 2020 23:00:09 +0200 Subject: Simplify HTML highlighter and add test case for highlight_injection logic --- .../ra_ide/src/snapshots/highlight_injection.html | 39 +++++++++++++ crates/ra_ide/src/snapshots/highlighting.html | 10 ++-- crates/ra_ide/src/syntax_highlighting.rs | 8 ++- crates/ra_ide/src/syntax_highlighting/html.rs | 66 ++++++++-------------- crates/ra_ide/src/syntax_highlighting/tests.rs | 33 +++++++---- 5 files changed, 97 insertions(+), 59 deletions(-) create mode 100644 crates/ra_ide/src/snapshots/highlight_injection.html (limited to 'crates/ra_ide/src') 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 @@ + + +
fn fixture(ra_fixture: &str) {}
+
+fn main() {
+    fixture(r#"
+        trait Foo {
+            fn foo() {
+                println!("2 + 2 = {}", 4);
+            }
+        }"#
+    );
+}
\ 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; } -
#[derive(Clone, Debug)]
+
#[derive(Clone, Debug)]
 struct Foo {
     pub x: i32,
     pub y: i32,
@@ -36,11 +36,11 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
     foo::<'a, i32>()
 }
 
-macro_rules! def_fn {
+macro_rules! def_fn {
     ($($tt:tt)*) => {$($tt)*}
 }
 
-def_fn! {
+def_fn! {
     fn bar() -> u32 {
         100
     }
@@ -48,7 +48,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 
 // comment
 fn main() {
-    println!("Hello, {}!", 92);
+    println!("Hello, {}!", 92);
 
     let mut vec = Vec::new();
     if true {
@@ -73,7 +73,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 impl<T> Option<T> {
     fn and<U>(self, other: Option<U>) -> Option<(T, U)> {
         match other {
-            None => unimplemented!(),
+            None => unimplemented!(),
             Nope => Nope,
         }
     }
diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs
index eb1c54639..7d908f987 100644
--- a/crates/ra_ide/src/syntax_highlighting.rs
+++ b/crates/ra_ide/src/syntax_highlighting.rs
@@ -174,7 +174,13 @@ pub(crate) fn highlight(
     }
 
     assert_eq!(res.len(), 1, "after DFS traversal, the stack should only contain a single element");
-    res.pop().unwrap()
+    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
 }
 
 fn macro_call_range(macro_call: &ast::MacroCall) -> Option {
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("
");
-    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::>();
-        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::>()
-                .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!("{}", 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!("{}", 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("
");
     buf
 }
diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs
index 7f442bd0f..110887c2a 100644
--- a/crates/ra_ide/src/syntax_highlighting/tests.rs
+++ b/crates/ra_ide/src/syntax_highlighting/tests.rs
@@ -134,16 +134,25 @@ fn test_ranges() {
 
 #[test]
 fn test_flattening() {
-    let (analysis, file_id) = single_file(r#"#[cfg(feature = "foo")]"#);
-
-    let highlights = analysis.highlight(file_id).unwrap();
-
-    // The source code snippet contains 2 nested highlights:
-    // 1) Attribute spanning the whole string
-    // 2) The string "foo"
-    // The resulting flattening splits the attribute range:
-    assert_eq!(highlights.len(), 3);
-    assert_eq!(&highlights[0].highlight.to_string(), "attribute");
-    assert_eq!(&highlights[1].highlight.to_string(), "string_literal");
-    assert_eq!(&highlights[2].highlight.to_string(), "attribute");
+    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);
 }
-- 
cgit v1.2.3


From d8f6013404a88d845cda6793162e7ac24b0ccbf2 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov 
Date: Tue, 7 Apr 2020 18:23:18 +0200
Subject: Fix names of test modules

---
 crates/ra_ide/src/completion/complete_record.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'crates/ra_ide/src')

diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs
index 79f5c8c8f..b180e2388 100644
--- a/crates/ra_ide/src/completion/complete_record.rs
+++ b/crates/ra_ide/src/completion/complete_record.rs
@@ -59,7 +59,7 @@ fn pattern_ascribed_fields(record_pat: &ast::RecordPat) -> Vec {
 
 #[cfg(test)]
 mod tests {
-    mod record_lit_tests {
+    mod record_pat_tests {
         use insta::assert_debug_snapshot;
 
         use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind};
@@ -205,7 +205,7 @@ mod tests {
         }
     }
 
-    mod record_pat_tests {
+    mod record_lit_tests {
         use insta::assert_debug_snapshot;
 
         use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind};
-- 
cgit v1.2.3


From 7819d99d6bc617ee8653e9dc2fa4d82072d6c594 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov 
Date: Tue, 7 Apr 2020 18:25:47 +0200
Subject: Add functional update test

---
 crates/ra_ide/src/completion/complete_record.rs | 33 +++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

(limited to 'crates/ra_ide/src')

diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs
index b180e2388..2352ced5f 100644
--- a/crates/ra_ide/src/completion/complete_record.rs
+++ b/crates/ra_ide/src/completion/complete_record.rs
@@ -410,5 +410,38 @@ mod tests {
         ]
         "###);
         }
+
+        #[test]
+        fn completes_functional_update() {
+            let completions = complete(
+                r"
+            struct S {
+                foo1: u32,
+                foo2: u32,
+            }
+
+            fn main() {
+                let foo1 = 1;
+                let s = S {
+                    foo1,
+                    <|>
+                    .. loop {}
+                }
+            }
+            ",
+            );
+            assert_debug_snapshot!(completions, @r###"
+        [
+            CompletionItem {
+                label: "foo2",
+                source_range: [221; 221),
+                delete: [221; 221),
+                insert: "foo2",
+                kind: Field,
+                detail: "u32",
+            },
+        ]
+        "###);
+        }
     }
 }
-- 
cgit v1.2.3


From 4c29214bba65d23e18875bd060325c489be5a8e4 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov 
Date: Tue, 7 Apr 2020 17:09:02 +0200
Subject: Move computation of missing fields into hir

---
 crates/ra_ide/src/completion/complete_record.rs | 59 ++++---------------------
 1 file changed, 9 insertions(+), 50 deletions(-)

(limited to 'crates/ra_ide/src')

diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs
index 2352ced5f..f46bcee5c 100644
--- a/crates/ra_ide/src/completion/complete_record.rs
+++ b/crates/ra_ide/src/completion/complete_record.rs
@@ -1,60 +1,19 @@
 //! Complete fields in record literals and patterns.
-use ra_syntax::{ast, ast::NameOwner, SmolStr};
-
 use crate::completion::{CompletionContext, Completions};
 
 pub(super) fn complete_record(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> {
-    let (ty, variant, already_present_fields) =
-        match (ctx.record_lit_pat.as_ref(), ctx.record_lit_syntax.as_ref()) {
-            (None, None) => return None,
-            (Some(_), Some(_)) => unreachable!("A record cannot be both a literal and a pattern"),
-            (Some(record_pat), _) => (
-                ctx.sema.type_of_pat(&record_pat.clone().into())?,
-                ctx.sema.resolve_record_pattern(record_pat)?,
-                pattern_ascribed_fields(record_pat),
-            ),
-            (_, Some(record_lit)) => (
-                ctx.sema.type_of_expr(&record_lit.clone().into())?,
-                ctx.sema.resolve_record_literal(record_lit)?,
-                literal_ascribed_fields(record_lit),
-            ),
-        };
+    let missing_fields = match (ctx.record_lit_pat.as_ref(), ctx.record_lit_syntax.as_ref()) {
+        (None, None) => return None,
+        (Some(_), Some(_)) => unreachable!("A record cannot be both a literal and a pattern"),
+        (Some(record_pat), _) => ctx.sema.record_pattern_missing_fields(record_pat),
+        (_, Some(record_lit)) => ctx.sema.record_literal_missing_fields(record_lit),
+    };
 
-    for (field, field_ty) in ty.variant_fields(ctx.db, variant).into_iter().filter(|(field, _)| {
-        // FIXME: already_present_names better be `Vec`
-        !already_present_fields.contains(&SmolStr::from(field.name(ctx.db).to_string()))
-    }) {
-        acc.add_field(ctx, field, &field_ty);
+    for (field, ty) in missing_fields {
+        acc.add_field(ctx, field, &ty)
     }
-    Some(())
-}
 
-fn literal_ascribed_fields(record_lit: &ast::RecordLit) -> Vec {
-    record_lit
-        .record_field_list()
-        .map(|field_list| field_list.fields())
-        .map(|fields| {
-            fields
-                .into_iter()
-                .filter_map(|field| field.name_ref())
-                .map(|name_ref| name_ref.text().clone())
-                .collect()
-        })
-        .unwrap_or_default()
-}
-
-fn pattern_ascribed_fields(record_pat: &ast::RecordPat) -> Vec {
-    record_pat
-        .record_field_pat_list()
-        .map(|pat_list| {
-            pat_list
-                .record_field_pats()
-                .filter_map(|fild_pat| fild_pat.name())
-                .chain(pat_list.bind_pats().filter_map(|bind_pat| bind_pat.name()))
-                .map(|name| name.text().clone())
-                .collect()
-        })
-        .unwrap_or_default()
+    Some(())
 }
 
 #[cfg(test)]
-- 
cgit v1.2.3