near / near-sdk-rs

Rust library for writing NEAR smart contracts
https://near-sdk.io
Apache License 2.0
452 stars 241 forks source link

`#[handle_result]` should not rely on `Result` being referred to using specific tokens #852

Open austinabell opened 2 years ago

austinabell commented 2 years ago

Currently, the attribute expects Result<T, E> specifically, which is a subset of syntax that this functionality allows.

The main cases that should be handled are:

Pulling a result type from another module

#[handle_result]
pub fn foo() -> ::core::result::Result<(), &'static str> {...}

or a type alias to result:

type Result<T> = core::result::Result<T, &'static str>;

#[handle_result]
pub fn foo() -> Result<()> {...}

Where having both of these would allow a common pattern like:

#[handle_result]
pub fn foo() -> anyhow::Result<()> {...}

Motivation: near.zulipchat.com/#narrow/stream/300659-Rust-.F0.9F.A6.80/topic/anyhow.3F/near/287320608

uint commented 1 year ago

At macro expansion time, the compiler operates on tokens and has no access to type information. That makes this whole deal rather hacky.

I'm toying with the idea of moving "Ok type" extraction to the type/trait system instead. I suspect that will work out much better.

uint commented 1 year ago

Resolving this ticket isn't enough to support anyhow. Opened another issue: #1055

frol commented 1 year ago

I'm toying with the idea of moving "Ok type" extraction to the type/trait system instead. I suspect that will work out much better.

This is exactly the direction I would love to see it going. Thanks for looking into that direction.

miraclx commented 5 months ago

Did exactly this last night for Calimero, figured I'd mention it here as well.

The idea is to inject scaffolding for any return type, and specialize for results using the trait domain. Rust treats direct association with a higher precedence than trait-enabled ones. It's the same magic that powers https://github.com/nvzqz/impls.

Once implemented, you'll no longer need the annotation, nor will you be forcing downstream users to return a Result.

Method Signature IntoResult Representation
fn() Result<(), Infallible>
fn() -> T Result<T, Infallible>
fn() -> Result<T, E> Result<T, E>
fn() -> anyhow::Result<T> Result<T, anyhow::Report>

At which point, you can do with the error whatever thou wilt.

The foundation of it (can be re-exported from near_sdk::__private)

struct WrappedReturn<T>(pub T);

impl<T, E> WrappedReturn<Result<T, E>> {
    fn into_result(self) -> Result<T, E> {
        let WrappedReturn(result) = self;
        result
    }
}

trait IntoResult {
    type Ok;
    type Err;

    fn into_result(self) -> Result<Self::Ok, Self::Err>;
}

impl<T> IntoResult for WrappedReturn<T> {
    type Ok = T;
    type Err = std::convert::Infallible;

    fn into_result(self) -> Result<Self::Ok, Self::Err> {
        let WrappedReturn(value) = self;
        Ok(value)
    }
}

then in the codegen (same as usual, with a little tweak)

let result = contract.method();

#[allow(unused_imports)]
use near_sdk::__private::IntoResult;
match near_sdk::__private::WrappedReturn(result).into_result() {
    Ok(result) => {
        let result = serde_json::to_vec(&result)
            .expect("Failed to serialize the return value using JSON.");
        near_sdk::env::value_return(&result);
    }
    Err(err) => near_sdk::FunctionError::panic(&err),
}

You might want to use a custom Infallible type with dummy impls of Serialize if you go with structured errors or at least AsRef<str> for FunctionError::panic.

Ah, just saw @uint alluded to this sometime back, noice - https://github.com/near/near-sdk-rs/issues/749#issuecomment-1722979660

faytey commented 1 month ago

I would like to hop on this @frol To tackle this issue I'll need to enhance the procedural macro to accommodate different Result types/forms, including those from other modules or using type aliases. Also, I'd add the dependencies to the cargo.toml file, then check the Handle_result attribute works with all forms of Result, I'd also closely work with the maintainer if I face any blockers

frol commented 1 month ago

@faytey Sure, please, go for it. Maybe you need to apply through ODHack website, so I will wait until that before I assign you to the issue, but feel free to start hacking!

onlydustapp[bot] commented 1 month ago

Hi @frol! Maintainers during the ODHack #6.0 will be tracking applications via OnlyDust. Therefore, in order for you to have a chance at being assigned to this issue, please apply directly here, or else your application may not be considered.

faytey commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience with react/nextjs, typescript, javascript, solidity, cairo, rust.

How I plan on tackling this issue

To tackle this issue I'll need to enhance the procedural macro to accommodate different Result types/forms, including those from other modules or using type aliases. Also, I'd add the dependencies to the cargo.toml file, then check the Handle_result attribute works with all forms of Result, I'd also closely work with the maintainer if I face any blockers

frol commented 1 month ago

@faytey this issue requires deep understanding of type system and fairly advanced Rust experience, I don't think you will succeed here

faytey commented 1 month ago

@faytey this issue requires deep understanding of type system and fairly advanced Rust experience, I don't think you will succeed here

Alright