Skip to content

Commit

Permalink
generate auto imports for core callables (#1861)
Browse files Browse the repository at this point in the history
Closes #1860

Currently, we have a separate code path for generating completions for
core callables. This code path just hard-codes the callables to
not include any auto-generated imports. This may have been necessary at
some point, but it no longer is and it is preventing us from generating
the correct auto-imports for `QIR.Runtime`. We now have a function that
generates completions from a package and is aware of if imports are
needed or not, so I just went ahead and used that instead of special
casing `Core`.

This PR:
- Uses the standard completion codepath to generate completions for
`Core`
- Adds the ability to unit test completions with the real stdlib, not
just the fake stdlib
- Adds tests to ensure auto-imports are generated for items that are not
already in scope (e.g. `QIR.Runtime.*`) and are _not_ generated for
items already
  in scope (e.g. `Length`).
  • Loading branch information
sezna authored Aug 20, 2024
1 parent df339be commit 1096d87
Show file tree
Hide file tree
Showing 11 changed files with 195 additions and 109 deletions.
10 changes: 10 additions & 0 deletions compiler/qsc_data_structures/src/index_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,16 @@ pub struct IterMut<'a, K, V> {
base: Enumerate<slice::IterMut<'a, Option<V>>>,
}

impl<K: From<usize>, V> DoubleEndedIterator for Iter<'_, K, V> {
fn next_back(&mut self) -> Option<Self::Item> {
loop {
if let (index, Some(value)) = self.base.next_back()? {
break Some((index.into(), value));
}
}
}
}

impl<'a, K: From<usize>, V> Iterator for IterMut<'a, K, V> {
type Item = (K, &'a mut V);

Expand Down
6 changes: 6 additions & 0 deletions compiler/qsc_frontend/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,12 @@ impl<'a> Iterator for Iter<'a> {
}
}

impl DoubleEndedIterator for Iter<'_> {
fn next_back(&mut self) -> Option<Self::Item> {
self.0.next_back()
}
}

pub(super) struct Offsetter(pub(super) u32);

impl MutVisitor for Offsetter {
Expand Down
2 changes: 1 addition & 1 deletion language_service/src/code_lens/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use expect_test::{expect, Expect};

fn check(source_with_markers: &str, expect: &Expect) {
let (compilation, expected_code_lens_ranges) =
compile_with_fake_stdlib_and_markers_no_cursor(source_with_markers);
compile_with_fake_stdlib_and_markers_no_cursor(source_with_markers, true);
let mut actual_code_lenses = get_code_lenses(&compilation, "<source>", Encoding::Utf8);

for expected_range in &expected_code_lens_ranges {
Expand Down
55 changes: 4 additions & 51 deletions language_service/src/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,40 +337,21 @@ impl CompletionListBuilder {
current_namespace_name: &Option<Vec<Rc<str>>>,
indent: &String,
) {
let core = &compilation
.package_store
.get(PackageId::CORE)
.expect("expected to find core package")
.package;

let mut all_except_core = compilation
.package_store
.iter()
.filter(|p| p.0 != PackageId::CORE)
.collect::<Vec<_>>();

// Reverse to collect symbols starting at the current package backwards
all_except_core.reverse();

for (package_id, _) in &all_except_core {
for (package_id, _) in compilation.package_store.iter().rev() {
self.push_sorted_completions(Self::get_callables(
compilation,
*package_id,
package_id,
imports,
insert_open_range,
current_namespace_name.as_deref(),
indent,
));
}

self.push_sorted_completions(Self::get_core_callables(compilation, core));

for (id, unit) in &all_except_core {
let alias = compilation.dependencies.get(id).cloned().flatten();
for (id, unit) in compilation.package_store.iter().rev() {
let alias = compilation.dependencies.get(&id).cloned().flatten();
self.push_completions(Self::get_namespaces(&unit.package, alias));
}

self.push_completions(Self::get_namespaces(core, None));
}

fn push_locals(
Expand Down Expand Up @@ -500,34 +481,6 @@ impl CompletionListBuilder {
})
}

/// Get all callables in the core package
fn get_core_callables<'a>(
compilation: &'a Compilation,
package: &'a Package,
) -> impl Iterator<Item = (CompletionItem, SortPriority)> + 'a {
let display = CodeDisplay { compilation };

package.items.values().filter_map(move |i| match &i.kind {
ItemKind::Callable(callable_decl) => {
let name = callable_decl.name.name.as_ref();
let detail = Some(display.hir_callable_decl(callable_decl).to_string());
// Everything that starts with a __ goes last in the list
let sort_group = SortPriority::from(name.starts_with("__"));
Some((
CompletionItem {
label: name.to_string(),
kind: CompletionItemKind::Function,
sort_text: None, // This will get filled in during `push_sorted_completions`
detail,
additional_text_edits: None,
},
sort_group,
))
}
_ => None,
})
}

fn get_namespaces(
package: &'_ Package,
package_alias: Option<Arc<str>>,
Expand Down
136 changes: 130 additions & 6 deletions language_service/src/completion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,31 @@ use super::{get_completions, CompletionItem};
use crate::{
protocol::CompletionList,
test_utils::{
compile_notebook_with_fake_stdlib_and_markers,
compile_project_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
compile_notebook_with_markers, compile_project_with_markers, compile_with_markers,
},
Encoding,
};
use indoc::indoc;

fn check(source_with_cursor: &str, completions_to_check: &[&str], expect: &Expect) {
let (compilation, cursor_position, _) =
compile_with_fake_stdlib_and_markers(source_with_cursor);
let (compilation, cursor_position, _) = compile_with_markers(source_with_cursor, true);
let actual_completions =
get_completions(&compilation, "<source>", cursor_position, Encoding::Utf8);
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_with_stdlib(source_with_cursor: &str, completions_to_check: &[&str], expect: &Expect) {
let (compilation, cursor_position, _) = compile_with_markers(source_with_cursor, false);
let actual_completions =
get_completions(&compilation, "<source>", cursor_position, Encoding::Utf8);
let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
Expand All @@ -40,7 +55,7 @@ fn check_project(
expect: &Expect,
) {
let (compilation, cursor_uri, cursor_position, _) =
compile_project_with_fake_stdlib_and_markers(sources_with_markers);
compile_project_with_markers(sources_with_markers, true);
let actual_completions =
get_completions(&compilation, &cursor_uri, cursor_position, Encoding::Utf8);
let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
Expand All @@ -63,7 +78,7 @@ fn check_notebook(
expect: &Expect,
) {
let (compilation, cell_uri, cursor_position, _) =
compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
compile_notebook_with_markers(cells_with_markers);
let actual_completions =
get_completions(&compilation, &cell_uri, cursor_position, Encoding::Utf8);
let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
Expand Down Expand Up @@ -1457,6 +1472,115 @@ fn dont_import_if_already_directly_imported() {
);
}

#[test]
fn auto_import_from_qir_runtime() {
check_with_stdlib(
r#"
namespace Test {
operation Main() : Unit {
AllocateQubitA↘
}
}"#,
&["AllocateQubitArray"],
&expect![[r#"
[
Some(
CompletionItem {
label: "AllocateQubitArray",
kind: Function,
sort_text: Some(
"0800AllocateQubitArray",
),
detail: Some(
"operation AllocateQubitArray(size : Int) : Qubit[]",
),
additional_text_edits: Some(
[
TextEdit {
new_text: "import QIR.Runtime.AllocateQubitArray;\n ",
range: Range {
start: Position {
line: 2,
column: 12,
},
end: Position {
line: 2,
column: 12,
},
},
},
],
),
},
),
]
"#]],
);
}

#[test]
fn dont_generate_import_for_core_prelude() {
check_with_stdlib(
r#"
namespace Test {
operation Main() : Unit {
Length↘
}
}"#,
&["Length"],
// additional text edits should be None because Length is in the core prelude
&expect![[r#"
[
Some(
CompletionItem {
label: "Length",
kind: Function,
sort_text: Some(
"0800Length",
),
detail: Some(
"function Length<'T>(a : 'T[]) : Int",
),
additional_text_edits: None,
},
),
]
"#]],
);
}

#[test]
fn dont_generate_import_for_stdlib_prelude() {
check_with_stdlib(
r#"
namespace Test {
operation Main() : Unit {
MResetZ↘
}
}"#,
&["MResetZ"],
// additional text edits should be None because MResetZ is in Std.Measurement, which
// is in the prelude.
&expect![[r#"
[
Some(
CompletionItem {
label: "MResetZ",
kind: Function,
sort_text: Some(
"0700MResetZ",
),
detail: Some(
"operation MResetZ(target : Qubit) : Result",
),
additional_text_edits: None,
},
),
]
"#]],
);
}

#[test]
fn callable_from_same_file() {
check(
Expand Down
11 changes: 4 additions & 7 deletions language_service/src/definition/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use qsc::location::Location;

use super::get_definition;
use crate::{
test_utils::{
compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
},
test_utils::{compile_notebook_with_markers, compile_with_markers},
Encoding,
};

Expand All @@ -19,7 +17,7 @@ use crate::{
/// The expected definition range is indicated by `◉` markers in the source text.
fn assert_definition(source_with_markers: &str) {
let (compilation, cursor_position, target_spans) =
compile_with_fake_stdlib_and_markers(source_with_markers);
compile_with_markers(source_with_markers, true);
let actual_definition =
get_definition(&compilation, "<source>", cursor_position, Encoding::Utf8);
let expected_definition = if target_spans.is_empty() {
Expand All @@ -35,7 +33,7 @@ fn assert_definition(source_with_markers: &str) {

fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) {
let (compilation, cell_uri, position, target_spans) =
compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
compile_notebook_with_markers(cells_with_markers);
let actual_definition = get_definition(&compilation, &cell_uri, position, Encoding::Utf8);
let expected_definition = if target_spans.is_empty() {
None
Expand All @@ -46,8 +44,7 @@ fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) {
}

fn check(source_with_markers: &str, expect: &Expect) {
let (compilation, cursor_position, _) =
compile_with_fake_stdlib_and_markers(source_with_markers);
let (compilation, cursor_position, _) = compile_with_markers(source_with_markers, true);
let actual_definition =
get_definition(&compilation, "<source>", cursor_position, Encoding::Utf8);
expect.assert_debug_eq(&actual_definition);
Expand Down
14 changes: 5 additions & 9 deletions language_service/src/hover/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
#![allow(clippy::needless_raw_string_hashes)]

use super::get_hover;
use crate::test_utils::{
compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers,
};
use crate::test_utils::{compile_notebook_with_markers, compile_with_markers};
use expect_test::{expect, Expect};
use indoc::indoc;
use qsc::line_column::Encoding;
Expand All @@ -16,7 +14,7 @@ use qsc::line_column::Encoding;
/// The expected hover span is indicated by two `◉` markers in the source text.
fn check(source_with_markers: &str, expect: &Expect) {
let (compilation, cursor_position, target_spans) =
compile_with_fake_stdlib_and_markers(source_with_markers);
compile_with_markers(source_with_markers, true);
let actual = get_hover(&compilation, "<source>", cursor_position, Encoding::Utf8)
.expect("Expected a hover.");
assert_eq!(&actual.span, &target_spans[0]);
Expand All @@ -25,15 +23,14 @@ fn check(source_with_markers: &str, expect: &Expect) {

/// Asserts that there is no hover for the given test case.
fn check_none(source_with_markers: &str) {
let (compilation, cursor_position, _) =
compile_with_fake_stdlib_and_markers(source_with_markers);
let (compilation, cursor_position, _) = compile_with_markers(source_with_markers, true);
let actual = get_hover(&compilation, "<source>", cursor_position, Encoding::Utf8);
assert!(actual.is_none());
}

fn check_notebook(cells_with_markers: &[(&str, &str)], expect: &Expect) {
let (compilation, cell_uri, position, target_spans) =
compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
compile_notebook_with_markers(cells_with_markers);

let actual =
get_hover(&compilation, &cell_uri, position, Encoding::Utf8).expect("Expected a hover.");
Expand All @@ -42,8 +39,7 @@ fn check_notebook(cells_with_markers: &[(&str, &str)], expect: &Expect) {
}

fn check_notebook_none(cells_with_markers: &[(&str, &str)]) {
let (compilation, cell_uri, position, _) =
compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
let (compilation, cell_uri, position, _) = compile_notebook_with_markers(cells_with_markers);

let actual = get_hover(&compilation, &cell_uri, position, Encoding::Utf8);
assert!(actual.is_none());
Expand Down
Loading

0 comments on commit 1096d87

Please sign in to comment.