Apply only allowed attributes to generated functions

[seancribbs]

This is a response to #3315. @daxpedda suggested that we pass down specific attributes and then allow the user to specify additional allowed attributes that could be attached to the bindgen and descriptor functions.

This is an initial implementation of the first part of this idea, namely, that only the cfg and cfg_attr attributes are allowed to be passed through. There may be others that should be allowed by default, but since these are involved in conditional compilation, I felt they would be most relevant to apply to the generated functions.

I'm mostly looking for feedback on the direction here before I dive too much deeper. Thanks!

[daxpedda]

So by default we have to pass through all attributes, because anything else is a breaking change.

What we can do is to only pass cfg and cfg_attr by default for the functions generated by wasm-bindgen, e.g. *_describe_*(). This is theoretically still a breaking change but I think proc-macros that generate cfgs are pretty rare (which are the only ones we really need to support here).

I believe these changes along should suffice to solve #3315, adding an attribute to allow passing down more specific user-specified attributes is not necessary until requested.

I'm not entirely sure but I think you have already achieved what I outlined above with this PR.
It would be great if we can add a test.

@Liamolucko considering this is theoretically a breaking change, I would very much like your approval as well here.

[seancribbs]

It would be great if we can add a test.

Yes, it wasn't clear where to add tests. It's not a compilation error/warning, so the ui-tests didn't seem to apply. Any advice?

[daxpedda]

Yes, it wasn't clear where to add tests. It's not a compilation error/warning, so the ui-tests didn't seem to apply. Any advice?

I would have imagined them to be added via compilation errors as UI tests. E.g. use cfg attributes and check if methods are still accessible. Serde could be used to test foreign attributes.

Of course ideally we would have proper proc-macro tests, if you feel like adding them that would be great. But I'm happy to accept UI tests instead of nothing.

[daxpedda]

Apologies, after further thinking I believe you are right, this doesn't make sense in a UI test (and Serde doesn't have any method related attributes I believe).

I believe non-UI tests can be added in /tests/wasm/file.rs (you don't have to add a corresponding .js file).

[seancribbs]

@daxpedda I was able to add a test to the existing test suite in tests/wasm. Please let me know if that is sufficient to demonstrate the efficacy of the change.

[daxpedda]

Can we add another test with custom proc-macros, to make sure they are skipped for the generated functions?
Feel free to add any reasonable dev-dependencies to make it happen, e.g. tracing.

[Liamolucko]

@Liamolucko considering this is theoretically a breaking change, I would very much like your approval as well here.

I think it should be fine, I'd be very surprised if anyone's relying on attributes getting copied to wasm-bindgen's internal functions.