From 3cdb6ef03a236a09a4551c8923796f47bfad9bea Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 11 Jul 2018 11:07:03 -0700 Subject: [PATCH] webidl: Turn the `[Throws]` extended attributes into `Result` This sets the `catch` flag on the emitted AST when an operation/attribute has the `[Throws]` extended attribute on it. Additionally, constructors aren't annotated with `[Throws]` but can still throw exceptions, so we must conservatively assume *every* constructor can throw an error. --- crates/backend/src/util.rs | 23 ++++++- crates/webidl/src/lib.rs | 77 +++++++++++++++++++---- crates/webidl/src/util.rs | 57 +++++++++++++++-- crates/webidl/tests/all/main.rs | 1 + crates/webidl/tests/all/simple.rs | 22 +++---- crates/webidl/tests/all/throws.rs | 100 ++++++++++++++++++++++++++++++ 6 files changed, 251 insertions(+), 29 deletions(-) create mode 100644 crates/webidl/tests/all/throws.rs diff --git a/crates/backend/src/util.rs b/crates/backend/src/util.rs index 8773c4abf..9bdb96a37 100644 --- a/crates/backend/src/util.rs +++ b/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(segments: I) -> syn::Type +where + I: IntoIterator, +{ + 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(segments: I) -> syn::Type +where + I: IntoIterator, +{ + path_ty(true, segments) +} + +fn path_ty(leading_colon: bool, segments: I) -> syn::Type where I: IntoIterator, { @@ -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() diff --git a/crates/webidl/src/lib.rs b/crates/webidl/src/lib.rs index 12469d150..c9678b944 100644 --- a/crates/webidl/src/lib.rs +++ b/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) - .map(wrap_import_function) + create_getter( + &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) - .map(wrap_import_function) + create_setter( + &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) - .map(wrap_import_function) + create_getter( + &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) - .map(wrap_import_function) + create_setter( + &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)); diff --git a/crates/webidl/src/util.rs b/crates/webidl/src/util.rs index 480aba66c..b8edae82e 100644 --- a/crates/webidl/src/util.rs +++ b/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, + mut ret: Option, kind: backend::ast::ImportFunctionKind, structural: bool, + catch: bool, ) -> Option where I: Iterator, @@ -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 { 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 { 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 { 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]) -> bool { } }) } + +pub fn throws(attrs: &[Box]) -> bool { + attrs.iter().any(|attr| { + if let ExtendedAttribute::NoArguments(webidl::ast::Other::Identifier(ref name)) = **attr { + name == "Throws" + } else { + false + } + }) +} diff --git a/crates/webidl/tests/all/main.rs b/crates/webidl/tests/all/main.rs index dffe6eccd..6cce8f3db 100644 --- a/crates/webidl/tests/all/main.rs +++ b/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; diff --git a/crates/webidl/tests/all/simple.rs b/crates/webidl/tests/all/simple.rs index 726375336..ad177a086 100644 --- a/crates/webidl/tests/all/simple.rs +++ b/crates/webidl/tests/all/simple.rs @@ -41,17 +41,17 @@ fn method() { #[wasm_bindgen] pub fn test() { - let pi = Foo::new(3.14159); - let e = Foo::new(2.71828); + let pi = Foo::new(3.14159).unwrap(); + let e = Foo::new(2.71828).unwrap(); // TODO: figure out why the following doesn't fail - // assert!(!pi.my_cmp(Foo::new(3.14159))); - let tmp = 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).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); } diff --git a/crates/webidl/tests/all/throws.rs b/crates/webidl/tests/all/throws.rs new file mode 100644 index 000000000..e930cc3ff --- /dev/null +++ b/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(); +}