Fix handling stability of use of "foreign" types (#2046)

This commit addresses an issue where stability attributes on a `use`
didn't quite work as expected when a type to another package was
referred to. This fix was to update the "merging" process to skip types
being processed in one more location which involved threading some more
contexts around. Additionally `use` items, when elaborated, now contain
their stability instead of the default stability to ensure that's
propagated correctly as well.

cc #1995 but doesn't fix it

Author: alexcrichton

crates/wit-parser/src/ast.rs

@@ -142,6 +142,7 @@ impl<'a> DeclList<'a> {
         &'b self,
         f: &mut dyn FnMut(
             Option<&'b Id<'a>>,
+            &'b [Attribute<'a>],
             &'b UsePath<'a>,
             Option<&'b [UseName<'a>]>,
             WorldOrInterface,
@@ -158,49 +159,66 @@ impl<'a> DeclList<'a> {
                     let mut exports = Vec::new();
                     for item in world.items.iter() {
                         match item {
-                            WorldItem::Use(u) => {
-                                f(None, &u.from, Some(&u.names), WorldOrInterface::Interface)?
-                            }
-                            WorldItem::Include(i) => {
-                                f(Some(&world.name), &i.from, None, WorldOrInterface::World)?
-                            }
+                            WorldItem::Use(u) => f(
+                                None,
+                                &u.attributes,
+                                &u.from,
+                                Some(&u.names),
+                                WorldOrInterface::Interface,
+                            )?,
+                            WorldItem::Include(i) => f(
+                                Some(&world.name),
+                                &i.attributes,
+                                &i.from,
+                                None,
+                                WorldOrInterface::World,
+                            )?,
                             WorldItem::Type(_) => {}
-                            WorldItem::Import(Import { kind, .. }) => imports.push(kind),
-                            WorldItem::Export(Export { kind, .. }) => exports.push(kind),
+                            WorldItem::Import(Import {
+                                kind, attributes, ..
+                            }) => imports.push((kind, attributes)),
+                            WorldItem::Export(Export {
+                                kind, attributes, ..
+                            }) => exports.push((kind, attributes)),
                         }
                     }
 
-                    let mut visit_kind = |kind: &'b ExternKind<'a>| match kind {
-                        ExternKind::Interface(_, items) => {
-                            for item in items {
-                                match item {
-                                    InterfaceItem::Use(u) => f(
-                                        None,
-                                        &u.from,
-                                        Some(&u.names),
-                                        WorldOrInterface::Interface,
-                                    )?,
-                                    _ => {}
+                    let mut visit_kind =
+                        |kind: &'b ExternKind<'a>, attrs: &'b [Attribute<'a>]| match kind {
+                            ExternKind::Interface(_, items) => {
+                                for item in items {
+                                    match item {
+                                        InterfaceItem::Use(u) => f(
+                                            None,
+                                            &u.attributes,
+                                            &u.from,
+                                            Some(&u.names),
+                                            WorldOrInterface::Interface,
+                                        )?,
+                                        _ => {}
+                                    }
                                 }
+                                Ok(())
                             }
-                            Ok(())
-                        }
-                        ExternKind::Path(path) => f(None, path, None, WorldOrInterface::Interface),
-                        ExternKind::Func(..) => Ok(()),
-                    };
+                            ExternKind::Path(path) => {
+                                f(None, attrs, path, None, WorldOrInterface::Interface)
+                            }
+                            ExternKind::Func(..) => Ok(()),
+                        };
 
-                    for kind in imports {
-                        visit_kind(kind)?;
+                    for (kind, attrs) in imports {
+                        visit_kind(kind, attrs)?;
                     }
-                    for kind in exports {
-                        visit_kind(kind)?;
+                    for (kind, attrs) in exports {
+                        visit_kind(kind, attrs)?;
                     }
                 }
                 AstItem::Interface(i) => {
                     for item in i.items.iter() {
                         match item {
                             InterfaceItem::Use(u) => f(
                                 Some(&i.name),
+                                &u.attributes,
                                 &u.from,
                                 Some(&u.names),
                                 WorldOrInterface::Interface,
@@ -212,7 +230,13 @@ impl<'a> DeclList<'a> {
                 AstItem::Use(u) => {
                     // At the top-level, we don't know if this is a world or an interface
                     // It is up to the resolver to decides how to handle this ambiguity.
-                    f(None, &u.item, None, WorldOrInterface::Unknown)?;
+                    f(
+                        None,
+                        &u.attributes,
+                        &u.item,
+                        None,
+                        WorldOrInterface::Unknown,
+                    )?;
                 }
 
                 AstItem::Package(pkg) => pkg.decl_list.for_each_path(f)?,

crates/wit-parser/src/ast/resolve.rs

@@ -257,7 +257,7 @@ 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 |_, 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(()),
@@ -433,7 +433,7 @@ impl<'a> Resolver<'a> {
 
             // With this file's namespace information look at all `use` paths
             // and record dependencies between interfaces.
-            decl_list.for_each_path(&mut |iface, path, _names, _| {
+            decl_list.for_each_path(&mut |iface, _attrs, path, _names, _| {
                 // If this import isn't contained within an interface then it's
                 // in a world and it doesn't need to participate in our
                 // topo-sort.
@@ -561,11 +561,12 @@ impl<'a> Resolver<'a> {
     fn populate_foreign_types(&mut self, decl_lists: &[ast::DeclList<'a>]) -> Result<()> {
         for (i, decl_list) in decl_lists.iter().enumerate() {
             self.cur_ast_index = i;
-            decl_list.for_each_path(&mut |_, path, names, _| {
+            decl_list.for_each_path(&mut |_, attrs, path, names, _| {
                 let names = match names {
                     Some(names) => names,
                     None => return Ok(()),
                 };
+                let stability = self.stability(attrs)?;
                 let (item, name, span) = self.resolve_ast_item_path(path)?;
                 let iface = self.extract_iface_from_item(&item, &name, span)?;
                 if !self.foreign_interfaces.contains(&iface) {
@@ -582,7 +583,7 @@ impl<'a> Resolver<'a> {
                     }
                     let id = self.types.alloc(TypeDef {
                         docs: Docs::default(),
-                        stability: Default::default(),
+                        stability: stability.clone(),
                         kind: TypeDefKind::Unknown,
                         name: Some(name.name.name.to_string()),
                         owner: TypeOwner::Interface(iface),

crates/wit-parser/src/resolve.rs

@@ -1848,6 +1848,19 @@ package {name} is defined in two different locations:\n\
         })
     }
 
+    /// Convenience wrapper around `include_stability` specialized for types
+    /// with a more targeted error message.
+    fn include_type(&self, ty: &TypeDef, pkgid: PackageId, span: Span) -> Result<bool> {
+        self.include_stability(&ty.stability, &pkgid, Some(span))
+            .with_context(|| {
+                format!(
+                    "failed to process feature gate for type [{}] in package [{}]",
+                    ty.name.as_ref().map(String::as_str).unwrap_or("<unknown>"),
+                    self.packages[pkgid].name,
+                )
+            })
+    }
+
     /// Performs the "elaboration process" necessary for the `world_id`
     /// specified to ensure that all of its transitive imports are listed.
     ///
@@ -2625,12 +2638,6 @@ impl Remap {
         resolve: &mut Resolve,
         unresolved: UnresolvedPackage,
     ) -> Result<PackageId> {
-        self.process_foreign_deps(resolve, &unresolved)?;
-
-        let foreign_types = self.types.len();
-        let foreign_interfaces = self.interfaces.len();
-        let foreign_worlds = self.worlds.len();
-
         let pkgid = resolve.packages.alloc(Package {
             name: unresolved.name.clone(),
             docs: unresolved.docs.clone(),
@@ -2640,6 +2647,12 @@ impl Remap {
         let prev = resolve.package_names.insert(unresolved.name.clone(), pkgid);
         assert!(prev.is_none());
 
+        self.process_foreign_deps(resolve, pkgid, &unresolved)?;
+
+        let foreign_types = self.types.len();
+        let foreign_interfaces = self.interfaces.len();
+        let foreign_worlds = self.worlds.len();
+
         // Copy over all types first, updating any intra-type references. Note
         // that types are sorted topologically which means this iteration
         // order should be sufficient. Also note though that the interface
@@ -2652,16 +2665,7 @@ impl Remap {
             .zip(&unresolved.type_spans)
             .skip(foreign_types)
         {
-            if !resolve
-                .include_stability(&ty.stability, &pkgid, Some(*span))
-                .with_context(|| {
-                    format!(
-                        "failed to process feature gate for type [{}] in package [{}]",
-                        ty.name.as_ref().map(String::as_str).unwrap_or("<unknown>"),
-                        resolve.packages[pkgid].name,
-                    )
-                })?
-            {
+            if !resolve.include_type(&ty, pkgid, *span)? {
                 self.types.push(None);
                 continue;
             }
@@ -2838,6 +2842,7 @@ impl Remap {
     fn process_foreign_deps(
         &mut self,
         resolve: &mut Resolve,
+        pkgid: PackageId,
         unresolved: &UnresolvedPackage,
     ) -> Result<()> {
         // Invert the `foreign_deps` map to be keyed by world id to get
@@ -2877,7 +2882,7 @@ impl Remap {
 
         // Finally, iterate over all foreign-defined types and determine
         // what they map to.
-        self.process_foreign_types(unresolved, resolve)?;
+        self.process_foreign_types(unresolved, pkgid, resolve)?;
 
         for (id, span) in unresolved.required_resource_types.iter() {
             let mut id = self.map_type(*id, Some(*span))?;
@@ -2988,16 +2993,25 @@ impl Remap {
     fn process_foreign_types(
         &mut self,
         unresolved: &UnresolvedPackage,
+        pkgid: PackageId,
         resolve: &mut Resolve,
     ) -> Result<(), anyhow::Error> {
-        for (unresolved_type_id, unresolved_ty) in unresolved.types.iter() {
+        for ((unresolved_type_id, unresolved_ty), span) in
+            unresolved.types.iter().zip(&unresolved.type_spans)
+        {
             // All "Unknown" types should appear first so once we're no longer
             // in unknown territory it's package-defined types so break out of
             // this loop.
             match unresolved_ty.kind {
                 TypeDefKind::Unknown => {}
                 _ => break,
             }
+
+            if !resolve.include_type(unresolved_ty, pkgid, *span)? {
+                self.types.push(None);
+                continue;
+            }
+
             let unresolved_iface_id = match unresolved_ty.owner {
                 TypeOwner::Interface(id) => id,
                 _ => unreachable!(),

crates/wit-parser/tests/ui/gated-use.wit

@@ -0,0 +1,13 @@
+package wasmtime:test;
+
+interface types {
+  @unstable(feature = inactive)
+  use wasi:dep2/stable@0.2.3.{unstable-resource};
+}
+
+package wasi:dep2@0.2.3 {
+  interface stable {
+    @unstable(feature = inactive)
+    resource unstable-resource;
+  }
+}

crates/wit-parser/tests/ui/gated-use.wit.json

@@ -0,0 +1,34 @@
+{
+  "worlds": [],
+  "interfaces": [
+    {
+      "name": "stable",
+      "types": {},
+      "functions": {},
+      "package": 0
+    },
+    {
+      "name": "types",
+      "types": {},
+      "functions": {},
+      "package": 1
+    }
+  ],
+  "types": [],
+  "packages": [
+    {
+      "name": "wasi:dep2@0.2.3",
+      "interfaces": {
+        "stable": 0
+      },
+      "worlds": {}
+    },
+    {
+      "name": "wasmtime:test",
+      "interfaces": {
+        "types": 1
+      },
+      "worlds": {}
+    }
+  ]
+}

crates/wit-parser/tests/ui/parse-fail/bad-pkg6.wit.result

@@ -1,5 +1,6 @@
 failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/bad-pkg6]: package 'foo:bar' not found. known packages:
     foo:baz
+    foo:foo
 
      --> tests/ui/parse-fail/bad-pkg6/root.wit:3:7
       |

crates/wit-parser/tests/ui/parse-fail/unresolved-interface4.wit.result

@@ -1,4 +1,6 @@
-package 'some:dependency' not found. no known packages.
+package 'some:dependency' not found. known packages:
+    foo:foo
+
      --> tests/ui/parse-fail/unresolved-interface4.wit:6:10
       |
     6 |   import some:dependency/iface;