Address review comments

CHANGELOG.md

@@ -23,10 +23,11 @@
   ```gleam
   import gleam/option.{type Option as Maybe}
 
-  const none = option.Some(1)
+  const value = option.Some(1)
   //    ^ hovering here shows `Maybe(Int)`
   ```
 
   ([Surya Rose](https://github.com/GearsDatapacks))
 
 ### Bug Fixes
+

compiler-core/src/analyse/imports.rs

@@ -258,10 +258,6 @@ impl<'context, 'problems> Importer<'context, 'problems> {
                 .imported_modules
                 .insert(used_name.clone(), (import.location, import_info));
 
-            self.environment
-                .names
-                .imported_module(import.module.clone(), used_name.clone());
-
             self.environment
                 .names
                 .imported_module(import.module.clone(), used_name);

compiler-core/src/ast.rs

@@ -37,12 +37,12 @@ pub trait HasLocation {
     fn location(&self) -> SrcSpan;
 }
 
-pub type TypedModule = Module<type_::ModuleInterface, TypedDefinition, Names>;
+pub type TypedModule = Module<type_::ModuleInterface, TypedDefinition>;
 
-pub type UntypedModule = Module<(), TargetedDefinition, ()>;
+pub type UntypedModule = Module<(), TargetedDefinition>;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Module<Info, Statements, Names> {
+pub struct Module<Info, Statements> {
     pub name: EcoString,
     pub documentation: Vec<EcoString>,
     pub type_info: Info,

compiler-core/src/exhaustiveness/printer.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 
 use ecow::EcoString;
 
-use crate::type_::printer::{NameQualifier, Names};
+use crate::type_::printer::{NameContextInformation, Names};
 
 use super::{missing_patterns::Term, Variable};
 
@@ -54,9 +54,9 @@ impl<'a> Printer<'a> {
                 ..
             } => {
                 let (module, name) = match self.names.named_constructor(module, name) {
-                    NameQualifier::Qualified(m, n) => (Some(m), n),
-                    NameQualifier::Unqualified(n) => (None, n),
-                    NameQualifier::Unimported(n) => {
+                    NameContextInformation::Qualified(m, n) => (Some(m), n),
+                    NameContextInformation::Unqualified(n) => (None, n),
+                    NameContextInformation::Unimported(n) => {
                         (Some(module.split('/').last().unwrap_or(module)), n)
                     }
                 };

compiler-core/src/parse.rs

@@ -242,7 +242,7 @@ where
             documentation: vec![],
             type_info: (),
             definitions,
-            names: (),
+            names: Default::default(),
         };
         Ok(Parsed {
             module,

compiler-core/src/parse/snapshots/gleam_core__parse__tests__const_string_concat.snap

@@ -95,7 +95,14 @@ Parsed {
                 target: None,
             },
         ],
-        names: (),
+        names: Names {
+            local_types: {},
+            unshadowed_prelude_types: {},
+            imported_modules: {},
+            type_variables: {},
+            local_value_constructors: {},
+            constructor_names: {},
+        },
     },
     extra: ModuleExtra {
         module_comments: [],

compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_type.snap

@@ -52,7 +52,14 @@ Parsed {
                 target: None,
             },
         ],
-        names: (),
+        names: Names {
+            local_types: {},
+            unshadowed_prelude_types: {},
+            imported_modules: {},
+            type_variables: {},
+            local_value_constructors: {},
+            constructor_names: {},
+        },
     },
     extra: ModuleExtra {
         module_comments: [],

compiler-core/src/parse/snapshots/gleam_core__parse__tests__record_access_no_label.snap

@@ -157,7 +157,14 @@ Parsed {
                 target: None,
             },
         ],
-        names: (),
+        names: Names {
+            local_types: {},
+            unshadowed_prelude_types: {},
+            imported_modules: {},
+            type_variables: {},
+            local_value_constructors: {},
+            constructor_names: {},
+        },
     },
     extra: ModuleExtra {
         module_comments: [],

compiler-core/src/type_/printer.rs

@@ -9,7 +9,7 @@ use super::PRELUDE_MODULE_NAME;
 /// This class keeps track of what names are used for modules in the current
 /// scope, so they can be printed in errors, etc.
 ///
-#[derive(Debug, Default)]
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
 pub struct Names {
     /// Types that exist in the current module, either defined or imported in an
     /// unqualified fashion.
@@ -119,7 +119,7 @@ pub struct Names {
     /// - key:   `("wibble", "Wobble")`
     /// - value: `"Woo"`
     ///
-    local_constructors: HashMap<(EcoString, EcoString), EcoString>,
+    local_value_constructors: HashMap<(EcoString, EcoString), EcoString>,
 
     /// A map from local constructor names to the modules which they refer to.
     /// This helps resolve cases like:
@@ -153,7 +153,7 @@ impl Names {
             unshadowed_prelude_types: Default::default(),
             imported_modules: Default::default(),
             type_variables: Default::default(),
-            local_constructors: Default::default(),
+            local_value_constructors: Default::default(),
             constructor_names: Default::default(),
         }
     }
@@ -203,33 +203,33 @@ impl Names {
         module: &'a EcoString,
         name: &'a EcoString,
         print_mode: PrintMode,
-    ) -> NameQualifier<'a> {
-        if print_mode == PrintMode::Raw {
+    ) -> NameContextInformation<'a> {
+        if print_mode == PrintMode::ExpandAliases {
             if let Some(module) = self.imported_modules.get(module) {
-                return NameQualifier::Qualified(module, name.as_str());
+                return NameContextInformation::Qualified(module, name.as_str());
             };
 
-            return NameQualifier::Unimported(name.as_str());
+            return NameContextInformation::Unimported(name.as_str());
         }
 
         let key = (module.clone(), name.clone());
 
         // Only check for local aliases if we want to print aliases
         // There is a local name for this type, use that.
         if let Some(name) = self.local_types.get(&key) {
-            return NameQualifier::Unqualified(name.as_str());
+            return NameContextInformation::Unqualified(name.as_str());
         }
 
         if module == PRELUDE_MODULE_NAME && self.unshadowed_prelude_types.contains(name) {
-            return NameQualifier::Unqualified(name.as_str());
+            return NameContextInformation::Unqualified(name.as_str());
         }
 
         // This type is from a module that has been imported
         if let Some(module) = self.imported_modules.get(module) {
-            return NameQualifier::Qualified(module, name.as_str());
+            return NameContextInformation::Qualified(module, name.as_str());
         };
 
-        return NameQualifier::Unimported(name.as_str());
+        return NameContextInformation::Unimported(name.as_str());
     }
 
     /// Record a named value in this module.
@@ -240,7 +240,7 @@ impl Names {
         local_alias: EcoString,
     ) {
         _ = self
-            .local_constructors
+            .local_value_constructors
             .insert((module_name.clone(), value_name), local_alias.clone());
         _ = self.constructor_names.insert(local_alias, module_name);
     }
@@ -250,11 +250,11 @@ impl Names {
         &'a self,
         module: &'a EcoString,
         name: &'a EcoString,
-    ) -> NameQualifier<'a> {
+    ) -> NameContextInformation<'a> {
         let key: (EcoString, EcoString) = (module.clone(), name.clone());
 
         // There is a local name for this value, use that.
-        if let Some(name) = self.local_constructors.get(&key) {
+        if let Some(name) = self.local_value_constructors.get(&key) {
             // Only return unqualified syntax if the constructor is not shadowed,
             // and unqualified syntax is valid.
             if self
@@ -263,21 +263,21 @@ impl Names {
                 .expect("Constructors must be added to both maps")
                 == module
             {
-                return NameQualifier::Unqualified(name.as_str());
+                return NameContextInformation::Unqualified(name.as_str());
             }
         }
 
         // This value is from a module that has been imported
         if let Some(module) = self.imported_modules.get(module) {
-            return NameQualifier::Qualified(module, name.as_str());
+            return NameContextInformation::Qualified(module, name.as_str());
         };
 
-        NameQualifier::Unimported(name.as_str())
+        NameContextInformation::Unimported(name.as_str())
     }
 }
 
 #[derive(Debug)]
-pub enum NameQualifier<'a> {
+pub enum NameContextInformation<'a> {
     /// This type is from a module that has not been imported in this module.
     Unimported(&'a str),
     /// This type has been imported in an unqualifid fashion in this module.
@@ -288,11 +288,24 @@ pub enum NameQualifier<'a> {
 
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum PrintMode {
-    /// Prints the context-specific representation of a type
+    /// Prints the context-specific representation of a type.
     Normal,
-    /// Prints the raw type, always qualified. Useful for providing
-    /// more detail to the user.
-    Raw,
+    /// Prints full detail of the given type, always qualified.
+    /// Useful for providing more detail to the user.
+    ///
+    /// For example, with this code:
+    /// ```gleam
+    /// type A = Int
+    /// ```
+    /// If the type `gleam.Int` were printed using the `Normal` mode,
+    /// we would print `A`, since that is the local alias for the `Int` type.
+    ///
+    /// However, if the user were hovering over the type `A` itself, it wouldn't be
+    /// particularly helpful to print `A`.
+    /// So with `ExpandAliases`, it would print `gleam.Int`,
+    /// which tells the user exactly what type `A` represents.
+    ///
+    ExpandAliases,
 }
 
 /// A type printer that does not wrap and indent, but does take into account the
@@ -335,7 +348,7 @@ impl<'a> Printer<'a> {
 
     pub fn print_type_without_aliases(&mut self, type_: &Type) -> EcoString {
         let mut buffer = EcoString::new();
-        self.print(type_, &mut buffer, PrintMode::Raw);
+        self.print(type_, &mut buffer, PrintMode::ExpandAliases);
         buffer
     }
 
@@ -345,11 +358,11 @@ impl<'a> Printer<'a> {
                 name, args, module, ..
             } => {
                 let (module, name) = match self.names.named_type(module, name, print_mode) {
-                    NameQualifier::Qualified(m, n) => (Some(m), n),
-                    NameQualifier::Unqualified(n) => (None, n),
+                    NameContextInformation::Qualified(m, n) => (Some(m), n),
+                    NameContextInformation::Unqualified(n) => (None, n),
                     // TODO: indicate that the module is not import and as such
                     // needs to be, as well as how.
-                    NameQualifier::Unimported(n) => {
+                    NameContextInformation::Unimported(n) => {
                         (Some(module.split('/').last().unwrap_or(module)), n)
                     }
                 };

compiler-core/src/type_/tests.rs

@@ -666,7 +666,7 @@ fn infer_module_type_retention_test() {
         name: "ok".into(),
         definitions: vec![],
         type_info: (),
-        names: (),
+        names: Default::default(),
     };
     let direct_dependencies = HashMap::from_iter(vec![]);
     let ids = UniqueIdGenerator::new();