diff --git a/lsp/lsp-harness/src/output.rs b/lsp/lsp-harness/src/output.rs index cc4416e39e..2c030bac02 100644 --- a/lsp/lsp-harness/src/output.rs +++ b/lsp/lsp-harness/src/output.rs @@ -106,7 +106,26 @@ impl LspDebug for lsp_types::GotoDefinitionResponse { impl LspDebug for lsp_types::CompletionItem { fn debug(&self, mut w: impl Write) -> std::io::Result<()> { - write!(w, "{}", self.label) + let detail = self + .detail + .as_ref() + .map(|d| format!(" ({d})")) + .unwrap_or_default(); + let doc = self + .documentation + .as_ref() + .map(|d| { + let s = match d { + lsp_types::Documentation::String(s) => s, + lsp_types::Documentation::MarkupContent(lsp_types::MarkupContent { + value, + .. + }) => value, + }; + format!(" [{}]", s) + }) + .unwrap_or_default(); + write!(w, "{}{}{}", self.label, detail, doc) } } diff --git a/lsp/nls/src/field_walker.rs b/lsp/nls/src/field_walker.rs index 7bdaa8a962..c4fe17c013 100644 --- a/lsp/nls/src/field_walker.rs +++ b/lsp/nls/src/field_walker.rs @@ -1,6 +1,5 @@ use std::{cell::RefCell, collections::HashSet}; -use lsp_types::{CompletionItemKind, Documentation, MarkupContent, MarkupKind}; use nickel_lang_core::{ identifier::Ident, pretty::ident_quoted, @@ -47,9 +46,7 @@ impl Record { .iter() .map(|(id, val)| CompletionItem { label: ident_quoted(id), - detail: metadata_detail(&val.metadata), - kind: Some(CompletionItemKind::PROPERTY), - documentation: metadata_doc(&val.metadata), + metadata: vec![val.metadata.clone()], ident: Some((*id).into()), }) .collect(), @@ -60,8 +57,17 @@ impl Record { RecordRowsIteratorItem::TailVar(_) => None, RecordRowsIteratorItem::Row(r) => Some(CompletionItem { label: ident_quoted(&r.id), - kind: Some(CompletionItemKind::PROPERTY), - detail: Some(r.typ.to_string()), + metadata: vec![FieldMetadata { + annotation: TypeAnnotation { + typ: Some(nickel_lang_core::term::LabeledType { + typ: r.typ.clone(), + label: Default::default(), + }), + contracts: vec![], + }, + ..Default::default() + }], + //detail: vec![r.typ.to_string()], ..Default::default() }), }) @@ -148,24 +154,6 @@ enum FieldContent { Type(Type), } -fn metadata_doc(m: &FieldMetadata) -> Option { - let doc = m.doc.as_ref()?; - Some(Documentation::MarkupContent(MarkupContent { - kind: MarkupKind::Markdown, - value: doc.clone(), - })) -} - -// If the field is annotated, returns its type annotation (preferred) or its -// contract annotation (fallback). -fn metadata_detail(m: &FieldMetadata) -> Option { - m.annotation - .typ - .as_ref() - .map(|ty| ty.typ.to_string()) - .or_else(|| m.annotation.contracts_to_string()) -} - /// The definition site of an identifier. #[derive(Clone, Debug, PartialEq)] pub enum Def { @@ -235,9 +223,7 @@ impl Def { pub fn completion_item(&self) -> CompletionItem { CompletionItem { label: ident_quoted(&self.ident().into()), - detail: self.metadata().and_then(metadata_detail), - kind: Some(CompletionItemKind::PROPERTY), - documentation: self.metadata().and_then(metadata_doc), + metadata: self.metadata().into_iter().cloned().collect(), ..Default::default() } } diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index ef3b1c15d9..a7deb28355 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -3,11 +3,12 @@ use lsp_server::{RequestId, Response, ResponseError}; use lsp_types::{CompletionItemKind, CompletionParams}; use nickel_lang_core::{ cache::{self, InputFormat}, + combine::Combine, identifier::Ident, position::RawPos, - term::{RichTerm, Term, UnaryOp}, + term::{record::FieldMetadata, RichTerm, Term, UnaryOp}, }; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::ffi::OsString; use std::io; use std::iter::Extend; @@ -23,24 +24,36 @@ use crate::{ world::World, }; -fn remove_duplicates_and_myself( - items: &[CompletionItem], +/// Filter out completion items that contain the cursor position. +/// +/// In situations like +/// ```nickel +/// { foo, ba } +/// # ^cursor +/// ``` +/// we don't want to offer "ba" as a completion. +fn remove_myself( + items: impl Iterator, cursor: RawPos, +) -> impl Iterator { + items.filter(move |it| it.ident.map_or(true, |ident| !ident.pos.contains(cursor))) +} + +/// Combine duplicate items: take all items that share the same completion text, and +/// combine their documentation strings by removing duplicate documentation and concatenating +/// what's left. +fn combine_duplicates( + items: impl Iterator, ) -> Vec { - let mut seen_labels = HashSet::new(); - let mut ret = Vec::new(); + let mut grouped = HashMap::::new(); for item in items { - if let Some(ident) = item.ident { - if ident.pos.contains(cursor) { - continue; - } - } - - if seen_labels.insert(&item.label) { - ret.push(item.clone().into()); - } + grouped + .entry(item.label.clone()) + .and_modify(|old| *old = Combine::combine(old.clone(), item.clone())) + .or_insert(item); } - ret + + grouped.into_values().map(From::from).collect() } fn extract_static_path(mut rt: RichTerm) -> (RichTerm, Vec) { @@ -85,22 +98,60 @@ fn sanitize_record_path_for_completion( } } -#[derive(Default, Debug, PartialEq, Eq, Clone)] +#[derive(Default, Debug, PartialEq, Clone)] pub struct CompletionItem { pub label: String, - pub detail: Option, - pub kind: Option, - pub documentation: Option, + pub metadata: Vec, pub ident: Option, } +impl Combine for CompletionItem { + fn combine(mut left: Self, mut right: Self) -> Self { + left.metadata.append(&mut right.metadata); + left.ident = left.ident.or(right.ident); + left + } +} + impl From for lsp_types::CompletionItem { fn from(my: CompletionItem) -> Self { + // The details are the type and contract annotations. + let mut detail: Vec<_> = my + .metadata + .iter() + .flat_map(|m| { + m.annotation + .typ + .iter() + .map(|ty| ty.typ.to_string()) + .chain(m.annotation.contracts_to_string()) + }) + .collect(); + detail.sort(); + detail.dedup(); + let detail = detail.join("\n"); + + let mut doc: Vec<_> = my + .metadata + .iter() + .filter_map(|m| m.doc.as_deref()) + .collect(); + doc.sort(); + doc.dedup(); + // Docs are likely to be longer than types/contracts, so put + // a blank line between them. + let doc = doc.join("\n\n"); + Self { label: my.label, - detail: my.detail, - kind: my.kind, - documentation: my.documentation, + detail: (!detail.is_empty()).then_some(detail), + kind: Some(CompletionItemKind::PROPERTY), + documentation: (!doc.is_empty()).then_some(lsp_types::Documentation::MarkupContent( + lsp_types::MarkupContent { + kind: lsp_types::MarkupKind::Markdown, + value: doc, + }, + )), ..Default::default() } } @@ -115,27 +166,21 @@ fn record_path_completion(term: RichTerm, world: &World) -> Vec defs.iter().flat_map(Record::completion_items).collect() } -fn env_completion(rt: &RichTerm, world: &World) -> Vec { - let env = world.analysis.get_env(rt).cloned().unwrap_or_default(); +// Try to complete a field name in a record, like in +// ``` +// { bar = 1, foo } +// ^cursor +// ``` +// In this situation we don't care about the environment, but we do care about +// contracts and merged records. +fn field_completion(rt: &RichTerm, world: &World) -> Vec { let resolver = FieldResolver::new(world); - let mut items: Vec<_> = env - .iter_elems() - .map(|(_, def_with_path)| def_with_path.completion_item()) + let mut items: Vec<_> = resolver + .resolve_record(rt) + .iter() + .flat_map(Record::completion_items) .collect(); - // If the current term is a record, add its fields. (They won't be in the environment, - // because that's the environment *of* the current term. And we don't want to treat - // all possible Containers here, because for example if the current term is a Term::Var - // that references a record, we don't want it.) - if matches!(rt.as_ref(), Term::RecRecord(..)) { - items.extend( - resolver - .resolve_record(rt) - .iter() - .flat_map(Record::completion_items), - ); - } - // Look for identifiers that are "in scope" because they're in a cousin that gets merged // into us. For example, when completing // @@ -148,6 +193,13 @@ fn env_completion(rt: &RichTerm, world: &World) -> Vec { items } +fn env_completion(rt: &RichTerm, world: &World) -> Vec { + let env = world.analysis.get_env(rt).cloned().unwrap_or_default(); + env.iter_elems() + .map(|(_, def_with_path)| def_with_path.completion_item()) + .collect() +} + pub fn handle_completion( params: CompletionParams, id: RequestId, @@ -175,6 +227,7 @@ pub fn handle_completion( .and_then(|context| context.trigger_character.as_deref()); let term = server.world.lookup_term_by_position(pos)?.cloned(); + let ident = server.world.lookup_ident_by_position(pos)?; if let Some(Term::Import(import)) = term.as_ref().map(|t| t.term.as_ref()) { // Don't respond with anything if trigger is a `.`, as that may be the @@ -186,18 +239,23 @@ pub fn handle_completion( return Ok(()); } - let sanitized_term = term + let path_term = term .as_ref() .and_then(|rt| sanitize_record_path_for_completion(rt, cursor, &mut server.world)); - #[allow(unused_mut)] // needs to be mut with feature = old-completer - let mut completions = match (sanitized_term, term) { - (Some(sanitized), _) => record_path_completion(sanitized, &server.world), - (_, Some(term)) => env_completion(&term, &server.world), - (None, None) => Vec::new(), + let completions = if let Some(path_term) = path_term { + record_path_completion(path_term, &server.world) + } else if let Some(term) = term { + if matches!(term.as_ref(), Term::RecRecord(..) | Term::Record(..)) && ident.is_some() { + field_completion(&term, &server.world) + } else { + env_completion(&term, &server.world) + } + } else { + Vec::new() }; - let completions = remove_duplicates_and_myself(&completions, pos); + let completions = combine_duplicates(remove_myself(completions.into_iter(), pos)); server.reply(Response::new_ok(id.clone(), completions)); Ok(()) diff --git a/lsp/nls/tests/inputs/completion-field-disambiguation.ncl b/lsp/nls/tests/inputs/completion-field-disambiguation.ncl new file mode 100644 index 0000000000..455c02487a --- /dev/null +++ b/lsp/nls/tests/inputs/completion-field-disambiguation.ncl @@ -0,0 +1,12 @@ +### /main.ncl +let C = { foo | Number | doc "Some doc" } in +{ + foo | String, + bar = { + f + } | C +} +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///main.ncl" +### position = { line = 4, character = 5 } diff --git a/lsp/nls/tests/inputs/completion-patterns.ncl b/lsp/nls/tests/inputs/completion-patterns.ncl index 1a14594d3d..6a53aa4db2 100644 --- a/lsp/nls/tests/inputs/completion-patterns.ncl +++ b/lsp/nls/tests/inputs/completion-patterns.ncl @@ -48,7 +48,7 @@ in ### textDocument.uri = "file:///input.ncl" ### position = { line = 17, character = 14 } ### -### # Env completions +### # Record field completions ### ### [[request]] ### type = "Completion" @@ -63,4 +63,21 @@ in ### [[request]] ### type = "Completion" ### textDocument.uri = "file:///input.ncl" -### position = { line = 5, character = 6 } +### position = { line = 5, character = 7 } +### +### # Env completions +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 1, character = 10 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 3, character = 11 } +### +### [[request]] +### type = "Completion" +### textDocument.uri = "file:///input.ncl" +### position = { line = 5, character = 13 } diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-array.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-array.ncl.snap index a79a06302e..4294eaee21 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-array.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-array.ncl.snap @@ -2,6 +2,5 @@ source: lsp/nls/tests/main.rs expression: output --- -[a, b, foo, std] -[a, b, bar, c, std] - +[foo] +[bar] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-basic.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-basic.ncl.snap index 197bbb83de..c074adedce 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-basic.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-basic.ncl.snap @@ -14,7 +14,6 @@ expression: output [field] [field] [subfield] -[foo] -[field] -[field] - +[foo (Number)] +[field (Number)] +[field (Number)] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-dict.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-dict.ncl.snap index 4ca1c4b19c..1fbf50a58c 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-dict.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-dict.ncl.snap @@ -2,6 +2,5 @@ source: lsp/nls/tests/main.rs expression: output --- -[foo] -[foo] - +[foo (Number)] +[foo (Number)] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-field-disambiguation.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-field-disambiguation.ncl.snap new file mode 100644 index 0000000000..a9c9c9ca2b --- /dev/null +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-field-disambiguation.ncl.snap @@ -0,0 +1,5 @@ +--- +source: lsp/nls/tests/main.rs +expression: output +--- +[foo (Number) [Some doc]] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-fun-parameter-contract.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-fun-parameter-contract.ncl.snap index 331e706cde..64c5dc275d 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-fun-parameter-contract.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-fun-parameter-contract.ncl.snap @@ -2,4 +2,4 @@ source: lsp/nls/tests/main.rs expression: output --- -[baz, foo] +[baz, foo ({ bar, })] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap index a79a763ad9..4e3c7a24ee 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-match-typed.ncl.snap @@ -2,4 +2,4 @@ source: lsp/nls/tests/main.rs expression: output --- -[fo, foo] +[fo (Number), foo (Number)] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-nested.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-nested.ncl.snap index e4696b609a..c90b35213c 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-nested.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-nested.ncl.snap @@ -2,7 +2,6 @@ source: lsp/nls/tests/main.rs expression: output --- -[bar, baz, foo, one, std] -[bar, baz, foo, std, two] -[bar, baz, foo, quux, std, three] - +[one] +[two] +[three] diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-patterns.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-patterns.ncl.snap index 170385c367..8c28c32efb 100644 --- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-patterns.ncl.snap +++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__completion-patterns.ncl.snap @@ -5,10 +5,12 @@ expression: output [extra, foo] [bar, more] [baz, most] -[bar, more] +[bar (Dyn), more] [baz, most] [baz, most] -[foo, inner, innermost, outer, std] -[bar, extra, foo, inner, innermost, outer, std] -[bar, baz, extra, foo, inner, innermost, more, most, outer, std] - +[foo] +[bar] +[baz] +[inner, innermost, outer, std] +[extra, foo, inner, innermost, outer, std] +[bar, extra, foo, inner, innermost, more, outer, std]