Fix span offsets in additional_text_edits in completions (#884)

minestarks · web-flow · commit 4cdde6addb5f · 2023-12-07T18:59:18.000Z
This bug was exposed while testing multi-file projects.

Plus, some test utilities for testing language service features with
multiple sources.
diff --git a/language_service/src/completion.rs b/language_service/src/completion.rs
@@ -8,8 +8,8 @@ use std::rc::Rc;
 
 use crate::compilation::{Compilation, CompilationKind};
 use crate::display::CodeDisplay;
-use crate::protocol::{self, CompletionItem, CompletionItemKind, CompletionList};
-use crate::qsc_utils::span_contains;
+use crate::protocol::{CompletionItem, CompletionItemKind, CompletionList};
+use crate::qsc_utils::{protocol_span, span_contains};
 use qsc::ast::visit::{self, Visitor};
 use qsc::hir::{ItemKind, Package, PackageId};
 
@@ -373,7 +373,13 @@ impl CompletionListBuilder {
                                             None => match start_of_namespace {
                                                 Some(start) => {
                                                     additional_edits.push((
-                                                        protocol::Span { start, end: start },
+                                                        protocol_span(
+                                                            qsc::Span {
+                                                                lo: start,
+                                                                hi: start,
+                                                            },
+                                                            &compilation.user_unit().sources,
+                                                        ),
                                                         format!(
                                                             "open {};{}",
                                                             namespace.name.clone(),
diff --git a/language_service/src/completion/tests.rs b/language_service/src/completion/tests.rs
@@ -7,15 +7,35 @@ use expect_test::{expect, Expect};
 
 use super::{get_completions, CompletionItem};
 use crate::test_utils::{
-    compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib,
-    get_source_and_marker_offsets,
+    compile_notebook_with_fake_stdlib_and_markers, compile_project_with_fake_stdlib_and_markers,
+    compile_with_fake_stdlib_and_markers,
 };
 use indoc::indoc;
 
 fn check(source_with_cursor: &str, completions_to_check: &[&str], expect: &Expect) {
-    let (source, cursor_offset, _) = get_source_and_marker_offsets(source_with_cursor);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual_completions = get_completions(&compilation, "<source>", cursor_offset[0]);
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_cursor);
+    let actual_completions = get_completions(&compilation, "<source>", cursor_offset);
+    let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
+        .iter()
+        .map(|comp| {
+            actual_completions
+                .items
+                .iter()
+                .find(|item| item.label == **comp)
+        })
+        .collect();
+
+    expect.assert_debug_eq(&checked_completions);
+}
+
+fn check_project(
+    sources_with_markers: &[(&str, &str)],
+    completions_to_check: &[&str],
+    expect: &Expect,
+) {
+    let (compilation, cursor_uri, cursor_offset, _) =
+        compile_project_with_fake_stdlib_and_markers(sources_with_markers);
+    let actual_completions = get_completions(&compilation, &cursor_uri, cursor_offset);
     let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
         .iter()
         .map(|comp| {
@@ -241,6 +261,52 @@ fn in_block_from_other_namespace() {
     );
 }
 
+#[test]
+fn auto_open_multiple_files() {
+    check_project(
+        &[
+            (
+                "foo.qs",
+                indoc! {r#"namespace Foo { operation FooOperation() : Unit {} }
+                "#},
+            ),
+            (
+                "bar.qs",
+                indoc! {r#"namespace Bar { operation BarOperation() : Unit { ↘ } }
+                "#},
+            ),
+        ],
+        &["FooOperation"],
+        &expect![[r#"
+            [
+                Some(
+                    CompletionItem {
+                        label: "FooOperation",
+                        kind: Function,
+                        sort_text: Some(
+                            "0500FooOperation",
+                        ),
+                        detail: Some(
+                            "operation FooOperation() : Unit",
+                        ),
+                        additional_text_edits: Some(
+                            [
+                                (
+                                    Span {
+                                        start: 16,
+                                        end: 16,
+                                    },
+                                    "open Foo;\n ",
+                                ),
+                            ],
+                        ),
+                    },
+                ),
+            ]
+        "#]],
+    );
+}
+
 #[ignore = "nested callables are not currently supported for completions"]
 #[test]
 fn in_block_nested_op() {
diff --git a/language_service/src/definition/tests.rs b/language_service/src/definition/tests.rs
@@ -9,20 +9,17 @@ use super::get_definition;
 use crate::{
     protocol::Location,
     test_utils::{
-        compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib,
-        get_source_and_marker_offsets, target_offsets_to_spans,
+        compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
     },
 };
 
 /// Asserts that the definition given at the cursor position matches the expected range.
 /// The cursor position is indicated by a `↘` marker in the source text.
 /// The expected definition range is indicated by `◉` markers in the source text.
 fn assert_definition(source_with_markers: &str) {
-    let (source, cursor_offsets, target_offsets) =
-        get_source_and_marker_offsets(source_with_markers);
-    let target_spans = target_offsets_to_spans(&target_offsets);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual_definition = get_definition(&compilation, "<source>", cursor_offsets[0]);
+    let (compilation, cursor_offset, target_spans) =
+        compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual_definition = get_definition(&compilation, "<source>", cursor_offset);
     let expected_definition = if target_spans.is_empty() {
         None
     } else {
@@ -50,9 +47,8 @@ fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) {
 }
 
 fn check(source_with_markers: &str, expect: &Expect) {
-    let (source, cursor_offsets, _) = get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual_definition = get_definition(&compilation, "<source>", cursor_offsets[0]);
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual_definition = get_definition(&compilation, "<source>", cursor_offset);
     expect.assert_debug_eq(&actual_definition);
 }
 
diff --git a/language_service/src/hover/tests.rs b/language_service/src/hover/tests.rs
@@ -4,12 +4,8 @@
 #![allow(clippy::needless_raw_string_hashes)]
 
 use super::get_hover;
-use crate::{
-    protocol,
-    test_utils::{
-        compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib,
-        get_source_and_marker_offsets,
-    },
+use crate::test_utils::{
+    compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
 };
 use expect_test::{expect, Expect};
 use indoc::indoc;
@@ -18,25 +14,17 @@ use indoc::indoc;
 /// The cursor position is indicated by a `↘` marker in the source text.
 /// The expected hover span is indicated by two `◉` markers in the source text.
 fn check(source_with_markers: &str, expect: &Expect) {
-    let (source, cursor_offsets, target_offsets) =
-        get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_hover(&compilation, "<source>", cursor_offsets[0]).expect("Expected a hover.");
-    assert_eq!(
-        &actual.span,
-        &protocol::Span {
-            start: target_offsets[0],
-            end: target_offsets[1],
-        }
-    );
+    let (compilation, cursor_offset, target_spans) =
+        compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_hover(&compilation, "<source>", cursor_offset).expect("Expected a hover.");
+    assert_eq!(&actual.span, &target_spans[0]);
     expect.assert_eq(&actual.contents);
 }
 
 /// Asserts that there is no hover for the given test case.
 fn check_none(source_with_markers: &str) {
-    let (source, cursor_offsets, _) = get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_hover(&compilation, "<source>", cursor_offsets[0]);
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_hover(&compilation, "<source>", cursor_offset);
     assert!(actual.is_none());
 }
 
diff --git a/language_service/src/references/tests.rs b/language_service/src/references/tests.rs
@@ -7,8 +7,7 @@ use super::get_references;
 use crate::{
     protocol,
     test_utils::{
-        compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib,
-        get_source_and_marker_offsets, target_offsets_to_spans,
+        compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
     },
 };
 use expect_test::{expect, Expect};
@@ -17,29 +16,21 @@ use indoc::indoc;
 /// Asserts that the reference locations given at the cursor position matches the expected reference locations.
 /// The cursor position is indicated by a `↘` marker in the source text.
 fn check_with_std(source_with_markers: &str, expect: &Expect) {
-    let (source, cursor_offsets, _) = get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_references(&compilation, "<source>", cursor_offsets[0], true);
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_references(&compilation, "<source>", cursor_offset, true);
     expect.assert_debug_eq(&actual);
 }
 
 /// Asserts that the reference locations given at the cursor position matches the expected reference locations.
 /// The cursor position is indicated by a `↘` marker in the source text.
 /// The expected reference location ranges are indicated by `◉` markers in the source text.
 fn check(source_with_markers: &str, include_declaration: bool) {
-    let (source, cursor_offsets, target_offsets) =
-        get_source_and_marker_offsets(source_with_markers);
-    let target_spans = target_offsets_to_spans(&target_offsets);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_references(
-        &compilation,
-        "<source>",
-        cursor_offsets[0],
-        include_declaration,
-    )
-    .into_iter()
-    .map(|l| l.span)
-    .collect::<Vec<_>>();
+    let (compilation, cursor_offset, target_spans) =
+        compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_references(&compilation, "<source>", cursor_offset, include_declaration)
+        .into_iter()
+        .map(|l| l.span)
+        .collect::<Vec<_>>();
     for target in &target_spans {
         assert!(
             actual.contains(target),
diff --git a/language_service/src/rename/tests.rs b/language_service/src/rename/tests.rs
@@ -5,20 +5,17 @@
 
 use super::{get_rename, prepare_rename};
 use crate::test_utils::{
-    compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib,
-    get_source_and_marker_offsets, target_offsets_to_spans,
+    compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
 };
 use expect_test::{expect, Expect};
 
 /// Asserts that the rename locations given at the cursor position matches the expected rename locations.
 /// The cursor position is indicated by a `↘` marker in the source text.
 /// The expected rename location ranges are indicated by `◉` markers in the source text.
 fn check(source_with_markers: &str) {
-    let (source, cursor_offsets, target_offsets) =
-        get_source_and_marker_offsets(source_with_markers);
-    let target_spans = target_offsets_to_spans(&target_offsets);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_rename(&compilation, "<source>", cursor_offsets[0])
+    let (compilation, cursor_offset, target_spans) =
+        compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_rename(&compilation, "<source>", cursor_offset)
         .into_iter()
         .map(|l| l.span)
         .collect::<Vec<_>>();
@@ -31,9 +28,8 @@ fn check(source_with_markers: &str) {
 /// Asserts that the prepare rename given at the cursor position returns None.
 /// The cursor position is indicated by a `↘` marker in the source text.
 fn assert_no_rename(source_with_markers: &str) {
-    let (source, cursor_offsets, _) = get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = prepare_rename(&compilation, "<source>", cursor_offsets[0]);
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = prepare_rename(&compilation, "<source>", cursor_offset);
     assert!(actual.is_none());
 }
 
diff --git a/language_service/src/signature_help/tests.rs b/language_service/src/signature_help/tests.rs
@@ -4,16 +4,15 @@
 #![allow(clippy::needless_raw_string_hashes)]
 
 use super::get_signature_help;
-use crate::test_utils::{compile_with_fake_stdlib, get_source_and_marker_offsets};
+use crate::test_utils::compile_with_fake_stdlib_and_markers;
 use expect_test::{expect, Expect};
 use indoc::indoc;
 
 /// Asserts that the signature help given at the cursor position matches the expected signature help.
 /// The cursor position is indicated by a `↘` marker in the source text.
 fn check(source_with_markers: &str, expect: &Expect) {
-    let (source, cursor_offsets, _) = get_source_and_marker_offsets(source_with_markers);
-    let compilation = compile_with_fake_stdlib("<source>", &source);
-    let actual = get_signature_help(&compilation, "<source>", cursor_offsets[0])
+    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual = get_signature_help(&compilation, "<source>", cursor_offset)
         .expect("Expected a signature help.");
     expect.assert_debug_eq(&actual);
 }
diff --git a/language_service/src/test_utils.rs b/language_service/src/test_utils.rs
