cleanup nitpicks and reorganising

crates/lang_srv/src/analysis.rs

@@ -40,6 +40,38 @@ pub(super) struct ModulesInfo {
     exposed: HashMap<ModuleId, Arc<Vec<(Symbol, Variable)>>>,
 }
 
+impl ModulesInfo {
+    fn with_subs<F, A>(&self, mod_id: &ModuleId, f: F) -> Option<A>
+    where
+        F: FnOnce(&mut Subs) -> A,
+    {
+        self.subs.lock().get_mut(mod_id).map(f)
+    }
+    ///Transforms some of the raw data from the analysis into a state that is more useful during processes like completion
+    fn from_analysis(
+        exposes: MutMap<ModuleId, Vec<(Symbol, Variable)>>,
+        typechecked: &MutMap<ModuleId, CheckedModule>,
+    ) -> ModulesInfo {
+        //We wrap this in arc because later we will go through each module's imports and store the full list of symbols that each imported module exposes.
+        //eg: 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)))
+            .collect::<HashMap<_, _>>();
+        //Combine the subs from all modules
+        let all_subs = Mutex::new(
+            typechecked
+                .iter()
+                .map(|(k, v)| (*k, v.solved_subs.0.clone()))
+                .collect::<HashMap<_, _>>(),
+        );
+        ModulesInfo {
+            subs: all_subs,
+            exposed,
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub(super) struct AnalyzedModule {
     exposed_imports: Vec<(Symbol, Variable)>,
@@ -122,7 +154,7 @@ pub(crate) fn global_analysis(doc_info: DocInfo) -> Vec<AnalyzedDocument> {
 
     let exposed_imports = resolve_exposed_imports(exposed_imports, &exposes);
 
-    let modules_info = Arc::new(make_modules_info(exposes, &typechecked));
+    let modules_info = Arc::new(ModulesInfo::from_analysis(exposes, &typechecked));
     let mut builder = AnalyzedDocumentBuilder {
         interns: &interns,
         module_id_to_url: module_id_to_url_from_sources(&sources),
@@ -150,6 +182,13 @@ fn resolve_exposed_imports(
     exposed_imports: MutMap<ModuleId, MutMap<Symbol, roc_region::all::Region>>,
     exposes: &MutMap<ModuleId, Vec<(Symbol, Variable)>>,
 ) -> HashMap<ModuleId, Vec<(Symbol, Variable)>> {
+    let get_exposed_symbol_info = |symbol: &Symbol, module_id: &ModuleId| {
+        exposes
+            .get(module_id)?
+            .iter()
+            .find(|(symb, _)| symb == symbol)
+    };
+
     exposed_imports
         .into_iter()
         .map(|(module_id, symbols)| {
@@ -157,41 +196,13 @@ fn resolve_exposed_imports(
                 module_id,
                 symbols
                     .into_iter()
-                    .filter_map(|(symbol, _)| {
+                    .filter_map(|(symbol, _)| get_exposed_symbol_info(&symbol, &module_id))
-                        exposes.get(&module_id)?.iter().find(|(symb, _)| {
-                            //TODO this seems to not be comparing properly so we aren't getting any exposed imports
-                            symb == &symbol
-                        })
-                    })
                     .cloned()
                     .collect::<Vec<_>>(),
             )
         })
         .collect()
 }
-///Transforms some of the raw data from the analysis into a state that is more useful during processes like completion
-fn make_modules_info(
-    exposes: MutMap<ModuleId, Vec<(Symbol, Variable)>>,
-    typechecked: &MutMap<ModuleId, CheckedModule>,
-) -> ModulesInfo {
-    //We wrap this in arc because later we will go through each module's imports and store the full list of symbols that each imported module exposes.
-    //eg: 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)))
-        .collect::<HashMap<_, _>>();
-    //Combine the subs from all modules
-    let all_subs = Mutex::new(
-        typechecked
-            .iter()
-            .map(|(k, v)| (*k, v.solved_subs.0.clone()))
-            .collect::<HashMap<_, _>>(),
-    );
-    ModulesInfo {
-        subs: all_subs,
-        exposed,
-    }
-}
 
 fn find_src_dir(path: &Path) -> &Path {
     path.parent().unwrap_or(path)
@@ -275,6 +286,7 @@ impl<'a> AnalyzedDocumentBuilder<'a> {
                 )
             })
             .collect::<HashMap<_, _>>();
+
         let exposed_imports = self.exposed_imports.remove(&module_id).unwrap_or_default();
 
         if let Some(m) = self.typechecked.remove(&module_id) {

crates/lang_srv/src/analysis/analysed_doc.rs

@@ -13,9 +13,7 @@ use tower_lsp::lsp_types::{
 };
 
 use crate::{
-    analysis::completion::{
+    analysis::completion::{field_completion, get_completion_items, get_module_completion_items},
-        field_completion, get_completion_items, get_upper_case_completion_items,
-    },
     convert::{ToRange, ToRocPosition},
 };
 
@@ -242,7 +240,7 @@ impl AnalyzedDocument {
                 .unwrap_or(false);
             if is_module_completion {
                 info!("Getting module dot completion");
-                Some(get_upper_case_completion_items(
+                Some(get_module_completion_items(
                     symbol_prefix,
                     interns,
                     imports,
@@ -267,7 +265,7 @@ impl AnalyzedDocument {
                 .map_or(false, |c| c.is_uppercase());
             if is_module_or_type_completion {
                 info!("Getting module completion");
-                let completions = get_upper_case_completion_items(
+                let completions = get_module_completion_items(
                     symbol_prefix,
                     interns,
                     imports,

crates/lang_srv/src/analysis/completion.rs

@@ -321,62 +321,70 @@ pub fn get_completion_items(
             .collect(),
     )
 }
-pub(super) fn get_upper_case_completion_items(
+pub(super) fn get_module_completion_items(
     prefix: String,
     interns: &Interns,
     imported_modules: &HashMap<ModuleId, Arc<Vec<(Symbol, Variable)>>>,
     modules_info: &ModulesInfo,
     just_modules: bool,
 ) -> Vec<CompletionItem> {
-    let module_completions = imported_modules.iter().flat_map(|(mod_id, vars)| {
+    let module_completions = imported_modules
-        let mod_name = mod_id.to_ident_str(interns).to_string();
+        .iter()
+        .flat_map(|(mod_id, exposed_symbols)| {
+            let mod_name = mod_id.to_ident_str(interns).to_string();
 
-        if mod_name.starts_with(&prefix) {
+            if mod_name.starts_with(&prefix) {
-            let item = CompletionItem {
+                let item = CompletionItem {
-                label: mod_name.clone(),
+                    label: mod_name.clone(),
-                kind: Some(CompletionItemKind::MODULE),
+                    kind: Some(CompletionItemKind::MODULE),
-                documentation: Some(formatting::module_documentation(
+                    documentation: Some(formatting::module_documentation(
-                    formatting::DescriptionsType::Exposes,
+                        formatting::DescriptionsType::Exposes,
-                    mod_id,
+                        mod_id,
-                    interns,
+                        interns,
-                    modules_info,
+                        exposed_symbols,
-                )),
+                        modules_info,
-                ..Default::default()
+                    )),
-            };
+                    ..Default::default()
-            vec![item]
+                };
-        //Complete dot completions
+                vec![item]
-        } else if prefix.starts_with(&(mod_name + ".")) {
+            //Complete dot completions
-            vars.clone()
+            } else if prefix.starts_with(&(mod_name + ".")) {
-                .iter()
+                get_module_exposed_completion(exposed_symbols, modules_info, mod_id, interns)
-                .map(|(sym, var)| {
+            } else {
-                    //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
+                vec![]
-                    modules_info
+            }
-                        .subs
+        });
-                        .lock()
-                        .get_mut(mod_id)
-                        .map(|subs| {
-                            make_completion_item(
-                                subs,
-                                mod_id,
-                                interns,
-                                sym.as_str(interns).to_string(),
-                                *var,
-                            )
-                        })
-                        .expect("Couldn't find subs for module during completion.")
-                })
-                .collect::<Vec<_>>()
-        } else {
-            vec![]
-        }
-    });
     if just_modules {
         return module_completions.collect();
     }
-
     module_completions.collect()
 }
 
+fn get_module_exposed_completion(
+    exposed_symbols: &Vec<(Symbol, Variable)>,
+    modules_info: &ModulesInfo,
+    mod_id: &ModuleId,
+    interns: &Interns,
+) -> Vec<CompletionItem> {
+    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
+            modules_info
+                .with_subs(mod_id, |subs| {
+                    make_completion_item(
+                        subs,
+                        mod_id,
+                        interns,
+                        sym.as_str(interns).to_string(),
+                        *var,
+                    )
+                })
+                .expect("Couldn't find subs for module during completion.")
+        })
+        .collect::<Vec<_>>()
+}
+
 ///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(

crates/lang_srv/src/analysis/completion/formatting.rs

@@ -1,5 +1,6 @@
-use roc_module::symbol::{Interns, ModuleId};
+use roc_module::symbol::{Interns, ModuleId, Symbol};
 
+use roc_types::subs::Variable;
 use tower_lsp::lsp_types::{Documentation, MarkupContent, MarkupKind};
 
 use crate::analysis::{utils::format_var_type, ModulesInfo};
@@ -7,20 +8,19 @@ use crate::analysis::{utils::format_var_type, ModulesInfo};
 fn module_exposed_list(
     module_id: &ModuleId,
     interns: &Interns,
-    ModulesInfo { subs, exposed }: &ModulesInfo,
+    modules_info: &ModulesInfo,
+    exposed: &Vec<(Symbol, Variable)>,
 ) -> Option<std::string::String> {
-    exposed.get(module_id).and_then(|exposed| {
+    modules_info.with_subs(module_id, |subs| {
-        subs.lock().get_mut(module_id).map(|subs| {
+        let items = exposed
-            let items = exposed
+            .iter()
-                .iter()
+            .map(|(symb, var)| {
-                .map(|(symb, var)| {
+                let var_str = format_var_type(*var, subs, module_id, interns);
-                    let var_str = format_var_type(*var, subs, module_id, interns);
+                format!("{0}: {1}", symb.as_str(interns), var_str)
-                    format!("{0}: {1}", symb.as_str(interns), var_str)
+            })
-                })
+            .collect::<Vec<_>>();
-                .collect::<Vec<_>>();
 
-            items.join("\n").to_string()
+        items.join("\n").to_string()
-        })
     })
 }
 pub(super) enum DescriptionsType {
@@ -38,11 +38,13 @@ pub(super) fn module_documentation(
     description_type: DescriptionsType,
     module_id: &ModuleId,
     interns: &Interns,
+    exposed: &Vec<(Symbol, Variable)>,
     modules_info: &ModulesInfo,
 ) -> Documentation {
-    let exposed = || module_exposed_list(module_id, interns, modules_info).unwrap_or_default();
+    let exposed_string =
+        module_exposed_list(module_id, interns, modules_info, exposed).unwrap_or_default();
 
     match description_type {
-        DescriptionsType::Exposes => md_doc(format!("```roc\n{0}\n```", exposed())),
+        DescriptionsType::Exposes => md_doc(format!("```roc\n{0}\n```", exposed_string)),
     }
 }