From 1a1d0c1bad04b7e0b30b1ba583d19c5cd168c34c Mon Sep 17 00:00:00 2001 From: Alex Hansen Date: Wed, 10 Jul 2024 15:13:42 -0700 Subject: [PATCH] In libraries, treat a top-level namespace called Main as the root of the library (#1705) Currently, if you have a library with the following structure: ``` src/ Foo.qs Bar.qs ``` And you import it: ``` { "dependencies": { "MyLibrary": ... } } ``` The API will always include at least two initial path items: the package alias (`MyLibrary`) and the namespace (`Foo` or `Bar`). Every exported item will be `MyLibrary.Foo.X()` or `MyLibrary.Bar.X()`. There's no way to construct a library export such that it is referenced at the top level, e.g. `MyLibrary.X()`. This PR solves this problem. In a library, if there's a namespace called `Main`, it is treated as the root of the library. So, if you have the below structure: ``` src/ Foo.qs Main.qs Bar.qs ``` Anything exported from `Main.qs`, say a callable `X`, will show up in the consumer of the library as `MyLibrary.X()`. The behavior of items exported from `Foo.qs` and `Bar.qs` is unchanged. --- .../qsc_data_structures/src/namespaces.rs | 6 +- compiler/qsc_frontend/src/compile/tests.rs | 2 + .../src/compile/tests/multiple_packages.rs | 169 ++++++++++++++++++ compiler/qsc_frontend/src/resolve.rs | 11 +- 4 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 compiler/qsc_frontend/src/compile/tests/multiple_packages.rs diff --git a/compiler/qsc_data_structures/src/namespaces.rs b/compiler/qsc_data_structures/src/namespaces.rs index ea9878bf0a..1a1ca33b1d 100644 --- a/compiler/qsc_data_structures/src/namespaces.rs +++ b/compiler/qsc_data_structures/src/namespaces.rs @@ -187,10 +187,14 @@ impl NamespaceTreeRoot { ns: impl Into>>, root: NamespaceId, ) -> NamespaceId { + let ns = ns.into(); + if ns.is_empty() { + return root; + } let (_root_name, root_contents) = self.find_namespace_by_id(&root); let id = root_contents .borrow_mut() - .insert_or_find_namespace(ns.into().into_iter().peekable(), &mut self.assigner); + .insert_or_find_namespace(ns.into_iter().peekable(), &mut self.assigner); id.expect("empty name should not be passed into namespace insertion") } diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index 3815e7ba8a..ad6c9c7c88 100644 --- a/compiler/qsc_frontend/src/compile/tests.rs +++ b/compiler/qsc_frontend/src/compile/tests.rs @@ -3,6 +3,8 @@ #![allow(clippy::needless_raw_string_hashes)] +mod multiple_packages; + use std::sync::Arc; use super::{compile, longest_common_prefix, CompileUnit, Error, PackageStore, SourceMap}; diff --git a/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs b/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs new file mode 100644 index 0000000000..c7eaa32516 --- /dev/null +++ b/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs @@ -0,0 +1,169 @@ +use std::sync::Arc; + +use super::{compile, PackageStore, SourceMap}; +use crate::compile::TargetCapabilityFlags; + +use crate::compile::core; +use expect_test::expect; +use expect_test::Expect; +use qsc_data_structures::language_features::LanguageFeatures; +use qsc_hir::hir::PackageId; + +/// Runs a test where each successive package relies solely on the package before it +/// useful for testing chains of reexports +fn multiple_package_check(packages: Vec<(&str, &str)>) { + multiple_package_check_inner(packages, None); +} + +fn multiple_package_check_expect_err(packages: Vec<(&str, &str)>, expect: &Expect) { + multiple_package_check_inner(packages, Some(expect)); +} + +fn multiple_package_check_inner(packages: Vec<(&str, &str)>, expect: Option<&Expect>) { + let mut store = PackageStore::new(core()); + let mut prev_id_and_name: Option<(PackageId, &str)> = None; + let num_packages = packages.len(); + for (ix, (package_name, package_source)) in packages.into_iter().enumerate() { + let is_last = ix == num_packages - 1; + let deps = if let Some((prev_id, prev_name)) = prev_id_and_name { + vec![(prev_id, Some(Arc::from(prev_name)))] + } else { + vec![] + }; + + let sources = SourceMap::new( + [( + Arc::from(format!("{package_name}.qs")), + Arc::from(package_source), + )], + None, + ); + + let unit = compile( + &store, + &deps[..], + sources, + TargetCapabilityFlags::all(), + LanguageFeatures::default(), + ); + + match (is_last, &expect) { + (true, Some(expect)) => { + expect.assert_eq(&format!("{:#?}", unit.errors)); + } + _ => { + assert!(unit.errors.is_empty(), "{:#?}", unit.errors); + } + } + + prev_id_and_name = Some((store.insert(unit), package_name)); + } +} + +#[test] +fn namespace_named_main_doesnt_create_main_namespace() { + multiple_package_check_expect_err( + vec![ + ( + "Main", + "operation Foo(x: Int, y: Bool) : Int { + x + } + export Foo;", + ), + ( + "C", + r#" + // this fails because `Main` is considered the "root" + import Main.Main.Foo; + @EntryPoint() + operation Main() : Unit { + Foo(10, true); + }"#, + ), + ], + &expect!([r#" + [ + Error( + Resolve( + NotFound( + "Main.Main.Foo", + Span { + lo: 86, + hi: 99, + }, + ), + ), + ), + Error( + Resolve( + NotFound( + "Foo", + Span { + lo: 205, + hi: 208, + }, + ), + ), + ), + Error( + Type( + Error( + AmbiguousTy( + Span { + lo: 205, + hi: 218, + }, + ), + ), + ), + ), + ]"#]), + ); +} + +#[test] +fn namespaces_named_main_treated_as_root() { + multiple_package_check(vec![ + ( + "Main", + "operation Foo(x: Int, y: Bool) : Int { + x + } + export Foo;", + ), + ( + "C", + " + // note that this is not Main.Main + // and that the namespace `Main` is omitted here + import Main.Foo; + @EntryPoint() + operation Main() : Unit { + Foo(10, true); + }", + ), + ]); +} + +#[test] +fn namespaces_named_lowercase_main_not_treated_as_root() { + multiple_package_check(vec![ + ( + "main", + "operation Foo(x: Int, y: Bool) : Int { + x + } + export Foo;", + ), + ( + "C", + " + import main.main.Foo; + @EntryPoint() + operation Main() : Unit { + Foo(10, true); + }", + ), + ]); +} diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 15f76216bc..3479bdf0b0 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -1376,9 +1376,18 @@ impl GlobalTable { global.visibility == hir::Visibility::Public || matches!(&global.kind, global::Kind::Term(t) if t.intrinsic) }) { + // If the namespace is `Main`, we treat it as the root of the package, so there's no + // namespace prefix. + let global_namespace = if global.namespace.len() == 1 && &*global.namespace[0] == "Main" + { + vec![] + } else { + global.namespace.clone() + }; + let namespace = self .scope - .insert_or_find_namespace_from_root(global.namespace.clone(), root); + .insert_or_find_namespace_from_root(global_namespace, root); match (global.kind, global.visibility) { (global::Kind::Ty(ty), hir::Visibility::Public) => {