fix: concretize Self references in method signatures

[miraclx]

Resolves: #972, #1000.

This patch replaces any Self references in method signatures with the concrete type of the impl block itself.

Example:

#[near_bindgen]
#[derive(Serialize, Deserialize)]
struct MyType {
    data: u8,
}

#[near_bindgen]
impl MyType {
    fn set_state(&mut self, new_state: Self) -> Self {
        std::mem::replace(self, new_state)
    }
}

the impl block is transformed into:

impl MyType {
    fn set_state(&mut self, new_state: MyType) -> MyType {
        std::mem::replace(self, new_state)
    }
}

this transformation is maintained regardless of type complexity or nesting level.

Worth noting, this does not transform Self references in generic bounds. But they're going to be prohibited following #980 anyway. So I left it out.

[willemneal]

Shouldn't this be fixed in the abi? Seems like it should be handled there and not banned. Witgen handles it.

[miraclx]

Shouldn't this be fixed in the abi? Seems like it should be handled there and not banned. Witgen handles it.

Well, the issue here is mostly that the intermediate code generated that eventually generates the ABI derives the schema for a type from a method call that is generic over the type. Specifically a line that does:

gen.subschema_for::<T>();

and in the case where T is Self, Self will be undefined because the generated code lives inside an extern "C" block which is unbound to an impl block.

At the moment, the example code just fails on master (See #972). This patch fixes that to make sure it both compiles and has ABI generated for the appropriate type.

An alternative as mentioned in #972 would just be to warn / fail with a useful error nudging users to use concrete types, but I wanted to submit this first so we know it's possible and not just jump head-first into disallowing it.

[itegulov]

LGTM, just a couple of nitpicks. This is a great fix for the current major version of SDK, but I wonder if we should still explore whether banning Self makes sense in 5.0. Not because this PR does an unsatisfactory job but rather because it seems like a potentially harmful practice as was pointed out here. I can create an issue if you agree that it is worth exploring.

[itegulov]

Sanitizing TokenStream instead of Type is a nice trick. I remember trying to sanitize Self in the past, and it required so much boilerplate to traverse all Type variants.

[miraclx]

Yeah, nightmarish 😨

[miraclx]

it seems like a potentially harmful practice as was pointed out #972 (comment). I can create an issue if you agree that it is worth exploring.

Yup, Sounds good to me!

[itegulov]

Done: #1005

[mitinarseny]

Hey Team,
This fix makes it impossible to define a trait with #[init] method that returns Self:

#[ext_contract(ext_my_contract)]
pub trait MyContract {
    #[init]
    fn new() -> Self;
}

as it expands into

pub trait MyContract {
    fn new() -> MyContract;
}

pub mod ext_my_contract {
    // ...
}

error[E0782]: trait objects must include the `dyn` keyword
 --> src/lib.rs:2:16:
  |
6 |     fn new() -> Self;
  |                 ^^^^
  |
help: add `dyn` keyword before this trait
  |
6 |     fn new() -> dyn Self;
  |                 +++