Warn when top-level definition shadows imported value

CHANGELOG.md

@@ -53,6 +53,10 @@
 
   ([Zij-IT](https://github.com/zij-it))
 
+- The compiler now emits a warning when a top-level constant or function declaration
+  shadows an imported name in the current module.
+  ([Aayush Tripathi](https://github.com/aayush-tripathi))
+
 ### Build tool
 
 - `gleam update`, `gleam deps update`, and `gleam deps download` will now print

compiler-core/src/analyse.rs

@@ -393,6 +393,8 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
             ..
         } = c;
         self.check_name_case(name_location, &name, Named::Constant);
+        // If the constant's name matches an unqualified import, emit a warning:
+        self.check_shadow_import(&name, c.location, environment);
 
         environment.references.begin_constant();
 
@@ -1422,6 +1424,8 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
         let (name_location, name) = name.as_ref().expect("A module's function must be named");
 
         self.check_name_case(*name_location, name, Named::Function);
+        // If the function's name matches an unqualified import, emit a warning:
+        self.check_shadow_import(name, f.location, environment);
 
         environment.references.register_value(
             name.clone(),
@@ -1551,6 +1555,21 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
             self.minimum_required_version = minimum_required_version;
         }
     }
+
+    fn check_shadow_import(
+        &mut self,
+        name: &EcoString,
+        location: SrcSpan,
+        environment: &mut Environment<'_>,
+    ) {
+        if environment.unqualified_imported_names.contains_key(name) {
+            self.problems
+                .warning(Warning::TopLevelDefinitionShadowsImport {
+                    location,
+                    name: name.clone(),
+                });
+        }
+    }
 }
 
 fn optionally_push<T>(vector: &mut Vec<T>, item: Option<T>) {

compiler-core/src/type_/error.rs

@@ -1015,6 +1015,13 @@ pub enum Warning {
         first: SrcSpan,
         second: SrcSpan,
     },
+
+    /// Top-level definition should not shadow an imported one.
+    /// This includes constant or function imports.
+    TopLevelDefinitionShadowsImport {
+        location: SrcSpan,
+        name: EcoString,
+    },
 }
 
 #[derive(Debug, Eq, Copy, PartialEq, Clone, serde::Serialize, serde::Deserialize)]
@@ -1240,6 +1247,7 @@ impl Warning {
             | Warning::JavaScriptIntUnsafe { location, .. }
             | Warning::AssertLiteralValue { location, .. }
             | Warning::BitArraySegmentTruncatedValue { location, .. }
+            | Warning::TopLevelDefinitionShadowsImport { location, .. }
             | Warning::ModuleImportedTwice {
                 second: location, ..
             } => *location,

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

@@ -445,7 +445,7 @@ pub const wibble = 2
 
 #[test]
 fn used_shadowed_imported_value() {
-    assert_no_warnings!(
+    assert_warning!(
         (
             "thepackage",
             "wibble",

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

@@ -1,6 +1,8 @@
 ---
 source: compiler-core/src/type_/tests/dead_code_detection.rs
+assertion_line: 431
 expression: "\nimport wibble.{wibble}\n\npub const wibble = 2\n"
+snapshot_kind: text
 ---
 ----- SOURCE CODE
 -- wibble.gleam
@@ -23,3 +25,13 @@ warning: Unused imported module
   │ ^^^^^^^^^^^^^^^^^^^^^^ This imported module is never used
 
 Hint: You can safely remove it.
+
+warning: Shadowed Import
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub const wibble = 2
+  │ ^^^^^^^^^^^^^^^^ `wibble` is defined here
+
+Definition of wibble shadows an imported value.
+The imported value could not be used in this module anyway.
+Hint: Either rename the definition or remove the import.

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

@@ -0,0 +1,29 @@
+---
+source: compiler-core/src/type_/tests/dead_code_detection.rs
+assertion_line: 448
+expression: "\nimport wibble.{wibble}\n\npub const wibble = wibble\n"
+snapshot_kind: text
+---
+----- SOURCE CODE
+-- wibble.gleam
+
+pub const wibble = 1
+
+
+-- main.gleam
+
+import wibble.{wibble}
+
+pub const wibble = wibble
+
+
+----- WARNING
+warning: Shadowed Import
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub const wibble = wibble
+  │ ^^^^^^^^^^^^^^^^ `wibble` is defined here
+
+Definition of wibble shadows an imported value.
+The imported value could not be used in this module anyway.
+Hint: Either rename the definition or remove the import.

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

@@ -0,0 +1,37 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+assertion_line: 3950
+expression: "\nimport module.{value}\n\npub const value = 1\n"
+snapshot_kind: text
+---
+----- SOURCE CODE
+-- module.gleam
+
+pub const value = 1
+
+
+-- main.gleam
+
+import module.{value}
+
+pub const value = 1
+
+
+----- WARNING
+warning: Unused imported module
+  ┌─ /src/warning/wrn.gleam:2:1
+  │
+2 │ import module.{value}
+  │ ^^^^^^^^^^^^^^^^^^^^^ This imported module is never used
+
+Hint: You can safely remove it.
+
+warning: Shadowed Import
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub const value = 1
+  │ ^^^^^^^^^^^^^^^ `value` is defined here
+
+Definition of value shadows an imported value.
+The imported value could not be used in this module anyway.
+Hint: Either rename the definition or remove the import.

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

@@ -0,0 +1,37 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+assertion_line: 3932
+expression: "\nimport module.{wibble}\n\npub fn wibble() { Nil }\n"
+snapshot_kind: text
+---
+----- SOURCE CODE
+-- module.gleam
+
+pub fn wibble() { Nil }
+
+
+-- main.gleam
+
+import module.{wibble}
+
+pub fn wibble() { Nil }
+
+
+----- WARNING
+warning: Unused imported module
+  ┌─ /src/warning/wrn.gleam:2:1
+  │
+2 │ import module.{wibble}
+  │ ^^^^^^^^^^^^^^^^^^^^^^ This imported module is never used
+
+Hint: You can safely remove it.
+
+warning: Shadowed Import
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub fn wibble() { Nil }
+  │ ^^^^^^^^^^^^^^^ `wibble` is defined here
+
+Definition of wibble shadows an imported value.
+The imported value could not be used in this module anyway.
+Hint: Either rename the definition or remove the import.

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

@@ -1,6 +1,8 @@
 ---
 source: compiler-core/src/type_/tests/warnings.rs
+assertion_line: 974
 expression: "\n            import gleam/wibble.{one} as wobble\n            pub const one = one\n        "
+snapshot_kind: text
 ---
 ----- SOURCE CODE
 -- gleam/wibble.gleam
@@ -22,3 +24,14 @@ warning: Unused imported module alias
 Hint: You can safely remove it.
 
     import gleam/wibble as _
+
+
+warning: Shadowed Import
+  ┌─ /src/warning/wrn.gleam:3:13
+  │
+3 │             pub const one = one
+  │             ^^^^^^^^^^^^^ `one` is defined here
+
+Definition of one shadows an imported value.
+The imported value could not be used in this module anyway.
+Hint: Either rename the definition or remove the import.

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

@@ -3926,3 +3926,39 @@ pub fn main() {
 "
     );
 }
+//https://github.com/gleam-lang/gleam/issues/4666
+#[test]
+fn shadow_imported_function() {
+    assert_warning!(
+        (
+            "thepackage",
+            "module",
+            r#"
+pub fn wibble() { Nil }
+"#
+        ),
+        r#"
+import module.{wibble}
+
+pub fn wibble() { Nil }
+"#
+    );
+}
+//https://github.com/gleam-lang/gleam/issues/4666
+#[test]
+fn shadow_imported_constant() {
+    assert_warning!(
+        (
+            "thepackage",
+            "module",
+            r#"
+pub const value = 1
+"#
+        ),
+        r#"
+import module.{value}
+
+pub const value = 1
+"#
+    );
+}

compiler-core/src/warning.rs

@@ -1302,6 +1302,29 @@ can already tell whether it will be true or false.",
                         }],
                     }),
                 },
+
+                type_::Warning::TopLevelDefinitionShadowsImport { location, name } => {
+                    let text = format!(
+                        "Definition of {} shadows an imported value.
+The imported value could not be used in this module anyway.",
+                        name
+                    );
+                    Diagnostic {
+                        title: "Shadowed Import".into(),
+                        text: wrap(&text),
+                        level: diagnostic::Level::Warning,
+                        location: Some(Location {
+                            path: path.clone(),
+                            src: src.clone(),
+                            label: diagnostic::Label {
+                                text: Some(wrap(&format!("`{}` is defined here", name))),
+                                span: *location,
+                            },
+                            extra_labels: Vec::new(),
+                        }),
+                        hint: Some("Either rename the definition or remove the import.".into()),
+                    }
+                }
             },
 
             Warning::DeprecatedEnvironmentVariable { variable } => {