google / tarpc

An RPC framework for Rust with a focus on ease of use.
MIT License
3.09k stars 189 forks source link

 Add support for #[cfg(…)] attributes on rpc trait definition #435

Closed paulfariello closed 4 months ago

paulfariello commented 5 months ago

Currently it's not possible to use a #[cfg(…)] attribute on a method.

#[tarpc::service]
pub trait World {
    #[cfg(feature = "hello")]
    async fn hello() -> String;
}

Would result in:

error[E0599]: no function or associated item named `hello` found for trait `World`
 --> rpc/src/lib.rs:8:14
  |
5 |   pub trait World {
  |  ___________-
6 | |     /// Returns a greeting for name.
7 | |     #[cfg(feature = "hello")]
8 | |     async fn hello() -> String;
  | |             -^^^^^ function or associated item not found in `World`
  | |_____________|
  |

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rpc` (lib) due to previous error

This PR propose to simply propagate all attributes to every enum, and impl that are linked to a given rpc. Obviously this might cause issue with attributes that are only supposed to be used on function.

Would this kind of solution be acceptable? If not, what kind of solution would you consider to enable #[cfg(…)] usage?

tikue commented 5 months ago

Hi, thanks for working to fix this! Interesting, I hadn't thought about supporting #[cfg()] on RPC methods. I'm curious (though doesn't affect what I think about this PR), what's your use case?

What about a more selective fix, to basically only add attributes to the enum variants if if they are #[cfg()] attributes? Right now you can see there is a failing clippy check due to doc comments (/// like this) being added to match arms.

paulfariello commented 5 months ago

Our main use case is to disable a specific RPC when building our app for production. This RPC is mainly used for simulation of a specific behavior.

Sure, trying to look for specific #[cfg(…)] attributes seems quite reasonable, I'll work on it.

tikue commented 5 months ago

Could you also add a test case in https://github.com/google/tarpc/blob/master/plugins/tests/service.rs?

tikue commented 4 months ago

Thanks very much for doing this!