Fix return type of WebIDL indexing getters (#1789)

* Wrap the return type of indexing getters as Option<T> if necessary.

* Update tests for indexing getters

* Fix typo

* Add comments describing what the code segment is doing

* Update indexing getter usage

* Revert "Add comments describing what the code segment is doing"

This reverts commit 624a14c0fffb78e8eaed21658ddddbad70b2462d.

* Revert "Fix typo"

This reverts commit 487fc307bc08c2a7778b2117fb03f0f5eb5a3c18.

* Revert "Wrap the return type of indexing getters as Option<T> if necessary."

This reverts commit 547f3dd36c1182928ff728a8452591a492b65e21.

* Update the return signatures of WebIDL indexing getters

crates/web-sys/tests/wasm/html_element.rs

@@ -33,6 +33,21 @@ fn test_html_element() {
     element.set_hidden(true);
     assert!(element.hidden(), "Should be hidden");
 
+    assert_eq!(element.class_list().get(0), None, "Shouldn't have class at index 0");
+    element.class_list().add_2("a", "b").unwrap();
+    assert_eq!(element.class_list().get(0).unwrap(), "a", "Should have class at index 0");
+    assert_eq!(element.class_list().get(1).unwrap(), "b", "Should have class at index 1");
+    assert_eq!(element.class_list().get(2), None, "Shouldn't have class at index 2");
+
+    assert_eq!(element.dataset().get("id"), None, "Shouldn't have data-id");
+    element.dataset().set("id", "123").unwrap();
+    assert_eq!(element.dataset().get("id").unwrap(), "123", "Should have data-id");
+
+    assert_eq!(element.style().get(0), None, "Shouldn't have style property name at index 0");
+    element.style().set_property("background-color", "red").unwrap();
+    assert_eq!(element.style().get(0).unwrap(), "background-color", "Should have style property at index 0");
+    assert_eq!(element.style().get_property_value("background-color").unwrap(), "red", "Should have style property");
+
     // TODO add a click handler here
     element.click();
 

crates/webidl-tests/simple.rs

@@ -86,11 +86,11 @@ fn global_method() {
 #[wasm_bindgen_test]
 fn indexing() {
     let f = Indexing::new().unwrap();
-    assert_eq!(f.get(123), -1);
+    assert_eq!(f.get(123).unwrap(), -1);
     f.set(123, 456);
-    assert_eq!(f.get(123), 456);
+    assert_eq!(f.get(123).unwrap(), 456);
     f.delete(123);
-    assert_eq!(f.get(123), -1);
+    assert_eq!(f.get(123).unwrap(), -1);
 }
 
 #[wasm_bindgen_test]

crates/webidl/src/util.rs

@@ -580,6 +580,20 @@ impl<'src> FirstPassRecord<'src> {
             let structural =
                 force_structural || is_structural(signature.orig.attrs.as_ref(), container_attrs);
             let catch = force_throws || throws(&signature.orig.attrs);
+            let ret_ty = if id == &OperationId::IndexingGetter {
+                // All indexing getters should return optional values (or
+                // otherwise be marked with catch).
+                match ret_ty {
+                    IdlType::Nullable(_) => ret_ty,
+                    ref ty @ _ => if catch {
+                        ret_ty
+                    } else {
+                        IdlType::Nullable(Box::new(ty.clone()))
+                    },
+                }
+            } else {
+                ret_ty
+            };
             let variadic = signature.args.len() == signature.orig.args.len()
                 && signature
                     .orig

examples/todomvc/src/element.rs

@@ -294,7 +294,9 @@ impl Element {
         if let Some(el) = self.el.take() {
             {
                 if let Some(el) = wasm_bindgen::JsCast::dyn_ref::<web_sys::HtmlElement>(&el) {
-                    text = el.dataset().get(key);
+                    if let Some(value) = el.dataset().get(key) {
+                        text = value;
+                    }
                 }
             }
             self.el = Some(el);