Replace all Self identifiers in method argument and return types (#…

…4155)

Co-authored-by: Oliver T <[email protected]>

CHANGELOG.md

@@ -8,6 +8,9 @@
 * Added support for implicit discriminants in enums.
   [#4152](https://github.com/rustwasm/wasm-bindgen/pull/4152)
 
+* Added support for `Self` in complex type expressions in methods.
+  [#4155](https://github.com/rustwasm/wasm-bindgen/pull/4155)
+
 --------------------------------------------------------------------------------
 
 ## [0.2.94](https://github.com/rustwasm/wasm-bindgen/compare/0.2.93...0.2.94)

crates/macro-support/Cargo.toml

@@ -21,6 +21,6 @@ strict-macro = []
 [dependencies]
 proc-macro2 = "1.0"
 quote = '1.0'
-syn = { version = '2.0', features = ['visit', 'full'] }
+syn = { version = '2.0', features = ['visit', 'visit-mut', 'full'] }
 wasm-bindgen-backend = { path = "../backend", version = "=0.2.94" }
 wasm-bindgen-shared = { path = "../shared", version = "=0.2.94" }

crates/macro-support/src/parser.rs

@@ -12,6 +12,7 @@ use quote::ToTokens;
 use syn::ext::IdentExt;
 use syn::parse::{Parse, ParseStream, Result as SynResult};
 use syn::spanned::Spanned;
+use syn::visit_mut::VisitMut;
 use syn::{ItemFn, Lit, MacroDelimiter, ReturnType};
 
 use crate::ClassMarker;
@@ -912,26 +913,33 @@ fn function_from_decl(
 
     let syn::Signature { inputs, output, .. } = sig;
 
-    let replace_self = |t: syn::Type| {
-        let self_ty = match self_ty {
-            Some(i) => i,
-            None => return t,
-        };
-        let path = match get_ty(&t) {
-            syn::Type::Path(syn::TypePath { qself: None, path }) => path.clone(),
-            other => return other.clone(),
-        };
-        let new_path = if path.segments.len() == 1 && path.segments[0].ident == "Self" {
-            self_ty.clone().into()
-        } else {
-            path
-        };
-        syn::Type::Path(syn::TypePath {
-            qself: None,
-            path: new_path,
-        })
+    // A helper function to replace `Self` in the function signature of methods.
+    // E.g. `fn get(&self) -> Option<Self>` to `fn get(&self) -> Option<MyType>`
+    // The following comment explains why this is necessary:
+    // https://github.com/rustwasm/wasm-bindgen/issues/3105#issuecomment-1275160744
+    let replace_self = |mut t: syn::Type| {
+        if let Some(self_ty) = self_ty {
+            // This uses a visitor to replace all occurrences of `Self` with
+            // the actual type identifier. The visitor guarantees that we find
+            // all occurrences of `Self`, even if deeply nested and even if
+            // future Rust versions add more places where `Self` can appear.
+            struct SelfReplace(Ident);
+            impl VisitMut for SelfReplace {
+                fn visit_ident_mut(&mut self, i: &mut proc_macro2::Ident) {
+                    if i == "Self" {
+                        *i = self.0.clone();
+                    }
+                }
+            }
+
+            let mut replace = SelfReplace(self_ty.clone());
+            replace.visit_type_mut(&mut t);
+        }
+        t
     };
 
+    // A helper function to replace argument names that are JS keywords.
+    // E.g. this will replace `fn foo(class: u32)` to `fn foo(_class: u32)`
     let replace_colliding_arg = |i: &mut syn::PatType| {
         if let syn::Pat::Ident(ref mut i) = *i.pat {
             let ident = i.ident.to_string();
@@ -946,14 +954,18 @@ fn function_from_decl(
         .into_iter()
         .filter_map(|arg| match arg {
             syn::FnArg::Typed(mut c) => {
+                // typical arguments like foo: u32
                 replace_colliding_arg(&mut c);
                 c.ty = Box::new(replace_self(*c.ty));
                 Some(c)
             }
             syn::FnArg::Receiver(r) => {
+                // the self argument, so self, &self, &mut self, self: Box<Self>, etc.
                 if !allow_self {
                     panic!("arguments cannot be `self`")
                 }
+
+                // write down the way in which `self` is passed for later
                 assert!(method_self.is_none());
                 if r.reference.is_none() {
                     method_self = Some(ast::MethodSelf::ByValue);
@@ -962,6 +974,7 @@ fn function_from_decl(
                 } else {
                     method_self = Some(ast::MethodSelf::RefShared);
                 }
+
                 None
             }
         })

tests/wasm/inner_self.rs

@@ -0,0 +1,28 @@
+//! This tests that the `wasm_bindgen` macro produces code that compiles for these use cases.
+//! `cargo test --target wasm32-unknown-unknown` will not run if any of these tests breaks.
+use wasm_bindgen::prelude::*;
+
+#[wasm_bindgen]
+pub struct A;
+
+#[wasm_bindgen]
+pub struct SelfPortrait;
+
+#[wasm_bindgen]
+impl A {
+    pub fn test_only_self() -> Self {
+        A
+    }
+    pub fn test_option_self() -> Option<Self> {
+        None
+    }
+    pub fn test_nested_self() -> Result<Option<Self>, String> {
+        Ok(None)
+    }
+    pub fn test_self_slice() -> Box<[Self]> {
+        Box::new([])
+    }
+    pub fn test_selfish() -> Result<Self, SelfPortrait> {
+        Ok(A)
+    }
+}

tests/wasm/main.rs

@@ -36,6 +36,7 @@ pub mod getters_and_setters;
 pub mod ignore;
 pub mod import_class;
 pub mod imports;
+pub mod inner_self;
 pub mod intrinsics;
 pub mod js_keywords;
 pub mod js_objects;