Add PR review updates

crates/macro-support/src/lib.rs

@@ -15,6 +15,7 @@ pub use crate::parser::BindgenAttrs;
 use crate::parser::{ConvertToAst, MacroParse};
 use backend::{Diagnostic, TryToTokens};
 use proc_macro2::TokenStream;
+use quote::quote;
 use quote::ToTokens;
 use quote::TokenStreamExt;
 use syn::parse::{Parse, ParseStream, Result as SynResult};
@@ -27,39 +28,22 @@ pub fn expand(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diag
     // if struct is encountered, add `derive` attribute and let everything happen there (workaround
     // to help parsing cfg_attr correctly).
     let item = syn::parse2::<syn::Item>(input)?;
-    if let syn::Item::Struct(mut s) = item {
-        s.attrs.insert(
-            0,
-            syn::Attribute {
-                pound_token: Default::default(),
-                style: syn::AttrStyle::Outer,
-                bracket_token: Default::default(),
-                meta: syn::parse_quote! {
-                    derive(wasm_bindgen::prelude::BindgenedStruct)
-                },
-            },
-        );
-        if !attr.is_empty() {
-            s.attrs.insert(
-                1,
-                syn::Attribute {
-                    pound_token: Default::default(),
-                    style: syn::AttrStyle::Outer,
-                    bracket_token: Default::default(),
-                    meta: syn::parse_quote! {
-                        wasm_bindgen(#attr)
-                    },
-                },
-            );
-        }
-
-        let mut tokens = proc_macro2::TokenStream::new();
-        s.to_tokens(&mut tokens);
-        return Ok(tokens);
+    if let syn::Item::Struct(s) = item {
+        let opts: BindgenAttrs = syn::parse2(attr.clone())?;
+        let wasm_bindgen = opts
+            .wasm_bindgen()
+            .cloned()
+            .unwrap_or_else(|| syn::parse_quote! { wasm_bindgen });
+
+        let item = quote! {
+            #[derive(#wasm_bindgen::__rt::BindgenedStruct)]
+            #[wasm_bindgen(#attr)]
+            #s
+        };
+        return Ok(item);
     }
 
     let opts = syn::parse2(attr)?;
-
     let mut tokens = proc_macro2::TokenStream::new();
     let mut program = backend::ast::Program::default();
     item.macro_parse(&mut program, (Some(opts), &mut tokens))?;

crates/macro-support/src/parser.rs

@@ -216,7 +216,7 @@ macro_rules! methods {
     };
 
     (@method $name:ident, $variant:ident(Span, String, Span)) => {
-        fn $name(&self) -> Option<(&str, Span)> {
+        pub(crate) fn $name(&self) -> Option<(&str, Span)> {
             self.attrs
                 .iter()
                 .find_map(|a| match &a.1 {
@@ -230,7 +230,7 @@ macro_rules! methods {
     };
 
     (@method $name:ident, $variant:ident(Span, JsNamespace, Vec<Span>)) => {
-        fn $name(&self) -> Option<(JsNamespace, &[Span])> {
+        pub(crate) fn $name(&self) -> Option<(JsNamespace, &[Span])> {
             self.attrs
                 .iter()
                 .find_map(|a| match &a.1 {
@@ -245,7 +245,7 @@ macro_rules! methods {
 
     (@method $name:ident, $variant:ident(Span, $($other:tt)*)) => {
         #[allow(unused)]
-        fn $name(&self) -> Option<&$($other)*> {
+        pub(crate) fn $name(&self) -> Option<&$($other)*> {
             self.attrs
                 .iter()
                 .find_map(|a| match &a.1 {
@@ -260,7 +260,7 @@ macro_rules! methods {
 
     (@method $name:ident, $variant:ident($($other:tt)*)) => {
         #[allow(unused)]
-        fn $name(&self) -> Option<&$($other)*> {
+        pub(crate) fn $name(&self) -> Option<&$($other)*> {
             self.attrs
                 .iter()
                 .find_map(|a| match &a.1 {
@@ -513,10 +513,10 @@ impl ConvertToAst<&ast::Program> for &mut syn::ItemStruct {
                  type parameters currently"
             );
         }
-        let struct_attrs = BindgenAttrs::find(&mut self.attrs)?;
+        let attrs = BindgenAttrs::find(&mut self.attrs)?;
 
         let mut fields = Vec::new();
-        let js_name = struct_attrs
+        let js_name = attrs
             .js_name()
             .map(|s| s.0.to_string())
             .unwrap_or(self.ident.unraw().to_string());
@@ -528,8 +528,8 @@ impl ConvertToAst<&ast::Program> for &mut syn::ItemStruct {
             );
         }
 
-        let is_inspectable = struct_attrs.inspectable().is_some();
-        let getter_with_clone = struct_attrs.getter_with_clone();
+        let is_inspectable = attrs.inspectable().is_some();
+        let getter_with_clone = attrs.getter_with_clone();
         for (i, field) in self.fields.iter_mut().enumerate() {
             match field.vis {
                 syn::Visibility::Public(..) => {}
@@ -571,9 +571,9 @@ impl ConvertToAst<&ast::Program> for &mut syn::ItemStruct {
             });
             attrs.check_used();
         }
-        let generate_typescript = struct_attrs.skip_typescript().is_none();
+        let generate_typescript = attrs.skip_typescript().is_none();
         let comments: Vec<String> = extract_doc_comments(&self.attrs);
-        struct_attrs.check_used();
+        attrs.check_used();
         Ok(ast::Struct {
             rust_name: self.ident.clone(),
             js_name,

crates/macro/ui-tests/pub-not-copy.stderr

@@ -12,4 +12,4 @@ note: required by a bound in `assert_copy`
   |
 3 | #[wasm_bindgen]
   | ^^^^^^^^^^^^^^^ required by this bound in `assert_copy`
-  = note: this error originates in the derive macro `wasm_bindgen::prelude::BindgenedStruct` (in Nightly builds, run with -Z macro-backtrace for more info)
+  = note: this error originates in the derive macro `wasm_bindgen::__rt::BindgenedStruct` (in Nightly builds, run with -Z macro-backtrace for more info)

crates/macro/ui-tests/struct-fields.stderr

@@ -12,7 +12,7 @@ note: required by a bound in `__wbg_get_bar_a::assert_copy`
    |
 8  | #[wasm_bindgen]
    | ^^^^^^^^^^^^^^^ required by this bound in `assert_copy`
-   = note: this error originates in the derive macro `wasm_bindgen::prelude::BindgenedStruct` (in Nightly builds, run with -Z macro-backtrace for more info)
+   = note: this error originates in the derive macro `wasm_bindgen::__rt::BindgenedStruct` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0277]: the trait bound `Foo: Clone` is not satisfied
   --> ui-tests/struct-fields.rs:12:12

src/lib.rs

@@ -101,7 +101,6 @@ pub mod prelude {
     pub use crate::JsCast;
     pub use crate::JsValue;
     pub use crate::UnwrapThrowExt;
-    pub use wasm_bindgen_macro::BindgenedStruct;
     #[doc(hidden)]
     pub use wasm_bindgen_macro::__wasm_bindgen_class_marker;
     pub use wasm_bindgen_macro::wasm_bindgen;

src/rt/mod.rs

@@ -19,6 +19,8 @@ pub extern crate std;
 
 pub mod marker;
 
+pub use wasm_bindgen_macro::BindgenedStruct;
+
 /// Wrapper around [`Lazy`] adding `Send + Sync` when `atomics` is not enabled.
 pub struct LazyCell<T, F = fn() -> T>(Wrapper<Lazy<T, F>>);
 

tests/wasm/classes.js

@@ -170,6 +170,13 @@ exports.js_renamed_field = () => {
     x.foo();
 }
 
+exports.js_conditional_skip = () => {
+    const x = new wasm.ConditionalSkipClass();
+    assert.strictEqual(x.skipped_field, undefined);
+    assert.ok(x.not_skipped_field === 42);
+    assert.strictEqual(b.needs_clone, 'foo');
+}
+
 exports.js_conditional_bindings = () => {
     const x = new wasm.ConditionalBindings();
     x.free();

tests/wasm/classes.rs

@@ -24,6 +24,7 @@ extern "C" {
     fn js_access_fields();
     fn js_renamed_export();
     fn js_renamed_field();
+    fn js_conditional_skip();
     fn js_conditional_bindings();
 
     fn js_assert_none(a: Option<OptionClass>);
@@ -485,14 +486,36 @@ fn renamed_field() {
     wasm_bindgen(inspectable, js_name = "ConditionalSkipClass")
 )]
 pub struct ConditionalSkip {
+    /// [u8; 8] cannot be passed to JS, so this won't compile without `skip`
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen(skip))]
     pub skipped_field: [u8; 8],
 
-    /// cfg_attr annotated field
+    /// this field shouldn't be skipped as predicate is false
+    #[cfg_attr(all(target_arch = "wasm32", target_arch = "x86"), wasm_bindgen(skip))]
+    pub not_skipped_field: u32,
+
+    /// String struct field requires `getter_with_clone` to compile
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen(getter_with_clone))]
     pub needs_clone: String,
 }
 
+#[wasm_bindgen(js_class = "ConditionalSkipClass")]
+impl ConditionalSkip {
+    #[wasm_bindgen(constructor)]
+    pub fn new() -> ConditionalSkip {
+        ConditionalSkip {
+            skipped_field: [0u8; 8],
+            not_skipped_field: 42,
+            needs_clone: "foo".to_string(),
+        }
+    }
+}
+
+#[wasm_bindgen_test]
+fn conditional_skip() {
+    js_conditional_skip();
+}
+
 #[cfg_attr(target_arch = "wasm32", wasm_bindgen)]
 pub struct ConditionalBindings {}