diff --git a/docs/manual/src/udl/interfaces.md b/docs/manual/src/udl/interfaces.md index 465dab869a..1f6deade2d 100644 --- a/docs/manual/src/udl/interfaces.md +++ b/docs/manual/src/udl/interfaces.md @@ -74,6 +74,27 @@ func display(list: TodoListProtocol) { } ``` +## Alternate Named Constructors + +In addition to the default constructor connected to the `::new()` method, you can specify +alternate named constructors to create object instances in different ways. Each such constructor +must be given an explicit name, provided in the UDL with the `[Name]` attribute like so: + +```idl +interface TodoList { + // The default constructor makes an empty list. + constructor(); + // This alternate constructor makes a new TodoList from a list of string items. + [Name=new_from_items] + constructor(sequence items) + ... +``` + +For each alternate constructor, UniFFI will expose an appropriate static-method, class-method or similar +in the foreign language binding, and will connect it to the Rust method of the same name on the underlying +Rust struct. + + ## Concurrent Access Since interfaces represent mutable data, uniffi has to take extra care diff --git a/examples/sprites/src/lib.rs b/examples/sprites/src/lib.rs index 3721e37a21..2b1a6b65b3 100644 --- a/examples/sprites/src/lib.rs +++ b/examples/sprites/src/lib.rs @@ -39,6 +39,12 @@ impl Sprite { } } + fn new_relative_to(reference: Point, direction: Vector) -> Sprite { + Sprite { + current_position: translate(&reference, direction), + } + } + fn get_position(&self) -> Point { self.current_position.clone() } diff --git a/examples/sprites/src/sprites.udl b/examples/sprites/src/sprites.udl index e76c33ac08..6781c6cee5 100644 --- a/examples/sprites/src/sprites.udl +++ b/examples/sprites/src/sprites.udl @@ -14,8 +14,8 @@ dictionary Vector { }; interface Sprite { - // Should be an optional, but I had to test nullable args :) constructor(Point? initial_position); + [Name=new_relative_to] constructor(Point reference, Vector direction); Point get_position(); void move_to(Point position); void move_by(Vector direction); diff --git a/examples/sprites/tests/bindings/test_sprites.kts b/examples/sprites/tests/bindings/test_sprites.kts index a023e11ab5..42451f28dd 100644 --- a/examples/sprites/tests/bindings/test_sprites.kts +++ b/examples/sprites/tests/bindings/test_sprites.kts @@ -13,9 +13,13 @@ s.moveBy(Vector(-4.0, 2.0)) assert( s.getPosition() == Point(-3.0, 4.0) ) s.destroy() -try { +try { s.moveBy(Vector(0.0, 0.0)) assert(false) { "Should not be able to call anything after `destroy`" } } catch(e: IllegalStateException) { assert(true) -} \ No newline at end of file +} + +val srel = Sprite.newRelativeTo(Point(0.0, 1.0), Vector(1.0, 1.5)) +assert( srel.getPosition() == Point(1.0, 2.5) ) + diff --git a/examples/sprites/tests/bindings/test_sprites.py b/examples/sprites/tests/bindings/test_sprites.py index 45c130c8c9..5142c2fc42 100644 --- a/examples/sprites/tests/bindings/test_sprites.py +++ b/examples/sprites/tests/bindings/test_sprites.py @@ -11,3 +11,7 @@ s.move_by(Vector(-4, 2)) assert s.get_position() == Point(-3, 4) + +srel = Sprite.new_relative_to(Point(0, 1), Vector(1, 1.5)) +assert srel.get_position() == Point(1, 2.5) + diff --git a/examples/sprites/tests/bindings/test_sprites.swift b/examples/sprites/tests/bindings/test_sprites.swift index 8ff51ac597..d5428ac679 100644 --- a/examples/sprites/tests/bindings/test_sprites.swift +++ b/examples/sprites/tests/bindings/test_sprites.swift @@ -12,4 +12,5 @@ assert( s.getPosition() == Point(x: 1, y: 2)) s.moveBy(direction: Vector(dx: -4, dy: 2)) assert( s.getPosition() == Point(x: -3, y: 4)) - +let srel = Sprite.newRelativeTo(reference: Point(x: 0.0, y: 1.0), direction: Vector(dx: 1, dy: 1.5)) +assert( srel.getPosition() == Point(x: 1.0, y: 2.5) ) diff --git a/examples/sprites/tests/test_generated_bindings.rs b/examples/sprites/tests/test_generated_bindings.rs index d9539977de..fa4354c096 100644 --- a/examples/sprites/tests/test_generated_bindings.rs +++ b/examples/sprites/tests/test_generated_bindings.rs @@ -1,7 +1,7 @@ uniffi_macros::build_foreign_language_testcases!( "src/sprites.udl", [ - // "tests/bindings/test_sprites.py", + "tests/bindings/test_sprites.py", "tests/bindings/test_sprites.kts", "tests/bindings/test_sprites.swift", ] diff --git a/uniffi_bindgen/src/bindings/gecko_js/templates/InterfaceTemplate.cpp b/uniffi_bindgen/src/bindings/gecko_js/templates/InterfaceTemplate.cpp index f1843a15ea..ec3aa0b76a 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/templates/InterfaceTemplate.cpp +++ b/uniffi_bindgen/src/bindings/gecko_js/templates/InterfaceTemplate.cpp @@ -39,7 +39,8 @@ JSObject* {{ obj.name()|class_name_cpp(context) }}::WrapObject( return dom::{{ obj.name()|class_name_cpp(context) }}_Binding::Wrap(aCx, this, aGivenProto); } -{%- for cons in obj.constructors() %} +{%- match obj.primary_constructor() %} +{%- when Some with (cons) %} /* static */ already_AddRefed<{{ obj.name()|class_name_cpp(context) }}> {{ obj.name()|class_name_cpp(context) }}::Constructor( @@ -61,10 +62,14 @@ already_AddRefed<{{ obj.name()|class_name_cpp(context) }}> {{ obj.name()|class_n auto result = MakeRefPtr<{{ obj.name()|class_name_cpp(context) }}>(global, handle); return result.forget(); } +{%- when None %} +{%- endmatch %} + +{%- for cons in obj.alternate_constructors() %} +MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support alternate constructors"); {%- endfor %} {%- for meth in obj.methods() %} - {% match meth.cpp_return_type() %}{% when Some with (type_) %}{{ type_|ret_type_cpp(context) }}{% else %}void{% endmatch %} {{ obj.name()|class_name_cpp(context) }}::{{ meth.name()|fn_name_cpp }}( {%- for arg in meth.cpp_arguments() %} {{ arg|arg_type_cpp(context) }} {{ arg.name() }}{%- if !loop.last %},{% endif %} diff --git a/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl b/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl index eddd93fdec..c27cb89729 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl +++ b/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl @@ -46,7 +46,9 @@ namespace {{ context.namespace()|class_name_webidl(context) }} { {%- for obj in ci.iter_object_definitions() %} [ChromeOnly, Exposed=Window] interface {{ obj.name()|class_name_webidl(context) }} { - {%- for cons in obj.constructors() %} + + {%- match obj.primary_constructor() %} + {%- when Some with (cons) %} {%- if cons.throws().is_some() %} [Throws] {% endif %} @@ -60,6 +62,23 @@ interface {{ obj.name()|class_name_webidl(context) }} { {%- if !loop.last %}, {% endif %} {%- endfor %} ); + {%- when None %} + {%- endmatch %} + + {%- for cons in obj.alternate_constructors() %} + {%- if cons.throws().is_some() %} + [Throws] + {% endif %} + {{ obj.name()|class_name_webidl(context) }} {{ cons.name()|fn_name_webidl }}( + {%- for arg in cons.arguments() %} + {% if arg.optional() -%}optional{%- else -%}{%- endif %} {{ arg.webidl_type()|type_webidl(context) }} {{ arg.name() }} + {%- match arg.webidl_default_value() %} + {%- when Some with (literal) %} = {{ literal|literal_webidl }} + {%- else %} + {%- endmatch %} + {%- if !loop.last %}, {% endif %} + {%- endfor %} + ); {%- endfor %} {% for meth in obj.methods() -%} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index c2ab9a223f..90e9fd4074 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -12,10 +12,12 @@ class {{ obj.name()|class_name_kt }}( handle: Long ) : FFIObject(AtomicLong(handle)), {{ obj.name()|class_name_kt }}Interface { - {%- for cons in obj.constructors() %} + {%- match obj.primary_constructor() %} + {%- when Some with (cons) %} constructor({% call kt::arg_list_decl(cons) -%}) : this({% call kt::to_ffi_call(cons) %}) - {%- endfor %} + {%- when None %} + {%- endmatch %} /** * Disconnect the object from the underlying Rust object. @@ -48,12 +50,21 @@ class {{ obj.name()|class_name_kt }}( }.let { {{ "it"|lift_kt(return_type) }} } - + {%- when None -%} override fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_protocol(meth) %}) = callWithHandle { - {%- call kt::to_ffi_call_with_prefix("it", meth) %} + {%- call kt::to_ffi_call_with_prefix("it", meth) %} } {% endmatch %} {% endfor %} + + {% if obj.constructors().len() > 1 -%} + companion object { + {% for cons in obj.alternate_constructors() -%} + fun {{ cons.name()|fn_name_kt }}({% call kt::arg_list_decl(cons) %}): {{ obj.name()|class_name_kt }} = + {{ obj.name()|class_name_kt }}({% call kt::to_ffi_call(cons) %}) + {% endfor %} + } + {%- endif %} } diff --git a/uniffi_bindgen/src/bindings/python/gen_python.rs b/uniffi_bindgen/src/bindings/python/gen_python.rs index 6c086c5995..bc06e6fb84 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python.rs @@ -183,7 +183,7 @@ mod filters { Type::Enum(name) => format!("{}({})", class_name_py(name)?, nm), Type::Object(_) => panic!("No support for lifting objects, yet"), Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"), - Type::Error(_) => panic!("No support for lowering errors, yet"), + Type::Error(_) => panic!("No support for lifting errors, yet"), Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => format!( "{}.consumeInto{}()", nm, diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 930e670635..cec5a695c0 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -1,10 +1,11 @@ class {{ obj.name()|class_name_py }}(object): - # XXX TODO: support for multiple constructors... - {%- for cons in obj.constructors() %} + {%- match obj.primary_constructor() %} + {%- when Some with (cons) %} def __init__(self, {% call py::arg_list_decl(cons) -%}): {%- call py::coerce_args_extra_indent(cons) %} - self._handle = {% call py::to_ffi_call(cons) %} - {%- endfor %} + self._handle = {% call py::to_ffi_call(cons) %}\ + {%- when None %} + {%- endmatch %} def __del__(self): rust_call_with_error( @@ -13,6 +14,19 @@ def __del__(self): self._handle ) + {% for cons in obj.alternate_constructors() -%} + @classmethod + def {{ cons.name()|fn_name_py }}(cls, {% call py::arg_list_decl(cons) %}): + {%- call py::coerce_args_extra_indent(cons) %} + # Call the (fallible) function before creating any half-baked object instances. + handle = {% call py::to_ffi_call(cons) %} + # Lightly yucky way to bypass the usual __init__ logic + # and just create a new instance with the required handle. + inst = cls.__new__(cls) + inst._handle = handle + return inst + {% endfor %} + {% for meth in obj.methods() -%} {%- match meth.return_type() -%} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 672f7b23b3..3b744b6d52 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -9,14 +9,20 @@ public protocol {{ obj.name() }}Protocol { {% endfor %} } -public class {{ obj.name() }}: {{ obj.name() }}Protocol { +public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol { private let handle: UInt64 - {%- for cons in obj.constructors() %} - public init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { - self.handle = {% call swift::to_ffi_call(cons) %} + private init(fromRawHandle handle: UInt64) { + self.handle = handle } - {%- endfor %} + + {%- match obj.primary_constructor() %} + {%- when Some with (cons) %} + public convenience init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { + self.init(fromRawHandle: {% call swift::to_ffi_call(cons) %}) + } + {%- when None %} + {%- endmatch %} deinit { try! rustCall(InternalError.unknown()) { err in @@ -24,7 +30,13 @@ public class {{ obj.name() }}: {{ obj.name() }}Protocol { } } - // TODO: Maybe merge the two templates (i.e the one with a return type and the one without) + {% for cons in obj.alternate_constructors() %} + public static func {{ cons.name()|fn_name_swift }}({% call swift::arg_list_decl(cons) %}) {% call swift::throws(cons) %} -> {{ obj.name()|class_name_swift }} { + return {{ obj.name()|class_name_swift }}(fromRawHandle: {% call swift::to_ffi_call(cons) %}) + } + {% endfor %} + + {# // TODO: Maybe merge the two templates (i.e the one with a return type and the one without) #} {% for meth in obj.methods() -%} {%- match meth.return_type() -%} diff --git a/uniffi_bindgen/src/interface/attributes.rs b/uniffi_bindgen/src/interface/attributes.rs index 0ba78fb2f0..a386934612 100644 --- a/uniffi_bindgen/src/interface/attributes.rs +++ b/uniffi_bindgen/src/interface/attributes.rs @@ -27,6 +27,7 @@ use anyhow::{bail, Result}; pub(super) enum Attribute { ByRef, Error, + Name(String), Threadsafe, Throws(String), } @@ -54,20 +55,13 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute { }, // Matches assignment-style attributes like ["Throws=Error"] weedle::attribute::ExtendedAttribute::Ident(identity) => { - if identity.lhs_identifier.0 == "Throws" { - Ok(Attribute::Throws(match identity.rhs { - weedle::attribute::IdentifierOrString::Identifier(identifier) => { - identifier.0.to_string() - } - weedle::attribute::IdentifierOrString::String(str_lit) => { - str_lit.0.to_string() - } - })) - } else { - anyhow::bail!( + match identity.lhs_identifier.0 { + "Name" => Ok(Attribute::Name(name_from_id_or_string(&identity.rhs))), + "Throws" => Ok(Attribute::Throws(name_from_id_or_string(&identity.rhs))), + _ => anyhow::bail!( "Attribute identity Identifier not supported: {:?}", identity.lhs_identifier.0 - ) + ), } } _ => anyhow::bail!("Attribute not supported: {:?}", weedle_attribute), @@ -75,6 +69,13 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute { } } +fn name_from_id_or_string(nm: &weedle::attribute::IdentifierOrString<'_>) -> String { + match nm { + weedle::attribute::IdentifierOrString::Identifier(identifier) => identifier.0.to_string(), + weedle::attribute::IdentifierOrString::String(str_lit) => str_lit.0.to_string(), + } +} + /// Parse a weedle `ExtendedAttributeList` into a list of `Attribute`s, /// erroring out on duplicates. fn parse_attributes( @@ -154,10 +155,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttribut ) -> Result { let attrs = parse_attributes(weedle_attributes, |attr| match attr { Attribute::Throws(_) => Ok(()), - _ => bail!(format!( - "{:?} not supported for functions, methods or constructors", - attr - )), + _ => bail!(format!("{:?} not supported for functions or methods", attr)), })?; Ok(Self(attrs)) } @@ -218,10 +216,44 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for InterfaceAttribu // but not yet. pub(super) type MethodAttributes = FunctionAttributes; -// Similarly, currently Constructors only support the same attributes as Functions. -// When this changes, (e.g. https://github.com/mozilla/uniffi-rs/issues/37 to support multiple -// named constructors), ConstructorAttributes will need its own implementation. -pub(super) type ConstructorAttributes = FunctionAttributes; +// Constructors can have a `Name` attribute, so need their own implementation. +/// Represents UDL attributes that might appear on a function. +/// +/// This supports the `[Throws=ErrorName]` attribute for functions that +/// can produce an error. +#[derive(Debug, Clone, Hash, Default)] +pub(super) struct ConstructorAttributes(Vec); + +impl ConstructorAttributes { + pub(super) fn get_throws_err(&self) -> Option<&str> { + self.0.iter().find_map(|attr| match attr { + // This will hopefully return a helpful compilation error + // if the error is not defined. + Attribute::Throws(inner) => Some(inner.as_ref()), + _ => None, + }) + } + pub(super) fn get_name(&self) -> Option<&str> { + self.0.iter().find_map(|attr| match attr { + Attribute::Name(inner) => Some(inner.as_ref()), + _ => None, + }) + } +} + +impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for ConstructorAttributes { + type Error = anyhow::Error; + fn try_from( + weedle_attributes: &weedle::attribute::ExtendedAttributeList<'_>, + ) -> Result { + let attrs = parse_attributes(weedle_attributes, |attr| match attr { + Attribute::Throws(_) => Ok(()), + Attribute::Name(_) => Ok(()), + _ => bail!(format!("{:?} not supported for constructors", attr)), + })?; + Ok(Self(attrs)) + } +} #[cfg(test)] mod test { @@ -245,6 +277,22 @@ mod test { Ok(()) } + #[test] + fn test_name() -> Result<()> { + let (_, node) = weedle::attribute::ExtendedAttribute::parse("Name=Value").unwrap(); + let attr = Attribute::try_from(&node)?; + assert!(matches!(attr, Attribute::Name(nm) if nm == "Value")); + + let (_, node) = weedle::attribute::ExtendedAttribute::parse("Name").unwrap(); + let err = Attribute::try_from(&node).unwrap_err(); + assert_eq!( + err.to_string(), + "ExtendedAttributeNoArgs not supported: \"Name\"" + ); + + Ok(()) + } + #[test] fn test_threadsafe() -> Result<()> { let (_, node) = weedle::attribute::ExtendedAttribute::parse("Threadsafe").unwrap(); @@ -317,11 +365,43 @@ mod test { let err = FunctionAttributes::try_from(&node).unwrap_err(); assert_eq!( err.to_string(), - "ByRef not supported for functions, methods or constructors" + "ByRef not supported for functions or methods" ); Ok(()) } + #[test] + fn test_constructor_attributes() -> Result<()> { + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Throws=Error]").unwrap(); + let attrs = ConstructorAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.get_throws_err(), Some("Error"))); + assert!(matches!(attrs.get_name(), None)); + + let (_, node) = + weedle::attribute::ExtendedAttributeList::parse("[Name=MyFactory]").unwrap(); + let attrs = ConstructorAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.get_throws_err(), None)); + assert!(matches!(attrs.get_name(), Some("MyFactory"))); + + let (_, node) = + weedle::attribute::ExtendedAttributeList::parse("[Throws=Error, Name=MyFactory]") + .unwrap(); + let attrs = ConstructorAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.get_throws_err(), Some("Error"))); + assert!(matches!(attrs.get_name(), Some("MyFactory"))); + + Ok(()) + } + + #[test] + fn test_other_attributes_not_supported_for_constructors() -> Result<()> { + let (_, node) = + weedle::attribute::ExtendedAttributeList::parse("[Throws=Error, ByRef]").unwrap(); + let err = ConstructorAttributes::try_from(&node).unwrap_err(); + assert_eq!(err.to_string(), "ByRef not supported for constructors"); + Ok(()) + } + #[test] fn test_byref_attribute() -> Result<()> { let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[ByRef]").unwrap(); diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 940de7045d..1b385691c6 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -45,6 +45,7 @@ //! # Ok::<(), anyhow::Error>(()) //! ``` +use std::collections::HashSet; use std::convert::TryFrom; use std::hash::{Hash, Hasher}; @@ -98,6 +99,19 @@ impl Object { self.constructors.iter().collect() } + pub fn primary_constructor(&self) -> Option<&Constructor> { + self.constructors + .iter() + .find(|cons| cons.is_primary_constructor()) + } + + pub fn alternate_constructors(&self) -> Vec<&Constructor> { + self.constructors + .iter() + .filter(|cons| !cons.is_primary_constructor()) + .collect() + } + pub fn methods(&self) -> Vec<&Method> { self.methods.iter().collect() } @@ -146,29 +160,38 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { if self.inheritance.is_some() { bail!("interface inheritence is not supported"); } + let mut object = Object::new(self.identifier.0.to_string()); let attributes = match &self.attributes { Some(attrs) => InterfaceAttributes::try_from(attrs)?, None => Default::default(), }; - let mut object = Object::new(self.identifier.0.to_string()); + object.threadsafe = attributes.threadsafe(); + // Convert each member into a constructor or method, guarding against duplicate names. + let mut member_names = HashSet::new(); for member in &self.members.body { match member { weedle::interface::InterfaceMember::Constructor(t) => { - object.constructors.push(t.convert(ci)?); + let cons = t.convert(ci)?; + if !member_names.insert(cons.name.clone()) { + bail!("Duplicate interface member name: \"{}\"", cons.name()) + } + object.constructors.push(cons); } weedle::interface::InterfaceMember::Operation(t) => { let mut method = t.convert(ci)?; + if !member_names.insert(method.name.clone()) { + bail!("Duplicate interface member name: \"{}\"", method.name()) + } method.object_name.push_str(object.name.as_str()); object.methods.push(method); } _ => bail!("no support for interface member type {:?} yet", member), } } - if object.constructors.is_empty() { + // Everyone gets a primary constructor, even if not declared explicitly. + if object.primary_constructor().is_none() { object.constructors.push(Default::default()); } - - object.threadsafe = attributes.threadsafe(); Ok(object) } } @@ -212,6 +235,10 @@ impl Constructor { self.ffi_func.return_type = Some(FFIType::UInt64); Ok(()) } + + fn is_primary_constructor(&self) -> bool { + self.name == "new" + } } impl Hash for Constructor { @@ -241,14 +268,15 @@ impl Default for Constructor { impl APIConverter for weedle::interface::ConstructorInterfaceMember<'_> { fn convert(&self, ci: &mut ComponentInterface) -> Result { + let attributes = match &self.attributes { + Some(attr) => ConstructorAttributes::try_from(attr)?, + None => Default::default(), + }; Ok(Constructor { - name: String::from("new"), // TODO: get the name from an attribute maybe? + name: String::from(attributes.get_name().unwrap_or("new")), arguments: self.args.body.list.convert(ci)?, ffi_func: Default::default(), - attributes: match &self.attributes { - Some(attr) => ConstructorAttributes::try_from(attr)?, - None => Default::default(), - }, + attributes, }) } } @@ -337,8 +365,8 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { if self.special.is_some() { bail!("special operations not supported"); } - if let Some(weedle::interface::StringifierOrStatic::Stringifier(_)) = self.modifier { - bail!("stringifiers are not supported"); + if self.modifier.is_some() { + bail!("method modifiers are not supported") } let return_type = ci.resolve_return_type_expression(&self.return_type)?; if let Some(Type::Object(_)) = return_type { @@ -347,7 +375,13 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { Ok(Method { name: match self.identifier { None => bail!("anonymous methods are not supported {:?}", self), - Some(id) => id.0.to_string(), + Some(id) => { + let name = id.0.to_string(); + if name == "new" { + bail!("the method name \"new\" is reserved for the default constructor"); + } + name + } }, // We don't know the name of the containing `Object` at this point, fill it in later. object_name: Default::default(), @@ -413,4 +447,106 @@ mod test { Ok(()) } + + #[test] + fn test_alternate_constructors() -> Result<()> { + const UDL: &str = r#" + namespace test{}; + interface Testing { + constructor(); + [Name=new_with_u32] + constructor(u32 v); + }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.iter_object_definitions().len(), 1); + + let obj = ci.get_object_definition("Testing").unwrap(); + assert!(obj.primary_constructor().is_some()); + assert_eq!(obj.alternate_constructors().len(), 1); + assert_eq!(obj.methods().len(), 0); + + let cons = obj.primary_constructor().unwrap(); + assert_eq!(cons.name(), "new"); + assert_eq!(cons.arguments.len(), 0); + assert_eq!(cons.ffi_func.arguments.len(), 0); + + let cons = obj.alternate_constructors()[0]; + assert_eq!(cons.name(), "new_with_u32"); + assert_eq!(cons.arguments.len(), 1); + assert_eq!(cons.ffi_func.arguments.len(), 1); + + Ok(()) + } + + #[test] + fn test_the_name_new_identifies_the_primary_constructor() -> Result<()> { + const UDL: &str = r#" + namespace test{}; + interface Testing { + [Name=newish] + constructor(); + [Name=new] + constructor(u32 v); + }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.iter_object_definitions().len(), 1); + + let obj = ci.get_object_definition("Testing").unwrap(); + assert!(obj.primary_constructor().is_some()); + assert_eq!(obj.alternate_constructors().len(), 1); + assert_eq!(obj.methods().len(), 0); + + let cons = obj.primary_constructor().unwrap(); + assert_eq!(cons.name(), "new"); + assert_eq!(cons.arguments.len(), 1); + + let cons = obj.alternate_constructors()[0]; + assert_eq!(cons.name(), "newish"); + assert_eq!(cons.arguments.len(), 0); + assert_eq!(cons.ffi_func.arguments.len(), 0); + + Ok(()) + } + + #[test] + fn test_the_name_new_is_reserved_for_constructors() { + const UDL: &str = r#" + namespace test{}; + interface Testing { + constructor(); + void new(u32 v); + }; + "#; + let err = ComponentInterface::from_webidl(UDL).unwrap_err(); + assert_eq!( + err.to_string(), + "the method name \"new\" is reserved for the default constructor" + ); + } + + #[test] + fn test_duplicate_primary_constructors_not_allowed() { + const UDL: &str = r#" + namespace test{}; + interface Testing { + constructor(); + constructor(u32 v); + }; + "#; + let err = ComponentInterface::from_webidl(UDL).unwrap_err(); + assert_eq!(err.to_string(), "Duplicate interface member name: \"new\""); + + const UDL2: &str = r#" + namespace test{}; + interface Testing { + constructor(); + [Name=new] + constructor(u32 v); + }; + "#; + let err = ComponentInterface::from_webidl(UDL2).unwrap_err(); + assert_eq!(err.to_string(), "Duplicate interface member name: \"new\""); + } }