Merge pull request #457 from fitzgen/turn-throws-extended-attribute-into-catch

webidl: Turn the `[Throws]` extended attributes into `Result<T, JsValue>`

crates/backend/src/util.rs

@@ -35,6 +35,23 @@ pub fn raw_ident(name: &str) -> Ident {
 /// Create a path type from the given segments. For example an iterator yielding
 /// the idents `[foo, bar, baz]` will result in the path type `foo::bar::baz`.
 pub fn simple_path_ty<I>(segments: I) -> syn::Type
+where
+    I: IntoIterator<Item = Ident>,
+{
+    path_ty(false, segments)
+}
+
+/// Create a global path type from the given segments. For example an iterator
+/// yielding the idents `[foo, bar, baz]` will result in the path type
+/// `::foo::bar::baz`.
+pub fn leading_colon_path_ty<I>(segments: I) -> syn::Type
+where
+    I: IntoIterator<Item = Ident>,
+{
+    path_ty(true, segments)
+}
+
+fn path_ty<I>(leading_colon: bool, segments: I) -> syn::Type
 where
     I: IntoIterator<Item = Ident>,
 {
@@ -49,7 +66,11 @@ where
     syn::TypePath {
         qself: None,
         path: syn::Path {
-            leading_colon: None,
+            leading_colon: if leading_colon {
+                Some(Default::default())
+            } else {
+                None
+            },
             segments: syn::punctuated::Punctuated::from_iter(segments),
         },
     }.into()

crates/webidl/src/lib.rs

@@ -77,7 +77,7 @@ fn compile_ast(mut ast: backend::ast::Program) -> String {
     let mut defined = BTreeSet::from_iter(
         vec![
             "str", "char", "bool", "JsValue", "u8", "i8", "u16", "i16", "u32", "i32", "u64", "i64",
-            "usize", "isize", "f32", "f64",
+            "usize", "isize", "f32", "f64", "Result",
         ].into_iter()
             .map(|id| proc_macro2::Ident::new(id, proc_macro2::Span::call_site())),
     );
@@ -210,11 +210,29 @@ impl<'a> WebidlParse<&'a webidl::ast::NonPartialInterface> for webidl::ast::Exte
     ) -> Result<()> {
         let mut add_constructor = |arguments: &[webidl::ast::Argument], class: &str| {
             let self_ty = ident_ty(rust_ident(&interface.name));
+
             let kind = backend::ast::ImportFunctionKind::Method {
                 class: class.to_string(),
                 ty: self_ty.clone(),
                 kind: backend::ast::MethodKind::Constructor,
             };
+
+            let structural = false;
+
+            // Constructors aren't annotated with `[Throws]` extended attributes
+            // (how could they be, since they themselves are extended
+            // attributes?) so we must conservatively assume that they can
+            // always throw.
+            //
+            // From https://heycam.github.io/webidl/#Constructor (emphasis
+            // mine):
+            //
+            // > The prose definition of a constructor must either return an IDL
+            // > value of a type corresponding to the interface the
+            // > `[Constructor]` extended attribute appears on, **or throw an
+            // > exception**.
+            let throws = true;
+
             create_function(
                 "new",
                 arguments
@@ -222,7 +240,8 @@ impl<'a> WebidlParse<&'a webidl::ast::NonPartialInterface> for webidl::ast::Exte
                     .map(|arg| (&*arg.name, &*arg.type_, arg.variadic)),
                 Some(self_ty),
                 kind,
-                false,
+                structural,
+                throws,
             ).map(|function| {
                 program.imports.push(backend::ast::Import {
                     module: None,
@@ -236,7 +255,8 @@ impl<'a> WebidlParse<&'a webidl::ast::NonPartialInterface> for webidl::ast::Exte
         match self {
             webidl::ast::ExtendedAttribute::ArgumentList(
                 webidl::ast::ArgumentListExtendedAttribute { arguments, name },
-            ) if name == "Constructor" =>
+            )
+                if name == "Constructor" =>
             {
                 add_constructor(arguments, &interface.name);
             }
@@ -251,7 +271,8 @@ impl<'a> WebidlParse<&'a webidl::ast::NonPartialInterface> for webidl::ast::Exte
                     rhs_arguments,
                     rhs_name,
                 },
-            ) if lhs_name == "NamedConstructor" =>
+            )
+                if lhs_name == "NamedConstructor" =>
             {
                 add_constructor(rhs_arguments, rhs_name);
             }
@@ -322,14 +343,27 @@ impl<'a> WebidlParse<&'a str> for webidl::ast::RegularAttribute {
         }
 
         let is_structural = util::is_structural(&self.extended_attributes);
+        let throws = util::throws(&self.extended_attributes);
 
-        create_getter(&self.name, &self.type_, self_name, false, is_structural)
+        create_getter(
-            .map(wrap_import_function)
+            &self.name,
+            &self.type_,
+            self_name,
+            false,
+            is_structural,
+            throws,
+        ).map(wrap_import_function)
             .map(|import| program.imports.push(import));
 
         if !self.read_only {
-            create_setter(&self.name, &self.type_, self_name, false, is_structural)
+            create_setter(
-                .map(wrap_import_function)
+                &self.name,
+                &self.type_,
+                self_name,
+                false,
+                is_structural,
+                throws,
+            ).map(wrap_import_function)
                 .map(|import| program.imports.push(import));
         }
 
@@ -344,14 +378,27 @@ impl<'a> WebidlParse<&'a str> for webidl::ast::StaticAttribute {
         }
 
         let is_structural = util::is_structural(&self.extended_attributes);
+        let throws = util::throws(&self.extended_attributes);
 
-        create_getter(&self.name, &self.type_, self_name, true, is_structural)
+        create_getter(
-            .map(wrap_import_function)
+            &self.name,
+            &self.type_,
+            self_name,
+            true,
+            is_structural,
+            throws,
+        ).map(wrap_import_function)
             .map(|import| program.imports.push(import));
 
         if !self.read_only {
-            create_setter(&self.name, &self.type_, self_name, true, is_structural)
+            create_setter(
-                .map(wrap_import_function)
+                &self.name,
+                &self.type_,
+                self_name,
+                true,
+                is_structural,
+                throws,
+            ).map(wrap_import_function)
                 .map(|import| program.imports.push(import));
         }
 
@@ -365,12 +412,15 @@ impl<'a> WebidlParse<&'a str> for webidl::ast::RegularOperation {
             return Ok(());
         }
 
+        let throws = util::throws(&self.extended_attributes);
+
         create_basic_method(
             &self.arguments,
             self.name.as_ref(),
             &self.return_type,
             self_name,
             false,
+            throws,
         ).map(wrap_import_function)
             .map(|import| program.imports.push(import));
 
@@ -384,12 +434,15 @@ impl<'a> WebidlParse<&'a str> for webidl::ast::StaticOperation {
             return Ok(());
         }
 
+        let throws = util::throws(&self.extended_attributes);
+
         create_basic_method(
             &self.arguments,
             self.name.as_ref(),
             &self.return_type,
             self_name,
             true,
+            throws,
         ).map(wrap_import_function)
             .map(|import| program.imports.push(import));
 

crates/webidl/src/util.rs

@@ -1,7 +1,7 @@
-use std::iter;
+use std::iter::{self, FromIterator};
 
 use backend;
-use backend::util::{ident_ty, raw_ident, rust_ident, simple_path_ty};
+use backend::util::{ident_ty, leading_colon_path_ty, raw_ident, rust_ident, simple_path_ty};
 use heck::SnakeCase;
 use proc_macro2::Ident;
 use syn;
@@ -146,12 +146,40 @@ where
     Some(res)
 }
 
+fn unit_ty() -> syn::Type {
+    syn::Type::Tuple(syn::TypeTuple {
+        paren_token: Default::default(),
+        elems: syn::punctuated::Punctuated::new(),
+    })
+}
+
+fn result_ty(t: syn::Type) -> syn::Type {
+    let js_value = leading_colon_path_ty(vec![rust_ident("wasm_bindgen"), rust_ident("JsValue")]);
+
+    let arguments = syn::PathArguments::AngleBracketed(syn::AngleBracketedGenericArguments {
+        colon2_token: None,
+        lt_token: Default::default(),
+        args: FromIterator::from_iter(vec![
+            syn::GenericArgument::Type(t),
+            syn::GenericArgument::Type(js_value),
+        ]),
+        gt_token: Default::default(),
+    });
+
+    let ident = raw_ident("Result");
+    let seg = syn::PathSegment { ident, arguments };
+    let path: syn::Path = seg.into();
+    let ty = syn::TypePath { qself: None, path };
+    ty.into()
+}
+
 pub fn create_function<'a, I>(
     name: &str,
     arguments: I,
-    ret: Option<syn::Type>,
+    mut ret: Option<syn::Type>,
     kind: backend::ast::ImportFunctionKind,
     structural: bool,
+    catch: bool,
 ) -> Option<backend::ast::ImportFunction>
 where
     I: Iterator<Item = (&'a str, &'a webidl::ast::Type, bool)>,
@@ -163,6 +191,10 @@ where
 
     let js_ret = ret.clone();
 
+    if catch {
+        ret = Some(ret.map_or_else(|| result_ty(unit_ty()), |ret| result_ty(ret)))
+    }
+
     let shim = {
         let ns = match kind {
             backend::ast::ImportFunctionKind::Normal => "",
@@ -184,7 +216,7 @@ where
         },
         rust_name,
         js_ret,
-        catch: false,
+        catch,
         structural,
         kind,
         shim,
@@ -197,6 +229,7 @@ pub fn create_basic_method(
     return_type: &webidl::ast::ReturnType,
     self_name: &str,
     is_static: bool,
+    catch: bool,
 ) -> Option<backend::ast::ImportFunction> {
     let name = match name {
         None => {
@@ -235,6 +268,7 @@ pub fn create_basic_method(
         ret,
         kind,
         false,
+        catch,
     )
 }
 
@@ -244,6 +278,7 @@ pub fn create_getter(
     self_name: &str,
     is_static: bool,
     is_structural: bool,
+    catch: bool,
 ) -> Option<backend::ast::ImportFunction> {
     let ret = match webidl_ty_to_syn_ty(ty, TypePosition::Return) {
         None => {
@@ -262,7 +297,7 @@ pub fn create_getter(
         }),
     };
 
-    create_function(name, iter::empty(), ret, kind, is_structural)
+    create_function(name, iter::empty(), ret, kind, is_structural, catch)
 }
 
 pub fn create_setter(
@@ -271,6 +306,7 @@ pub fn create_setter(
     self_name: &str,
     is_static: bool,
     is_structural: bool,
+    catch: bool,
 ) -> Option<backend::ast::ImportFunction> {
     let kind = backend::ast::ImportFunctionKind::Method {
         class: self_name.to_string(),
@@ -287,6 +323,7 @@ pub fn create_setter(
         None,
         kind,
         is_structural,
+        catch,
     )
 }
 
@@ -315,3 +352,13 @@ pub fn is_structural(attrs: &[Box<ExtendedAttribute>]) -> bool {
         }
     })
 }
+
+pub fn throws(attrs: &[Box<ExtendedAttribute>]) -> bool {
+    attrs.iter().any(|attr| {
+        if let ExtendedAttribute::NoArguments(webidl::ast::Other::Identifier(ref name)) = **attr {
+            name == "Throws"
+        } else {
+            false
+        }
+    })
+}

crates/webidl/tests/all/main.rs

@@ -2,3 +2,4 @@ extern crate wasm_bindgen_test_project_builder as project_builder;
 use project_builder::project;
 
 mod simple;
+mod throws;

crates/webidl/tests/all/simple.rs

@@ -41,17 +41,17 @@ fn method() {
 
                 #[wasm_bindgen]
                 pub fn test() {
-                    let pi = Foo::new(3.14159);
+                    let pi = Foo::new(3.14159).unwrap();
-                    let e = Foo::new(2.71828);
+                    let e = Foo::new(2.71828).unwrap();
                     // TODO: figure out why the following doesn't fail
-                    // assert!(!pi.my_cmp(Foo::new(3.14159)));
+                    // assert!(!pi.my_cmp(Foo::new(3.14159).unwrap()));
-                    let tmp = pi.my_cmp(Foo::new(3.14159));
+                    let tmp = pi.my_cmp(Foo::new(3.14159).unwrap());
                     assert!(tmp);
-                    let tmp =!pi.my_cmp(Foo::new(2.71828));
+                    let tmp =!pi.my_cmp(Foo::new(2.71828).unwrap());
                     assert!(tmp);
-                    let tmp = !e.my_cmp(Foo::new(3.14159));
+                    let tmp = !e.my_cmp(Foo::new(3.14159).unwrap());
                     assert!(tmp);
-                    let tmp = e.my_cmp(Foo::new(2.71828));
+                    let tmp = e.my_cmp(Foo::new(2.71828).unwrap());
                     assert!(tmp);
                 }
             "#,
@@ -105,7 +105,7 @@ fn property() {
 
                 #[wasm_bindgen]
                 pub fn test() {
-                    let x = Foo::new(3.14159);
+                    let x = Foo::new(3.14159).unwrap();
                     assert_eq!(x.value(), 3.14159);
                     assert_ne!(x.value(), 2.71828);
                     x.set_value(2.71828);
@@ -167,7 +167,7 @@ fn named_constructor() {
 
                 #[wasm_bindgen]
                 pub fn test() {
-                    let x = Foo::new(3.14159);
+                    let x = Foo::new(3.14159).unwrap();
                     assert_eq!(x.value(), 3.14159);
                     assert_ne!(x.value(), 0.);
                 }
@@ -317,7 +317,7 @@ fn one_method_using_an_undefined_import_doesnt_break_all_other_methods() {
 
                 #[wasm_bindgen]
                 pub fn test() {
-                    let f = foo::Foo::new();
+                    let f = foo::Foo::new().unwrap();
                     assert!(f.ok_method());
                 }
             "#,
@@ -362,7 +362,7 @@ fn unforgeable_is_structural() {
 
                 #[wasm_bindgen]
                 pub fn test() {
-                    let f = foo::Foo::new();
+                    let f = foo::Foo::new().unwrap();
                     assert_eq!(f.uno(), 1);
                     assert_eq!(f.dos(), 2);
                 }

crates/webidl/tests/all/throws.rs

@@ -0,0 +1,100 @@
+use super::project;
+
+#[test]
+fn throws() {
+    project()
+        .file(
+            "thang.webidl",
+            r#"
+                [Constructor(long value)]
+                interface Thang {
+                    [Throws]
+                    attribute long ok_attr;
+                    [Throws]
+                    attribute long err_attr;
+
+                    [Throws]
+                    long ok_method();
+                    [Throws]
+                    long err_method();
+
+                    [Throws]
+                    static long ok_static_method();
+                    [Throws]
+                    static long err_static_method();
+
+                    [Throws]
+                    static attribute long ok_static_attr;
+                    [Throws]
+                    static attribute long err_static_attr;
+                };
+            "#,
+        )
+        .file(
+            "thang.js",
+            r#"
+                export class Thang {
+                    constructor(value) {
+                        if (value % 2 == 0) {
+                            throw new Error("only odd allowed");
+                        }
+                        this.value = value;
+                    }
+
+                    get ok_attr() { return this.value; }
+                    set ok_attr(x) { }
+
+                    get err_attr() { throw new Error("bad"); }
+                    set err_attr(x) { throw new Error("bad"); }
+
+                    ok_method() { return this.value + 1; }
+                    err_method() { throw new Error("bad"); }
+
+                    static ok_static_method() { return 1; }
+                    static err_static_method() { throw new Error("bad"); }
+
+                    static get ok_static_attr() { return 1; }
+                    static set ok_static_attr(x) { }
+
+                    static get err_static_attr() { throw new Error("bad"); }
+                    static set err_static_attr(x) { throw new Error("bad"); }
+                }
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                #![feature(proc_macro, wasm_custom_section, wasm_import_module)]
+                extern crate wasm_bindgen;
+                use wasm_bindgen::prelude::*;
+
+                pub mod thang;
+                use thang::Thang;
+
+                #[wasm_bindgen]
+                pub fn test() {
+                    assert!(Thang::new(0).is_err());
+                    let thang = Thang::new(1).unwrap();
+
+                    assert!(thang.ok_attr().is_ok());
+                    assert!(thang.set_ok_attr(0).is_ok());
+
+                    assert!(thang.err_attr().is_err());
+                    assert!(thang.set_err_attr(0).is_err());
+
+                    assert!(thang.ok_method().is_ok());
+                    assert!(thang.err_method().is_err());
+
+                    assert!(Thang::ok_static_method().is_ok());
+                    assert!(Thang::err_static_method().is_err());
+
+                    assert!(Thang::ok_static_attr().is_ok());
+                    assert!(Thang::set_ok_static_attr(0).is_ok());
+
+                    assert!(Thang::err_static_attr().is_err());
+                    assert!(Thang::set_err_static_attr(0).is_err());
+                }
+            "#,
+        )
+        .test();
+}