Repository: bytecodealliance/wasm-tools
Pull request #1910 (2024-11-19 14:30:20)

wit-encoder: Implement world uses and sort interface and world items, uses
Wit-encoder changes:
 - implement world uses for exports and imports following the pattern used in interfaces
 - sort output items that are converted from IndexMap parser sources, so the ordering is stable


Patch:
diff --git a/crates/wit-encoder/src/from_parser.rs b/crates/wit-encoder/src/from_parser.rs
index 98fded85..db5ecd29 100644
--- a/crates/wit-encoder/src/from_parser.rs
+++ b/crates/wit-encoder/src/from_parser.rs
@@ -84,11 +84,13 @@ impl<'a> Converter<'a> {
                         output.item(WorldItem::function_import(func));
                     }
                 }
-                wit_parser::WorldItem::Type(_) => {
-                    todo!();
+                wit_parser::WorldItem::Type(type_id) => {
+                    let (target, item, rename) = self.convert_world_item_type(package_id, *type_id);
+                    output.use_type(target, item, rename)
                 }
             }
         }
+
         for (key, item) in &world.exports {
             match item {
                 wit_parser::WorldItem::Interface { id, .. } => {
@@ -110,15 +112,42 @@ impl<'a> Converter<'a> {
                         output.item(WorldItem::function_export(func));
                     }
                 }
-                wit_parser::WorldItem::Type(_) => {
-                    todo!();
+                wit_parser::WorldItem::Type(type_id) => {
+                    let (target, item, rename) = self.convert_world_item_type(package_id, *type_id);
+                    output.use_type(target, item, rename)
                 }
             }
         }
 
+        output.uses_mut().sort();
+        output.items_mut().sort();
+
         output
     }
 
+    fn convert_world_item_type(
+        &self,
+        package_id: PackageId,
+        type_id: wit_parser::TypeId,
+    ) -> (Ident, Ident, Option<Ident>) {
+        let type_def = self.resolve.types.get(type_id).unwrap();
+        let underlying_type_def = self.underlying_type_def(&type_def);
+        let interface_ident = self
+            .type_def_owner_interface_ident(package_id, underlying_type_def)
+            .expect("Type not part of an interface");
+        let local_type_name = type_def.name.clone().unwrap();
+        let underlying_local_type_name = underlying_type_def.name.clone().unwrap();
+        if underlying_local_type_name == local_type_name {
+            (interface_ident, local_type_name.into(), None)
+        } else {
+            (
+                interface_ident,
+                underlying_local_type_name.into(),
+                Some(local_type_name.into()),
+            )
+        }
+    }
+
     fn convert_interface(
         &self,
         package_id: PackageId,
@@ -140,31 +169,16 @@ impl<'a> Converter<'a> {
         }
         for (_, type_id) in &interface.types {
             let type_def = self.resolve.types.get(*type_id).unwrap();
-
-            let underlying_type_def = match &type_def.kind {
-                wit_parser::TypeDefKind::Type(type_) => match &type_ {
-                    wit_parser::Type::Id(type_id) => {
-                        let type_def = self.resolve.types.get(*type_id).unwrap();
-                        type_def
-                    }
-                    _ => type_def,
-                },
-                _ => type_def,
-            };
+            let underlying_type_def = self.underlying_type_def(&type_def);
 
             if underlying_type_def.owner == owner {
                 if let Some(type_def) = self.convert_type_def(type_def, *type_id) {
                     output.item(InterfaceItem::TypeDef(type_def));
                 }
             } else {
-                let interface_ident = match underlying_type_def.owner {
-                    wit_parser::TypeOwner::Interface(id) => self.interface_ident(
-                        package_id,
-                        None,
-                        self.resolve.interfaces.get(id).unwrap(),
-                    ),
-                    _ => panic!("Type not part of an interface"),
-                };
+                let interface_ident = self
+                    .type_def_owner_interface_ident(package_id, underlying_type_def)
+                    .expect("Type not part of an interface");
                 let local_type_name = type_def.name.clone().unwrap();
                 let underlying_local_type_name = underlying_type_def.name.clone().unwrap();
                 if underlying_local_type_name == local_type_name {
@@ -179,6 +193,9 @@ impl<'a> Converter<'a> {
             }
         }
 
+        output.uses_mut().sort();
+        output.items_mut().sort();
+
         output
     }
 
@@ -521,6 +538,31 @@ impl<'a> Converter<'a> {
             },
         }
     }
+
+    fn underlying_type_def(&'a self, type_def: &'a wit_parser::TypeDef) -> &'a wit_parser::TypeDef {
+        match &type_def.kind {
+            wit_parser::TypeDefKind::Type(type_) => match &type_ {
+                wit_parser::Type::Id(type_id) => {
+                    let type_def = self.resolve.types.get(*type_id).unwrap();
+                    type_def
+                }
+                _ => type_def,
+            },
+            _ => type_def,
+        }
+    }
+
+    fn type_def_owner_interface_ident(
+        &self,
+        package_id: PackageId,
+        type_def: &wit_parser::TypeDef,
+    ) -> Option<Ident> {
+        if let wit_parser::TypeOwner::Interface(id) = type_def.owner {
+            Some(self.interface_ident(package_id, None, self.resolve.interfaces.get(id).unwrap()))
+        } else {
+            None
+        }
+    }
 }
 
 fn clean_func_name(resource_name: &str, method_name: &str) -> String {
diff --git a/crates/wit-encoder/src/world.rs b/crates/wit-encoder/src/world.rs
index 2eaeeba8..9aebb00d 100644
--- a/crates/wit-encoder/src/world.rs
+++ b/crates/wit-encoder/src/world.rs
@@ -9,6 +9,9 @@ pub struct World {
     /// The WIT identifier name of this world.
     name: Ident,
 
+    /// Interface uses
+    uses: Vec<Use>,
+
     /// All imported and exported items into this world.
     items: Vec<WorldItem>,
 
@@ -21,6 +24,7 @@ impl World {
     pub fn new(name: impl Into<Ident>) -> Self {
         Self {
             name: name.into(),
+            uses: vec![],
             items: vec![],
             docs: None,
         }
@@ -69,8 +73,34 @@ impl World {
     pub fn include(&mut self, include: Include) {
         self.item(WorldItem::Include(include));
     }
+
+    pub fn uses(&self) -> &[Use] {
+        &self.uses
+    }
+    pub fn uses_mut(&mut self) -> &mut [Use] {
+        &mut self.uses
+    }
     pub fn use_(&mut self, use_: Use) {
-        self.item(WorldItem::Use(use_));
+        self.uses.push(use_);
+    }
+    pub fn use_type(
+        &mut self,
+        target: impl Into<Ident>,
+        item: impl Into<Ident>,
+        rename: Option<Ident>,
+    ) {
+        let target = target.into();
+        let use_ = self.uses.iter_mut().find(|u| u.target() == &target);
+        match use_ {
+            Some(use_) => use_.item(item, rename),
+            None => {
+                self.use_({
+                    let mut use_ = Use::new(target);
+                    use_.item(item, rename);
+                    use_
+                });
+            }
+        }
     }
 
     pub fn docs(&self) -> Option<&Docs> {
@@ -105,6 +135,7 @@ impl Render for World {
         }
         write!(f, "{}world {} {{\n", opts.spaces(), self.name)?;
         let opts = &opts.indent();
+        self.uses.render(f, opts)?;
         for item in &self.items {
             match item {
                 WorldItem::InlineInterfaceImport(interface) => {
@@ -165,7 +196,6 @@ impl Render for World {
                     render_function(f, opts, function)?;
                 }
                 WorldItem::Include(include) => include.render(f, opts)?,
-                WorldItem::Use(use_) => use_.render(f, opts)?,
             }
         }
         let opts = &opts.outdent();
@@ -198,9 +228,6 @@ pub enum WorldItem {
 
     /// Include type
     Include(Include),
-
-    /// Use
-    Use(Use),
 }
 
 impl WorldItem {


Test patch:
diff --git a/crates/wit-encoder/tests/deps.rs b/crates/wit-encoder/tests/deps.rs
index 428f17a1..36cc145e 100644
--- a/crates/wit-encoder/tests/deps.rs
+++ b/crates/wit-encoder/tests/deps.rs
@@ -10,7 +10,11 @@ const MAIN_PACKAGE_SOURCE: &str = indoc::indoc! {"
       use foo:dep-a/dep-a-interface-b.{ ra };
     }
 
-    interface main-interface-b {}
+    interface main-interface-b {
+        record mb {
+            x: f32,
+        }
+    }
 
     interface main-interface-c {}
 
@@ -19,14 +23,17 @@ const MAIN_PACKAGE_SOURCE: &str = indoc::indoc! {"
     interface main-interface-e {}
 
     world main-world-a {
+      use foo:dep-c/dep-c-interface-b.{ cb, cb as cbcb };
       import foo:dep-c/dep-c-interface-a;
       export foo:dep-c/dep-c-interface-b;
       include foo:dep-c/dep-c-world-a;
 
+      use foo:dep-b/dep-b-interface-b@1.2.3.{ bb, bb as bbbb };
       import foo:dep-b/dep-b-interface-a@1.2.3;
       export foo:dep-b/dep-b-interface-b@1.2.3;
       include foo:dep-b/dep-b-world-b@1.2.3;
 
+      use main-interface-b.{ mb, mb as mbmb };
       import main-interface-b;
       export main-interface-a;
       include main-world-b;
@@ -45,7 +52,11 @@ const MAIN_PACKAGE_RESOLVED_ENCODED: &str = indoc::indoc! {"
       use foo:dep-a/dep-a-interface-b.{ ra };
     }
 
-    interface main-interface-b {}
+    interface main-interface-b {
+      record mb {
+        x: f32,
+      }
+    }
 
     interface main-interface-c {}
 
@@ -59,16 +70,21 @@ const MAIN_PACKAGE_RESOLVED_ENCODED: &str = indoc::indoc! {"
     }
 
     world main-world-a {
-      import foo:dep-c/dep-c-interface-a;
+      use foo:dep-b/dep-b-interface-b@1.2.3.{ bb, bb as bbbb };
+      use foo:dep-c/dep-c-interface-b.{ cb, cb as cbcb };
+      use main-interface-b.{ mb, mb as mbmb };
+      import foo:dep-a/dep-a-interface-b;
       import foo:dep-b/dep-b-interface-a@1.2.3;
-      import main-interface-b;
+      import foo:dep-b/dep-b-interface-b@1.2.3;
+      import foo:dep-c/dep-c-interface-a;
+      import foo:dep-c/dep-c-interface-b;
       import foo:dep-c/dep-c-interface-c;
+      import main-interface-b;
       import main-interface-d;
-      import foo:dep-a/dep-a-interface-b;
-      export foo:dep-c/dep-c-interface-b;
       export foo:dep-b/dep-b-interface-b@1.2.3;
-      export main-interface-a;
+      export foo:dep-c/dep-c-interface-b;
       export foo:dep-c/dep-c-interface-d;
+      export main-interface-a;
       export main-interface-d;
     }
 "};
@@ -96,6 +112,10 @@ const DEP_PACKAGE_B: &str = indoc::indoc! {"
 
     interface dep-b-interface-b {
       use foo:dep-c/dep-c-interface-a.{a};
+
+      record bb {
+        x: f32,
+      }
     }
 
     world dep-b-world-a {
@@ -115,7 +135,11 @@ const DEP_PACKAGE_C: &str = indoc::indoc! {"
       }
     }
 
-    interface dep-c-interface-b {}
+    interface dep-c-interface-b {
+      record cb {
+        x: f32,
+      }
+    }
 
     interface dep-c-interface-c {}
 
diff --git a/crates/wit-encoder/tests/parse-to-encoder.rs b/crates/wit-encoder/tests/parse-to-encoder.rs
index 45cb39b6..ae93fb67 100644
--- a/crates/wit-encoder/tests/parse-to-encoder.rs
+++ b/crates/wit-encoder/tests/parse-to-encoder.rs
@@ -20,20 +20,15 @@ interface bar-interface {
 
 interface %interface {
   use bar-interface.{ some-enum, external-type };
-  statndalone-func: func(a: u32, b: s32) -> f32;
-  variant %variant {
-    empty-case,
-    valued-case(u32),
+  resource other-resource {
+    constructor(values: list<u32>);
+    static-method: func(arg: tuple<u32, u32>);
   }
   resource %resource {
     constructor();
     method: func(arg: list<u32>) -> char;
     static-method: static func(arg: tuple<u32, u32>) -> list<u32>;
   }
-  resource other-resource {
-    constructor(values: list<u32>);
-    static-method: func(arg: tuple<u32, u32>);
-  }
   record some-record {
     optinal: option<string>,
     %own: %resource,
@@ -44,6 +39,8 @@ interface %interface {
     result-d: result<f64, f32>,
     result-e: result<string, list<u8>>,
   }
+  type type-handle-borrow = borrow<%resource>;
+  type type-handle-own = %resource;
   type type-list = list<f64>;
   type type-option = option<f64>;
   type type-result-a = result;
@@ -51,34 +48,47 @@ interface %interface {
   type type-result-c = result<_, f64>;
   type type-result-d = result<f64, f32>;
   type type-result-e = result<string, list<u8>>;
-  type type-handle-own = %resource;
-  type type-handle-borrow = borrow<%resource>;
   type type-tuple = tuple<u32, list<u32>, borrow<%resource>>;
+  variant %variant {
+    empty-case,
+    valued-case(u32),
+  }
+  statndalone-func: func(a: u32, b: s32) -> f32;
 }
 
 world my-world {
-  import foo-interface;
-  import bar-interface;
-  import %interface;
   import inline-import: interface {
     do-something: func(a: string) -> string;
   }
-  import import-func: func();
-  export export-func: func(a: u32, b: s32) -> f32;
-  export %interface;
   export inline-export: interface {
     do-nothing: func();
   }
+  import bar-interface;
+  import foo-interface;
+  import %interface;
+  export %interface;
+  import import-func: func();
+  export export-func: func(a: u32, b: s32) -> f32;
 }
 "#;
 
 #[test]
 fn round_trip() {
+    // First round trip
     let mut resolve = wit_parser::Resolve::new();
     resolve.push_str("", WIT).unwrap();
     let packages = packages_from_parsed(&resolve);
 
-    assert!(packages.len() == 1, "Should create exactly one package");
+    assert_eq!(packages.len(), 1, "Should create exactly one package");
+    let package = &packages[0];
+    assert_eq!(WIT, package.to_string());
+
+    // Second round tip
+    let mut resolve = wit_parser::Resolve::new();
+    resolve.push_str("", &package.to_string()).unwrap();
+    let packages = packages_from_parsed(&resolve);
+
+    assert_eq!(packages.len(), 1, "Should create exactly one package");
     let package = &packages[0];
     assert_eq!(WIT, package.to_string());
 }