Merge pull request #1572 from alexcrichton/dictionary-fields

Fix small issues around web-sys dictionaries

Author: fitzgen

crates/backend/src/ast.rs

@@ -292,6 +292,9 @@ pub enum ConstValue {
 pub struct Dictionary {
     pub name: Ident,
     pub fields: Vec<DictionaryField>,
+    pub ctor: bool,
+    pub doc_comment: Option<String>,
+    pub ctor_doc_comment: Option<String>,
 }
 
 #[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))]
@@ -301,6 +304,7 @@ pub struct DictionaryField {
     pub js_name: String,
     pub required: bool,
     pub ty: syn::Type,
+    pub doc_comment: Option<String>,
 }
 
 impl Export {

crates/backend/src/codegen.rs

@@ -1281,24 +1281,41 @@ impl ToTokens for ast::Dictionary {
             .collect::<Vec<_>>();
         let required_names2 = required_names;
         let required_names3 = required_names;
+        let doc_comment = match &self.doc_comment {
+            None => "",
+            Some(doc_string) => doc_string,
+        };
+
+        let ctor = if self.ctor {
+            let doc_comment = match &self.ctor_doc_comment {
+                None => "",
+                Some(doc_string) => doc_string,
+            };
+            quote! {
+                #[doc = #doc_comment]
+                pub fn new(#(#required_names: #required_types),*) -> #name {
+                    let mut _ret = #name { obj: ::js_sys::Object::new() };
+                    #(_ret.#required_names2(#required_names3);)*
+                    return _ret
+                }
+            }
+        } else {
+            quote! {}
+        };
 
         let const_name = Ident::new(&format!("_CONST_{}", name), Span::call_site());
         (quote! {
             #[derive(Clone, Debug)]
             #[repr(transparent)]
             #[allow(clippy::all)]
+            #[doc = #doc_comment]
             pub struct #name {
                 obj: ::js_sys::Object,
             }
 
             #[allow(clippy::all)]
             impl #name {
-                pub fn new(#(#required_names: #required_types),*) -> #name {
-                    let mut _ret = #name { obj: ::js_sys::Object::new() };
-                    #(_ret.#required_names2(#required_names3);)*
-                    return _ret
-                }
-
+                #ctor
                 #methods
             }
 
@@ -1407,8 +1424,13 @@ impl ToTokens for ast::DictionaryField {
         let rust_name = &self.rust_name;
         let js_name = &self.js_name;
         let ty = &self.ty;
+        let doc_comment = match &self.doc_comment {
+            None => "",
+            Some(doc_string) => doc_string,
+        };
         (quote! {
             #[allow(clippy::all)]
+            #[doc = #doc_comment]
             pub fn #rust_name(&mut self, val: #ty) -> &mut Self {
                 use wasm_bindgen::JsValue;
                 let r = ::js_sys::Reflect::set(

crates/backend/src/defined.rs

@@ -348,25 +348,20 @@ impl RemoveUndefinedImports for ast::Program {
         let mut changed = self.imports.remove_undefined_imports(is_defined);
         changed = self.consts.remove_undefined_imports(is_defined) || changed;
 
-        let mut dictionaries_to_remove = Vec::new();
-        for (i, dictionary) in self.dictionaries.iter_mut().enumerate() {
+        for dictionary in self.dictionaries.iter_mut() {
             let num_required =
                 |dict: &ast::Dictionary| dict.fields.iter().filter(|f| f.required).count();
             let before = num_required(dictionary);
             changed = dictionary.fields.remove_undefined_imports(is_defined) || changed;
+
+            // If a required field was removed we can no longer construct this
+            // dictionary so disable the constructor.
             if before != num_required(dictionary) {
-                log::warn!(
-                    "removing {} due to a required field being removed",
-                    dictionary.name
-                );
-                dictionaries_to_remove.push(i);
+                dictionary.ctor = false;
             }
         }
-        for i in dictionaries_to_remove.iter().rev() {
-            self.dictionaries.swap_remove(*i);
-        }
 
-        changed || dictionaries_to_remove.len() > 0
+        changed
     }
 }
 

crates/webidl/src/lib.rs

@@ -26,6 +26,7 @@ use proc_macro2::{Ident, Span};
 use quote::{quote, ToTokens};
 use std::collections::{BTreeSet, HashSet};
 use std::env;
+use std::fmt::Display;
 use std::fs;
 use std::iter::FromIterator;
 use wasm_bindgen_backend::ast;
@@ -313,11 +314,33 @@ impl<'src> FirstPassRecord<'src> {
         if !self.append_dictionary_members(def.identifier.0, &mut fields) {
             return;
         }
+        let name = rust_ident(&camel_case_ident(def.identifier.0));
+        let extra_feature = name.to_string();
+        for field in fields.iter_mut() {
+            let mut doc_comment = Some(format!(
+                "Configure the `{}` field of this object\n",
+                field.js_name,
+            ));
+            self.append_required_features_doc(&*field, &mut doc_comment, &[&extra_feature]);
+            field.doc_comment = doc_comment;
+        }
 
-        program.dictionaries.push(ast::Dictionary {
-            name: rust_ident(&camel_case_ident(def.identifier.0)),
+        let mut doc_comment = format!("The `{}` dictionary\n", def.identifier.0);
+        if let Some(s) = self.required_doc_string(vec![name.clone()]) {
+            doc_comment.push_str(&s);
+        }
+        let mut dict = ast::Dictionary {
+            name,
             fields,
-        });
+            ctor: true,
+            doc_comment: Some(doc_comment),
+            ctor_doc_comment: None,
+        };
+        let mut ctor_doc_comment = Some(format!("Construct a new `{}`\n", def.identifier.0));
+        self.append_required_features_doc(&dict, &mut ctor_doc_comment, &[&extra_feature]);
+        dict.ctor_doc_comment = ctor_doc_comment;
+
+        program.dictionaries.push(dict);
     }
 
     fn append_dictionary_members(
@@ -418,6 +441,7 @@ impl<'src> FirstPassRecord<'src> {
             rust_name: rust_ident(&snake_case_ident(field.identifier.0)),
             js_name: field.identifier.0.to_string(),
             ty,
+            doc_comment: None,
         })
     }
 
@@ -739,16 +763,29 @@ impl<'src> FirstPassRecord<'src> {
         if required.len() == 0 {
             return;
         }
-        let list = required
+        if let Some(extra) = self.required_doc_string(required) {
+            doc.push_str(&extra);
+        }
+    }
+
+    fn required_doc_string<T: Display>(
+        &self,
+        features: impl IntoIterator<Item = T>,
+    ) -> Option<String> {
+        let features = features.into_iter().collect::<Vec<_>>();
+        if features.len() == 0 {
+            return None;
+        }
+        let list = features
             .iter()
             .map(|ident| format!("`{}`", ident))
             .collect::<Vec<_>>()
             .join(", ");
-        doc.push_str(&format!(
+        Some(format!(
             "\n\n*This API requires the following crate features \
              to be activated: {}*",
             list,
-        ));
+        ))
     }
 
     fn append_callback_interface(
@@ -770,6 +807,7 @@ impl<'src> FirstPassRecord<'src> {
                         rust_name: rust_ident(&snake_case_ident(identifier)),
                         js_name: identifier.to_string(),
                         ty: idl_type::IdlType::Callback.to_syn_type(pos).unwrap(),
+                        doc_comment: None,
                     });
                 }
                 _ => {
@@ -784,6 +822,9 @@ impl<'src> FirstPassRecord<'src> {
         program.dictionaries.push(ast::Dictionary {
             name: rust_ident(&camel_case_ident(item.definition.identifier.0)),
             fields,
+            ctor: true,
+            doc_comment: None,
+            ctor_doc_comment: None,
         });
     }
 }