Use Option<T> to return from getters

This removes the cumbersome &mut bool argument and offers overall
a more readable code.

components/script/dom/bindings/codegen/CodegenRust.py

@@ -18,6 +18,7 @@ from WebIDL import (
     BuiltinTypes,
     IDLBuiltinType,
     IDLNullValue,
+    IDLNullableType,
     IDLType,
     IDLInterfaceMember,
     IDLUndefinedValue,
@@ -4555,6 +4556,8 @@ class CGProxySpecialOperation(CGPerSignatureCall):
         signature = operation.signatures()[0]
 
         (returnType, arguments) = signature
+        if operation.isGetter() and not returnType.nullable():
+            returnType = IDLNullableType(returnType.location, returnType)
 
         # We pass len(arguments) as the final argument so that the
         # CGPerSignatureCall won't do any argument conversion of its own.
@@ -4577,8 +4580,6 @@ class CGProxySpecialOperation(CGPerSignatureCall):
             self.cgRoot.prepend(instantiateJSToNativeConversionTemplate(
                 template, templateValues, declType, argument.identifier.name))
             self.cgRoot.prepend(CGGeneric("rooted!(in(cx) let value = desc.value);"))
-        elif operation.isGetter():
-            self.cgRoot.prepend(CGGeneric("let mut found = false;"))
 
     def getArguments(self):
         def process(arg):
@@ -4587,10 +4588,6 @@ class CGProxySpecialOperation(CGPerSignatureCall):
                 argVal += ".r()"
             return argVal
         args = [(a, process(a)) for a in self.arguments]
-        if self.idlNode.isGetter():
-            args.append((FakeArgument(BuiltinTypes[IDLBuiltinType.Types.boolean],
-                                      self.idlNode),
-                         "&mut found"))
         return args
 
     def wrap_return_value(self):
@@ -4598,7 +4595,7 @@ class CGProxySpecialOperation(CGPerSignatureCall):
             return ""
 
         wrap = CGGeneric(wrapForType(**self.templateValues))
-        wrap = CGIfWrapper("found", wrap)
+        wrap = CGIfWrapper("let Some(result) = result", wrap)
         return "\n" + wrap.define()
 
 
@@ -4974,7 +4971,7 @@ class CGDOMJSProxyHandler_hasOwn(CGAbstractExternMethod):
                        "    let this = UnwrapProxy(proxy);\n" +
                        "    let this = &*this;\n" +
                        CGIndenter(CGProxyIndexedGetter(self.descriptor)).define() + "\n" +
-                       "    *bp = found;\n" +
+                       "    *bp = result.is_some();\n" +
                        "    return true;\n" +
                        "}\n\n")
         else:
@@ -4990,7 +4987,7 @@ if RUST_JSID_IS_STRING(id) {
     }
     if !has_on_proto {
         %s
-        *bp = found;
+        *bp = result.is_some();
         return true;
     }
 }
@@ -5274,7 +5271,9 @@ class CGInterfaceTrait(CGThing):
 
                     infallible = 'infallible' in descriptor.getExtendedAttributes(operation)
                     if operation.isGetter():
-                        arguments = method_arguments(descriptor, rettype, arguments, trailing=("found", "&mut bool"))
+                        if not rettype.nullable():
+                            rettype = IDLNullableType(rettype.location, rettype)
+                        arguments = method_arguments(descriptor, rettype, arguments)
 
                         # If this interface 'supports named properties', then we
                         # should be able to access 'supported property names'

components/script/dom/cssstyledeclaration.rs

@@ -100,18 +100,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-item
     fn Item(&self, index: u32) -> DOMString {
-        let index = index as usize;
-        let elem = self.owner.upcast::<Element>();
-        let style_attribute = elem.style_attribute().borrow();
-        style_attribute.as_ref().and_then(|declarations| {
-            declarations.declarations.get(index)
-        }).map(|&(ref declaration, importance)| {
-            let mut css = declaration.to_css_string();
-            if importance.important() {
-                css += " !important";
-            }
-            DOMString::from(css)
-        }).unwrap_or_else(DOMString::new)
+        self.IndexedGetter(index).unwrap_or_default()
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
@@ -333,10 +322,19 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> DOMString {
-        let rval = self.Item(index);
-        *found = index < self.Length();
-        rval
+    fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
+        let index = index as usize;
+        let elem = self.owner.upcast::<Element>();
+        let style_attribute = elem.style_attribute().borrow();
+        style_attribute.as_ref().and_then(|declarations| {
+            declarations.declarations.get(index)
+        }).map(|&(ref declaration, importance)| {
+            let mut css = declaration.to_css_string();
+            if importance.important() {
+                css += " !important";
+            }
+            DOMString::from(css)
+        })
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext

components/script/dom/document.rs

@@ -91,7 +91,7 @@ use euclid::point::Point2D;
 use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
 use ipc_channel::ipc::{self, IpcSender};
 use js::jsapi::JS_GetRuntime;
-use js::jsapi::{JSContext, JSObject, JSRuntime, JS_NewPlainObject};
+use js::jsapi::{JSContext, JSObject, JSRuntime};
 use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
 use msg::constellation_msg::{Key, KeyModifiers, KeyState};
 use msg::constellation_msg::{PipelineId, ReferrerPolicy, SubpageId};
@@ -2691,8 +2691,7 @@ impl DocumentMethods for Document {
 
     #[allow(unsafe_code)]
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
-    fn NamedGetter(&self, cx: *mut JSContext, name: DOMString, found: &mut bool)
-                   -> NonZero<*mut JSObject> {
+    fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option<NonZero<*mut JSObject>> {
         #[derive(JSTraceable, HeapSizeOf)]
         struct NamedElementFilter {
             name: Atom,
@@ -2758,28 +2757,23 @@ impl DocumentMethods for Document {
                                    .peekable();
             if let Some(first) = elements.next() {
                 if elements.peek().is_none() {
-                    *found = true;
                     // TODO: Step 2.
                     // Step 3.
                     return unsafe {
-                        NonZero::new(first.reflector().get_jsobject().get())
+                        Some(NonZero::new(first.reflector().get_jsobject().get()))
                     };
                 }
             } else {
-                *found = false;
-                return unsafe {
-                    NonZero::new(JS_NewPlainObject(cx))
-                };
+                return None;
             }
         }
         // Step 4.
-        *found = true;
         let filter = NamedElementFilter {
             name: name,
         };
         let collection = HTMLCollection::create(self.window(), root, box filter);
         unsafe {
-            NonZero::new(collection.reflector().get_jsobject().get())
+            Some(NonZero::new(collection.reflector().get_jsobject().get()))
         }
     }
 

components/script/dom/domrectlist.rs

@@ -52,8 +52,7 @@ impl DOMRectListMethods for DOMRectList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<DOMRect>> {
-        *found = index < self.rects.len() as u32;
+    fn IndexedGetter(&self, index: u32) -> Option<Root<DOMRect>> {
         self.Item(index)
     }
 }

components/script/dom/domstringmap.rs

@@ -47,10 +47,8 @@ impl DOMStringMapMethods for DOMStringMap {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-domstringmap-nameditem
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> DOMString {
-        let attr = self.element.get_custom_attr(name);
-        *found = attr.is_some();
-        attr.unwrap_or_default()
+    fn NamedGetter(&self, name: DOMString) -> Option<DOMString> {
+        self.element.get_custom_attr(name)
     }
 
     // https://html.spec.whatwg.org/multipage/#the-domstringmap-interface:supported-property-names

components/script/dom/domtokenlist.rs

@@ -171,9 +171,7 @@ impl DOMTokenListMethods for DOMTokenList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<DOMString> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
+        self.Item(index)
     }
 }

components/script/dom/filelist.rs

@@ -55,9 +55,7 @@ impl FileListMethods for FileList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<File>> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<File>> {
+        self.Item(index)
     }
 }

components/script/dom/htmlcollection.rs

@@ -317,17 +317,13 @@ impl HTMLCollectionMethods for HTMLCollection {
     }
 
     // https://dom.spec.whatwg.org/#dom-htmlcollection-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Element>> {
-        let maybe_elem = self.Item(index);
-        *found = maybe_elem.is_some();
-        maybe_elem
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Element>> {
+        self.Item(index)
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option<Root<Element>> {
-        let maybe_elem = self.NamedItem(name);
-        *found = maybe_elem.is_some();
-        maybe_elem
+    fn NamedGetter(&self, name: DOMString) -> Option<Root<Element>> {
+        self.NamedItem(name)
     }
 
     // https://dom.spec.whatwg.org/#interface-htmlcollection

components/script/dom/htmlformcontrolscollection.rs

@@ -77,10 +77,8 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-htmlformcontrolscollection-nameditem
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option<RadioNodeListOrElement> {
-        let maybe_elem = self.NamedItem(name);
-        *found = maybe_elem.is_some();
-        maybe_elem
+    fn NamedGetter(&self, name: DOMString) -> Option<RadioNodeListOrElement> {
+        self.NamedItem(name)
     }
 
     // https://html.spec.whatwg.org/multipage/#the-htmlformcontrolscollection-interface:supported-property-names
@@ -93,7 +91,7 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
     // https://github.com/servo/servo/issues/5875
     //
     // https://dom.spec.whatwg.org/#dom-htmlcollection-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Element>> {
-        self.collection.IndexedGetter(index, found)
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Element>> {
+        self.collection.IndexedGetter(index)
     }
 }

components/script/dom/htmlformelement.rs

@@ -230,9 +230,9 @@ impl HTMLFormElementMethods for HTMLFormElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-form-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Element>> {
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Element>> {
         let elements = self.Elements();
-        elements.IndexedGetter(index, found)
+        elements.IndexedGetter(index)
     }
 }
 

components/script/dom/mimetypearray.rs

@@ -46,12 +46,12 @@ impl MimeTypeArrayMethods for MimeTypeArray {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-mimetypearray-item
-    fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<Root<MimeType>> {
+    fn IndexedGetter(&self, _index: u32) -> Option<Root<MimeType>> {
         None
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option<Root<MimeType>> {
+    fn NamedGetter(&self, _name: DOMString) -> Option<Root<MimeType>> {
         None
     }
 

components/script/dom/namednodemap.rs

@@ -85,17 +85,13 @@ impl NamedNodeMapMethods for NamedNodeMap {
     }
 
     // https://dom.spec.whatwg.org/#dom-namednodemap-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Attr>> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Attr>> {
+        self.Item(index)
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option<Root<Attr>> {
-        let item = self.GetNamedItem(name);
-        *found = item.is_some();
-        item
+    fn NamedGetter(&self, name: DOMString) -> Option<Root<Attr>> {
+        self.GetNamedItem(name)
     }
 
     // https://heycam.github.io/webidl/#dfn-supported-property-names

components/script/dom/nodelist.rs

@@ -75,10 +75,8 @@ impl NodeListMethods for NodeList {
     }
 
     // https://dom.spec.whatwg.org/#dom-nodelist-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Node>> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Node>> {
+        self.Item(index)
     }
 }
 

components/script/dom/plugin.rs

@@ -45,12 +45,12 @@ impl PluginMethods for Plugin {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-plugin-item
-    fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<Root<MimeType>> {
+    fn IndexedGetter(&self, _index: u32) -> Option<Root<MimeType>> {
         unreachable!()
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option<Root<MimeType>> {
+    fn NamedGetter(&self, _name: DOMString) -> Option<Root<MimeType>> {
         unreachable!()
     }
 

components/script/dom/pluginarray.rs

@@ -50,12 +50,12 @@ impl PluginArrayMethods for PluginArray {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-pluginarray-item
-    fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<Root<Plugin>> {
+    fn IndexedGetter(&self, _index: u32) -> Option<Root<Plugin>> {
         None
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, _name: DOMString, _found: &mut bool) -> Option<Root<Plugin>> {
+    fn NamedGetter(&self, _name: DOMString) -> Option<Root<Plugin>> {
         None
     }
 

components/script/dom/radionodelist.rs

@@ -105,7 +105,7 @@ impl RadioNodeListMethods for RadioNodeList {
     // https://github.com/servo/servo/issues/5875
     //
     // https://dom.spec.whatwg.org/#dom-nodelist-item
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Node>> {
-        self.node_list.IndexedGetter(index, found)
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Node>> {
+        self.node_list.IndexedGetter(index)
     }
 }

components/script/dom/storage.rs

@@ -136,10 +136,8 @@ impl StorageMethods for Storage {
     }
 
     // check-tidy: no specs after this line
-    fn NamedGetter(&self, name: DOMString, found: &mut bool) -> Option<DOMString> {
-        let item = self.GetItem(name);
-        *found = item.is_some();
-        item
+    fn NamedGetter(&self, name: DOMString) -> Option<DOMString> {
+        self.GetItem(name)
     }
 
     fn NamedSetter(&self, name: DOMString, value: DOMString) -> ErrorResult {

components/script/dom/stylesheetlist.rs

@@ -46,9 +46,7 @@ impl StyleSheetListMethods for StyleSheetList {
     }
 
     // check-tidy: no specs after this line
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<StyleSheet>>{
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<StyleSheet>> {
+        self.Item(index)
     }
 }

components/script/dom/testbindingiterable.rs

@@ -34,10 +34,8 @@ impl TestBindingIterable {
 impl TestBindingIterableMethods for TestBindingIterable {
     fn Add(&self, v: DOMString) { self.vals.borrow_mut().push(v); }
     fn Length(&self) -> u32 { self.vals.borrow().len() as u32 }
-    fn GetItem(&self, n: u32) -> DOMString { self.vals.borrow().get(n as usize).unwrap().clone() }
-    fn IndexedGetter(&self, n: u32, found: &mut bool) -> DOMString {
-        let s = self.GetItem(n);
-        *found = true;
-        s
+    fn GetItem(&self, n: u32) -> DOMString { self.IndexedGetter(n).unwrap_or_default() }
+    fn IndexedGetter(&self, n: u32) -> Option<DOMString> {
+        self.vals.borrow().get(n as usize).cloned()
     }
 }

components/script/dom/testbindingproxy.rs

@@ -23,10 +23,10 @@ impl TestBindingProxyMethods for TestBindingProxy {
     fn SetItem(&self, _: u32, _: DOMString) -> () {}
     fn RemoveItem(&self, _: DOMString) -> () {}
     fn Stringifier(&self) -> DOMString { DOMString::new() }
-    fn IndexedGetter(&self, _: u32, _: &mut bool) -> DOMString { DOMString::new() }
+    fn IndexedGetter(&self, _: u32) -> Option<DOMString> { None }
     fn NamedDeleter(&self, _: DOMString) -> () {}
     fn IndexedSetter(&self, _: u32, _: DOMString) -> () {}
     fn NamedSetter(&self, _: DOMString, _: DOMString) -> () {}
-    fn NamedGetter(&self, _: DOMString, _: &mut bool) -> DOMString { DOMString::new() }
+    fn NamedGetter(&self, _: DOMString) -> Option<DOMString> { None }
 
 }

components/script/dom/touchlist.rs

@@ -42,9 +42,7 @@ impl TouchListMethods for TouchList {
     }
 
     /// https://w3c.github.io/touch-events/#widl-TouchList-item-getter-Touch-unsigned-long-index
-    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Touch>> {
-        let item = self.Item(index);
-        *found = item.is_some();
-        item
+    fn IndexedGetter(&self, index: u32) -> Option<Root<Touch>> {
+        self.Item(index)
     }
 }

components/script/dom/xmldocument.rs

@@ -88,8 +88,7 @@ impl XMLDocumentMethods for XMLDocument {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
-    fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString, found: &mut bool)
-                   -> NonZero<*mut JSObject> {
-        self.upcast::<Document>().NamedGetter(_cx, name, found)
+    fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option<NonZero<*mut JSObject>> {
+        self.upcast::<Document>().NamedGetter(_cx, name)
     }
 }