moar cleanup

compiler/qsc_data_structures/src/line_column.rs

@@ -26,7 +26,7 @@ pub enum Encoding {
 
 impl Position {
     /// For a given string and utf-8 byte offset, returns a [`Position`]
-    /// that corresponds to that byte offset.
+    /// that corresponds to that offset.
     ///
     /// The column information is expressed in code units, depending on the passed in `encoding`.
     /// [`Encoding::Utf8`] will use byte offsets, and [`Encoding::Utf16`] will use
@@ -126,7 +126,7 @@ impl Range {
     }
 }
 
-/// For a given string and an array utf-8 byte offsets, returns the [`Position`]s
+/// For a given string and array of utf-8 byte offsets, returns the [`Position`]s
 /// corresponding to the byte offsets.
 ///
 /// Since this function iterates over the string once, the array *MUST* be in sorted order.

compiler/qsc_data_structures/src/line_column/tests.rs

@@ -297,13 +297,13 @@ fn range_across_lines() {
 #[test]
 fn range_out_of_bounds() {
     let contents = "hello";
-    let span = Span { lo: 2, hi: 10 };
+    let span = Span { lo: 6, hi: 10 };
     let range = Range::from_span(Encoding::Utf8, contents, &span);
     expect![[r"
         Range {
             start: Position {
                 line: 0,
-                column: 2,
+                column: 5,
             },
             end: Position {
                 line: 0,

language_service/src/completion.rs

@@ -7,10 +7,10 @@ mod tests;
 use crate::compilation::{Compilation, CompilationKind, Lookup};
 use crate::display::CodeDisplay;
 use crate::protocol::{CompletionItem, CompletionItemKind, CompletionList};
-use crate::qsc_utils::{protocol_span, span_contains};
+use crate::qsc_utils::{into_range, span_contains};
 use qsc::ast::visit::{self, Visitor};
 use qsc::hir::{ItemKind, Package, PackageId, Visibility};
-use qsc::line_column::{Encoding, Position};
+use qsc::line_column::{Encoding, Position, Range};
 use qsc::resolve::{Local, LocalKind};
 use rustc_hash::FxHashSet;
 use std::rc::Rc;
@@ -22,10 +22,10 @@ const PRELUDE: [&str; 3] = [
 ];
 
 pub(crate) fn get_completions(
-    position_encoding: Encoding,
     compilation: &Compilation,
     source_name: &str,
     position: Position,
+    position_encoding: Encoding,
 ) -> CompletionList {
     let offset =
         compilation.source_position_to_package_offset(source_name, position, position_encoding);
@@ -55,6 +55,14 @@ pub(crate) fn get_completions(
         CompilationKind::Notebook => Some(get_first_non_whitespace_in_source(compilation, offset)),
     };
 
+    let insert_open_range = insert_open_at.map(|o| {
+        into_range(
+            position_encoding,
+            qsc::Span { lo: o, hi: o },
+            &compilation.user_unit().sources,
+        )
+    });
+
     let indent = match insert_open_at {
         Some(start) => get_indent(compilation, start),
         None => String::new(),
@@ -65,7 +73,7 @@ pub(crate) fn get_completions(
         .opens
         .extend(PRELUDE.into_iter().map(|ns| (Rc::from(ns), None)));
 
-    let mut builder = CompletionListBuilder::new(position_encoding);
+    let mut builder = CompletionListBuilder::new();
 
     match context_finder.context {
         Context::Namespace => {
@@ -82,7 +90,7 @@ pub(crate) fn get_completions(
             builder.push_globals(
                 compilation,
                 &context_finder.opens,
-                insert_open_at,
+                insert_open_range,
                 &context_finder.current_namespace_name,
                 &indent,
             );
@@ -101,7 +109,7 @@ pub(crate) fn get_completions(
             builder.push_globals(
                 compilation,
                 &context_finder.opens,
-                insert_open_at,
+                insert_open_range,
                 &context_finder.current_namespace_name,
                 &indent,
             );
@@ -127,7 +135,7 @@ pub(crate) fn get_completions(
                 builder.push_globals(
                     compilation,
                     &context_finder.opens,
-                    insert_open_at,
+                    insert_open_range,
                     &context_finder.current_namespace_name,
                     &indent,
                 );
@@ -185,15 +193,13 @@ fn get_indent(compilation: &Compilation, package_offset: u32) -> String {
 }
 
 struct CompletionListBuilder {
-    position_encoding: Encoding,
     current_sort_group: u32,
     items: FxHashSet<CompletionItem>,
 }
 
 impl CompletionListBuilder {
-    fn new(position_encoding: Encoding) -> Self {
+    fn new() -> Self {
         CompletionListBuilder {
-            position_encoding,
             current_sort_group: 1,
             items: FxHashSet::default(),
         }
@@ -257,7 +263,7 @@ impl CompletionListBuilder {
         &mut self,
         compilation: &Compilation,
         opens: &[(Rc<str>, Option<Rc<str>>)],
-        insert_open_at: Option<u32>,
+        insert_open_range: Option<Range>,
         current_namespace_name: &Option<Rc<str>>,
         indent: &String,
     ) {
@@ -278,11 +284,10 @@ impl CompletionListBuilder {
 
         for (package_id, _) in &all_except_core {
             self.push_sorted_completions(Self::get_callables(
-                self.position_encoding,
                 compilation,
                 *package_id,
                 opens,
-                insert_open_at,
+                insert_open_range,
                 current_namespace_name.clone(),
                 indent,
             ));
@@ -376,13 +381,11 @@ impl CompletionListBuilder {
         self.current_sort_group += 1;
     }
 
-    #[allow(clippy::too_many_lines)]
     fn get_callables<'a>(
-        position_encoding: Encoding,
         compilation: &'a Compilation,
         package_id: PackageId,
         opens: &'a [(Rc<str>, Option<Rc<str>>)],
-        insert_open_at: Option<u32>,
+        insert_open_at: Option<Range>,
         current_namespace_name: Option<Rc<str>>,
         indent: &'a String,
     ) -> impl Iterator<Item = (CompletionItem, u32)> + 'a {
@@ -447,14 +450,7 @@ impl CompletionListBuilder {
                                             None => match insert_open_at {
                                                 Some(start) => {
                                                     additional_edits.push((
-                                                        protocol_span(
-                                                            position_encoding,
-                                                            qsc::Span {
-                                                                lo: start,
-                                                                hi: start,
-                                                            },
-                                                            &compilation.user_unit().sources,
-                                                        ),
+                                                        start,
                                                         format!(
                                                             "open {};{}",
                                                             namespace.name.clone(),

language_service/src/completion/tests.rs

@@ -17,9 +17,10 @@ use crate::{
 use indoc::indoc;
 
 fn check(source_with_cursor: &str, completions_to_check: &[&str], expect: &Expect) {
-    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_cursor);
+    let (compilation, cursor_position, _) =
+        compile_with_fake_stdlib_and_markers(source_with_cursor);
     let actual_completions =
-        get_completions(Encoding::Utf8, &compilation, "<source>", cursor_offset);
+        get_completions(&compilation, "<source>", cursor_position, Encoding::Utf8);
     let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
         .iter()
         .map(|comp| {
@@ -38,10 +39,10 @@ fn check_project(
     completions_to_check: &[&str],
     expect: &Expect,
 ) {
-    let (compilation, cursor_uri, cursor_offset, _) =
+    let (compilation, cursor_uri, cursor_position, _) =
         compile_project_with_fake_stdlib_and_markers(sources_with_markers);
     let actual_completions =
-        get_completions(Encoding::Utf8, &compilation, &cursor_uri, cursor_offset);
+        get_completions(&compilation, &cursor_uri, cursor_position, Encoding::Utf8);
     let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
         .iter()
         .map(|comp| {
@@ -61,9 +62,10 @@ fn check_notebook(
     completions_to_check: &[&str],
     expect: &Expect,
 ) {
-    let (compilation, cell_uri, offset, _) =
+    let (compilation, cell_uri, cursor_position, _) =
         compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
-    let actual_completions = get_completions(Encoding::Utf8, &compilation, &cell_uri, offset);
+    let actual_completions =
+        get_completions(&compilation, &cell_uri, cursor_position, Encoding::Utf8);
     let checked_completions: Vec<Option<&CompletionItem>> = completions_to_check
         .iter()
         .map(|comp| {

language_service/src/definition.rs

@@ -7,16 +7,16 @@ mod tests;
 use crate::compilation::Compilation;
 use crate::name_locator::{Handler, Locator, LocatorContext};
 use crate::protocol::Location;
-use crate::qsc_utils::protocol_location;
+use crate::qsc_utils::into_location;
 use qsc::ast::visit::Visitor;
 use qsc::line_column::{Encoding, Position};
 use qsc::{ast, hir};
 
 pub(crate) fn get_definition(
-    position_encoding: Encoding,
     compilation: &Compilation,
     source_name: &str,
     position: Position,
+    position_encoding: Encoding,
 ) -> Option<Location> {
     let offset =
         compilation.source_position_to_package_offset(source_name, position, position_encoding);
@@ -47,7 +47,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         name: &'a ast::Ident,
         _: &'a ast::CallableDecl,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             name.span,
@@ -63,7 +63,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         _: &'a hir::Package,
         decl: &'a hir::CallableDecl,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             decl.name.span,
@@ -77,7 +77,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         def_name: &'a ast::Ident,
         _: hir::ty::ParamId,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             def_name.span,
@@ -92,7 +92,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         _: hir::ty::ParamId,
         definition: &'a ast::Ident,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             definition.span,
@@ -101,7 +101,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
     }
 
     fn at_new_type_def(&mut self, type_name: &'a ast::Ident, _: &'a ast::TyDef) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             type_name.span,
@@ -117,7 +117,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         type_name: &'a hir::Ident,
         _: &'a hir::ty::Udt,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             type_name.span,
@@ -126,7 +126,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
     }
 
     fn at_field_def(&mut self, _: &LocatorContext<'a>, field_name: &'a ast::Ident, _: &'a ast::Ty) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             field_name.span,
@@ -144,7 +144,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         let span = field_def
             .name_span
             .expect("field found via name should have a name");
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             span,
@@ -153,7 +153,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
     }
 
     fn at_local_def(&mut self, _: &LocatorContext<'a>, ident: &'a ast::Ident, _: &'a ast::Pat) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             ident.span,
@@ -168,7 +168,7 @@ impl<'a> Handler<'a> for DefinitionFinder<'a> {
         _: &'a ast::NodeId,
         definition: &'a ast::Ident,
     ) {
-        self.definition = Some(protocol_location(
+        self.definition = Some(into_location(
             self.position_encoding,
             self.compilation,
             definition.span,

language_service/src/definition/tests.rs

@@ -18,9 +18,10 @@ use crate::{
 /// 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 (compilation, cursor_offset, target_spans) =
+    let (compilation, cursor_position, target_spans) =
         compile_with_fake_stdlib_and_markers(source_with_markers);
-    let actual_definition = get_definition(Encoding::Utf8, &compilation, "<source>", cursor_offset);
+    let actual_definition =
+        get_definition(&compilation, "<source>", cursor_position, Encoding::Utf8);
     let expected_definition = if target_spans.is_empty() {
         None
     } else {
@@ -33,9 +34,9 @@ fn assert_definition(source_with_markers: &str) {
 }
 
 fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) {
-    let (compilation, cell_uri, offset, target_spans) =
+    let (compilation, cell_uri, position, target_spans) =
         compile_notebook_with_fake_stdlib_and_markers(cells_with_markers);
-    let actual_definition = get_definition(Encoding::Utf8, &compilation, &cell_uri, offset);
+    let actual_definition = get_definition(&compilation, &cell_uri, position, Encoding::Utf8);
     let expected_definition = if target_spans.is_empty() {
         None
     } else {
@@ -48,8 +49,10 @@ fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) {
 }
 
 fn check(source_with_markers: &str, expect: &Expect) {
-    let (compilation, cursor_offset, _) = compile_with_fake_stdlib_and_markers(source_with_markers);
-    let actual_definition = get_definition(Encoding::Utf8, &compilation, "<source>", cursor_offset);
+    let (compilation, cursor_position, _) =
+        compile_with_fake_stdlib_and_markers(source_with_markers);
+    let actual_definition =
+        get_definition(&compilation, "<source>", cursor_position, Encoding::Utf8);
     expect.assert_debug_eq(&actual_definition);
 }