From 3fadcf7bcb1edea677f28ff268ccc5117db0ee40 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Sat, 16 Mar 2024 14:41:08 +0100 Subject: [PATCH] style changes --- crates/compiler/load_internal/src/docs.rs | 60 ++++++---- .../compiler/load_internal/tests/test_load.rs | 16 ++- crates/compiler/parse/src/header.rs | 1 + crates/docs/src/lib.rs | 7 +- crates/lang_srv/src/analysis.rs | 11 +- crates/lang_srv/src/analysis/analysed_doc.rs | 4 +- crates/lang_srv/src/analysis/completion.rs | 105 +++++++++++------- .../src/analysis/completion/formatting.rs | 1 + .../src/analysis/completion/visitor.rs | 60 ++++++---- crates/lang_srv/src/server.rs | 37 +++--- 10 files changed, 181 insertions(+), 121 deletions(-) diff --git a/crates/compiler/load_internal/src/docs.rs b/crates/compiler/load_internal/src/docs.rs index 51a4a50ff6..8b89042c04 100644 --- a/crates/compiler/load_internal/src/docs.rs +++ b/crates/compiler/load_internal/src/docs.rs @@ -17,10 +17,13 @@ pub struct ModuleDocumentation { pub scope: Scope, pub exposed_symbols: VecSet, } + impl ModuleDocumentation { - pub fn get_doc_for_symbol(&self, symb: &Symbol) -> Option { + pub fn get_doc_for_symbol(&self, symbol_to_match: &Symbol) -> Option { self.entries.iter().find_map(|doc| match doc { - DocEntry::DocDef(DocDef { symbol, docs, .. }) if symbol == symb => docs.clone(), + DocEntry::DocDef(DocDef { symbol, docs, .. }) if symbol == symbol_to_match => { + docs.clone() + } _ => None, }) } @@ -183,10 +186,10 @@ fn generate_entry_docs( ) -> Vec { use roc_parse::ast::Pattern; - let mut acc = Vec::with_capacity(defs.tags.len() + 1); + let mut doc_entries = Vec::with_capacity(defs.tags.len() + 1); if let Some(docs) = comments_or_new_lines_to_docs(header_comments) { - acc.push(DocEntry::ModuleDoc(docs)); + doc_entries.push(DocEntry::ModuleDoc(docs)); } let mut before_comments_or_new_lines: Option<&[CommentOrNewline]> = None; @@ -220,7 +223,7 @@ fn generate_entry_docs( type_vars: Vec::new(), docs, }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } } } @@ -240,7 +243,7 @@ fn generate_entry_docs( symbol: Symbol::new(home, ident_id), docs, }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } } } @@ -256,7 +259,7 @@ fn generate_entry_docs( symbol: Symbol::new(home, ident_id), docs, }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } } } @@ -273,6 +276,7 @@ fn generate_entry_docs( // Don't generate docs for `expect-fx`s } }, + Ok(type_index) => match &defs.type_defs[type_index.index()] { TypeDef::Alias { header: TypeHeader { name, vars }, @@ -305,7 +309,7 @@ fn generate_entry_docs( docs, symbol: Symbol::new(home, ident_id), }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } TypeDef::Opaque { @@ -328,7 +332,7 @@ fn generate_entry_docs( docs, symbol: Symbol::new(home, ident_id), }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } TypeDef::Ability { @@ -368,7 +372,7 @@ fn generate_entry_docs( type_vars, docs, }; - acc.push(DocEntry::DocDef(doc_def)); + doc_entries.push(DocEntry::DocDef(doc_def)); } }, } @@ -380,42 +384,49 @@ fn generate_entry_docs( let it = before_comments_or_new_lines.iter().flat_map(|e| e.iter()); for detached_doc in detached_docs_from_comments_and_new_lines(it) { - acc.push(DetachedDoc(detached_doc)); + doc_entries.push(DetachedDoc(detached_doc)); } - acc + doc_entries } /// Does this type contain any types which are not exposed outside the package? /// (If so, we shouldn't try to render a type annotation for it.) fn contains_unexposed_type( - ann: &ast::TypeAnnotation, + type_ann: &ast::TypeAnnotation, exposed_module_ids: &[ModuleId], module_ids: &ModuleIds, ) -> bool { use ast::TypeAnnotation::*; - match ann { + match type_ann { // Apply is the one case that can directly return true. Apply(module_name, _ident, loc_args) => { + let apply_module_id = module_ids.get_id(&(*module_name).into()); + let loc_args_contains_unexposed_type = loc_args.iter().any(|loc_arg| { + contains_unexposed_type(&loc_arg.value, exposed_module_ids, module_ids) + }); + // If the *ident* was unexposed, we would have gotten a naming error // during canonicalization, so all we need to check is the module. - if let Some(module_id) = module_ids.get_id(&(*module_name).into()) { - !exposed_module_ids.contains(&module_id) - || loc_args.iter().any(|loc_arg| { - contains_unexposed_type(&loc_arg.value, exposed_module_ids, module_ids) - }) + if let Some(module_id) = apply_module_id { + !exposed_module_ids.contains(&module_id) || loc_args_contains_unexposed_type } else { true } } + Malformed(_) | Inferred | Wildcard | BoundVariable(_) => false, + Function(loc_args, loc_ret) => { + let loc_args_contains_unexposed_type = loc_args.iter().any(|loc_arg| { + contains_unexposed_type(&loc_arg.value, exposed_module_ids, module_ids) + }); + contains_unexposed_type(&loc_ret.value, exposed_module_ids, module_ids) - || loc_args.iter().any(|loc_arg| { - contains_unexposed_type(&loc_arg.value, exposed_module_ids, module_ids) - }) + || loc_args_contains_unexposed_type } + Record { fields, ext } => { if let Some(loc_ext) = ext { if contains_unexposed_type(&loc_ext.value, exposed_module_ids, module_ids) { @@ -445,6 +456,7 @@ fn contains_unexposed_type( false } + Tuple { elems: fields, ext } => { if let Some(loc_ext) = ext { if contains_unexposed_type(&loc_ext.value, exposed_module_ids, module_ids) { @@ -456,6 +468,7 @@ fn contains_unexposed_type( contains_unexposed_type(&loc_field.value, exposed_module_ids, module_ids) }) } + TagUnion { ext, tags } => { use ast::Tag; @@ -491,14 +504,17 @@ fn contains_unexposed_type( false } + Where(loc_ann, _loc_has_clauses) => { // We assume all the abilities in the `implements` clause are from exported modules. // TODO don't assume this! Instead, look them up and verify. contains_unexposed_type(&loc_ann.value, exposed_module_ids, module_ids) } + As(loc_ann, _spaces, _type_header) => { contains_unexposed_type(&loc_ann.value, exposed_module_ids, module_ids) } + SpaceBefore(ann, _) | ast::TypeAnnotation::SpaceAfter(ann, _) => { contains_unexposed_type(ann, exposed_module_ids, module_ids) } diff --git a/crates/compiler/load_internal/tests/test_load.rs b/crates/compiler/load_internal/tests/test_load.rs index c0c1b3b45f..bc1667f18b 100644 --- a/crates/compiler/load_internal/tests/test_load.rs +++ b/crates/compiler/load_internal/tests/test_load.rs @@ -423,22 +423,25 @@ fn load_unit() { }, ); } + #[test] fn load_docs() { let subs_by_module = Default::default(); let loaded_module = load_fixture("no_deps", "Docs", subs_by_module); - let docs = loaded_module + let module_docs = loaded_module .docs_by_module .get(&loaded_module.module_id) .expect("module should have docs"); - let docs = docs + + let all_docs = module_docs .entries .iter() .map(|a| match a { roc_load_internal::docs::DocEntry::DocDef(DocDef { name, docs, .. }) => { (Some(name.clone()), docs.clone().map(|a| a.to_string())) } + roc_load_internal::docs::DocEntry::ModuleDoc(docs) | roc_load_internal::docs::DocEntry::DetachedDoc(docs) => (None, Some(docs.clone())), }) @@ -452,12 +455,17 @@ fn load_docs() { (Some("getNameExposed"), None), ] .into_iter() - .map(|(a, b)| (a.map(|a| a.to_string()), b.map(|b| b.to_string()))) + .map(|(ident_str_opt, doc_str_opt)| { + ( + ident_str_opt.map(|a| a.to_string()), + doc_str_opt.map(|b| b.to_string()), + ) + }) .collect::>(); // let has_all_docs = expected.map(|a| docs.contains(&a)).all(|a| a); // assert!(has_all_docs, "Some of the expected docs were not created") - assert_eq!(expected, docs); + assert_eq!(expected, all_docs); } #[test] diff --git a/crates/compiler/parse/src/header.rs b/crates/compiler/parse/src/header.rs index 04020572d4..3929100556 100644 --- a/crates/compiler/parse/src/header.rs +++ b/crates/compiler/parse/src/header.rs @@ -78,6 +78,7 @@ pub enum HeaderType<'a> { exposes: &'a [Loc>], }, } + impl<'a> HeaderType<'a> { pub fn get_name(self) -> Option<&'a str> { match self { diff --git a/crates/docs/src/lib.rs b/crates/docs/src/lib.rs index 88d4d5ea2e..54c01c633e 100644 --- a/crates/docs/src/lib.rs +++ b/crates/docs/src/lib.rs @@ -175,7 +175,7 @@ pub fn generate_docs_html(root_file: PathBuf, build_dir: &Path) { println!("🎉 Docs generated in {}", build_dir.display()); } -///Gives only the module docs for modules that are exposed by the platform or package +/// Gives only the module docs for modules that are exposed by the platform or package. fn get_exposed_module_docs( loaded_module: &mut LoadedModule, ) -> Vec<(ModuleId, ModuleDocumentation)> { @@ -183,7 +183,8 @@ fn get_exposed_module_docs( // let mut docs_by_module = Vec::with_capacity(state.exposed_modules.len()); for module_id in loaded_module.exposed_modules.iter() { - let docs = loaded_module.docs_by_module.remove(module_id).unwrap_or_else(|| { + let docs = + loaded_module.docs_by_module.remove(module_id).unwrap_or_else(|| { panic!("A module was exposed but didn't have an entry in `documentation` somehow: {module_id:?}"); }); @@ -196,7 +197,7 @@ fn page_title(package_name: &str, module_name: &str) -> String { format!("{module_name} - {package_name}") } -fn render_package_index(docs_by_module: &Vec<(ModuleId, ModuleDocumentation)>) -> String { +fn render_package_index(docs_by_module: &[(ModuleId, ModuleDocumentation)]) -> String { // The list items containing module links let mut module_list_buf = String::new(); diff --git a/crates/lang_srv/src/analysis.rs b/crates/lang_srv/src/analysis.rs index 6df2d46c12..9c498492d8 100644 --- a/crates/lang_srv/src/analysis.rs +++ b/crates/lang_srv/src/analysis.rs @@ -60,15 +60,19 @@ impl ModulesInfo { // example: A imports B. B exposes [add, multiply, divide] and A will store a reference to that list. let exposed = exposes .into_iter() - .map(|(id, symbols)| (id, Arc::new(symbols))) + .map(|(module_id, symbols)| (module_id, Arc::new(symbols))) .collect::>(); - //Combine the subs from all modules + + // Combine the subs from all modules let all_subs = Mutex::new( typechecked .iter() - .map(|(k, v)| (*k, v.solved_subs.0.clone())) + .map(|(module_id, checked_module)| { + (*module_id, checked_module.solved_subs.0.clone()) + }) .collect::>(), ); + ModulesInfo { subs: all_subs, exposed, @@ -165,6 +169,7 @@ pub(crate) fn global_analysis(doc_info: DocInfo) -> Vec { &typechecked, docs_by_module, )); + let mut builder = AnalyzedDocumentBuilder { interns: &interns, module_id_to_url: module_id_to_url_from_sources(&sources), diff --git a/crates/lang_srv/src/analysis/analysed_doc.rs b/crates/lang_srv/src/analysis/analysed_doc.rs index 31e8f419f8..b97e355d1e 100644 --- a/crates/lang_srv/src/analysis/analysed_doc.rs +++ b/crates/lang_srv/src/analysis/analysed_doc.rs @@ -176,7 +176,7 @@ impl AnalyzedDocument { let (region, var) = roc_can::traverse::find_closest_type_at(pos, declarations)?; //TODO:Can this be integrated into find closest type? is it even worth it? - let docs = self + let docs_opt = self .symbol_at(position) .and_then(|symb| modules_info.docs.get(module_id)?.get_doc_for_symbol(&symb)); @@ -189,7 +189,7 @@ impl AnalyzedDocument { value: type_str, }); - let content = vec![Some(type_content), docs.map(MarkedString::String)] + let content = vec![Some(type_content), docs_opt.map(MarkedString::String)] .into_iter() .flatten() .collect::>(); diff --git a/crates/lang_srv/src/analysis/completion.rs b/crates/lang_srv/src/analysis/completion.rs index 80fa255167..cfe16478f9 100644 --- a/crates/lang_srv/src/analysis/completion.rs +++ b/crates/lang_srv/src/analysis/completion.rs @@ -27,15 +27,18 @@ fn get_completions( ) -> Vec<(Symbol, Variable)> { let mut visitor = CompletionVisitor { position, - found_decls: Vec::new(), + found_declarations: Vec::new(), interns, prefix, }; visitor.visit_decls(decls); - visitor.found_decls + visitor.found_declarations } -/// Walks through declarations that would be accessible from the provided position adding them to a list of completion items until all accessible declarations have been fully explored +#[allow(clippy::too_many_arguments)] +/// Walks through declarations that would be accessible from the provided +/// position adding them to a list of completion items until all accessible +/// declarations have been fully explored. pub fn get_completion_items( position: Position, prefix: String, @@ -64,7 +67,7 @@ pub(super) fn get_module_completion_items( .flat_map(|(mod_id, exposed_symbols)| { let mod_name = mod_id.to_ident_str(interns).to_string(); - //Completion for modules themselves + // Completion for modules themselves if mod_name.starts_with(&prefix) { let item = CompletionItem { label: mod_name.clone(), @@ -79,8 +82,9 @@ pub(super) fn get_module_completion_items( )), ..Default::default() }; + vec![item] - //Complete dot completions for module exports + // Complete dot completions for module exports } else if prefix.starts_with(&(mod_name + ".")) { get_module_exposed_completion( exposed_symbols, @@ -93,9 +97,11 @@ pub(super) fn get_module_completion_items( vec![] } }); + if just_modules { return module_completions.collect(); } + module_completions.collect() } @@ -109,18 +115,20 @@ fn get_module_exposed_completion( let mut completion_docs = docs.map_or(Default::default(), |docs| { get_completion_docs(exposed_symbols, docs) }); + exposed_symbols .iter() - .map(|(sym, var)| { - //We need to fetch the subs for the module that is exposing what we are trying to complete because that will have the type info we need + .map(|(symbol, var)| { + // We need to fetch the subs for the module that is exposing what we + // are trying to complete because that will have the type info we need. modules_info .with_subs(mod_id, |subs| { make_completion_item( subs, mod_id, interns, - completion_docs.remove(sym), - sym.as_str(interns).to_string(), + completion_docs.remove(symbol), + symbol.as_str(interns).to_string(), *var, ) }) @@ -129,27 +137,37 @@ fn get_module_exposed_completion( .collect::>() } -///Efficiently walks the list of docs collecting the docs for completions as we go. Should be faster than re-walking for every completion +/// Efficiently walks the list of docs collecting the docs for completions as we go. +/// Should be faster than re-walking for every completion. fn get_completion_docs( completions: &[(Symbol, Variable)], docs: &ModuleDocumentation, ) -> HashMap { - let mut symbols = completions.iter().map(|(s, _)| s).collect::>(); + let mut symbols = completions + .iter() + .map(|(symbol, _)| symbol) + .collect::>(); + docs.entries .iter() .filter_map(|doc| match doc { roc_load::docs::DocEntry::DocDef(DocDef { docs, symbol, .. }) => { - let docs = docs.as_ref().map(|a| a.trim().to_string())?; - let (idx, _s) = symbols.iter().enumerate().find(|(_i, s)| s == &&symbol)?; - symbols.swap_remove(idx); - Some((*symbol, docs)) + let docs_str = docs.as_ref().map(|str| str.trim().to_string())?; + let (index, _symbol) = symbols + .iter() + .enumerate() + .find(|(_index, symb)| symb == &&symbol)?; + + symbols.swap_remove(index); + + Some((*symbol, docs_str)) } _ => None, }) .collect() } -///Provides a list of completions for Type aliases within the scope. +/// Provides a list of completions for Type aliases within the scope. ///TODO: Use this when we know we are within a type definition fn _alias_completions( aliases: &MutMap, @@ -178,8 +196,8 @@ fn make_completion_items( docs: Option<&ModuleDocumentation>, completions: Vec<(Symbol, Variable)>, ) -> Vec { - let mut completion_docs = docs.map_or(Default::default(), |docs| { - get_completion_docs(&completions, docs) + let mut completion_docs = docs.map_or(Default::default(), |mod_docs| { + get_completion_docs(&completions, mod_docs) }); completions @@ -213,8 +231,8 @@ fn make_completion_item( subs: &mut Subs, module_id: &ModuleId, interns: &Interns, - docs: Option, - str: String, + docs_opt: Option, + symbol_str: String, var: Variable, ) -> CompletionItem { let type_str = format_var_type(var, subs, module_id, interns); @@ -226,38 +244,39 @@ fn make_completion_item( | roc_types::subs::FlatType::TagUnion(_, _) => CompletionItemKind::ENUM, _ => CompletionItemKind::VARIABLE, }, - a => { + other => { debug!( "No specific completionKind for variable type: {:?} defaulting to 'Variable'", - a + other ); CompletionItemKind::VARIABLE } }; CompletionItem { - label: str, + label: symbol_str, detail: Some(type_str), kind: Some(typ), - documentation: docs.map(|d| { + documentation: docs_opt.map(|docs| { lsp_types::Documentation::MarkupContent(lsp_types::MarkupContent { kind: lsp_types::MarkupKind::Markdown, - value: d, + value: docs, }) }), ..Default::default() } } -struct FieldCompletion { - ///The name of the variable that is a record +/// E.g. a.b.c.d->{variable_name:"a", field:"d", middle_fields:["b","c"]} +struct RecFieldCompletion { + /// name of variable that is a record variable_name: String, field: String, middle_fields: Vec, } -///Finds the types of and names of all the fields of a record -///`var` should be a `Variable` that you know is a record's type or else it will return an empty list +/// Finds the types of and names of all the fields of a record. +/// `var` should be a `Variable` that you know is of type record or else it will return an empty list. fn find_record_fields(var: Variable, subs: &mut Subs) -> Vec<(String, Variable)> { let content = subs.get(var); match content.content { @@ -311,16 +330,16 @@ fn find_record_fields(var: Variable, subs: &mut Subs) -> Vec<(String, Variable)> } } -///Splits a completion prefix for a field into its components -///E.g. a.b.c.d->{variable_name:"a",middle_fields:["b","c"],field:"d"} -fn get_field_completion_parts(symbol_prefix: &str) -> Option { +/// Splits a completion prefix for a field into its components. +/// E.g. a.b.c.d->{variable_name:"a",middle_fields:["b","c"],field:"d"} +fn get_field_completion_parts(symbol_prefix: &str) -> Option { let mut parts = symbol_prefix.split('.').collect::>(); let field = parts.pop().unwrap_or("").to_string(); let variable_name = parts.remove(0).to_string(); - //Now that we have the head and tail removed this is all the intermediate fields + // Now that we have the head and tail removed this is all the intermediate fields. let middle_fields = parts.into_iter().map(ToString::to_string).collect(); - Some(FieldCompletion { + Some(RecFieldCompletion { variable_name, field, middle_fields, @@ -334,7 +353,7 @@ pub fn field_completion( subs: &mut Subs, module_id: &ModuleId, ) -> Option> { - let FieldCompletion { + let RecFieldCompletion { variable_name, field, middle_fields, @@ -345,16 +364,19 @@ pub fn field_completion( variable_name, field, middle_fields ); - //We get completions here, but all we really want is the info about the variable that is the first part of our record completion. - //We are completing the full name of the variable so we should only have one match + // We get completions here, but all we really want is the info about the variable that + // is the first part of our record completion. + // We are completing the full name of the variable so we should only have one match. let completion = get_completions(position, declarations, variable_name, interns) .into_iter() - .map(|(symb, var)| (symb.as_str(interns).to_string(), var)) + .map(|(symbol, var)| (symbol.as_str(interns).to_string(), var)) .next()?; - //If we have a type that has nested records we could have a completion prefix like: "var.field1.field2.fi" - //If the document isn't fully typechecked we won't know what the type of field2 is for us to offer completions based on it's fields - //Instead we get the type of "var" and then the type of "field1" within var's type and then "field2" within field1's type etc etc, until we have the type of the record we are actually looking for field completions for. + // If we have a type that has nested records we could have a completion prefix like: "var.field1.field2.fi". + // If the document isn't fully typechecked we won't know what the type of field2 is for us to offer + // completions based on it's fields. Instead we get the type of "var" and then the type of "field1" within + // var's type and then "field2" within field1's type etc etc, until we have the type of the record we are + // actually looking for field completions for. let completion_record = middle_fields.iter().fold(completion, |state, chain_field| { let fields_vars = find_record_fields(state.1, subs); fields_vars @@ -370,5 +392,6 @@ pub fn field_completion( let field_completions = make_completion_items_string(subs, module_id, interns, field_completions); + Some(field_completions) } diff --git a/crates/lang_srv/src/analysis/completion/formatting.rs b/crates/lang_srv/src/analysis/completion/formatting.rs index 24b55c75c2..e2f4be3028 100644 --- a/crates/lang_srv/src/analysis/completion/formatting.rs +++ b/crates/lang_srv/src/analysis/completion/formatting.rs @@ -46,6 +46,7 @@ pub(super) fn module_documentation( ) -> Documentation { let exposed_string = get_module_exposed_list(module_id, interns, modules_info, exposed).unwrap_or_default(); + let module_doc = module_docs .and_then(|docs| { docs.entries.first().and_then(|first_doc| match first_doc { diff --git a/crates/lang_srv/src/analysis/completion/visitor.rs b/crates/lang_srv/src/analysis/completion/visitor.rs index fbec5e1eb2..ad0fb8905c 100644 --- a/crates/lang_srv/src/analysis/completion/visitor.rs +++ b/crates/lang_srv/src/analysis/completion/visitor.rs @@ -13,7 +13,7 @@ use roc_types::subs::Variable; pub(crate) struct CompletionVisitor<'a> { pub(crate) position: Position, - pub(crate) found_decls: Vec<(Symbol, Variable)>, + pub(crate) found_declarations: Vec<(Symbol, Variable)>, pub(crate) interns: &'a Interns, pub(crate) prefix: String, } @@ -26,7 +26,7 @@ impl Visitor for CompletionVisitor<'_> { fn visit_expr(&mut self, expr: &Expr, region: Region, var: Variable) { if region.contains_pos(self.position) { let mut res = self.expression_defs(expr); - self.found_decls.append(&mut res); + self.found_declarations.append(&mut res); walk_expr(self, expr, var); } @@ -40,7 +40,8 @@ impl Visitor for CompletionVisitor<'_> { } | DeclarationInfo::Destructure { loc_expr, .. } => { let res = self.decl_to_completion_item(&decl); - self.found_decls.extend(res); + self.found_declarations.extend(res); + if loc_expr.region.contains_pos(self.position) { walk_decl(self, decl); }; @@ -52,19 +53,23 @@ impl Visitor for CompletionVisitor<'_> { } fn visit_def(&mut self, def: &Def) { - let res = self.extract_defs(def); - self.found_decls.extend(res); + let sym_var_vec = self.extract_defs(def); + self.found_declarations.extend(sym_var_vec); + walk_def(self, def); } } + impl CompletionVisitor<'_> { fn extract_defs(&mut self, def: &Def) -> Vec<(Symbol, Variable)> { trace!("Completion begin"); + def.pattern_vars .iter() .map(|(symbol, var)| (*symbol, *var)) .collect() } + fn expression_defs(&self, expr: &Expr) -> Vec<(Symbol, Variable)> { match expr { Expr::When { @@ -75,7 +80,7 @@ impl CompletionVisitor<'_> { loc_body, .. }) => { - //if we are inside the closure complete it's vars + // if we are inside the closure complete it's vars if loc_body.region.contains_pos(self.position) { arguments .iter() @@ -89,7 +94,7 @@ impl CompletionVisitor<'_> { } } - ///Extract any variables made available by the branch of a when_is expression that contains `self.position` + /// Extract any variables made available by the branch of a when_is expression that contains `self.position` fn when_is_expr( &self, branches: &[WhenBranch], @@ -117,10 +122,10 @@ impl CompletionVisitor<'_> { fn record_destructure(&self, destructs: &[Loc]) -> Vec<(Symbol, Variable)> { destructs .iter() - .flat_map(|a| match &a.value.typ { + .flat_map(|loc| match &loc.value.typ { roc_can::pattern::DestructType::Required | roc_can::pattern::DestructType::Optional(_, _) => { - vec![(a.value.symbol, a.value.var)] + vec![(loc.value.symbol, loc.value.var)] } roc_can::pattern::DestructType::Guard(var, pat) => self.patterns(&pat.value, var), }) @@ -130,8 +135,8 @@ impl CompletionVisitor<'_> { fn tuple_destructure(&self, destructs: &[Loc]) -> Vec<(Symbol, Variable)> { destructs .iter() - .flat_map(|a| { - let (var, pattern) = &a.value.typ; + .flat_map(|loc| { + let (var, pattern) = &loc.value.typ; self.patterns(&pattern.value, var) }) .collect() @@ -141,7 +146,7 @@ impl CompletionVisitor<'_> { list_elems .patterns .iter() - .flat_map(|a| self.patterns(&a.value, var)) + .flat_map(|loc| self.patterns(&loc.value, var)) .collect() } fn tag_pattern(&self, arguments: &[(Variable, Loc)]) -> Vec<(Symbol, Variable)> { @@ -157,14 +162,16 @@ impl CompletionVisitor<'_> { as_symbol: Symbol, var: &Variable, ) -> Vec<(Symbol, Variable)> { - //Get the variables introduced within the pattern + // get the variables introduced within the pattern let mut patterns = self.patterns(as_pat, var); - //Add the "as" that wraps the whole pattern + // add the "as" that wraps the whole pattern patterns.push((as_symbol, *var)); patterns } - ///Returns a list of symbols defined by this pattern. - ///`pattern_var`: Variable type of the entire pattern. This will be returned if the pattern turns out to be an identifier + + /// Returns a list of symbols defined by this pattern. + /// `pattern_var`: Variable type of the entire pattern. This will be returned if + /// the pattern turns out to be an identifier. fn patterns( &self, pattern: &roc_can::pattern::Pattern, @@ -214,22 +221,27 @@ impl CompletionVisitor<'_> { loc_body, .. } => { - let mut out = vec![]; - //Append the function declaration itself for recursive calls - out.extend(self.patterns(pattern, expr_var)); + let mut sym_var_vec = vec![]; + // append the function declaration itself for recursive calls + sym_var_vec.extend(self.patterns(pattern, expr_var)); if loc_body.region.contains_pos(self.position) { - //also add the arguments if we are inside the function + // also add the arguments if we are inside the function let args = function .value .arguments .iter() .flat_map(|(var, _, pat)| self.patterns(&pat.value, var)); - //We add in the pattern for the function declaration - out.extend(args); - trace!("Added function args to completion output =:{:#?}", out); + // we add in the pattern for the function declaration + sym_var_vec.extend(args); + + trace!( + "Added function args to completion output =:{:#?}", + sym_var_vec + ); } - out + + sym_var_vec } DeclarationInfo::Destructure { loc_pattern, diff --git a/crates/lang_srv/src/server.rs b/crates/lang_srv/src/server.rs index a37b497110..9933c98e5c 100644 --- a/crates/lang_srv/src/server.rs +++ b/crates/lang_srv/src/server.rs @@ -364,7 +364,8 @@ mod tests { .map(|item| (item.label, item.documentation)) .collect::>() } - ///Gets completion and returns only the label and docs for each completion + + /// gets completion and returns only the label and docs for each completion async fn get_basic_completion_info( reg: &Registry, url: &Url, @@ -375,18 +376,12 @@ mod tests { .map(completion_resp_to_strings) } - ///Gets completion and returns only the label and docs for each completion + /// gets completion and returns only the label for each completion fn comp_labels( completions: Option)>>, ) -> Option> { completions.map(|list| list.into_iter().map(|(labels, _)| labels).collect()) } - ///Gets completion and returns only the label and docs for each completion - fn comp_docs( - completions: Option)>>, - ) -> Option>> { - completions.map(|list| list.into_iter().map(|(_, docs)| docs).collect()) - } const DOC_LIT: &str = indoc! {r#" interface Test @@ -420,15 +415,16 @@ mod tests { ) -> Option)>> { let doc = DOC_LIT.to_string() + initial; let (inner, url) = test_setup(doc.clone()).await; - let reg = &inner.registry; + let registry = &inner.registry; let change = doc.clone() + addition; info!("doc is:\n{0}", change); inner.change(&url, change, 1).await.unwrap(); - get_basic_completion_info(reg, &url, position).await + get_basic_completion_info(registry, &url, position).await } + async fn completion_test_labels( initial: &str, addition: &str, @@ -436,15 +432,8 @@ mod tests { ) -> Option> { comp_labels(completion_test(initial, addition, position).await) } - async fn completion_test_docs( - initial: &str, - addition: &str, - position: Position, - ) -> Option>> { - comp_docs(completion_test(initial, addition, position).await) - } - ///Test that completion works properly when we apply an "as" pattern to an identifier + /// Test that completion works properly when we apply an "as" pattern to an identifier #[tokio::test] async fn test_completion_as_identifier() { let suffix = DOC_LIT.to_string() @@ -456,15 +445,15 @@ mod tests { let (inner, url) = test_setup(suffix.clone()).await; let position = Position::new(6, 7); - let reg = &inner.registry; + let registry = &inner.registry; let change = suffix.clone() + "o"; inner.change(&url, change, 1).await.unwrap(); - let comp1 = comp_labels(get_basic_completion_info(reg, &url, position).await); + let comp1 = comp_labels(get_basic_completion_info(registry, &url, position).await); let c = suffix.clone() + "i"; inner.change(&url, c, 2).await.unwrap(); - let comp2 = comp_labels(get_basic_completion_info(reg, &url, position).await); + let comp2 = comp_labels(get_basic_completion_info(registry, &url, position).await); let actual = [comp1, comp2]; @@ -529,7 +518,8 @@ mod tests { "#]] .assert_debug_eq(&actual); } - ///Test that completion works properly when we apply an "as" pattern to a record + + /// Test that completion works properly when we apply an "as" pattern to a record #[tokio::test] async fn test_completion_fun_params() { let actual = completion_test_labels( @@ -551,6 +541,7 @@ mod tests { "#]] .assert_debug_eq(&actual); } + #[tokio::test] async fn test_completion_closure() { let actual = completion_test_labels( @@ -571,6 +562,7 @@ mod tests { "#]] .assert_debug_eq(&actual); } + #[tokio::test] async fn test_completion_with_docs() { let actual = completion_test( @@ -582,6 +574,7 @@ mod tests { Position::new(4, 10), ) .await; + expect![[r#" Some( [