From 8106f2fde4e1db9a071c466fe86d6e3a94a468fc Mon Sep 17 00:00:00 2001 From: Alex Hansen Date: Thu, 30 Jan 2025 14:30:38 -0800 Subject: [PATCH] Fix UDT re-exports (#2137) Closes #2122 --- .../src/compile/tests/multiple_packages.rs | 93 +++++++++++++++++++ compiler/qsc_frontend/src/resolve.rs | 8 +- compiler/qsc_frontend/src/typeck/convert.rs | 11 +-- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs b/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs index 7fb0eb5e91..d471b8527d 100644 --- a/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs +++ b/compiler/qsc_frontend/src/compile/tests/multiple_packages.rs @@ -61,6 +61,54 @@ fn multiple_package_check_inner(packages: Vec<(&str, &str)>, expect: Option<&Exp } } +/// This can be used to test multiple packages which internally have multiple source files, as opposed to the more simple `multiple_package_check` +/// which only allows one source file per package (for easy and quick test creation). +fn multiple_package_multiple_source_check( + packages: Vec<(&str, 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, sources)) 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( + sources.iter().map(|(name, source)| { + ( + Arc::from(format!("{package_name}/{name}.qs")), + Arc::from(*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( @@ -509,3 +557,48 @@ fn aliased_export_via_aliased_import() { ), ]); } + +#[test] +fn udt_reexport_with_alias() { + multiple_package_multiple_source_check( + vec![ + ( + "A", + vec![ + ("FileOne", "struct Foo { content: Bool }"), + ("FileTwo", "export FileOne.Foo as Bar;"), + ], + ), + ("B", vec![("FileThree", "export A.FileTwo.Bar as Baz;")]), + ( + "C", + vec![( + "FileFour", + "@EntryPoint() + operation Main() : Unit { + let x = new B.FileThree.Baz { content = true }; + }", + )], + ), + ], + Some(&expect!["[]"]), + ); +} + +#[test] +fn callable_reexport() { + multiple_package_check(vec![ + ( + "A", + "function Foo() : Unit { } + export Foo as Bar;", + ), + ( + "B", + " @EntryPoint() + operation Main() : Unit { + let x = A.A.Bar(); + }", + ), + ]); +} diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index f02f1113ed..5c3eeb0185 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -1676,10 +1676,10 @@ impl GlobalTable { .insert_or_find_namespace(ns.iter().map(|s| s.name.clone())); } hir::ItemKind::Ty(..) => { - self.scope - .tys - .get_mut_or_default(namespace) - .insert(global.name.clone(), Res::ExportedItem(item_id, None)); + self.scope.tys.get_mut_or_default(namespace).insert( + global.name.clone(), + Res::Item(item_id, ItemStatus::Available), + ); } hir::ItemKind::Export(_, _) => { unreachable!("find_item will never return an Export") diff --git a/compiler/qsc_frontend/src/typeck/convert.rs b/compiler/qsc_frontend/src/typeck/convert.rs index 17cfc4324f..f02adc397e 100644 --- a/compiler/qsc_frontend/src/typeck/convert.rs +++ b/compiler/qsc_frontend/src/typeck/convert.rs @@ -117,13 +117,12 @@ pub(super) fn ty_from_path(names: &Names, path: &Path) -> Ty { // So realistically, by construction, `Param` here is unreachable. // A path can also never resolve to an export, because in typeck/check, // we resolve exports to their original definition. - Some( - resolve::Res::Local(_) | resolve::Res::Param { .. } | resolve::Res::ExportedItem(_, _), - ) => { - unreachable!( - "A path should never resolve \ + Some(resolve::Res::Local(_) | resolve::Res::Param { .. }) => unreachable!( + " A path should never resolve \ to a local or a parameter, as there is syntactic differentiation." - ) + ), + Some(resolve::Res::ExportedItem(item_id, alias)) => { + unreachable!("Exported items should have been resolved to their original definition in type checking. Found {:?} with alias {:?}", item_id, alias); } None => Ty::Err, }