Warn if a top-level definition shadows an imported one

CHANGELOG.md

@@ -185,6 +185,29 @@
 - The compiler will not generate needless code for unused pattern variables.
   ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
 
+- The compiler will now emit a warning when a top-level function/constant definition shadows
+  an imported value with the same name.
+
+  ```gleam
+  import module.{fun}
+
+  fn fun() {}
+  ```
+
+  Will produce the following warning:
+
+  ```
+  warning: Definition of "fun" shadows an imported value
+    ┌─ /src/main.gleam:3:4
+    │
+  3 │ fn fun() {}
+    │    ^^^
+
+  The imported value could not be used in functions anyway.
+  ```
+
+  ([Ariel Parker](https://github.com/arielherself))
+
 ### Build tool
 
 - The build tool now supports placing modules in a directory called `dev`,

compiler-core/src/analyse.rs

@@ -7,7 +7,7 @@ mod tests;
 use crate::{
     GLEAM_CORE_PACKAGE_NAME,
     ast::{
-        self, Arg, BitArrayOption, CustomType, Definition, DefinitionLocation, Function,
+        self, Arg, BitArrayOption, Constant, CustomType, Definition, DefinitionLocation, Function,
         GroupedStatements, Import, ModuleConstant, Publicity, RecordConstructor,
         RecordConstructorArg, SrcSpan, Statement, TypeAlias, TypeAst, TypeAstConstructor,
         TypeAstFn, TypeAstHole, TypeAstTuple, TypeAstVar, TypedDefinition, TypedExpr,
@@ -395,6 +395,14 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
         } = c;
         self.check_name_case(name_location, &name, Named::Constant);
 
+        if environment.unqualified_imported_names.contains_key(&name) {
+            self.problems
+                .warning(Warning::TopLevelDefinitionShadowsImport {
+                    location: c.location,
+                    name: name.clone(),
+                })
+        }
+
         environment.references.begin_constant();
 
         let definition = FunctionDefinition {
@@ -1424,6 +1432,14 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
 
         self.check_name_case(*name_location, name, Named::Function);
 
+        if environment.unqualified_imported_names.contains_key(name) {
+            self.problems
+                .warning(Warning::TopLevelDefinitionShadowsImport {
+                    location: f.location,
+                    name: name.clone(),
+                });
+        }
+
         environment.references.register_value(
             name.clone(),
             EntityKind::Function,

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/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

@@ -23,3 +23,11 @@ warning: Unused imported value
   │                ^^^^^^ This imported value is never used
 
 Hint: You can safely remove it.
+
+warning: Definition of "wibble" shadows an imported value
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub const wibble = 2
+  │ ^^^^^^^^^^^^^^^^
+
+The imported value could not be used in functions anyway.

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

@@ -0,0 +1,25 @@
+---
+source: compiler-core/src/type_/tests/dead_code_detection.rs
+expression: "\nimport wibble.{wibble}\n\npub const wibble = wibble\n"
+---
+----- SOURCE CODE
+-- wibble.gleam
+
+pub const wibble = 1
+
+
+-- main.gleam
+
+import wibble.{wibble}
+
+pub const wibble = wibble
+
+
+----- WARNING
+warning: Definition of "wibble" shadows an imported value
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ pub const wibble = wibble
+  │ ^^^^^^^^^^^^^^^^
+
+The imported value could not be used in functions anyway.

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

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

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

@@ -0,0 +1,39 @@
+---
+source: compiler-core/src/type_/tests/warnings.rs
+expression: "\nimport one.{x}\n\nconst x = 9\n"
+---
+----- SOURCE CODE
+-- one.gleam
+pub const x = 10
+
+-- main.gleam
+
+import one.{x}
+
+const x = 9
+
+
+----- WARNING
+warning: Unused imported value
+  ┌─ /src/warning/wrn.gleam:2:13
+  │
+2 │ import one.{x}
+  │             ^ This imported value is never used
+
+Hint: You can safely remove it.
+
+warning: Definition of "x" shadows an imported value
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ const x = 9
+  │ ^^^^^^^
+
+The imported value could not be used in functions anyway.
+
+warning: Unused private constant
+  ┌─ /src/warning/wrn.gleam:4:1
+  │
+4 │ const x = 9
+  │ ^^^^^^^ This private constant is never used
+
+Hint: You can safely remove it.

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

@@ -22,3 +22,12 @@ warning: Unused imported module alias
 Hint: You can safely remove it.
 
     import gleam/wibble as _
+
+
+warning: Definition of "one" shadows an imported value
+  ┌─ /src/warning/wrn.gleam:3:13
+  │
+3 │             pub const one = one
+  │             ^^^^^^^^^^^^^
+
+The imported value could not be used in functions anyway.

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

@@ -458,6 +458,34 @@ pub const make_two = one.Two
     );
 }
 
+#[test]
+fn shadow_imported_function() {
+    assert_warning!(
+        ("thepackage", "one", "pub fn fun(x) { let _ = x }"),
+        "
+import one.{fun}
+fn fun(x) {
+    let _ = x + 1
+}
+pub fn main() {
+    let _ = fun(1)
+}
+"
+    );
+}
+
+#[test]
+fn shadow_imported_variable() {
+    assert_warning!(
+        ("thepackage", "one", "pub const x = 10"),
+        "
+import one.{x}
+
+const x = 9
+"
+    );
+}
+
 // 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 value"),
+                    text: wrap("The imported value could not be used in functions anyway."),
+                    hint: None,
+                    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 } => {