Skip to content

Commit 387eafe

Browse files
committed
Trigger warnings for unused wasm-bindgen attributes
This attempts to do something similar to wasm-bindgen#3070, but without potentially dangerous fallout from strict-mode failing on all the existing code out there. Instead of forcing a compiler error like strict-mode does, this PR will generate unused variables with spans pointing to unused attributes, so that users get a usable warning. Here's how the result looks like on example from wasm-bindgen#2874: ``` warning: unused variable: `typescript_type` --> tests\headless\snippets.rs:67:28 | 67 | #[wasm_bindgen(getter, typescript_type = "Thing[]")] | ^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_typescript_type` | = note: `#[warn(unused_variables)]` on by default ``` This is not 100% perfect - until Rust has built-in `compile_warning!`, nothing is - but is a better status quo than the current one and can help users find problematic attributes without actually breaking their builds. Fixes wasm-bindgen#3038.
1 parent 5c28993 commit 387eafe

File tree

2 files changed

+44
-34
lines changed

2 files changed

+44
-34
lines changed

crates/macro-support/src/lib.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ mod parser;
2323

2424
/// Takes the parsed input from a `#[wasm_bindgen]` macro and returns the generated bindings
2525
pub fn expand(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diagnostic> {
26-
parser::reset_attrs_used();
2726
let item = syn::parse2::<syn::Item>(input)?;
2827
let opts = syn::parse2(attr)?;
2928

@@ -35,7 +34,7 @@ pub fn expand(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diag
3534
// If we successfully got here then we should have used up all attributes
3635
// and considered all of them to see if they were used. If one was forgotten
3736
// that's a bug on our end, so sanity check here.
38-
parser::assert_all_attrs_checked();
37+
parser::check_and_reset_used_attrs(&mut tokens);
3938

4039
Ok(tokens)
4140
}
@@ -45,13 +44,11 @@ pub fn expand_class_marker(
4544
attr: TokenStream,
4645
input: TokenStream,
4746
) -> Result<TokenStream, Diagnostic> {
48-
parser::reset_attrs_used();
4947
let mut item = syn::parse2::<syn::ImplItemMethod>(input)?;
5048
let opts: ClassMarker = syn::parse2(attr)?;
5149

5250
let mut program = backend::ast::Program::default();
5351
item.macro_parse(&mut program, (&opts.class, &opts.js_class))?;
54-
parser::assert_all_attrs_checked(); // same as above
5552

5653
// This is where things are slightly different, we are being expanded in the
5754
// context of an impl so we can't inject arbitrary item-like tokens into the
@@ -76,6 +73,7 @@ pub fn expand_class_marker(
7673
if let Err(e) = program.try_to_tokens(tokens) {
7774
err = Some(e);
7875
}
76+
parser::check_and_reset_used_attrs(tokens); // same as above
7977
tokens.append_all(item.attrs.iter().filter(|attr| match attr.style {
8078
syn::AttrStyle::Inner(_) => true,
8179
_ => false,

crates/macro-support/src/parser.rs

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ const JS_KEYWORDS: [&str; 20] = [
4141
struct AttributeParseState {
4242
parsed: Cell<usize>,
4343
checks: Cell<usize>,
44+
#[cfg(not(feature = "strict-macro"))]
45+
unused_attrs: std::cell::RefCell<Vec<Ident>>,
4446
}
4547

4648
/// Parsed attributes from a `#[wasm_bindgen(..)]`.
@@ -94,35 +96,40 @@ macro_rules! methods {
9496
($(($name:ident, $variant:ident($($contents:tt)*)),)*) => {
9597
$(methods!(@method $name, $variant($($contents)*));)*
9698

97-
#[cfg(feature = "strict-macro")]
9899
fn check_used(self) -> Result<(), Diagnostic> {
99100
// Account for the fact this method was called
100101
ATTRS.with(|state| state.checks.set(state.checks.get() + 1));
101102

102-
let mut errors = Vec::new();
103-
for (used, attr) in self.attrs.iter() {
104-
if used.get() {
105-
continue
106-
}
107-
// The check below causes rustc to crash on powerpc64 platforms
108-
// with an LLVM error. To avoid this, we instead use #[cfg()]
109-
// and duplicate the function below. See #58516 for details.
110-
/*if !cfg!(feature = "strict-macro") {
111-
continue
112-
}*/
113-
let span = match attr {
114-
$(BindgenAttr::$variant(span, ..) => span,)*
115-
};
116-
errors.push(Diagnostic::span_error(*span, "unused #[wasm_bindgen] attribute"));
103+
let unused =
104+
self.attrs
105+
.iter()
106+
.filter_map(|(used, attr)| if used.get() { None } else { Some(attr) })
107+
.map(|attr| {
108+
match attr {
109+
$(BindgenAttr::$variant(span, ..) => {
110+
#[cfg(feature = "strict-macro")]
111+
{
112+
Diagnostic::span_error(span, "unused #[wasm_bindgen] attribute")
113+
}
114+
115+
#[cfg(not(feature = "strict-macro"))]
116+
{
117+
Ident::new(stringify!($name), *span)
118+
}
119+
},)*
120+
}
121+
});
122+
123+
#[cfg(feature = "strict-macro")]
124+
{
125+
Diagnostic::from_vec(unused.collect())
117126
}
118-
Diagnostic::from_vec(errors)
119-
}
120127

121-
#[cfg(not(feature = "strict-macro"))]
122-
fn check_used(self) -> Result<(), Diagnostic> {
123-
// Account for the fact this method was called
124-
ATTRS.with(|state| state.checks.set(state.checks.get() + 1));
125-
Ok(())
128+
#[cfg(not(feature = "strict-macro"))]
129+
{
130+
ATTRS.with(|state| state.unused_attrs.borrow_mut().extend(unused));
131+
Ok(())
132+
}
126133
}
127134
};
128135

@@ -1613,16 +1620,21 @@ fn extract_path_ident(path: &syn::Path) -> Result<Ident, Diagnostic> {
16131620
}
16141621
}
16151622

1616-
pub fn reset_attrs_used() {
1623+
pub fn check_and_reset_used_attrs(tokens: &mut TokenStream) {
16171624
ATTRS.with(|state| {
1625+
assert_eq!(state.parsed.get(), state.checks.get());
16181626
state.parsed.set(0);
16191627
state.checks.set(0);
1620-
})
1621-
}
1622-
1623-
pub fn assert_all_attrs_checked() {
1624-
ATTRS.with(|state| {
1625-
assert_eq!(state.parsed.get(), state.checks.get());
1628+
#[cfg(not(feature = "strict-macro"))]
1629+
{
1630+
let unused = std::mem::take(&mut *state.unused_attrs.borrow_mut());
1631+
tokens.extend(quote::quote! {
1632+
// Anonymous scope to prevent name clashes.
1633+
const _: () = {
1634+
#(let #unused: ();)*
1635+
};
1636+
});
1637+
}
16261638
})
16271639
}
16281640

0 commit comments

Comments
 (0)