address review comment

Author: giacomocavalieri

compiler-cli/src/compile_package.rs

@@ -9,7 +9,6 @@ use gleam_core::{
     build::{
         Mode, NullTelemetry, PackageCompiler, StaleTracker, Target, TargetCodegenConfiguration,
     },
-    config::PackageKind,
     metadata,
     paths::{self, ProjectPaths},
     type_::ModuleInterface,
@@ -39,7 +38,6 @@ pub fn command(options: CompilePackage) -> Result<()> {
     tracing::info!("Compiling package");
 
     let mut compiler = PackageCompiler::new(
-        PackageKind::Root,
         &config,
         Mode::Dev,
         &options.package_directory,

compiler-core/src/build/package_compiler.rs

@@ -3,7 +3,7 @@ mod tests;
 
 use crate::analyse::{ModuleAnalyzerConstructor, TargetSupport};
 use crate::build::package_loader::CacheFiles;
-use crate::config::PackageKind;
+
 use crate::error::DefinedModuleOrigin;
 use crate::inline;
 use crate::io::files_with_extension;
@@ -54,7 +54,6 @@ pub struct PackageCompiler<'a, IO> {
     pub root: &'a Utf8Path,
     pub mode: Mode,
     pub target: &'a TargetCodegenConfiguration,
-    pub package_kind: PackageKind,
     pub config: &'a PackageConfig,
     pub ids: UniqueIdGenerator,
     pub write_metadata: bool,
@@ -80,7 +79,6 @@ where
     IO: FileSystemReader + FileSystemWriter + CommandExecutor + BeamCompiler + Clone,
 {
     pub fn new(
-        package_kind: PackageKind,
         config: &'a PackageConfig,
         mode: Mode,
         root: &'a Utf8Path,
@@ -97,7 +95,6 @@ where
             lib,
             root,
             mode,
-            package_kind,
             config,
             target,
             write_metadata: true,
@@ -145,7 +142,6 @@ where
             self.io.clone(),
             self.ids.clone(),
             self.mode,
-            self.package_kind.clone(),
             self.root,
             self.cached_warnings,
             warnings,

…g_same_module_as_another_dependency.snap …_packages_defining_duplicate_module.snapcompiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__dependency_defining_same_module_as_another_dependency.snap renamed to compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__different_packages_defining_duplicate_module.snap compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__dependency_defining_same_module_as_another_dependency.snap renamed to compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__different_packages_defining_duplicate_module.snap

@@ -6,5 +6,5 @@ error: Duplicate module
 
 The module `a_module` is defined multiple times.
 
-It is first defined by dependency `another_dependency`
-It is defined a second time by dependency `a_dependency`
+It is first defined by the package dep1
+It is defined a second time by the package a_package

…efining_same_module_as_root_package.snap …e_package_defining_duplicate_module.snapcompiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__dependency_defining_same_module_as_root_package.snap renamed to compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__same_package_defining_duplicate_module.snap compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__dependency_defining_same_module_as_root_package.snap renamed to compiler-core/src/build/package_compiler/snapshots/gleam_core__build__package_compiler__tests__same_package_defining_duplicate_module.snap

@@ -6,6 +6,5 @@ error: Duplicate module
 
 The module `a_module` is defined multiple times.
 
-It is first defined by dependency `dep1`
-It is defined a second time at `/src/a_module.gleam`
-Hint: consider renaming the `a_module` module in your project
+It is first defined at a_package/a_module.gleam
+It is defined a second time at /src/a_module.gleam

compiler-core/src/build/package_compiler/tests.rs

@@ -9,17 +9,17 @@ use crate::{
         self, NullTelemetry, Outcome, PackageCompiler, StaleTracker, Target,
         TargetCodegenConfiguration, Telemetry, package_compiler::Compiled,
     },
-    config::{PackageConfig, PackageKind},
+    config::PackageConfig,
     error::DefinedModuleOrigin,
     io::{FileSystemWriter, memory::InMemoryFileSystem},
     uid::UniqueIdGenerator,
     warning::WarningEmitter,
 };
 
 fn compile_modules(
-    package_kind: PackageKind,
+    package_name: &str,
     module: &str,
-    dependency_modules: Vec<(&str, &str)>,
+    existing_modules: Vec<(&str, &str)>,
 ) -> Outcome<Compiled, Error> {
     let mut fs = InMemoryFileSystem::new();
     fs.write(
@@ -28,9 +28,10 @@ fn compile_modules(
     )
     .expect("write module");
 
-    let config = PackageConfig::default();
+    let mut config = PackageConfig::default();
+    config.name = package_name.into();
+
     let compiler = PackageCompiler::new(
-        package_kind,
         &config,
         build::Mode::Dev,
         Utf8Path::new("/"),
@@ -41,13 +42,15 @@ fn compile_modules(
         fs,
     );
 
-    let mut already_defined_modules = dependency_modules
+    let mut already_defined_modules = existing_modules
         .into_iter()
         .map(|(package_name, module_name)| {
             (
                 EcoString::from(module_name),
-                DefinedModuleOrigin::Dependency {
+                DefinedModuleOrigin {
                     package_name: EcoString::from(package_name),
+                    path: Utf8Path::new(&format!("{package_name}/{module_name}.gleam"))
+                        .to_path_buf(),
                 },
             )
         })
@@ -64,8 +67,8 @@ fn compile_modules(
 }
 
 #[test]
-pub fn dependency_defining_same_module_as_root_package() {
-    let output = compile_modules(PackageKind::Root, "a_module", vec![("dep1", "a_module")])
+pub fn different_packages_defining_duplicate_module() {
+    let output = compile_modules("a_package", "a_module", vec![("dep1", "a_module")])
         .into_result()
         .expect_err("should produce an error")
         .pretty_string();
@@ -74,17 +77,11 @@ pub fn dependency_defining_same_module_as_root_package() {
 }
 
 #[test]
-pub fn dependency_defining_same_module_as_another_dependency() {
-    let output = compile_modules(
-        PackageKind::Dependency {
-            package_name: EcoString::from("a_dependency"),
-        },
-        "a_module",
-        vec![("another_dependency", "a_module")],
-    )
-    .into_result()
-    .expect_err("should produce an error")
-    .pretty_string();
+pub fn same_package_defining_duplicate_module() {
+    let output = compile_modules("a_package", "a_module", vec![("a_package", "a_module")])
+        .into_result()
+        .expect_err("should produce an error")
+        .pretty_string();
 
     insta::assert_snapshot!(insta::internals::AutoName, output);
 }

compiler-core/src/build/package_loader.rs

@@ -18,7 +18,7 @@ use crate::{
     Error, Result,
     ast::SrcSpan,
     build::{Module, Origin, module_loader::ModuleLoader},
-    config::{PackageConfig, PackageKind},
+    config::PackageConfig,
     dep_tree,
     error::{DefinedModuleOrigin, FileIoAction, FileKind, ImportCycleLocationDetails},
     io::{self, CommandExecutor, FileSystemReader, FileSystemWriter, files_with_extension},
@@ -58,7 +58,6 @@ pub struct PackageLoader<'a, IO> {
     io: IO,
     ids: UniqueIdGenerator,
     mode: Mode,
-    package_kind: PackageKind,
     paths: ProjectPaths,
     warnings: &'a WarningEmitter,
     codegen: CodegenRequired,
@@ -79,7 +78,6 @@ where
         io: IO,
         ids: UniqueIdGenerator,
         mode: Mode,
-        package_kind: PackageKind,
         root: &'a Utf8Path,
         cached_warnings: CachedWarnings,
         warnings: &'a WarningEmitter,
@@ -95,7 +93,6 @@ where
             io,
             ids,
             mode,
-            package_kind,
             paths: ProjectPaths::new(root.into()),
             warnings,
             codegen,
@@ -234,7 +231,7 @@ where
         let span = tracing::info_span!("load");
         let _enter = span.enter();
 
-        let mut inputs = Inputs::new(self.already_defined_modules);
+        let mut inputs = Inputs::new(self.package_name.clone(), self.already_defined_modules);
 
         let src = self.paths.src_directory();
         let mut loader = ModuleLoader {
@@ -254,7 +251,7 @@ where
             match file {
                 Ok(file) => {
                     let input = loader.load(file)?;
-                    inputs.insert(input, self.package_kind.clone())?;
+                    inputs.insert(input)?;
                 }
                 Err(warning) => self.warnings.emit(warning),
             }
@@ -269,7 +266,7 @@ where
                 match file {
                     Ok(file) => {
                         let input = loader.load(file)?;
-                        inputs.insert(input, self.package_kind.clone())?;
+                        inputs.insert(input)?;
                     }
                     Err(warning) => self.warnings.emit(warning),
                 }
@@ -282,7 +279,7 @@ where
                 match file {
                     Ok(file) => {
                         let input = loader.load(file)?;
-                        inputs.insert(input, self.package_kind.clone())?;
+                        inputs.insert(input)?;
                     }
                     Err(warning) => self.warnings.emit(warning),
                 }
@@ -1691,30 +1688,32 @@ impl StaleTracker {
 
 #[derive(Debug)]
 pub struct Inputs<'a> {
+    /// The name of the package for which we're loading the inputs.
+    package: EcoString,
     collection: HashMap<EcoString, (DefinedModuleOrigin, Input)>,
     already_defined_modules: &'a mut im::HashMap<EcoString, DefinedModuleOrigin>,
 }
 
 impl<'a> Inputs<'a> {
-    fn new(already_defined_modules: &'a mut im::HashMap<EcoString, DefinedModuleOrigin>) -> Self {
+    fn new(
+        package: EcoString,
+        already_defined_modules: &'a mut im::HashMap<EcoString, DefinedModuleOrigin>,
+    ) -> Self {
         Self {
+            package,
             collection: Default::default(),
             already_defined_modules,
         }
     }
 
     /// Insert a module into the hashmap. If there is already a module with the
     /// same name then an error is returned.
-    fn insert(&mut self, input: Input, origin: PackageKind) -> Result<()> {
+    fn insert(&mut self, input: Input) -> Result<()> {
         let name = input.name().clone();
 
-        let origin = match origin {
-            PackageKind::Root => DefinedModuleOrigin::RootProject {
-                path: input.source_path().to_path_buf(),
-            },
-            PackageKind::Dependency { package_name } => {
-                DefinedModuleOrigin::Dependency { package_name }
-            }
+        let origin = DefinedModuleOrigin {
+            package_name: self.package.clone(),
+            path: input.source_path().to_path_buf(),
         };
 
         if let Some(first) = self

compiler-core/src/build/package_loader/tests.rs

@@ -5,7 +5,6 @@ use super::*;
 use crate::{
     Warning,
     build::SourceFingerprint,
-    config::PackageKind,
     io::{FileSystemWriter, memory::InMemoryFileSystem},
     line_numbers,
     parse::extra::ModuleExtra,
@@ -84,7 +83,6 @@ fn run_loader(fs: InMemoryFileSystem, root: &Utf8Path, artefact: &Utf8Path) -> L
     let (emitter, warnings) = WarningEmitter::vector();
 
     let loader = PackageLoader {
-        package_kind: PackageKind::Root,
         io: fs.clone(),
         ids,
         mode: Mode::Dev,

compiler-core/src/build/project_compiler.rs

@@ -9,7 +9,7 @@ use crate::{
         telemetry::Telemetry,
     },
     codegen::{self, ErlangApp},
-    config::{PackageConfig, PackageKind},
+    config::PackageConfig,
     dep_tree,
     error::{DefinedModuleOrigin, FileIoAction, FileKind, ShellCommandFailureReason},
     io::{BeamCompiler, Command, CommandExecutor, FileSystemReader, FileSystemWriter, Stdio},
@@ -584,16 +584,7 @@ where
             },
         };
 
-        let package_kind = if is_root {
-            PackageKind::Root
-        } else {
-            PackageKind::Dependency {
-                package_name: config.name.clone(),
-            }
-        };
-
         let mut compiler = PackageCompiler::new(
-            package_kind,
             config,
             mode,
             &root_path,

compiler-core/src/config.rs

@@ -188,12 +188,6 @@ pub struct PackageConfig {
     pub tools: Table,
 }
 
-#[derive(Debug, Clone)]
-pub enum PackageKind {
-    Root,
-    Dependency { package_name: EcoString },
-}
-
 pub fn serialise_gleam_version<S>(
     gleam_gersion: &Option<GleamVersion>,
     serializer: S,

compiler-core/src/docs/tests.rs

@@ -14,7 +14,7 @@ use crate::{
         self, Mode, NullTelemetry, Origin, PackageCompiler, StaleTracker,
         TargetCodegenConfiguration,
     },
-    config::{DocsPage, PackageConfig, PackageKind, Repository},
+    config::{DocsPage, PackageConfig, Repository},
     docs::{DocContext, search_item_for_module, search_item_for_type, search_item_for_value},
     io::{FileSystemWriter, memory::InMemoryFileSystem},
     paths::ProjectPaths,
@@ -69,17 +69,8 @@ fn compile_with_markdown_pages(
     let build = root.join("build");
     let lib = root.join("lib");
     let paths = ProjectPaths::new(root.clone());
-    let mut compiler = PackageCompiler::new(
-        PackageKind::Root,
-        &config,
-        Mode::Dev,
-        &root,
-        &build,
-        &lib,
-        &target,
-        ids,
-        fs,
-    );
+    let mut compiler =
+        PackageCompiler::new(&config, Mode::Dev, &root, &build, &lib, &target, ids, fs);
     compiler.write_entrypoint = false;
     compiler.write_metadata = false;
     compiler.compile_beam_bytecode = true;