diff --git a/CHANGELOG.md b/CHANGELOG.md index eb17e58be6f..7469195fe4c 100644 --- a/CHANGELOG.md +++ b/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`, diff --git a/compiler-core/src/analyse.rs b/compiler-core/src/analyse.rs index f9bccfe4c46..f4f64fbbf8b 100644 --- a/compiler-core/src/analyse.rs +++ b/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, diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index 66bab2e98d9..d3ff012ecbb 100644 --- a/compiler-core/src/type_/error.rs +++ b/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, } } diff --git a/compiler-core/src/type_/tests/dead_code_detection.rs b/compiler-core/src/type_/tests/dead_code_detection.rs index 3f23c6007dd..45051150293 100644 --- a/compiler-core/src/type_/tests/dead_code_detection.rs +++ b/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", diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__shadowed_imported_value_marked_unused.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__shadowed_imported_value_marked_unused.snap index 9f1a4562045..dccf1cb97a0 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__shadowed_imported_value_marked_unused.snap +++ b/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. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__used_shadowed_imported_value.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__dead_code_detection__used_shadowed_imported_value.snap new file mode 100644 index 00000000000..02cf6ffd10a --- /dev/null +++ b/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. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__shadow_imported_function.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__shadow_imported_function.snap new file mode 100644 index 00000000000..3a3c879e97c --- /dev/null +++ b/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. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__shadow_imported_variable.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__shadow_imported_variable.snap new file mode 100644 index 00000000000..227846fdbe5 --- /dev/null +++ b/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. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_alias_warning_test.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_alias_warning_test.snap index 1dde799623d..39fda4ef6f7 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_alias_warning_test.snap +++ b/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. diff --git a/compiler-core/src/type_/tests/warnings.rs b/compiler-core/src/type_/tests/warnings.rs index 545ae29d301..ab57201b1c9 100644 --- a/compiler-core/src/type_/tests/warnings.rs +++ b/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() { diff --git a/compiler-core/src/warning.rs b/compiler-core/src/warning.rs index 51e73094f58..b6b574d8a92 100644 --- a/compiler-core/src/warning.rs +++ b/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 } => {