From 0e90872f0c93324bd4c1cc3ea750c6d57c643c17 Mon Sep 17 00:00:00 2001 From: Julien Debon Date: Fri, 16 Dec 2022 13:47:49 +0100 Subject: [PATCH] Display meta information when providing completion - Rebase by Julien (#966) Co-authored-by: Oghenevwogaga Ebresafe --- lsp/nls/src/linearization/completed.rs | 25 ++- lsp/nls/src/requests/completion.rs | 206 ++++++++++++++++++++----- src/term/mod.rs | 14 ++ src/types.rs | 9 ++ 4 files changed, 197 insertions(+), 57 deletions(-) diff --git a/lsp/nls/src/linearization/completed.rs b/lsp/nls/src/linearization/completed.rs index 2582ee1048..f65df0d4e8 100644 --- a/lsp/nls/src/linearization/completed.rs +++ b/lsp/nls/src/linearization/completed.rs @@ -127,13 +127,14 @@ impl Completed { _ => item, }; - if let Some(MetaValue { - ref doc, - ref types, - ref contracts, - priority, - .. - }) = item.meta.as_ref() + if let Some( + meta @ MetaValue { + ref doc, + ref types, + priority, + .. + }, + ) = item.meta.as_ref() { if let Some(doc) = doc { extra.push(doc.to_owned()); @@ -141,14 +142,8 @@ impl Completed { if let Some(types) = types { extra.push(types.label.tag.to_string()); } - if !contracts.is_empty() { - extra.push( - contracts - .iter() - .map(|contract| format!("{}", contract.label.types,)) - .collect::>() - .join(","), - ); + if let Some(contracts) = meta.contracts_to_string() { + extra.push(contracts); } extra.push(format!("Merge Priority: {:?}", priority)); diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index 768e0aa321..31bc438331 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -3,7 +3,9 @@ use codespan_lsp::position_to_byte_index; use lazy_static::lazy_static; use log::debug; use lsp_server::{RequestId, Response, ResponseError}; -use lsp_types::{CompletionItem, CompletionParams}; +use lsp_types::{ + CompletionItem, CompletionItemKind, CompletionParams, Documentation, MarkupContent, MarkupKind, +}; use nickel_lang::{ identifier::Ident, term::{MetaValue, RichTerm, Term}, @@ -29,18 +31,112 @@ use crate::{ // We follow the path by traversing a term, type or contract which represents a record // and stop when there is nothing else on the path +#[derive(Debug)] +struct IdentWithType { + ident: Ident, + ty: Types, + item: Option>, +} + +impl From for IdentWithType { + fn from(ident: Ident) -> Self { + IdentWithType { + ident, + ty: Types(TypeF::Dyn), + item: None, + } + } +} + +impl From<&str> for IdentWithType { + fn from(ident: &str) -> Self { + IdentWithType { + ident: Ident::from(ident), + ty: Types(TypeF::Dyn), + item: None, + } + } +} + +impl IdentWithType { + fn detail(&self) -> String { + self.item + .as_ref() + .and_then(|item| { + item.meta.as_ref().and_then(|meta| { + meta.types + .as_ref() + .map(|ty| ty.types.to_string()) + .or_else(|| meta.contracts_to_string()) + }) + }) + .unwrap_or_else(|| self.ty.to_string()) + } + + fn completion_item_kind(&self) -> CompletionItemKind { + match &self.ty { + ty if ty.is_function_type() => CompletionItemKind::Function, + _ => CompletionItemKind::Property, + } + } + + fn to_lsp_completion_item(&self) -> CompletionItem { + /// Attach quotes to a non-ASCII string + fn adjust_name(name: &str) -> String { + if name.is_ascii() { + String::from(name) + } else { + format!("\"{}\"", name) + } + } + let doc = || { + let item = self.item.as_ref()?; + let meta = item.meta.as_ref()?; + let doc = meta.doc.as_ref()?; + let doc = Documentation::MarkupContent(MarkupContent { + kind: MarkupKind::Markdown, + value: format!("{}\nMerge Priority {}", doc.clone(), meta.priority), + }); + Some(doc) + }; + CompletionItem { + label: adjust_name(self.ident.label()), + detail: Some(self.detail()), + kind: Some(self.completion_item_kind()), + documentation: doc(), + ..Default::default() + } + } +} + /// Find the record field associated with a particular ID in the linearization /// using lexical scoping rules. fn find_fields_from_term_kind( linearization: &Completed, id: ItemId, path: &mut Vec, -) -> Option> { +) -> Option> { let item = linearization.get_item(id)?; match item.kind { TermKind::Record(ref fields) => { if path.is_empty() { - Some(fields.keys().cloned().collect()) + Some( + fields + .iter() + .map(|(&ident, &id)| { + // This unwrap is safe because, `id` is the field of the record + // we're currently analyzing. We're sure that the linearization + // phase doesn't produce wrong or invalid ids. + let item = linearization.get_item(id).unwrap(); + let (ty, _) = linearization.resolve_item_type_meta(item); + IdentWithType { + ident, + ty, + item: Some(item.clone()), + } + }) + .collect(), + ) } else { let name = path.pop()?; let new_id = fields.get(&name)?; @@ -65,7 +161,7 @@ fn find_fields_from_contract( linearization: &Completed, id: ItemId, path: &mut Vec, -) -> Option> { +) -> Option> { let item = linearization.get_item(id)?; match &item.meta { Some(meta_value) => Some(find_fields_from_meta_value(meta_value, path)), @@ -81,7 +177,10 @@ fn find_fields_from_contract( /// Find record field associated associated with a MetaValue. /// This can be gotten from the type or the contracts. -fn find_fields_from_meta_value(meta_value: &MetaValue, path: &mut Vec) -> Vec { +fn find_fields_from_meta_value( + meta_value: &MetaValue, + path: &mut Vec, +) -> Vec { meta_value .contracts .iter() @@ -95,7 +194,7 @@ fn find_fields_from_meta_value(meta_value: &MetaValue, path: &mut Vec) -> } /// Extract the fields from a given record type. -fn find_fields_from_type(rrows: &RecordRows, path: &mut Vec) -> Vec { +fn find_fields_from_type(rrows: &RecordRows, path: &mut Vec) -> Vec { if let Some(current) = path.pop() { let type_of_current = rrows.iter().find_map(|item| match item { RecordRowsIteratorItem::Row(row) if row.id == current => Some(row.types.clone()), @@ -115,20 +214,33 @@ fn find_fields_from_type(rrows: &RecordRows, path: &mut Vec) -> Vec Some(row.id), + RecordRowsIteratorItem::Row(row) => Some((row.id, row.types)), _ => None, }) - .collect::>() + .map(|(ident, types)| IdentWithType { + ident, + item: None, + ty: types.clone(), + }) + .collect() } } /// Extract record fields from a record term. -fn find_fields_from_term(term: &RichTerm, path: &mut Vec) -> Option> { +fn find_fields_from_term(term: &RichTerm, path: &mut Vec) -> Option> { let current = path.pop(); match (term.as_ref(), current) { - (Term::Record(data) | Term::RecRecord(data, ..), None) => { - Some(data.fields.keys().cloned().collect()) - } + (Term::Record(data) | Term::RecRecord(data, ..), None) => Some( + data.fields + .keys() + .copied() + .map(|ident| IdentWithType { + ident, + ty: Types(TypeF::Flat(term.clone())), + item: None, + }) + .collect(), + ), (Term::Record(data) | Term::RecRecord(data, ..), Some(name)) => { let term = data.fields.get(&name)?; find_fields_from_term(term, path) @@ -203,7 +315,11 @@ fn remove_duplicates(items: &Vec) -> Vec { /// Search the linearization to find the record information associated with a /// partiular ID, and in the scope of a given linearization item. -fn collect_record_info(linearization: &Completed, id: ItemId, path: &mut Vec) -> Vec { +fn collect_record_info( + linearization: &Completed, + id: ItemId, + path: &mut Vec, +) -> Vec { linearization .get_item(id) .map(|item| { @@ -243,21 +359,12 @@ fn get_completion_identifiers( name: Ident, server: &Server, path: &mut Vec, - ) -> Option> { + ) -> Option> { let item_id = item.env.get(&name)?; let lin = server.lin_cache_get(&item_id.file_id).unwrap(); Some(collect_record_info(lin, *item_id, path)) } - /// Attach quotes to a non-ASCII string - fn adjust_name(name: String) -> String { - if name.is_ascii() { - name - } else { - format!("\"{}\"", name) - } - } - let in_scope = match trigger { // Record Completion Some(server::DOT_COMPL_TRIGGER) => { @@ -283,11 +390,16 @@ fn get_completion_identifiers( complete(item, name, server, &mut path).unwrap_or_default() } else { // variable name completion + let (ty, _) = linearization.resolve_item_type_meta(item); linearization .get_in_scope(item) .iter() .filter_map(|i| match i.kind { - TermKind::Declaration(ref ident, _, _) => Some(*ident), + TermKind::Declaration(ident, _, _) => Some(IdentWithType { + ident, + item: Some(item.clone()), + ty: ty.clone(), + }), _ => None, }) .collect::>() @@ -297,10 +409,7 @@ fn get_completion_identifiers( let in_scope: Vec<_> = in_scope .iter() - .map(|ident| CompletionItem { - label: adjust_name(ident.into_label()), - ..Default::default() - }) + .map(|ident_meta| ident_meta.to_lsp_completion_item()) .collect(); Ok(remove_duplicates(&in_scope)) } @@ -435,12 +544,16 @@ mod tests { let mut path = vec![Ident::from("b"), Ident::from("a")]; // unwrap: the conversion must succeed because we built a type without unification variable // nor type constants - let result = find_fields_from_type(&Box::new(a_record_type.try_into().unwrap()), &mut path); - let expected = vec![Ident::from("c1"), Ident::from("c2")]; - assert_eq!( - result.iter().map(Ident::label).collect::>(), - expected.iter().map(Ident::label).collect::>() - ) + let result: Vec<_> = + find_fields_from_type(&Box::new(a_record_type.try_into().unwrap()), &mut path) + .iter() + .map(|iwm| iwm.ident) + .collect(); + let expected: Vec<_> = vec![IdentWithType::from("c1"), IdentWithType::from("c2")] + .iter() + .map(|iwm| iwm.ident) + .collect(); + assert_eq!(result, expected) } #[test] @@ -515,13 +628,18 @@ mod tests { fn single_case( linearization: Vec>, ids: [ItemId; N], - mut expected: Vec, + expected: Vec, ) { + let mut expected: Vec<_> = expected.iter().map(|iwm| iwm.ident).collect(); expected.sort(); let completed = make_completed(linearization); for id in ids { - let mut actual = find_fields_from_term_kind(&completed, id, &mut Vec::new()) - .expect("Expected Some"); + let mut actual: Vec<_> = + find_fields_from_term_kind(&completed, id, &mut Vec::new()) + .unwrap() + .iter() + .map(|iwm| iwm.ident) + .collect(); actual.sort(); assert_eq!(actual, expected) } @@ -560,7 +678,11 @@ mod tests { TermKind::Usage(UsageState::Resolved(ItemId { file_id, index: 0 })), ); let linearization = vec![a, b, c, d, e]; - let expected = vec![Ident::from("foo"), Ident::from("bar"), Ident::from("baz")]; + let expected = vec![ + IdentWithType::from("foo"), + IdentWithType::from("bar"), + IdentWithType::from("baz"), + ]; single_case( linearization, [ItemId { file_id, index: 0 }, ItemId { file_id, index: 3 }], @@ -622,10 +744,10 @@ mod tests { TermKind::Usage(UsageState::Resolved(ItemId { file_id, index: 4 })), ); let expected = vec![ - Ident::from("one"), - Ident::from("two"), - Ident::from("three"), - Ident::from("four"), + IdentWithType::from("one"), + IdentWithType::from("two"), + IdentWithType::from("three"), + IdentWithType::from("four"), ]; let linearization = vec![a, b, c, d, e, f, g, h, i]; single_case( diff --git a/src/term/mod.rs b/src/term/mod.rs index b51d858fa1..92a8fb7f13 100644 --- a/src/term/mod.rs +++ b/src/term/mod.rs @@ -390,6 +390,20 @@ impl MetaValue { Default::default() } + pub fn contracts_to_string(&self) -> Option { + if !self.contracts.is_empty() { + Some( + self.contracts + .iter() + .map(|contract| format!("{}", contract.label.types,)) + .collect::>() + .join(","), + ) + } else { + None + } + } + /// Flatten two nested metavalues into one, combining their metadata. If data that can't be /// combined (typically, the documentation or the type annotation) are set by both metavalues, /// outer's one are kept. diff --git a/src/types.rs b/src/types.rs index 2260361e80..de4f6dce0d 100644 --- a/src/types.rs +++ b/src/types.rs @@ -820,6 +820,15 @@ impl Types { self.subcontract(HashMap::new(), true, &mut sy) } + /// Returns true if this type is a function type, false otherwise. + pub fn is_function_type(&self) -> bool { + match self { + Types(TypeF::Forall { body, .. }) => body.is_function_type(), + Types(TypeF::Arrow(..)) => true, + Types(_) => false, + } + } + /// Return the contract corresponding to a subtype. /// /// # Arguments