Merge pull request #1550 from alexcrichton/more-better-errors

Improve diagnostics with missing trait implementations

crates/backend/src/codegen.rs

@@ -197,7 +197,6 @@ impl ToTokens for ast::Struct {
             impl wasm_bindgen::__rt::core::convert::From<#name> for
                 wasm_bindgen::JsValue
             {
-                #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                 fn from(value: #name) -> Self {
                     let ptr = wasm_bindgen::convert::IntoWasmAbi::into_abi(
                         value,
@@ -205,10 +204,16 @@ impl ToTokens for ast::Struct {
                     );
 
                     #[link(wasm_import_module = "__wbindgen_placeholder__")]
+                    #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                     extern "C" {
                         fn #new_fn(ptr: u32) -> u32;
                     }
 
+                    #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
+                    unsafe fn #new_fn(ptr: u32) -> u32 {
+                        panic!("cannot convert to JsValue outside of the wasm target")
+                    }
+
                     unsafe {
                         <wasm_bindgen::JsValue as wasm_bindgen::convert::FromWasmAbi>
                             ::from_abi(
@@ -217,11 +222,6 @@ impl ToTokens for ast::Struct {
                             )
                     }
                 }
-
-                #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
-                fn from(_value: #name) -> Self {
-                    panic!("cannot convert to JsValue outside of the wasm target")
-                }
             }
 
             #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
@@ -712,24 +712,22 @@ impl ToTokens for ast::ImportType {
                 }
 
                 impl JsCast for #rust_name {
-                    #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                     fn instanceof(val: &JsValue) -> bool {
                         #[link(wasm_import_module = "__wbindgen_placeholder__")]
+                        #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                         extern "C" {
                             fn #instanceof_shim(val: u32) -> u32;
                         }
+                        #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
+                        unsafe fn #instanceof_shim(val: u32) -> u32 {
+                            panic!("cannot check instanceof on non-wasm targets");
+                        }
                         unsafe {
                             let idx = val.into_abi(&mut wasm_bindgen::convert::GlobalStack::new());
                             #instanceof_shim(idx) != 0
                         }
                     }
 
-                    #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
-                    fn instanceof(val: &JsValue) -> bool {
-                        drop(val);
-                        panic!("cannot check instanceof on non-wasm targets");
-                    }
-
                     #is_type_of
 
                     #[inline]
@@ -998,6 +996,7 @@ impl TryToTokens for ast::ImportFunction {
         let import_name = &self.shim;
         let attrs = &self.function.rust_attrs;
         let arguments = &arguments;
+        let abi_arguments = &abi_arguments;
 
         let doc_comment = match &self.doc_comment {
             None => "",
@@ -1009,17 +1008,45 @@ impl TryToTokens for ast::ImportFunction {
             quote!()
         };
 
-        let invocation = quote! {
+        // Route any errors pointing to this imported function to the identifier
-            #(#attrs)*
+        // of the function we're imported from so we at least know what function
-            #[allow(bad_style)]
+        // is causing issues.
-            #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
+        //
-            #[doc = #doc_comment]
+        // Note that this is where type errors like "doesn't implement
-            #[allow(clippy::all)]
+        // FromWasmAbi" or "doesn't implement IntoWasmAbi" currently get routed.
-            #vis fn #rust_name(#me #(#arguments),*) #ret {
+        // I suspect that's because they show up in the signature via trait
+        // projections as types of arguments, and all that needs to typecheck
+        // before the body can be typechecked. Due to rust-lang/rust#60980 (and
+        // probably related issues) we can't really get a precise span.
+        //
+        // Ideally what we want is to point errors for particular types back to
+        // the specific argument/type that generated the error, but it looks
+        // like rustc itself doesn't do great in that regard so let's just do
+        // the best we can in the meantime.
+        let extern_fn = respan(
+            quote! {
                 #[link(wasm_import_module = "__wbindgen_placeholder__")]
+                #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                 extern "C" {
                     fn #import_name(#(#abi_arguments),*) -> #abi_ret;
                 }
+                #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
+                unsafe fn #import_name(#(#abi_arguments),*) -> #abi_ret {
+                    panic!("cannot call wasm-bindgen imported functions on \
+                            non-wasm targets");
+                }
+            },
+            &self.rust_name,
+        );
+
+        let invocation = quote! {
+            #(#attrs)*
+            #[allow(bad_style)]
+            #[doc = #doc_comment]
+            #[allow(clippy::all)]
+            #vis fn #rust_name(#me #(#arguments),*) #ret {
+                #extern_fn
+
                 unsafe {
                     #exn_data
                     let #ret_ident = {
@@ -1031,17 +1058,6 @@ impl TryToTokens for ast::ImportFunction {
                     #convert_ret
                 }
             }
-
-            #(#attrs)*
-            #[allow(bad_style, unused_variables)]
-            #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
-            #[doc = #doc_comment]
-            #[allow(clippy::all)]
-            #vis fn #rust_name(#me #(#arguments),*) #ret {
-                panic!("cannot call wasm-bindgen imported functions on \
-                        non-wasm targets");
-            }
-
         };
 
         if let Some(class) = class_ty {
@@ -1164,12 +1180,17 @@ impl ToTokens for ast::ImportStatic {
             #[allow(bad_style)]
             #[allow(clippy::all)]
             #vis static #name: wasm_bindgen::JsStatic<#ty> = {
-                #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                 fn init() -> #ty {
                     #[link(wasm_import_module = "__wbindgen_placeholder__")]
+                    #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
                     extern "C" {
                         fn #shim_name() -> <#ty as wasm_bindgen::convert::FromWasmAbi>::Abi;
                     }
+                    #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
+                    unsafe fn #shim_name() -> <#ty as wasm_bindgen::convert::FromWasmAbi>::Abi {
+                        panic!("cannot access imported statics on non-wasm targets")
+                    }
+
                     unsafe {
                         <#ty as wasm_bindgen::convert::FromWasmAbi>::from_abi(
                             #shim_name(),
@@ -1178,10 +1199,6 @@ impl ToTokens for ast::ImportStatic {
 
                     }
                 }
-                #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
-                fn init() -> #ty {
-                    panic!("cannot access imported statics on non-wasm targets")
-                }
                 thread_local!(static _VAL: #ty = init(););
                 wasm_bindgen::JsStatic {
                     __inner: &_VAL,
@@ -1449,6 +1466,8 @@ impl<'a, T: ToTokens> ToTokens for Descriptor<'a, T> {
     }
 }
 
+/// Converts `span` into a stream of tokens, and attempts to ensure that `input`
+/// has all the appropriate span information so errors in it point to `span`.
 fn respan(input: TokenStream, span: &dyn ToTokens) -> TokenStream {
     let mut first_span = Span::call_site();
     let mut last_span = Span::call_site();

crates/macro/ui-tests/missing-catch.rs

@@ -0,0 +1,9 @@
+use wasm_bindgen::prelude::*;
+
+#[wasm_bindgen]
+extern "C" {
+	#[wasm_bindgen]
+	pub fn foo() -> Result<JsValue, JsValue>;
+}
+
+fn main() {}

crates/macro/ui-tests/missing-catch.stderr

@@ -0,0 +1,7 @@
+error[E0277]: the trait bound `std::result::Result<wasm_bindgen::JsValue, wasm_bindgen::JsValue>: wasm_bindgen::convert::traits::FromWasmAbi` is not satisfied
+ --> $DIR/missing-catch.rs:6:9
+  |
+6 |     pub fn foo() -> Result<JsValue, JsValue>;
+  |            ^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::result::Result<wasm_bindgen::JsValue, wasm_bindgen::JsValue>`
+
+For more information about this error, try `rustc --explain E0277`.

crates/macro/ui-tests/traits-not-implemented.rs

@@ -0,0 +1,11 @@
+use wasm_bindgen::prelude::*;
+
+struct A;
+
+#[wasm_bindgen]
+extern "C" {
+    #[wasm_bindgen]
+    pub fn foo(a: A);
+}
+
+fn main() {}

crates/macro/ui-tests/traits-not-implemented.stderr

@@ -0,0 +1,7 @@
+error[E0277]: the trait bound `A: wasm_bindgen::convert::traits::IntoWasmAbi` is not satisfied
+ --> $DIR/traits-not-implemented.rs:8:12
+  |
+8 |     pub fn foo(a: A);
+  |            ^^^ the trait `wasm_bindgen::convert::traits::IntoWasmAbi` is not implemented for `A`
+
+For more information about this error, try `rustc --explain E0277`.

crates/macro/ui-tests/unknown-type-in-import.rs

@@ -0,0 +1,10 @@
+use wasm_bindgen::prelude::*;
+
+#[wasm_bindgen]
+extern "C" {
+    #[wasm_bindgen]
+    pub fn foo(a: A);
+}
+
+fn main() {}
+

crates/macro/ui-tests/unknown-type-in-import.stderr

@@ -0,0 +1,7 @@
+error[E0412]: cannot find type `A` in this scope
+ --> $DIR/unknown-type-in-import.rs:6:19
+  |
+6 |     pub fn foo(a: A);
+  |                   ^ not found in this scope
+
+For more information about this error, try `rustc --explain E0412`.