webidl: use Results for enum returns

Use `Result`s for enum returns so that non-valid strings can be handled
without panicking.

Author: Slowki

crates/backend/src/codegen.rs

@@ -594,7 +594,7 @@ impl ToTokens for ast::ImportEnum {
     fn to_tokens(&self, tokens: &mut TokenStream) {
         let vis = &self.vis;
         let name = &self.name;
-        let expect_string = format!("attempted to convert invalid JSValue into {}", name);
+        let panic_message = format!("attempted to convert a non-string JSValue into {}", name);
         let variants = &self.variants;
         let variant_strings = &self.variant_values;
 
@@ -623,17 +623,26 @@ impl ToTokens for ast::ImportEnum {
 
         (quote! {
             #[allow(bad_style)]
-            #[derive(Copy, Clone, Debug)]
+            #[derive(Copy, Clone, PartialEq, Debug)]
             #vis enum #name {
                 #(#variants = #variant_indexes_ref,)*
             }
 
             impl #name {
-                #vis fn from_js_value(obj: ::wasm_bindgen::JsValue) -> Option<#name> {
-                    obj.as_string().and_then(|obj_str| match obj_str.as_str() {
-                        #(#variant_strings => Some(#variant_paths_ref),)*
-                        _ => None,
-                    })
+                /// Attempt to convert a `JSValue` to an enum, returning Ok if
+                /// the value maps to the enum's variants, or an `Err` containing
+                /// the `JsValue`'s string value if it doesn't map to any of the
+                /// variants.
+                /// 
+                /// # Panics
+                /// This function will panic if the `JsValue` isn't a string
+                #vis fn from_js_value(obj: ::wasm_bindgen::JsValue) -> Result<#name, String> {
+                    let js_string_value = obj.as_string().expect(#panic_message);
+
+                    match js_string_value.as_str() {
+                        #(#variant_strings => Ok(#variant_paths_ref),)*
+                        _ => Err(js_string_value),
+                    }
                 }
             }
 
@@ -652,15 +661,17 @@ impl ToTokens for ast::ImportEnum {
                 }
             }
 
-            impl ::wasm_bindgen::convert::FromWasmAbi for #name {
+            impl ::wasm_bindgen::convert::TryFromWasmAbi for #name {
                 type Abi = <::wasm_bindgen::JsValue as
                     ::wasm_bindgen::convert::FromWasmAbi>::Abi;
+                type Error = String;
 
-                unsafe fn from_abi(
+                unsafe fn try_from_abi(
                     js: Self::Abi,
                     extra: &mut ::wasm_bindgen::convert::Stack,
-                ) -> Self {
-                    #name::from_js_value(::wasm_bindgen::JsValue::from_abi(js, extra)).expect(#expect_string)
+                ) -> Result<#name, Self::Error> {
+                    #name::from_js_value(<::wasm_bindgen::JsValue as
+                    ::wasm_bindgen::convert::FromWasmAbi>::from_abi(js, extra))
                 }
             }
 

crates/webidl/src/lib.rs

@@ -13,6 +13,7 @@ extern crate heck;
 #[macro_use]
 extern crate log;
 extern crate proc_macro2;
+#[macro_use]
 extern crate quote;
 extern crate syn;
 extern crate wasm_bindgen_backend as backend;

crates/webidl/src/util.rs

@@ -123,7 +123,14 @@ impl<'a> FirstPassRecord<'a> {
                 } else if self.dictionaries.contains(id) {
                     ty
                 } else if self.enums.contains(id) {
-                    ty
+                    if pos == TypePosition::Return {
+                        // Enums are returned as Results in case their value doesn't match the WebIDL schema
+                        syn::TypeVerbatim {
+                            tts: quote!(Result<#ty, String>),
+                        }.into()
+                    } else {
+                        ty
+                    }
                 } else {
                     warn!("unrecognized type {}", id);
                     ty

crates/webidl/tests/all/enums.rs

@@ -1,23 +1,78 @@
 use super::project;
 
+static SHAPE_IDL: &'static str = r#"
+    enum ShapeType { "circle", "square" };
+    
+    [Constructor(ShapeType kind)]
+    interface Shape {
+        [Pure]
+        boolean isSquare();
+
+        [Pure]
+        boolean isCircle();
+
+        [Pure]
+        ShapeType getShape();
+    };
+"#;
+
 #[test]
 fn top_level_enum() {
     project()
+        .file("shape.webidl", SHAPE_IDL)
+        .file(
+            "shape.mjs",
+            r#"
+            export class Shape {
+                constructor(kind) {
+                    this.kind = kind;
+                }
+
+                isSquare() {
+                    return this.kind === 'square';
+                }
+
+                isCircle() {
+                    return this.kind === 'circle';
+                }
+
+                getShape() {
+                    return this.kind;
+                }
+            }
+            "#,
+        )
         .file(
-            "shape.webidl",
+            "src/lib.rs",
             r#"
-            enum ShapeType { "circle", "square" };
-            
-            [Constructor(ShapeType kind)]
-            interface Shape {
-                [Pure]
-                boolean isSquare();
-
-                [Pure]
-                boolean isCircle();
-            };
+            #![feature(proc_macro, wasm_custom_section, wasm_import_module)]
+
+            extern crate wasm_bindgen;
+
+            use wasm_bindgen::prelude::*;
+
+            pub mod shape;
+
+            use shape::{Shape, ShapeType};
+
+            #[wasm_bindgen]
+            pub fn test() {
+                let circle = Shape::new(ShapeType::Circle).unwrap();
+                let square = Shape::new(ShapeType::Square).unwrap();
+                assert!(circle.is_circle());
+                assert!(!circle.is_square());
+                assert!(square.is_square());
+                assert!(!square.is_circle());
+            }
         "#,
         )
+        .test();
+}
+
+#[test]
+fn valid_enum_return() {
+    project()
+        .file("shape.webidl", SHAPE_IDL)
         .file(
             "shape.mjs",
             r#"
@@ -33,6 +88,10 @@ fn top_level_enum() {
                 isCircle() {
                     return this.kind === 'circle';
                 }
+
+                getShape() {
+                    return this.kind;
+                }
             }
             "#,
         )
@@ -55,8 +114,61 @@ fn top_level_enum() {
                 let square = Shape::new(ShapeType::Square).unwrap();
                 assert!(circle.is_circle());
                 assert!(!circle.is_square());
+                assert_eq!(circle.get_shape(), Ok(ShapeType::Circle));
                 assert!(square.is_square());
                 assert!(!square.is_circle());
+                assert_eq!(square.get_shape(), Ok(ShapeType::Square));
+            }
+        "#,
+        )
+        .test();
+}
+
+#[test]
+fn invalid_enum_return() {
+    project()
+        .file("shape.webidl", SHAPE_IDL)
+        .file(
+            "shape.mjs",
+            r#"
+            export class Shape {
+                constructor(kind) {
+                    this.kind = 'triangle'; // <-- invalid ShapeType
+                }
+
+                isSquare() {
+                    return this.kind === 'square';
+                }
+
+                isCircle() {
+                    return this.kind === 'circle';
+                }
+
+                getShape() {
+                    return this.kind;
+                }
+            }
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+            #![feature(proc_macro, wasm_custom_section, wasm_import_module)]
+
+            extern crate wasm_bindgen;
+
+            use wasm_bindgen::prelude::*;
+
+            pub mod shape;
+
+            use shape::{Shape, ShapeType};
+
+            #[wasm_bindgen]
+            pub fn test() {
+                let actually_a_triangle = Shape::new(ShapeType::Circle).unwrap();
+                assert!(!actually_a_triangle.is_circle());
+                assert!(!actually_a_triangle.is_square());
+                assert_eq!(actually_a_triangle.get_shape(), Err(String::from("triangle")));
             }
         "#,
         )

src/convert.rs

@@ -46,6 +46,27 @@ pub trait FromWasmAbi: WasmDescribe {
     unsafe fn from_abi(js: Self::Abi, extra: &mut Stack) -> Self;
 }
 
+/// A trait for anything that can be recovered by-value from the wasm ABI
+/// but whose conversion might error, for example a JS string being converted
+/// to a Rust enum.
+pub trait TryFromWasmAbi: WasmDescribe + Sized {
+    /// The wasm ABI type that this converts from when coming back out from the
+    /// ABI boundary.
+    type Abi: WasmAbi;
+
+    /// The error type for Result if the conversion fails
+    type Error: Sized;
+
+    /// Attempt to recover a `Self` from `Self::Abi`.
+    ///
+    /// # Safety
+    ///
+    /// This is only safe to call when -- and implementations may assume that --
+    /// the supplied `Self::Abi` was previously generated by a call to `<Self as
+    /// IntoWasmAbi>::into_abi()` or the moral equivalent in JS.
+    unsafe fn try_from_abi(js: Self::Abi, extra: &mut Stack) -> Result<Self, Self::Error>;
+}
+
 /// A trait for anything that can be recovered as some sort of shared reference
 /// from the wasm ABI boundary.
 ///
@@ -395,6 +416,21 @@ impl FromWasmAbi for JsValue {
     }
 }
 
+impl<T: TryFromWasmAbi> WasmDescribe for Result<T, T::Error> {
+    fn describe() {
+        T::describe()
+    }
+}
+
+impl<T: TryFromWasmAbi> FromWasmAbi for Result<T, T::Error> {
+    type Abi = T::Abi;
+
+    #[inline]
+    unsafe fn from_abi(js: Self::Abi, extra: &mut Stack) -> Result<T, T::Error> {
+        T::try_from_abi(js, extra)
+    }
+}
+
 impl<'a> IntoWasmAbi for &'a JsValue {
     type Abi = u32;