Repository: bytecodealliance/wasm-tools
Pull request #2078 (2025-02-27 23:09:18)

Preserve the version metadata in wit merge
Follow up to #2076 where I found this issue. 

It is expected that when merging two worlds the versions are preserved.  For instance the merge should preserve the `@since` for the into and the from.

given shared world 1:
```
world shared-world-with-versions-and-include {
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}
```

And shared world 2:
```
world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  import shared-version-on-into;
}
```

It is expected to see:

```
world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}
```

jsturtevant: @alexcrichton This is ready for a review, I wasn't able to refactor as much as I initiall thought since the semantics in the various places were slightly different even though the code looked similar.  If you have ideas or suggestions be happy to try again.

Patch:
diff --git a/crates/wit-parser/src/resolve.rs b/crates/wit-parser/src/resolve.rs
index f579d67e..aa64565b 100644
--- a/crates/wit-parser/src/resolve.rs
+++ b/crates/wit-parser/src/resolve.rs
@@ -692,9 +692,23 @@ package {name} is defined in two different locations:\n\
         let mut moved_worlds = Vec::new();
         for (id, mut world) in worlds {
             let new_id = match world_map.get(&id).copied() {
-                Some(id) => {
-                    update_stability(&world.stability, &mut self.worlds[id].stability)?;
-                    id
+                Some(world_id) => {
+                    update_stability(&world.stability, &mut self.worlds[world_id].stability)?;
+                    for from_import in world.imports.iter() {
+                        Resolve::update_world_imports_stability(
+                            from_import,
+                            &mut self.worlds[world_id].imports,
+                            &interface_map,
+                        )?;
+                    }
+                    for from_export in world.exports.iter() {
+                        Resolve::update_world_imports_stability(
+                            from_export,
+                            &mut self.worlds[world_id].exports,
+                            &interface_map,
+                        )?;
+                    }
+                    world_id
                 }
                 None => {
                     log::debug!("moving world {}", world.name);
@@ -798,6 +812,46 @@ package {name} is defined in two different locations:\n\
         Ok(remap)
     }
 
+    fn update_world_imports_stability(
+        from_item: (&WorldKey, &WorldItem),
+        into_items: &mut IndexMap<WorldKey, WorldItem>,
+        interface_map: &HashMap<Id<Interface>, Id<Interface>>,
+    ) -> Result<()> {
+        match from_item.0 {
+            WorldKey::Name(_) => {
+                // No stability info to update here, only updating import/include stability
+                Ok(())
+            }
+            key @ WorldKey::Interface(_) => {
+                let new_key = MergeMap::map_name(key, interface_map);
+                if let Some(into) = into_items.get_mut(&new_key) {
+                    match (from_item.1, into) {
+                        (
+                            WorldItem::Interface {
+                                id: aid,
+                                stability: astability,
+                            },
+                            WorldItem::Interface {
+                                id: bid,
+                                stability: bstability,
+                            },
+                        ) => {
+                            let aid = interface_map.get(aid).copied().unwrap_or(*aid);
+                            assert_eq!(aid, *bid);
+                            update_stability(astability, bstability)?;
+                            Ok(())
+                        }
+                        _ => unreachable!(),
+                    }
+                } else {
+                    // we've already matched all the imports/exports by the time we are calling this
+                    // so this is unreachable since we should always find the item
+                    unreachable!()
+                }
+            }
+        }
+    }
+
     /// Merges the world `from` into the world `into`.
     ///
     /// This will attempt to merge one world into another, unioning all of its
@@ -3739,7 +3793,7 @@ impl<'a> MergeMap<'a> {
         }
 
         for (from_name, from) in from_world.imports.iter() {
-            let into_name = self.map_name(from_name);
+            let into_name = MergeMap::map_name(from_name, &self.interface_map);
             let name_str = self.from.name_world_key(from_name);
             let into = into_world
                 .imports
@@ -3750,7 +3804,7 @@ impl<'a> MergeMap<'a> {
         }
 
         for (from_name, from) in from_world.exports.iter() {
-            let into_name = self.map_name(from_name);
+            let into_name = MergeMap::map_name(from_name, &self.interface_map);
             let name_str = self.from.name_world_key(from_name);
             let into = into_world
                 .exports
@@ -3763,11 +3817,14 @@ impl<'a> MergeMap<'a> {
         Ok(())
     }
 
-    fn map_name(&self, from_name: &WorldKey) -> WorldKey {
+    fn map_name(
+        from_name: &WorldKey,
+        interface_map: &HashMap<InterfaceId, InterfaceId>,
+    ) -> WorldKey {
         match from_name {
             WorldKey::Name(s) => WorldKey::Name(s.clone()),
             WorldKey::Interface(id) => {
-                WorldKey::Interface(self.interface_map.get(id).copied().unwrap_or(*id))
+                WorldKey::Interface(interface_map.get(id).copied().unwrap_or(*id))
             }
         }
     }


Test patch:
diff --git a/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit b/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit
index c2df6de7..9bd8bce6 100644
--- a/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit
+++ b/crates/wit-component/tests/merge/success/from/deps/foo/shared.wit
@@ -33,6 +33,10 @@ world shared-world-with-versions {
   @since(version = 1.0.0)
   import shared-version-on-from;
   import shared-version-on-into;
+
+  @since(version = 1.0.0)
+  export shared-version-on-from;
+  export shared-version-on-into;
 }
 
 world shared-world-with-versions-and-include {
diff --git a/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit b/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit
index 5a97ef93..9cdae206 100644
--- a/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit
+++ b/crates/wit-component/tests/merge/success/into/deps/foo/shared.wit
@@ -31,6 +31,10 @@ world shared-world-with-versions {
   import shared-version-on-from;
   @since(version = 1.0.0)
   import shared-version-on-into;
+
+  export shared-version-on-from;
+  @since(version = 1.0.0)
+  export shared-version-on-into;
 }
 
 world shared-world-with-versions-and-include {
diff --git a/crates/wit-component/tests/merge/success/merge/foo.wit b/crates/wit-component/tests/merge/success/merge/foo.wit
index c8b446a9..86caa391 100644
--- a/crates/wit-component/tests/merge/success/merge/foo.wit
+++ b/crates/wit-component/tests/merge/success/merge/foo.wit
@@ -54,12 +54,24 @@ world shared-world {
   export shared-items;
 }
 world shared-world-with-versions {
+  @since(version = 1.0.0)
   import shared-version-on-from;
   @since(version = 1.0.0)
   import shared-version-on-into;
+
+  @since(version = 1.0.0)
+  export shared-version-on-from;
+  @since(version = 1.0.0)
+  export shared-version-on-into;
 }
 world shared-world-with-versions-and-include {
+  @since(version = 1.0.0)
   import shared-version-on-from;
   @since(version = 1.0.0)
   import shared-version-on-into;
+
+  @since(version = 1.0.0)
+  export shared-version-on-from;
+  @since(version = 1.0.0)
+  export shared-version-on-into;
 }