From cd0638f6c9788393f006541ff6731af301c218ed Mon Sep 17 00:00:00 2001 From: Daniel Vallance Date: Sun, 30 Nov 2025 11:59:43 +0000 Subject: [PATCH] wit-parser: Use stability info when processing foreign deps Previously, the process_foreign_interfaces function would receive unresolved interfaces with their stability values set to Unknown This made it hard to distinguish between interfaces which had been feature gated out, and genuinely nonexistent interfaces This commit records the stability of each import of a foreign dependency when populating the self.foreign_{deps, interfaces, worlds} maps. This enables process_foreign_interfaces to push None as a placeholder if it sees that all of its imports have been feature-gated out. Otherwise, it will allow the resolver to try and resolve the import, and if the interface genuinely does not exist, it will fail. Fixes issue 2285 --- crates/wit-parser/src/ast/resolve.rs | 29 +++++--- crates/wit-parser/src/lib.rs | 2 +- crates/wit-parser/src/resolve.rs | 73 ++++++++++++++----- .../tests/ui/foreign-interface-dep-gated.wit | 17 +++++ .../ui/foreign-interface-dep-gated.wit.json | 39 ++++++++++ .../tests/ui/foreign-world-dep-gated.wit | 17 +++++ .../tests/ui/foreign-world-dep-gated.wit.json | 39 ++++++++++ tests/cli/bad-stability.wit.stderr | 13 ++-- 8 files changed, 190 insertions(+), 39 deletions(-) create mode 100644 crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit create mode 100644 crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit.json create mode 100644 crates/wit-parser/tests/ui/foreign-world-dep-gated.wit create mode 100644 crates/wit-parser/tests/ui/foreign-world-dep-gated.wit.json diff --git a/crates/wit-parser/src/ast/resolve.rs b/crates/wit-parser/src/ast/resolve.rs index a997add506..e18d73018f 100644 --- a/crates/wit-parser/src/ast/resolve.rs +++ b/crates/wit-parser/src/ast/resolve.rs @@ -46,8 +46,10 @@ pub struct Resolver<'a> { /// Metadata about foreign dependencies which are not defined in this /// package. This map is keyed by the name of the package being imported /// from. The next level of key is the name of the interface being imported - /// from, and the final value is the assigned ID of the interface. - foreign_deps: IndexMap>, + /// from, and the final value is a tuple containing the assigned ID of the + /// dependency, and a Vector of the Stability attributes associated with each + /// of its imports. + foreign_deps: IndexMap)>>, /// All interfaces that are present within `self.foreign_deps`. foreign_interfaces: HashSet, @@ -233,7 +235,9 @@ impl<'a> Resolver<'a> { ( name.clone(), deps.iter() - .map(|(name, id)| (name.to_string(), *id)) + .map(|(name, (id, stabilities))| { + (name.to_string(), (*id, stabilities.clone())) + }) .collect(), ) }) @@ -257,18 +261,20 @@ impl<'a> Resolver<'a> { let mut foreign_worlds = mem::take(&mut self.foreign_worlds); for decl_list in decl_lists { decl_list - .for_each_path(&mut |_, _attrs, path, _names, world_or_iface| { + .for_each_path(&mut |_, attrs, path, _names, world_or_iface| { let (id, name) = match path { ast::UsePath::Package { id, name } => (id, name), _ => return Ok(()), }; + let stability = self.stability(attrs)?; + let deps = foreign_deps.entry(id.package_name()).or_insert_with(|| { self.foreign_dep_spans.push(id.span); IndexMap::new() }); - let id = *deps.entry(name.name).or_insert_with(|| { - match world_or_iface { + let (id, stabilities) = deps.entry(name.name).or_insert_with(|| { + let id = match world_or_iface { WorldOrInterface::World => { log::trace!( "creating a world for foreign dep: {}/{}", @@ -287,10 +293,13 @@ impl<'a> Resolver<'a> { ); AstItem::Interface(self.alloc_interface(name.span)) } - } + }; + (id, Vec::new()) }); - let _ = match id { + stabilities.push(stability); + + let _ = match *id { AstItem::Interface(id) => foreign_interfaces.insert(id), AstItem::World(id) => foreign_worlds.insert(id), }; @@ -520,7 +529,7 @@ impl<'a> Resolver<'a> { ) })?, ast::UsePath::Package { id, name } => { - self.foreign_deps[&id.package_name()][name.name] + self.foreign_deps[&id.package_name()][name.name].0 } }; (name.name, item) @@ -1104,7 +1113,7 @@ impl<'a> Resolver<'a> { } } ast::UsePath::Package { id, name } => Ok(( - self.foreign_deps[&id.package_name()][name.name], + self.foreign_deps[&id.package_name()][name.name].0, name.name.into(), name.span, )), diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index e591e775d1..00fa2d84a4 100644 --- a/crates/wit-parser/src/lib.rs +++ b/crates/wit-parser/src/lib.rs @@ -103,7 +103,7 @@ pub struct UnresolvedPackage { /// interface name followed by the identifier within `self.interfaces`. The /// fields of `self.interfaces` describes the required types that are from /// each foreign interface. - pub foreign_deps: IndexMap>, + pub foreign_deps: IndexMap)>>, /// Doc comments for this package. pub docs: Docs, diff --git a/crates/wit-parser/src/resolve.rs b/crates/wit-parser/src/resolve.rs index 617996da3b..d073675599 100644 --- a/crates/wit-parser/src/resolve.rs +++ b/crates/wit-parser/src/resolve.rs @@ -3083,19 +3083,19 @@ impl Remap { let mut world_to_package = HashMap::new(); let mut interface_to_package = HashMap::new(); for (i, (pkg_name, worlds_or_ifaces)) in unresolved.foreign_deps.iter().enumerate() { - for (name, item) in worlds_or_ifaces { + for (name, (item, stabilities)) in worlds_or_ifaces { match item { AstItem::Interface(unresolved_interface_id) => { let prev = interface_to_package.insert( *unresolved_interface_id, - (pkg_name, name, unresolved.foreign_dep_spans[i]), + (pkg_name, name, unresolved.foreign_dep_spans[i], stabilities), ); assert!(prev.is_none()); } AstItem::World(unresolved_world_id) => { let prev = world_to_package.insert( *unresolved_world_id, - (pkg_name, name, unresolved.foreign_dep_spans[i]), + (pkg_name, name, unresolved.foreign_dep_spans[i], stabilities), ); assert!(prev.is_none()); } @@ -3106,12 +3106,12 @@ impl Remap { // Connect all interfaces referred to in `interface_to_package`, which // are at the front of `unresolved.interfaces`, to interfaces already // contained within `resolve`. - self.process_foreign_interfaces(unresolved, &interface_to_package, resolve)?; + self.process_foreign_interfaces(unresolved, &interface_to_package, resolve, &pkgid)?; // Connect all worlds referred to in `world_to_package`, which // are at the front of `unresolved.worlds`, to worlds already // contained within `resolve`. - self.process_foreign_worlds(unresolved, &world_to_package, resolve)?; + self.process_foreign_worlds(unresolved, &world_to_package, resolve, &pkgid)?; // Finally, iterate over all foreign-defined types and determine // what they map to. @@ -3146,17 +3146,20 @@ impl Remap { fn process_foreign_interfaces( &mut self, unresolved: &UnresolvedPackage, - interface_to_package: &HashMap, + interface_to_package: &HashMap)>, resolve: &mut Resolve, + parent_pkg_id: &PackageId, ) -> Result<(), anyhow::Error> { for (unresolved_iface_id, unresolved_iface) in unresolved.interfaces.iter() { - let (pkg_name, interface, span) = match interface_to_package.get(&unresolved_iface_id) { - Some(items) => *items, - // All foreign interfaces are defined first, so the first one - // which is defined in a non-foreign document means that all - // further interfaces will be non-foreign as well. - None => break, - }; + let (pkg_name, interface, span, stabilities) = + match interface_to_package.get(&unresolved_iface_id) { + Some(items) => *items, + // All foreign interfaces are defined first, so the first one + // which is defined in a non-foreign document means that all + // further interfaces will be non-foreign as well. + None => break, + }; + let pkgid = resolve .package_names .get(pkg_name) @@ -3174,6 +3177,20 @@ impl Remap { let pkg = &resolve.packages[pkgid]; let span = &unresolved.interface_spans[unresolved_iface_id.index()]; + + let mut enabled = false; + for stability in stabilities { + if resolve.include_stability(stability, parent_pkg_id, Some(span.span))? { + enabled = true; + break; + } + } + + if !enabled { + self.interfaces.push(None); + continue; + } + let iface_id = pkg .interfaces .get(interface) @@ -3194,16 +3211,18 @@ impl Remap { fn process_foreign_worlds( &mut self, unresolved: &UnresolvedPackage, - world_to_package: &HashMap, + world_to_package: &HashMap)>, resolve: &mut Resolve, + parent_pkg_id: &PackageId, ) -> Result<(), anyhow::Error> { for (unresolved_world_id, _) in unresolved.worlds.iter() { - let (pkg_name, world, span) = match world_to_package.get(&unresolved_world_id) { - Some(items) => *items, - // Same as above, all worlds are foreign until we find a - // non-foreign one. - None => break, - }; + let (pkg_name, world, span, stabilities) = + match world_to_package.get(&unresolved_world_id) { + Some(items) => *items, + // Same as above, all worlds are foreign until we find a + // non-foreign one. + None => break, + }; let pkgid = resolve .package_names @@ -3212,6 +3231,20 @@ impl Remap { .ok_or_else(|| Error::new(span, "package not found"))?; let pkg = &resolve.packages[pkgid]; let span = &unresolved.world_spans[unresolved_world_id.index()]; + + let mut enabled = false; + for stability in stabilities { + if resolve.include_stability(stability, parent_pkg_id, Some(span.span))? { + enabled = true; + break; + } + } + + if !enabled { + self.worlds.push(None); + continue; + } + let world_id = pkg .worlds .get(world) diff --git a/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit b/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit new file mode 100644 index 0000000000..25bbaf6334 --- /dev/null +++ b/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit @@ -0,0 +1,17 @@ +package a:b1; + +world the-world { + include a:b2/the-world; +} + +package a:b2 { + world the-world { + @unstable(feature = disabled) + import a:b3/thing; + } +} + +package a:b3 { + @unstable(feature = disabled) + interface thing {} +} diff --git a/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit.json b/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit.json new file mode 100644 index 0000000000..58e43977ea --- /dev/null +++ b/crates/wit-parser/tests/ui/foreign-interface-dep-gated.wit.json @@ -0,0 +1,39 @@ +{ + "worlds": [ + { + "name": "the-world", + "imports": {}, + "exports": {}, + "package": 1 + }, + { + "name": "the-world", + "imports": {}, + "exports": {}, + "package": 2 + } + ], + "interfaces": [], + "types": [], + "packages": [ + { + "name": "a:b3", + "interfaces": {}, + "worlds": {} + }, + { + "name": "a:b2", + "interfaces": {}, + "worlds": { + "the-world": 0 + } + }, + { + "name": "a:b1", + "interfaces": {}, + "worlds": { + "the-world": 1 + } + } + ] +} \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit b/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit new file mode 100644 index 0000000000..b7a55f20a1 --- /dev/null +++ b/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit @@ -0,0 +1,17 @@ +package a:b1; + +world the-world { + include a:b2/the-world; +} + +package a:b2 { + world the-world { + @unstable(feature = disabled) + import a:b3/another-world; + } +} + +package a:b3 { + @unstable(feature = disabled) + world another-world {} +} diff --git a/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit.json b/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit.json new file mode 100644 index 0000000000..58e43977ea --- /dev/null +++ b/crates/wit-parser/tests/ui/foreign-world-dep-gated.wit.json @@ -0,0 +1,39 @@ +{ + "worlds": [ + { + "name": "the-world", + "imports": {}, + "exports": {}, + "package": 1 + }, + { + "name": "the-world", + "imports": {}, + "exports": {}, + "package": 2 + } + ], + "interfaces": [], + "types": [], + "packages": [ + { + "name": "a:b3", + "interfaces": {}, + "worlds": {} + }, + { + "name": "a:b2", + "interfaces": {}, + "worlds": { + "the-world": 0 + } + }, + { + "name": "a:b1", + "interfaces": {}, + "worlds": { + "the-world": 1 + } + } + ] +} \ No newline at end of file diff --git a/tests/cli/bad-stability.wit.stderr b/tests/cli/bad-stability.wit.stderr index e7176904bf..a1a58637a0 100644 --- a/tests/cli/bad-stability.wit.stderr +++ b/tests/cli/bad-stability.wit.stderr @@ -1,8 +1,5 @@ -error: failed to process world item in `c` - -Caused by: - 0: package [a:b] contains a feature gate with a version specifier, so it must have a version - --> tests/cli/bad-stability.wit:7:14 - | - 7 | import d:e/f-g-h@0.2.3; - | ^---- +error: package [a:b] contains a feature gate with a version specifier, so it must have a version + --> tests/cli/bad-stability.wit:7:14 + | + 7 | import d:e/f-g-h@0.2.3; + | ^----