Fixed enum variant name collisions with object prototype fields (#4137)

- Fixed returning `Option<Enum>` now correctly has the `| undefined` type in TS bindings. - Improve enum Wasm to JS conversion code.
rustwasm · Oct 7, 2024 · 6505bf4 · 6505bf4
1 parent 7b4bb18
commit 6505bf4
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,12 @@
 * Fixed emitted `package.json` structure to correctly specify its dependencies
   [#4091](https://github.com/rustwasm/wasm-bindgen/pull/4091)
 
+* Fixed returning `Option<Enum>` now correctly has the `| undefined` type in TS bindings.
+  [#4137](https://github.com/rustwasm/wasm-bindgen/pull/4137)
+
+* Fixed enum variant name collisions with object prototype fields.
+  [#4137](https://github.com/rustwasm/wasm-bindgen/pull/4137)
+
 --------------------------------------------------------------------------------
 
 ## [0.2.93](https://github.com/rustwasm/wasm-bindgen/compare/0.2.92...0.2.93)

diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs
@@ -576,6 +576,40 @@ fn instruction(
     log_error: &mut bool,
     constructor: &Option<String>,
 ) -> Result<(), Error> {
+    fn wasm_to_string_enum(variant_values: &[String], index: &str) -> String {
+        // e.g. ["a","b","c"][someIndex]
+        let mut enum_val_expr = String::new();
+        enum_val_expr.push('[');
+        for variant in variant_values {
+            enum_val_expr.push_str(&format!("\"{variant}\","));
+        }
+        enum_val_expr.push(']');
+        enum_val_expr.push('[');
+        enum_val_expr.push_str(index);
+        enum_val_expr.push(']');
+        enum_val_expr
+    }
+    fn string_enum_to_wasm(variant_values: &[String], invalid: u32, enum_val: &str) -> String {
+        // e.g. (["a","b","c"].indexOf(someEnumVal) + 1 || 4) - 1
+        //                                                 |
+        //                                           invalid + 1
+        //
+        // The idea is that `indexOf` returns -1 if someEnumVal is invalid,
+        // and with +1 we get 0 which is falsey, so we can use || to
+        // substitute invalid+1. Finally, we just do -1 to get the correct
+        // values for everything.
+        let mut enum_val_expr = String::new();
+        enum_val_expr.push_str("([");
+        for variant in variant_values.iter() {
+            enum_val_expr.push_str(&format!("\"{variant}\","));
+        }
+        enum_val_expr.push_str(&format!(
+            "].indexOf({enum_val}) + 1 || {invalid}) - 1",
+            invalid = invalid + 1
+        ));
+        enum_val_expr
+    }
+
     match instr {
         Instruction::ArgGet(n) => {
             let arg = js.arg(*n).to_string();
@@ -670,66 +704,23 @@ fn instruction(
 
         Instruction::WasmToStringEnum { variant_values } => {
             let index = js.pop();
-
-            // e.g. ["a","b","c"][someIndex]
-            let mut enum_val_expr = String::new();
-            enum_val_expr.push('[');
-            for variant in variant_values {
-                enum_val_expr.push_str(&format!("\"{variant}\","));
-            }
-            enum_val_expr.push(']');
-            enum_val_expr.push('[');
-            enum_val_expr.push_str(&index);
-            enum_val_expr.push(']');
-
-            js.push(enum_val_expr)
+            js.push(wasm_to_string_enum(variant_values, &index))
         }
 
-        Instruction::OptionWasmToStringEnum {
-            variant_values,
-            hole,
-        } => {
+        Instruction::OptionWasmToStringEnum { variant_values, .. } => {
+            // Since hole is currently variant_count+1 and the lookup is
+            // ["a","b","c"][index], the lookup will implicitly return map
+            // the hole to undefined, because OOB indexes will return undefined.
             let index = js.pop();
-
-            let mut enum_val_expr = String::new();
-            enum_val_expr.push('[');
-            for variant in variant_values {
-                enum_val_expr.push_str(&format!("\"{variant}\","));
-            }
-            enum_val_expr.push(']');
-            enum_val_expr.push('[');
-            enum_val_expr.push_str(&index);
-            enum_val_expr.push(']');
-
-            // e.g. someIndex === 4 ? undefined : (["a","b","c"][someIndex])
-            //                    |
-            //      currently, hole = variant_count + 1
-            js.push(format!(
-                "{index} === {hole} ? undefined : ({enum_val_expr})"
-            ))
+            js.push(wasm_to_string_enum(variant_values, &index))
         }
 
         Instruction::StringEnumToWasm {
             variant_values,
             invalid,
         } => {
             let enum_val = js.pop();
-
-            // e.g. {"a":0,"b":1,"c":2}[someEnumVal] ?? 3
-            //                                          |
-            //                          currently, invalid = variant_count
-            let mut enum_val_expr = String::new();
-            enum_val_expr.push('{');
-            for (i, variant) in variant_values.iter().enumerate() {
-                enum_val_expr.push_str(&format!("\"{variant}\":{i},"));
-            }
-            enum_val_expr.push('}');
-            enum_val_expr.push('[');
-            enum_val_expr.push_str(&enum_val);
-            enum_val_expr.push(']');
-            enum_val_expr.push_str(&format!(" ?? {invalid}"));
-
-            js.push(enum_val_expr)
+            js.push(string_enum_to_wasm(variant_values, *invalid, &enum_val))
         }
 
         Instruction::OptionStringEnumToWasm {
@@ -738,19 +729,9 @@ fn instruction(
             hole,
         } => {
             let enum_val = js.pop();
+            let enum_val_expr = string_enum_to_wasm(variant_values, *invalid, &enum_val);
 
-            let mut enum_val_expr = String::new();
-            enum_val_expr.push('{');
-            for (i, variant) in variant_values.iter().enumerate() {
-                enum_val_expr.push_str(&format!("\"{variant}\":{i},"));
-            }
-            enum_val_expr.push('}');
-            enum_val_expr.push('[');
-            enum_val_expr.push_str(&enum_val);
-            enum_val_expr.push(']');
-            enum_val_expr.push_str(&format!(" ?? {invalid}"));
-
-            // e.g. someEnumVal == undefined ? 4 : ({"a":0,"b":1,"c":2}[someEnumVal] ?? 3)
+            // e.g. someEnumVal == undefined ? 4 : (string_enum_to_wasm(someEnumVal))
             //                  |
             //    double equals here in case it's null
             js.push(format!(

diff --git a/crates/cli-support/src/wit/outgoing.rs b/crates/cli-support/src/wit/outgoing.rs
@@ -305,7 +305,7 @@ impl InstructionBuilder<'_, '_> {
                         variant_values: variant_values.to_vec(),
                         hole: *hole,
                     },
-                    &[AdapterType::StringEnum(String::from(name))],
+                    &[AdapterType::StringEnum(String::from(name)).option()],
                 );
             }
             Descriptor::RustStruct(name) => {

diff --git a/crates/cli/tests/reference/enums.d.ts b/crates/cli/tests/reference/enums.d.ts
@@ -11,6 +11,16 @@ export function enum_echo(color: Color): Color;
 */
 export function option_enum_echo(color?: Color): Color | undefined;
 /**
+* @param {Color} color
+* @returns {ColorName}
+*/
+export function get_name(color: Color): ColorName;
+/**
+* @param {ColorName | undefined} [color]
+* @returns {ColorName | undefined}
+*/
+export function option_string_enum_echo(color?: ColorName): ColorName | undefined;
+/**
 */
 export enum Color {
   Green = 0,

diff --git a/crates/cli/tests/reference/enums.js b/crates/cli/tests/reference/enums.js
@@ -44,6 +44,24 @@ export function option_enum_echo(color) {
     return ret === 3 ? undefined : ret;
 }
 
+/**
+* @param {Color} color
+* @returns {ColorName}
+*/
+export function get_name(color) {
+    const ret = wasm.get_name(color);
+    return ["green","yellow","red",][ret];
+}
+
+/**
+* @param {ColorName | undefined} [color]
+* @returns {ColorName | undefined}
+*/
+export function option_string_enum_echo(color) {
+    const ret = wasm.option_string_enum_echo(color == undefined ? 4 : ((["green","yellow","red",].indexOf(color) + 1 || 4) - 1));
+    return ["green","yellow","red",][ret];
+}
+
 /**
 */
 export const Color = Object.freeze({ Green:0,"0":"Green",Yellow:1,"1":"Yellow",Red:2,"2":"Red", });

diff --git a/crates/cli/tests/reference/enums.rs b/crates/cli/tests/reference/enums.rs
@@ -17,3 +17,25 @@ pub fn enum_echo(color: Color) -> Color {
 pub fn option_enum_echo(color: Option<Color>) -> Option<Color> {
     color
 }
+
+#[wasm_bindgen]
+#[derive(PartialEq, Debug)]
+pub enum ColorName {
+    Green = "green",
+    Yellow = "yellow",
+    Red = "red",
+}
+
+#[wasm_bindgen]
+pub fn get_name(color: Color) -> ColorName {
+    match color {
+        Color::Red => ColorName::Red,
+        Color::Green => ColorName::Green,
+        Color::Yellow => ColorName::Yellow,
+    }
+}
+
+#[wasm_bindgen]
+pub fn option_string_enum_echo(color: Option<ColorName>) -> Option<ColorName> {
+    color
+}
diff --git a/crates/cli/tests/reference/enums.wat b/crates/cli/tests/reference/enums.wat
@@ -2,10 +2,14 @@
   (type (;0;) (func (param i32) (result i32)))
   (func $enum_echo (;0;) (type 0) (param i32) (result i32))
   (func $option_enum_echo (;1;) (type 0) (param i32) (result i32))
+  (func $get_name (;2;) (type 0) (param i32) (result i32))
+  (func $option_string_enum_echo (;3;) (type 0) (param i32) (result i32))
   (memory (;0;) 17)
   (export "memory" (memory 0))
   (export "enum_echo" (func $enum_echo))
   (export "option_enum_echo" (func $option_enum_echo))
+  (export "get_name" (func $get_name))
+  (export "option_string_enum_echo" (func $option_string_enum_echo))
   (@custom "target_features" (after code) "\02+\0fmutable-globals+\08sign-ext")
 )