Skip to content

Refactor Macros to Apply to Impl Blocks for Ergonomics (#43) #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

BigFish2086
Copy link

This PR addresses Issue #43 by refactoring the restate-sdk-rust macros to apply directly to impl blocks instead of requiring separate trait definitions. The change improves ergonomics, eliminates the need for FooBar/FooBarImpl naming conventions, and aligns the SDK with more idiomatic Rust patterns.

Current Macros:

  • #[service], #[object], and #[workflow] with support for optional attributes like
    vis = "pub" which is needed to control the visibility of the generated Serve and Client structs and
    name = "my_service" for overriding the service name.

  • #[handler] to mark which of the methods are service handlers. It also supports optional attributes like
    name = "my_handler" to override the handler name and shared in case of having an Object or a Workflow to support concurrent use of the handler.

Example:

use restate_sdk::prelude::*;

pub struct Counter;

#[restate_sdk::object(vis = "pub")]
impl Counter {
    #[handler(name = "add_value")]
    async fn add(&self, _ctx: ObjectContext<'_>, val: u64) -> Result<u64, TerminalError> {
        unimplemented!()
    }
    #[handler(shared, name = "get_counter")]
    async fn get(&self, _ctx: SharedObjectContext<'_>,) -> Result<u64, TerminalError> {
        unimplemented!()
    }
}

Modify the procedural macros in ast.rs, gen.rs, and lib.rs to apply
directly to impl blocks instead of traits, per Issue restatedev#43. This eliminates
the need for separate trait definitions and simplifies the SDK's usage.

New macro syntax examples:
- #[restate_sdk::service] or #[restate_sdk::service(vis = "pub", name = "my_service")]
- #[restate_sdk::object] or #[restate_sdk::object(vis = "pub")]
- #[restate_sdk::workflow(name = "my_workflow")]
- Method attributes: #[handler], #[handler(shared)], or #[handler(name = "my_handler")]
Copy link

github-actions bot commented Mar 19, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@BigFish2086
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Collaborator

@slinkydeveloper slinkydeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly that the changes to the macro won't support both the impl block and the trait? Because I think we should support both impl block and trait.

The trait variant is perfect when you need to strongly separate in different modules of your project the "API" of the service (containing, among the others, the macro generated client) and the "IMPL" of the service (which perhaps you want it to live in a separate module).

@BigFish2086
Copy link
Author

BigFish2086 commented Mar 19, 2025

Hello, @slinkydeveloper,

I think I misunderstood the intent of this issue and removed trait support—will add it back.

Questions:

  1. Is this proposed impl macro syntax (e.g., #[restate_sdk::service(name = "my_service")],
    #[handler(shared)], etc.) good as-is?
  2. Should the trait variant use similar syntax?

Let me know.

@musjj
Copy link

musjj commented Apr 22, 2025

Any progress here? The new macro looks so much more ergonomic.

@BigFish2086
Copy link
Author

BigFish2086 commented Apr 23, 2025

Hi @musjj,

Haven't progressed since due to questions on new/old syntax. Like the #[restate_sdk::object(vis = "pub")] visibility trick feels clunky—any better ideas?

@BigFish2086 BigFish2086 marked this pull request as draft April 23, 2025 09:55
@slinkydeveloper
Copy link
Collaborator

slinkydeveloper commented Apr 23, 2025

Any progress here? The new macro looks so much more ergonomic.

Hi, sorry but TBH I won't have much time for this PR any time soon, we've other priorities atm and before this PR I would like to complete #33 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants