Fix unused module name warning

CHANGELOG.md

@@ -612,3 +612,7 @@
 - Fixed a bug where an incorrect `package-interface.json` would be generated for
   certain type aliases.
   ([Surya Rose](https://github.com/GearsDatapacks))
+
+- Fixed a bug where the compiler would report a module import as unused when
+  its local name matched another imported module's full name.
+  ([John Downey](https://github.com/jtdowney))

compiler-core/src/reference.rs

@@ -305,8 +305,11 @@ pub struct ReferenceTracker {
     /// variant that defines it, used for renaming and go-to definition.
     pub label_definitions: HashMap<RecordLabel, Vec<LabelDefinition>>,
 
-    /// This map is used to access the nodes of modules that were not
-    /// aliased, given their name.
+    /// Maps a module's canonical name to the node of the import it was brought
+    /// in by. Every import is inserted here (aliased or not), keyed by its full
+    /// module name, _except_ imports with a discarded alias (`as _name`), which
+    /// have no reachable node and so are absent.
+    ///
     /// We need this to keep track of references made to imports by unqualified
     /// values/types: when an unqualified item is used we want to add an edge
     /// pointing to the import it comes from, so that if the item is used the
@@ -321,7 +324,8 @@ pub struct ReferenceTracker {
     /// ```
     ///
     /// And each imported entity carries around the _name of the module_ and not
-    /// just the alias (here it would be `wibble/wobble` and not just `wobble`).
+    /// just the alias (here it would be `wibble/wobble` and not just `wobble`),
+    /// so this map is keyed by that full name rather than the local name.
     ///
     module_name_to_node: HashMap<EcoString, NodeIndex>,
 }
@@ -376,6 +380,15 @@ impl ReferenceTracker {
         }
     }
 
+    /// Add a call-graph edge from the current node to the entity with the given
+    /// name and layer, creating its node if it doesn't exist yet. If the target
+    /// is used then the current node counts as used too.
+    ///
+    fn add_reference_edge(&mut self, name: EcoString, layer: EntityLayer) {
+        let target = self.get_or_create_node(name, layer);
+        _ = self.graph.add_edge(self.current_node, target, ());
+    }
+
     /// This function exists because of a specific edge-case where constants
     /// can shadow imported values. For example:
     /// ```gleam
@@ -505,7 +518,7 @@ impl ReferenceTracker {
         // Also we want to register the fact that if this alias is used then the
         // import is used: so we add a reference from the alias to the import
         // we've just added.
-        self.register_module_reference(module_name.clone());
+        self.register_module_reference_by_module_name(module_name.clone());
 
         // Finally we can add information for this alias:
         let entity = Entity {
@@ -577,7 +590,7 @@ impl ReferenceTracker {
             EntityKind::ImportedConstructor { module }
             | EntityKind::ImportedType { module }
             | EntityKind::ImportedValue { module } => {
-                self.register_module_reference(module.clone());
+                self.register_module_reference_by_module_name(module.clone());
             }
         }
     }
@@ -605,8 +618,7 @@ impl ReferenceTracker {
             }
             ReferenceKind::Import(_) | ReferenceKind::Definition => {}
             ReferenceKind::Alias | ReferenceKind::Unqualified => {
-                let target = self.get_or_create_node(referenced_name.clone(), EntityLayer::Value);
-                _ = self.graph.add_edge(self.current_node, target, ());
+                self.add_reference_edge(referenced_name.clone(), EntityLayer::Value);
             }
         }
 
@@ -649,11 +661,12 @@ impl ReferenceTracker {
             .push(Reference { location, kind });
     }
 
-    /// Register a reference to a module in the code. This is separate to
-    /// `register_module_reference`, as references to modules can be created
-    /// implicitly, for example when using unqualified imports. This only register
-    /// explicit references in the source code, when the module name or local
-    /// alias is written.
+    /// Register a reference to a module name written explicitly in the source
+    /// code, when the module name or local alias appears. This is separate from
+    /// the call-graph edges added by `register_module_reference_by_alias`
+    /// and `register_module_reference_by_module_name`, which track references
+    /// created implicitly, for example when using unqualified imports.
+    ///
     pub fn register_module_name_reference(
         &mut self,
         module: EcoString,
@@ -717,14 +730,28 @@ impl ReferenceTracker {
     /// is to make a connection between them in the call graph.
     ///
     pub fn register_type_reference_in_call_graph(&mut self, name: EcoString) {
-        let target = self.get_or_create_node(name, EntityLayer::Type);
-        _ = self.graph.add_edge(self.current_node, target, ());
+        self.add_reference_edge(name, EntityLayer::Type);
+    }
+
+    /// Add a call-graph edge to a module resolved by its local name: the
+    /// alias, or the last segment of the module path when there is no alias.
+    /// Use this for references that name the module as written at the use site,
+    /// such as a qualified `wobble.thing`.
+    ///
+    pub fn register_module_reference_by_alias(&mut self, name: EcoString) {
+        self.add_reference_edge(name, EntityLayer::Module);
     }
 
-    pub fn register_module_reference(&mut self, name: EcoString) {
-        let target = match self.module_name_to_node.get(&name) {
-            Some(target) => *target,
-            None => self.get_or_create_node(name, EntityLayer::Module),
+    /// Add a call-graph edge to an import resolved by its canonical module name
+    /// via `module_name_to_node`. Use this for references that carry the full
+    /// module name rather than the local alias, such as an unqualified item
+    /// that remembers which module it came from.
+    ///
+    fn register_module_reference_by_module_name(&mut self, name: EcoString) {
+        // If the module has no node then it was imported with a discarded
+        // alias, meaning there's no import for the reference to point at.
+        let Some(target) = self.module_name_to_node.get(&name).copied() else {
+            return;
         };
         _ = self.graph.add_edge(self.current_node, target, ());
     }

compiler-core/src/type_/environment.rs

@@ -470,7 +470,7 @@ impl Environment<'_> {
                     }
                 })?;
                 self.references
-                    .register_module_reference(module_name.clone());
+                    .register_module_reference_by_alias(module_name.clone());
                 module.get_importable_type(name).ok_or_else(|| {
                     UnknownTypeConstructorError::ModuleType {
                         name: name.clone(),
@@ -556,7 +556,7 @@ impl Environment<'_> {
                     }
                 })?;
                 self.references
-                    .register_module_reference(module_name.clone());
+                    .register_module_reference_by_alias(module_name.clone());
                 module.get_importable_value(name).ok_or_else(|| {
                     UnknownValueConstructorError::ModuleValue {
                         name: name.clone(),

compiler-core/src/type_/expression.rs

@@ -2954,7 +2954,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
 
             self.environment
                 .references
-                .register_module_reference(module_alias.clone());
+                .register_module_reference_by_alias(module_alias.clone());
 
             (module.name.clone(), constructor.clone())
         };
@@ -3701,7 +3701,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
 
                 self.environment
                     .references
-                    .register_module_reference(module_name.clone());
+                    .register_module_reference_by_alias(module_name.clone());
 
                 module
                     .values

compiler-core/src/type_/tests.rs

@@ -271,7 +271,7 @@ macro_rules! assert_warning {
         insta::assert_snapshot!(insta::internals::AutoName, output, $src);
     };
 
-    ($(($package:expr, $name:expr, $module_src:literal)),+, $src:expr) => {
+    ($(($package:expr, $name:expr, $module_src:literal)),+, $src:literal $(,)?) => {
         let warning = $crate::type_::tests::get_printed_warnings(
             $src,
             vec![$(($package, $name, $module_src)),*],
@@ -374,7 +374,7 @@ macro_rules! assert_no_warnings {
         let warnings = $crate::type_::tests::get_warnings($src, vec![], crate::build::Target::Erlang, None);
         assert_eq!(warnings, vec![]);
     };
-    ($(($name:expr, $module_src:literal)),+, $src:expr $(,)?) => {
+    ($(($name:expr, $module_src:literal)),+, $src:literal $(,)?) => {
         let warnings = $crate::type_::tests::get_warnings(
             $src,
             vec![$(("thepackage", $name, $module_src)),*],
@@ -383,7 +383,7 @@ macro_rules! assert_no_warnings {
         );
         assert_eq!(warnings, vec![]);
     };
-    ($(($package:expr, $name:expr, $module_src:literal)),+, $src:expr $(,)?) => {
+    ($(($package:expr, $name:expr, $module_src:literal)),+, $src:literal $(,)?) => {
         let warnings = $crate::type_::tests::get_warnings(
             $src,
             vec![$(($package, $name, $module_src)),*],

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_module_alias_not_confused_with_another_module_name.snap

@@ -0,0 +1,26 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+expression: "\n            import wibble/wobble\n            import wobble as my_wobble\n            pub const one = wobble.one\n        "
+---
+----- SOURCE CODE
+-- wibble/wobble.gleam
+pub const one = 1
+
+-- wobble.gleam
+pub const two = 2
+
+-- main.gleam
+
+            import wibble/wobble
+            import wobble as my_wobble
+            pub const one = wobble.one
+
+
+----- WARNING
+warning: Unused imported module
+  ┌─ /src/warning/wrn.gleam:3:13
+  │
+3 │             import wobble as my_wobble
+  │             ^^^^^^^^^^^^^^^^^^^^^^^^^^ This imported module is never used
+
+Hint: You can safely remove it.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_module_not_marked_as_used_by_discard_aliased_import_with_same_name.snap

@@ -0,0 +1,26 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+expression: "\n            import wibble/wobble\n            import wobble.{two} as _wobble\n            pub fn main() { two }\n        "
+---
+----- SOURCE CODE
+-- wibble/wobble.gleam
+pub const one = 1
+
+-- wobble.gleam
+pub const two = 2
+
+-- main.gleam
+
+            import wibble/wobble
+            import wobble.{two} as _wobble
+            pub fn main() { two }
+
+
+----- WARNING
+warning: Unused imported module
+  ┌─ /src/warning/wrn.gleam:2:13
+  │
+2 │             import wibble/wobble
+  │             ^^^^^^^^^^^^^^^^^^^^ This imported module is never used
+
+Hint: You can safely remove it.

compiler-core/src/type_/tests/warnings.rs

@@ -1145,6 +1145,63 @@ fn unused_alias_for_duplicate_module_no_warning_for_alias_test() {
     );
 }
 
+// https://github.com/gleam-lang/gleam/issues/5975
+#[test]
+fn imported_module_usage_not_confused_with_another_module_name() {
+    assert_no_warnings!(
+        ("thepackage", "wibble/wobble", "pub const one = 1"),
+        ("thepackage", "wobble", "pub const two = 2"),
+        r#"
+            import wibble/wobble
+            import wobble as my_wobble
+            pub const one = wobble.one
+            pub const two = my_wobble.two
+        "#
+    );
+}
+
+// https://github.com/gleam-lang/gleam/issues/5975
+#[test]
+fn imported_module_type_usage_not_confused_with_another_module_name() {
+    assert_no_warnings!(
+        ("thepackage", "wibble/wobble", "pub type One = Int"),
+        ("thepackage", "wobble", "pub type Two = Int"),
+        r#"
+            import wibble/wobble
+            import wobble as my_wobble
+            pub fn one(value: wobble.One) -> wobble.One { value }
+            pub fn two(value: my_wobble.Two) -> my_wobble.Two { value }
+        "#
+    );
+}
+
+// https://github.com/gleam-lang/gleam/issues/5975
+#[test]
+fn unused_module_alias_not_confused_with_another_module_name() {
+    assert_warning!(
+        ("thepackage", "wibble/wobble", "pub const one = 1"),
+        ("thepackage", "wobble", "pub const two = 2"),
+        r#"
+            import wibble/wobble
+            import wobble as my_wobble
+            pub const one = wobble.one
+        "#
+    );
+}
+
+#[test]
+fn unused_module_not_marked_as_used_by_discard_aliased_import_with_same_name() {
+    assert_warning!(
+        ("thepackage", "wibble/wobble", "pub const one = 1"),
+        ("thepackage", "wobble", "pub const two = 2"),
+        r#"
+            import wibble/wobble
+            import wobble.{two} as _wobble
+            pub fn main() { two }
+        "#
+    );
+}
+
 #[test]
 fn result_in_case_discarded() {
     assert_warning!(