Add support for cfg_attr for struct fields (#4351)

Author: fl0rek

CHANGELOG.md

@@ -23,6 +23,10 @@
 * Add bindings to `Date.to_locale_time_string_with_options`.
   [#4384](https://github.com/rustwasm/wasm-bindgen/pull/4384)
 
+* `#[wasm_bindgen]` now correctly applies `#[cfg(...)]`s in `struct`s.
+  [#4351](https://github.com/rustwasm/wasm-bindgen/pull/4351)
+
+
 ### Changed
 
 * Optional parameters are now typed as `T | undefined | null` to reflect the actual JS behavior.

crates/macro-support/src/lib.rs

@@ -12,9 +12,10 @@ extern crate wasm_bindgen_backend as backend;
 extern crate wasm_bindgen_shared as shared;
 
 pub use crate::parser::BindgenAttrs;
-use crate::parser::MacroParse;
+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};
@@ -24,9 +25,25 @@ mod parser;
 /// Takes the parsed input from a `#[wasm_bindgen]` macro and returns the generated bindings
 pub fn expand(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diagnostic> {
     parser::reset_attrs_used();
+    // 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)?;
-    let opts = syn::parse2(attr)?;
+    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))?;
@@ -168,3 +185,19 @@ impl Parse for ClassMarker {
         })
     }
 }
+
+pub fn expand_struct_marker(item: TokenStream) -> Result<TokenStream, Diagnostic> {
+    parser::reset_attrs_used();
+
+    let mut s: syn::ItemStruct = syn::parse2(item)?;
+
+    let mut program = backend::ast::Program::default();
+    program.structs.push((&mut s).convert(&program)?);
+
+    let mut tokens = proc_macro2::TokenStream::new();
+    program.try_to_tokens(&mut tokens)?;
+
+    parser::check_unused_attrs(&mut tokens);
+
+    Ok(tokens)
+}

crates/macro-support/src/parser.rs

@@ -250,7 +250,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 {
@@ -264,7 +264,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 {
@@ -279,7 +279,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 {
@@ -294,7 +294,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 {
@@ -527,7 +527,7 @@ impl Parse for AnyIdent {
 ///
 /// Used to convert syn tokens into an AST, that we can then use to generate glue code. The context
 /// (`Ctx`) is used to pass in the attributes from the `#[wasm_bindgen]`, if needed.
-trait ConvertToAst<Ctx> {
+pub(crate) trait ConvertToAst<Ctx> {
     /// What we are converting to.
     type Target;
     /// Convert into our target.
@@ -536,20 +536,19 @@ trait ConvertToAst<Ctx> {
     fn convert(self, context: Ctx) -> Result<Self::Target, Diagnostic>;
 }
 
-impl ConvertToAst<(&ast::Program, BindgenAttrs)> for &mut syn::ItemStruct {
+impl ConvertToAst<&ast::Program> for &mut syn::ItemStruct {
     type Target = ast::Struct;
 
-    fn convert(
-        self,
-        (program, attrs): (&ast::Program, BindgenAttrs),
-    ) -> Result<Self::Target, Diagnostic> {
+    fn convert(self, program: &ast::Program) -> Result<Self::Target, Diagnostic> {
         if !self.generics.params.is_empty() {
             bail_span!(
                 self.generics,
                 "structs with #[wasm_bindgen] cannot have lifetime or \
                  type parameters currently"
             );
         }
+        let attrs = BindgenAttrs::find(&mut self.attrs)?;
+
         let mut fields = Vec::new();
         let js_name = attrs
             .js_name()
@@ -1366,11 +1365,6 @@ impl<'a> MacroParse<(Option<BindgenAttrs>, &'a mut TokenStream)> for syn::Item {
                     wasm_bindgen_futures: program.wasm_bindgen_futures.clone(),
                 });
             }
-            syn::Item::Struct(mut s) => {
-                let opts = opts.unwrap_or_default();
-                program.structs.push((&mut s).convert((program, opts))?);
-                s.to_tokens(tokens);
-            }
             syn::Item::Impl(mut i) => {
                 let opts = opts.unwrap_or_default();
                 (&mut i).macro_parse(program, opts)?;

crates/macro/src/lib.rs

@@ -60,3 +60,16 @@ pub fn __wasm_bindgen_class_marker(attr: TokenStream, input: TokenStream) -> Tok
         Err(diagnostic) => (quote! { #diagnostic }).into(),
     }
 }
+
+#[proc_macro_derive(BindgenedStruct, attributes(wasm_bindgen))]
+pub fn __wasm_bindgen_struct_marker(item: TokenStream) -> TokenStream {
+    match wasm_bindgen_macro_support::expand_struct_marker(item.into()) {
+        Ok(tokens) => {
+            if cfg!(feature = "xxx_debug_only_print_generated_code") {
+                println!("{}", tokens);
+            }
+            tokens.into()
+        }
+        Err(diagnostic) => (quote! { #diagnostic }).into(),
+    }
+}

crates/macro/ui-tests/import-keyword.stderr

@@ -57,3 +57,17 @@ error: enum cannot use the JS keyword `switch` as its name
    |
 63 | pub enum switch {
    |          ^^^^^^
+
+warning: type `class` should have an upper camel case name
+  --> ui-tests/import-keyword.rs:59:12
+   |
+59 | pub struct class;
+   |            ^^^^^ help: convert the identifier to upper camel case (notice the capitalization): `Class`
+   |
+   = note: `#[warn(non_camel_case_types)]` on by default
+
+warning: type `true` should have an upper camel case name
+  --> ui-tests/import-keyword.rs:61:12
+   |
+61 | pub struct r#true; // forbid value-like keywords
+   |            ^^^^^^ help: convert the identifier to upper camel case: `True`

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 attribute macro `wasm_bindgen` (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 attribute macro `wasm_bindgen` (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/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(x.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>);
@@ -480,6 +481,41 @@ fn renamed_field() {
     js_renamed_field();
 }
 
+#[cfg_attr(
+    target_arch = "wasm32",
+    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],
+
+    /// 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 {}