From 70a4a9a4ada69e4ac11c70b5d8bbbab9af2958b5 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 7 Sep 2022 00:06:54 +0100 Subject: [PATCH] Trigger warnings for unused wasm-bindgen attributes This attempts to do something similar to #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 internally generate unused variables with spans pointing to unused attributes, so that users get a relatively meaningful warning. Here's how the result looks like on example from #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 a 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 #3038. --- crates/macro-support/src/lib.rs | 6 +-- crates/macro-support/src/parser.rs | 72 +++++++++++++++++------------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/crates/macro-support/src/lib.rs b/crates/macro-support/src/lib.rs index 9996849596af..54c0bc96361c 100644 --- a/crates/macro-support/src/lib.rs +++ b/crates/macro-support/src/lib.rs @@ -23,7 +23,6 @@ mod parser; /// Takes the parsed input from a `#[wasm_bindgen]` macro and returns the generated bindings pub fn expand(attr: TokenStream, input: TokenStream) -> Result { - parser::reset_attrs_used(); let item = syn::parse2::(input)?; let opts = syn::parse2(attr)?; @@ -35,7 +34,7 @@ pub fn expand(attr: TokenStream, input: TokenStream) -> Result Result { - parser::reset_attrs_used(); let mut item = syn::parse2::(input)?; let opts: ClassMarker = syn::parse2(attr)?; let mut program = backend::ast::Program::default(); item.macro_parse(&mut program, (&opts.class, &opts.js_class))?; - parser::assert_all_attrs_checked(); // same as above // This is where things are slightly different, we are being expanded in the // context of an impl so we can't inject arbitrary item-like tokens into the @@ -76,6 +73,7 @@ pub fn expand_class_marker( if let Err(e) = program.try_to_tokens(tokens) { err = Some(e); } + parser::check_and_reset_used_attrs(tokens); // same as above tokens.append_all(item.attrs.iter().filter(|attr| match attr.style { syn::AttrStyle::Inner(_) => true, _ => false, diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index a3148fdb1842..89cba4dee42d 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -41,6 +41,8 @@ const JS_KEYWORDS: [&str; 20] = [ struct AttributeParseState { parsed: Cell, checks: Cell, + #[cfg(not(feature = "strict-macro"))] + unused_attrs: std::cell::RefCell>, } /// Parsed attributes from a `#[wasm_bindgen(..)]`. @@ -94,35 +96,40 @@ macro_rules! methods { ($(($name:ident, $variant:ident($($contents:tt)*)),)*) => { $(methods!(@method $name, $variant($($contents)*));)* - #[cfg(feature = "strict-macro")] fn check_used(self) -> Result<(), Diagnostic> { // Account for the fact this method was called ATTRS.with(|state| state.checks.set(state.checks.get() + 1)); - let mut errors = Vec::new(); - for (used, attr) in self.attrs.iter() { - if used.get() { - continue - } - // The check below causes rustc to crash on powerpc64 platforms - // with an LLVM error. To avoid this, we instead use #[cfg()] - // and duplicate the function below. See #58516 for details. - /*if !cfg!(feature = "strict-macro") { - continue - }*/ - let span = match attr { - $(BindgenAttr::$variant(span, ..) => span,)* - }; - errors.push(Diagnostic::span_error(*span, "unused #[wasm_bindgen] attribute")); + let unused = + self.attrs + .iter() + .filter_map(|(used, attr)| if used.get() { None } else { Some(attr) }) + .map(|attr| { + match attr { + $(BindgenAttr::$variant(span, ..) => { + #[cfg(feature = "strict-macro")] + { + Diagnostic::span_error(span, "unused #[wasm_bindgen] attribute") + } + + #[cfg(not(feature = "strict-macro"))] + { + Ident::new(stringify!($name), *span) + } + },)* + } + }); + + #[cfg(feature = "strict-macro")] + { + Diagnostic::from_vec(unused.collect()) } - Diagnostic::from_vec(errors) - } - #[cfg(not(feature = "strict-macro"))] - fn check_used(self) -> Result<(), Diagnostic> { - // Account for the fact this method was called - ATTRS.with(|state| state.checks.set(state.checks.get() + 1)); - Ok(()) + #[cfg(not(feature = "strict-macro"))] + { + ATTRS.with(|state| state.unused_attrs.borrow_mut().extend(unused)); + Ok(()) + } } }; @@ -1613,16 +1620,21 @@ fn extract_path_ident(path: &syn::Path) -> Result { } } -pub fn reset_attrs_used() { +pub fn check_and_reset_used_attrs(tokens: &mut TokenStream) { ATTRS.with(|state| { + assert_eq!(state.parsed.get(), state.checks.get()); state.parsed.set(0); state.checks.set(0); - }) -} - -pub fn assert_all_attrs_checked() { - ATTRS.with(|state| { - assert_eq!(state.parsed.get(), state.checks.get()); + #[cfg(not(feature = "strict-macro"))] + { + let unused = std::mem::take(&mut *state.unused_attrs.borrow_mut()); + tokens.extend(quote::quote! { + // Anonymous scope to prevent name clashes. + const _: () = { + #(let #unused: ();)* + }; + }); + } }) }