fix(analyse): warn if a function definition shadows an imported one

Author: arielherself

compiler-core/src/analyse.rs

@@ -248,6 +248,15 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
         }
 
         for f in &statements.functions {
+            let (name_location, name) = f.name.clone().expect("A module's function must be named");
+            if env.unqualified_imported_names.contains_key(&name) {
+                self.problems
+                    .warning(Warning::TopLevelDefinitionShadowsImport {
+                        location: name_location,
+                        name,
+                    });
+            }
+
             if let Err(error) = self.register_value_from_function(f, &mut env) {
                 return self.all_errors(error);
             }

compiler-core/src/type_/error.rs

@@ -979,6 +979,13 @@ pub enum Warning {
         truncation: BitArraySegmentTruncation,
         location: SrcSpan,
     },
+
+    /// Top-level definition should not shadow an imported one.
+    /// This includes type imports and function imports.
+    TopLevelDefinitionShadowsImport {
+        location: SrcSpan,
+        name: EcoString,
+    },
 }
 
 #[derive(Debug, Eq, Copy, PartialEq, Clone, serde::Serialize, serde::Deserialize)]
@@ -1202,6 +1209,7 @@ impl Warning {
             | Warning::FeatureRequiresHigherGleamVersion { location, .. }
             | Warning::JavaScriptIntUnsafe { location, .. }
             | Warning::AssertLiteralValue { location, .. }
+            | Warning::TopLevelDefinitionShadowsImport { location, .. }
             | Warning::BitArraySegmentTruncatedValue { location, .. } => *location,
         }
     }

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

@@ -0,0 +1,36 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+expression: "\nimport one.{foo}\nfn foo(x) {\n    let _ = x + 1\n}\npub fn main() {\n    let _ = foo(1)\n}\n"
+---
+----- SOURCE CODE
+-- one.gleam
+pub fn foo(x) { let _ = x }
+
+-- main.gleam
+
+import one.{foo}
+fn foo(x) {
+    let _ = x + 1
+}
+pub fn main() {
+    let _ = foo(1)
+}
+
+
+----- WARNING
+warning: Unused imported value
+  ┌─ /src/warning/wrn.gleam:2:13
+  │
+2 │ import one.{foo}
+  │             ^^^ This imported value is never used
+
+Hint: You can safely remove it.
+
+warning: Definition of "foo" shadows an imported item
+  ┌─ /src/warning/wrn.gleam:3:4
+  │
+3 │ fn foo(x) {
+  │    ^^^
+
+The imported item could not be used anyway.
+Hint: You could remove the imported item.

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

@@ -458,6 +458,22 @@ pub const make_two = one.Two
     );
 }
 
+#[test]
+fn shadow_imported_function() {
+    assert_warning!(
+        ("thepackage", "one", "pub fn foo(x) { let _ = x }"),
+        "
+import one.{foo}
+fn foo(x) {
+    let _ = x + 1
+}
+pub fn main() {
+    let _ = foo(1)
+}
+"
+    );
+}
+
 // https://github.com/gleam-lang/gleam/issues/2050
 #[test]
 fn double_unary_integer_literal() {

compiler-core/src/warning.rs

@@ -1276,6 +1276,21 @@ can already tell whether it will be true or false.",
                         extra_labels: Vec::new(),
                     }),
                 },
+                type_::Warning::TopLevelDefinitionShadowsImport { location, name } => Diagnostic {
+                    title: format!("Definition of \"{name}\" shadows an imported item"),
+                    text: wrap("The imported item could not be used anyway."),
+                    hint: Some("You could remove the imported item.".into()),
+                    level: diagnostic::Level::Warning,
+                    location: Some(Location {
+                        src: src.clone(),
+                        path: path.to_path_buf(),
+                        label: diagnostic::Label {
+                            text: None,
+                            span: *location,
+                        },
+                        extra_labels: vec![],
+                    }),
+                },
             },
 
             Warning::DeprecatedEnvironmentVariable { variable } => {