diff --git a/.changes/unreleased/Fixed-20260926-190020.yaml b/.changes/unreleased/Fixed-20260926-190020.yaml new file mode 100644 index 00000000..60762e45 --- /dev/null +++ b/.changes/unreleased/Fixed-20260926-190020.yaml @@ -0,0 +1,18 @@ +kind: Fixed +body: |- + **RPCs named after Rust methods, such as `Register`, `Clone` and `Into`, + now compile** ([#309]). The generated server code called each handler with + method-call syntax on the `Arc` that holds the service, so an RPC whose + Rust name is also a method of that `Arc` resolved to the wrong method and + failed to compile, with the error pointing at generated code. That + covered the generated `register` and the methods of prelude traits that + `Arc` implements, such as `clone`, `into`, `as_ref` and `drop`. Generated + code now calls handlers through the service trait. In your own code, + method-call syntax can still resolve to the other method: for example, + call an RPC named `Into` as `FooClient::into(&client, request)`. An RPC + named `New`, `Config` or `ConfigMut` still fails with E0592, because the + generated client has a method of that name; the only workaround is to + rename the RPC. + + [#309]: https://github.com/connectrpc/connect-rust/issues/309 +time: 2026-09-26T19:00:20.000000000+00:00 diff --git a/benches/rpc/src/generated/connect/bench.__connect.rs b/benches/rpc/src/generated/connect/bench.__connect.rs index d9c7b562..b300b94b 100644 --- a/benches/rpc/src/generated/connect/bench.__connect.rs +++ b/benches/rpc/src/generated/connect/bench.__connect.rs @@ -420,7 +420,7 @@ impl BenchServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BenchRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.unary(ctx, sreq) + BenchService::unary(&*svc, ctx, sreq) .await? .encode::(format) } @@ -450,7 +450,7 @@ impl BenchServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BenchRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.server_stream(ctx, sreq).await + BenchService::server_stream(&*svc, ctx, sreq).await } } }), @@ -467,7 +467,7 @@ impl BenchServiceExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::bench::v1::BenchRequest, >(req); - svc.client_stream(ctx, req) + BenchService::client_stream(&*svc, ctx, req) .await? .encode::(format) } @@ -490,7 +490,7 @@ impl BenchServiceExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::bench::v1::BenchRequest, >(req); - svc.bidi_stream(ctx, req).await + BenchService::bidi_stream(&*svc, ctx, req).await } } }), @@ -515,7 +515,7 @@ impl BenchServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.log_unary(ctx, sreq) + BenchService::log_unary(&*svc, ctx, sreq) .await? .encode::(format) } @@ -542,7 +542,7 @@ impl BenchServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.log_unary_owned(ctx, sreq) + BenchService::log_unary_owned(&*svc, ctx, sreq) .await? .encode::(format) } @@ -670,7 +670,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BenchRequest, >::from_parts(&req, &body); - svc.unary(ctx, req) + BenchService::unary(&*svc, ctx, req) .await? .encode::(format) }) @@ -690,7 +690,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(&req, &body); - svc.log_unary(ctx, req) + BenchService::log_unary(&*svc, ctx, req) .await? .encode::(format) }) @@ -710,7 +710,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(&req, &body); - svc.log_unary_owned(ctx, req) + BenchService::log_unary_owned(&*svc, ctx, req) .await? .encode::(format) }) @@ -745,7 +745,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BenchRequest, >::from_parts(&req, &body); - let resp = svc.server_stream(ctx, req).await?; + let resp = BenchService::server_stream(&*svc, ctx, req).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< @@ -777,7 +777,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::bench::v1::BenchRequest, >(requests, format, ctx.decode_options().clone()); - svc.client_stream(ctx, req_stream) + BenchService::client_stream(&*svc, ctx, req_stream) .await? .encode::(format) }) @@ -803,7 +803,7 @@ impl ::connectrpc::Dispatcher for BenchServiceServer { let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::bench::v1::BenchRequest, >(requests, format, ctx.decode_options().clone()); - let resp = svc.bidi_stream(ctx, req_stream).await?; + let resp = BenchService::bidi_stream(&*svc, ctx, req_stream).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< @@ -1299,7 +1299,7 @@ impl EchoServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::EchoRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.echo(ctx, sreq) + EchoService::echo(&*svc, ctx, sreq) .await? .encode::(format) } @@ -1397,7 +1397,7 @@ impl ::connectrpc::Dispatcher for EchoServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::EchoRequest, >::from_parts(&req, &body); - svc.echo(ctx, req) + EchoService::echo(&*svc, ctx, req) .await? .encode::(format) }) @@ -1704,7 +1704,7 @@ impl LogIngestServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.ingest(ctx, sreq) + LogIngestService::ingest(&*svc, ctx, sreq) .await? .encode::< crate::proto::bench::v1::LogIngestResponse, @@ -1804,7 +1804,7 @@ impl ::connectrpc::Dispatcher for LogIngestServiceServer let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::LogRequest, >::from_parts(&req, &body); - svc.ingest(ctx, req) + LogIngestService::ingest(&*svc, ctx, req) .await? .encode::(format) }) diff --git a/benches/rpc/src/generated/connect/bench_noutf8.__connect.rs b/benches/rpc/src/generated/connect/bench_noutf8.__connect.rs index dcd57bcb..09286d55 100644 --- a/benches/rpc/src/generated/connect/bench_noutf8.__connect.rs +++ b/benches/rpc/src/generated/connect/bench_noutf8.__connect.rs @@ -173,7 +173,7 @@ impl LogIngestServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::noutf8::v1::LogRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.ingest(ctx, sreq) + LogIngestService::ingest(&*svc, ctx, sreq) .await? .encode::< crate::proto::bench::noutf8::v1::LogIngestResponse, @@ -273,7 +273,7 @@ impl ::connectrpc::Dispatcher for LogIngestServiceServer let req = ::connectrpc::ServiceRequest::< crate::proto::bench::noutf8::v1::LogRequest, >::from_parts(&req, &body); - svc.ingest(ctx, req) + LogIngestService::ingest(&*svc, ctx, req) .await? .encode::< crate::proto::bench::noutf8::v1::LogIngestResponse, diff --git a/benches/rpc/src/generated/connect/echo_bloat.__connect.rs b/benches/rpc/src/generated/connect/echo_bloat.__connect.rs index 15784b02..607d11ef 100644 --- a/benches/rpc/src/generated/connect/echo_bloat.__connect.rs +++ b/benches/rpc/src/generated/connect/echo_bloat.__connect.rs @@ -172,7 +172,7 @@ impl BloatEchoServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BloatEcho, >::from_parts(req.reborrow(), req.bytes()); - svc.echo(ctx, sreq) + BloatEchoService::echo(&*svc, ctx, sreq) .await? .encode::(format) } @@ -268,7 +268,7 @@ impl ::connectrpc::Dispatcher for BloatEchoServiceServer let req = ::connectrpc::ServiceRequest::< crate::proto::bench::v1::BloatEcho, >::from_parts(&req, &body); - svc.echo(ctx, req) + BloatEchoService::echo(&*svc, ctx, req) .await? .encode::(format) }) diff --git a/benches/rpc/src/generated/connect/filter.__connect.rs b/benches/rpc/src/generated/connect/filter.__connect.rs index 67b9e6b6..378f7193 100644 --- a/benches/rpc/src/generated/connect/filter.__connect.rs +++ b/benches/rpc/src/generated/connect/filter.__connect.rs @@ -171,7 +171,7 @@ impl FilterServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::filter::v1::Record, >::from_parts(req.reborrow(), req.bytes()); - svc.redact(ctx, sreq) + FilterService::redact(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::filter::v1::Record, @@ -272,7 +272,7 @@ impl ::connectrpc::Dispatcher for FilterServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::filter::v1::Record, >::from_parts(&req, &body); - svc.redact(ctx, req) + FilterService::redact(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::filter::v1::Record, diff --git a/benches/rpc/src/generated/connect/fortune.__connect.rs b/benches/rpc/src/generated/connect/fortune.__connect.rs index 0e58701c..8cd40748 100644 --- a/benches/rpc/src/generated/connect/fortune.__connect.rs +++ b/benches/rpc/src/generated/connect/fortune.__connect.rs @@ -173,7 +173,7 @@ impl FortuneServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::fortune::v1::GetFortunesRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.get_fortunes(ctx, sreq) + FortuneService::get_fortunes(&*svc, ctx, sreq) .await? .encode::< crate::proto::fortune::v1::GetFortunesResponse, @@ -273,7 +273,7 @@ impl ::connectrpc::Dispatcher for FortuneServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::fortune::v1::GetFortunesRequest, >::from_parts(&req, &body); - svc.get_fortunes(ctx, req) + FortuneService::get_fortunes(&*svc, ctx, req) .await? .encode::(format) }) diff --git a/conformance/src/generated/connect/connectrpc.conformance.v1.service.__connect.rs b/conformance/src/generated/connect/connectrpc.conformance.v1.service.__connect.rs index 0c2d4a07..0fb8a5a2 100644 --- a/conformance/src/generated/connect/connectrpc.conformance.v1.service.__connect.rs +++ b/conformance/src/generated/connect/connectrpc.conformance.v1.service.__connect.rs @@ -686,7 +686,7 @@ impl ConformanceServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::UnaryRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.unary(ctx, sreq) + ConformanceService::unary(&*svc, ctx, sreq) .await? .encode::< crate::proto::connectrpc::conformance::v1::UnaryResponse, @@ -718,7 +718,7 @@ impl ConformanceServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::ServerStreamRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.server_stream(ctx, sreq).await + ConformanceService::server_stream(&*svc, ctx, sreq).await } } }), @@ -735,7 +735,7 @@ impl ConformanceServiceExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::connectrpc::conformance::v1::ClientStreamRequest, >(req); - svc.client_stream(ctx, req) + ConformanceService::client_stream(&*svc, ctx, req) .await? .encode::< crate::proto::connectrpc::conformance::v1::ClientStreamResponse, @@ -760,7 +760,7 @@ impl ConformanceServiceExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::connectrpc::conformance::v1::BidiStreamRequest, >(req); - svc.bidi_stream(ctx, req).await + ConformanceService::bidi_stream(&*svc, ctx, req).await } } }), @@ -785,7 +785,7 @@ impl ConformanceServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::UnimplementedRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.unimplemented(ctx, sreq) + ConformanceService::unimplemented(&*svc, ctx, sreq) .await? .encode::< crate::proto::connectrpc::conformance::v1::UnimplementedResponse, @@ -814,7 +814,7 @@ impl ConformanceServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::IdempotentUnaryRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.idempotent_unary(ctx, sreq) + ConformanceService::idempotent_unary(&*svc, ctx, sreq) .await? .encode::< crate::proto::connectrpc::conformance::v1::IdempotentUnaryResponse, @@ -947,7 +947,7 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::UnaryRequest, >::from_parts(&req, &body); - svc.unary(ctx, req) + ConformanceService::unary(&*svc, ctx, req) .await? .encode::< crate::proto::connectrpc::conformance::v1::UnaryResponse, @@ -969,7 +969,7 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::UnimplementedRequest, >::from_parts(&req, &body); - svc.unimplemented(ctx, req) + ConformanceService::unimplemented(&*svc, ctx, req) .await? .encode::< crate::proto::connectrpc::conformance::v1::UnimplementedResponse, @@ -991,7 +991,7 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::IdempotentUnaryRequest, >::from_parts(&req, &body); - svc.idempotent_unary(ctx, req) + ConformanceService::idempotent_unary(&*svc, ctx, req) .await? .encode::< crate::proto::connectrpc::conformance::v1::IdempotentUnaryResponse, @@ -1029,7 +1029,7 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::conformance::v1::ServerStreamRequest, >::from_parts(&req, &body); - let resp = svc.server_stream(ctx, req).await?; + let resp = ConformanceService::server_stream(&*svc, ctx, req).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< @@ -1062,7 +1062,7 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::connectrpc::conformance::v1::ClientStreamRequest, >(requests, format, ctx.decode_options().clone()); - svc.client_stream(ctx, req_stream) + ConformanceService::client_stream(&*svc, ctx, req_stream) .await? .encode::< crate::proto::connectrpc::conformance::v1::ClientStreamResponse, @@ -1091,7 +1091,8 @@ impl ::connectrpc::Dispatcher for ConformanceServiceServe let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::connectrpc::conformance::v1::BidiStreamRequest, >(requests, format, ctx.decode_options().clone()); - let resp = svc.bidi_stream(ctx, req_stream).await?; + let resp = ConformanceService::bidi_stream(&*svc, ctx, req_stream) + .await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< diff --git a/connectrpc-codegen/src/codegen.rs b/connectrpc-codegen/src/codegen.rs index 75d745f8..3f7dae52 100644 --- a/connectrpc-codegen/src/codegen.rs +++ b/connectrpc-codegen/src/codegen.rs @@ -1465,6 +1465,20 @@ fn check_module_collisions( Ok(()) } +/// The call to a service's handler: `Trait::method(&*svc, ctx, request)`. +/// +/// `svc` is an `Arc`, and method-call syntax on it finds the `Arc`'s own +/// methods before those of `S`. An RPC named like one of them, such as the +/// extension trait's `register` or a prelude trait's `clone`, would resolve +/// to that method (issue [#309]). The `&*` makes `Self` the service and not +/// the `Arc`. +/// +/// [#309]: https://github.com/connectrpc/connect-rust/issues/309 +fn handler_call(trait_name: &Ident, method: &Ident, request: &str) -> TokenStream { + let request = format_ident!("{request}"); + quote! { #trait_name::#method(&*svc, ctx, #request) } +} + /// Generate code for a single service. fn generate_service( file: &FileDescriptorProto, @@ -1560,6 +1574,8 @@ fn generate_service( .map(|m| { let method_name = m.name.as_deref().unwrap_or(""); let method_snake = make_field_ident(&method_name.to_snake_case()); + let call_sreq = handler_call(&trait_name, &method_snake, "sreq"); + let call_req = handler_call(&trait_name, &method_snake, "req"); // Attach the per-method `Spec` const so the dynamic `Router` // surfaces `RequestContext::spec()` exactly like the // monomorphic `FooServiceServer` dispatcher does. @@ -1581,7 +1597,7 @@ fn generate_service( let input_owned = resolver.rust_type(input_fqn, package).unwrap(); let call_handler = quote! { let sreq = ::connectrpc::ServiceRequest::<#input_owned>::from_parts(req.reborrow(), req.bytes()); - svc.#method_snake(ctx, sreq).await + #call_sreq.await }; quote! { .route_view_server_stream::<_, _, #output_type>( @@ -1616,7 +1632,7 @@ fn generate_service( let svc = ::std::sync::Arc::clone(&svc); async move { #into_items - svc.#method_snake(ctx, req).await?.encode::<#output_type>(format) + #call_req.await?.encode::<#output_type>(format) } } }), @@ -1639,7 +1655,7 @@ fn generate_service( let svc = ::std::sync::Arc::clone(&svc); async move { #into_items - svc.#method_snake(ctx, req).await + #call_req.await } } }), @@ -1669,7 +1685,7 @@ fn generate_service( let input_owned = resolver.rust_type(input_fqn, package).unwrap(); let call_handler = quote! { let sreq = ::connectrpc::ServiceRequest::<#input_owned>::from_parts(req.reborrow(), req.bytes()); - svc.#method_snake(ctx, sreq).await?.encode::<#output_type>(format) + #call_sreq.await?.encode::<#output_type>(format) }; quote! { @@ -2024,6 +2040,8 @@ fn generate_service_server( for m in &service.method { let method_name = m.name.as_deref().unwrap_or(""); let method_snake = make_field_ident(&method_name.to_snake_case()); + let call_req = handler_call(trait_name, &method_snake, "req"); + let call_req_stream = handler_call(trait_name, &method_snake, "req_stream"); let input_view = resolver.rust_view_type(m.input_type.as_deref().unwrap_or(""), package)?; let output_type = resolver.rust_type(m.output_type.as_deref().unwrap_or(""), package)?; let cs = m.client_streaming.unwrap_or(false); @@ -2044,7 +2062,7 @@ fn generate_service_server( let svc = ::std::sync::Arc::clone(&self.inner); Box::pin(async move { let req_stream = #stream_decode; - let resp = svc.#method_snake(ctx, req_stream).await?; + let resp = #call_req_stream.await?; Ok(resp.map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::<#output_type, _, _>(s, format))) }) } @@ -2056,7 +2074,7 @@ fn generate_service_server( let svc = ::std::sync::Arc::clone(&self.inner); Box::pin(async move { let req_stream = #stream_decode; - svc.#method_snake(ctx, req_stream).await?.encode::<#output_type>(format) + #call_req_stream.await?.encode::<#output_type>(format) }) } }); @@ -2066,7 +2084,7 @@ fn generate_service_server( let input_owned = resolver.rust_type(input_fqn, package)?; let call_handler = quote! { let req = ::connectrpc::ServiceRequest::<#input_owned>::from_parts(&req, &body); - let resp = svc.#method_snake(ctx, req).await?; + let resp = #call_req.await?; }; call_ss_arms.push(quote! { #method_name => { @@ -2087,7 +2105,7 @@ fn generate_service_server( let input_owned = resolver.rust_type(input_fqn, package)?; let call_handler = quote! { let req = ::connectrpc::ServiceRequest::<#input_owned>::from_parts(&req, &body); - svc.#method_snake(ctx, req).await?.encode::<#output_type>(format) + #call_req.await?.encode::<#output_type>(format) }; call_unary_arms.push(quote! { #method_name => { @@ -3876,6 +3894,38 @@ mod tests { syn::parse_str::(&code).expect("generated code parses"); } + /// An RPC may share its name with a method that an `Arc` already has: + /// the extension trait's `register` (issue #309), or a prelude trait's + /// `clone`, `into` or `drop`. Method-call syntax on the `Arc` would + /// resolve to that method, so every handler call names the service trait. + #[test] + fn handler_calls_name_the_service_trait() { + let mut file = + minimal_file_with_methods("example.v1", &["Register", "Clone", "Into", "Drop"]); + let methods = &mut file.service[0].method; + methods[1].server_streaming = Some(true); + methods[2].client_streaming = Some(true); + methods[3].client_streaming = Some(true); + methods[3].server_streaming = Some(true); + + let code = gen_service(std::slice::from_ref(&file), 0, &[], false).unwrap(); + let code = prettyplease::unparse(&syn::parse_str(&code).expect("generated code parses")); + + for method in ["register", "clone", "into", "drop"] { + let call = format!("PingService::{method}(&*svc, ctx, "); + // Once in the `Router` registration and once in the dispatcher. + assert_eq!(code.matches(&call).count(), 2, "{method}: {code}"); + assert!( + !code.contains(&format!("svc.{method}(")), + "{method}: {code}" + ); + assert!( + !code.contains(&format!(".{method}(ctx")), + "{method}: {code}" + ); + } + } + /// Build a proto file holding several services, each with the given /// method names, all typed `Empty` -> `Empty`. Used for the cross-service /// collision tests, where the service and method *names* are what's diff --git a/connectrpc-health/src/generated/connect/grpc.health.v1.health.__connect.rs b/connectrpc-health/src/generated/connect/grpc.health.v1.health.__connect.rs index 1213942f..e52097fb 100644 --- a/connectrpc-health/src/generated/connect/grpc.health.v1.health.__connect.rs +++ b/connectrpc-health/src/generated/connect/grpc.health.v1.health.__connect.rs @@ -217,7 +217,7 @@ impl HealthExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::grpc::health::v1::HealthCheckRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.check(ctx, sreq) + Health::check(&*svc, ctx, sreq) .await? .encode::< crate::proto::grpc::health::v1::HealthCheckResponse, @@ -249,7 +249,7 @@ impl HealthExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::grpc::health::v1::HealthCheckRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.watch(ctx, sreq).await + Health::watch(&*svc, ctx, sreq).await } } }), @@ -351,7 +351,7 @@ impl ::connectrpc::Dispatcher for HealthServer { let req = ::connectrpc::ServiceRequest::< crate::proto::grpc::health::v1::HealthCheckRequest, >::from_parts(&req, &body); - svc.check(ctx, req) + Health::check(&*svc, ctx, req) .await? .encode::< crate::proto::grpc::health::v1::HealthCheckResponse, @@ -388,7 +388,7 @@ impl ::connectrpc::Dispatcher for HealthServer { let req = ::connectrpc::ServiceRequest::< crate::proto::grpc::health::v1::HealthCheckRequest, >::from_parts(&req, &body); - let resp = svc.watch(ctx, req).await?; + let resp = Health::watch(&*svc, ctx, req).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< diff --git a/connectrpc-reflection/src/generated/connect/grpc.reflection.v1.reflection.__connect.rs b/connectrpc-reflection/src/generated/connect/grpc.reflection.v1.reflection.__connect.rs index 375fe1fa..23703838 100644 --- a/connectrpc-reflection/src/generated/connect/grpc.reflection.v1.reflection.__connect.rs +++ b/connectrpc-reflection/src/generated/connect/grpc.reflection.v1.reflection.__connect.rs @@ -178,7 +178,8 @@ impl ServerReflectionExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::grpc::reflection::v1::ServerReflectionRequest, >(req); - svc.server_reflection_info(ctx, req).await + ServerReflection::server_reflection_info(&*svc, ctx, req) + .await } } }), @@ -314,7 +315,12 @@ impl ::connectrpc::Dispatcher for ServerReflectionServer let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::grpc::reflection::v1::ServerReflectionRequest, >(requests, format, ctx.decode_options().clone()); - let resp = svc.server_reflection_info(ctx, req_stream).await?; + let resp = ServerReflection::server_reflection_info( + &*svc, + ctx, + req_stream, + ) + .await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< diff --git a/connectrpc-reflection/src/generated/connect/grpc.reflection.v1alpha.reflection.__connect.rs b/connectrpc-reflection/src/generated/connect/grpc.reflection.v1alpha.reflection.__connect.rs index 9a5e63a3..dbbe1510 100644 --- a/connectrpc-reflection/src/generated/connect/grpc.reflection.v1alpha.reflection.__connect.rs +++ b/connectrpc-reflection/src/generated/connect/grpc.reflection.v1alpha.reflection.__connect.rs @@ -180,7 +180,8 @@ impl ServerReflectionExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::grpc::reflection::v1alpha::ServerReflectionRequest, >(req); - svc.server_reflection_info(ctx, req).await + ServerReflection::server_reflection_info(&*svc, ctx, req) + .await } } }), @@ -316,7 +317,12 @@ impl ::connectrpc::Dispatcher for ServerReflectionServer let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::grpc::reflection::v1alpha::ServerReflectionRequest, >(requests, format, ctx.decode_options().clone()); - let resp = svc.server_reflection_info(ctx, req_stream).await?; + let resp = ServerReflection::server_reflection_info( + &*svc, + ctx, + req_stream, + ) + .await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< diff --git a/docs/guide.md b/docs/guide.md index 4d8cbb47..7214891f 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -411,6 +411,31 @@ from another's (`XGet.Foo` and `X.GetFoo` both name the `X_GET_FOO_SPEC` constant), are rejected at generation time with a message naming both sides. Rename one of them in the proto. +An RPC's Rust method name can also be the name of another method on +the value you call it on: the generated `register` on an `Arc`, or a +prelude trait method such as `clone`, `into`, or `drop`. The generated +code compiles, but method-call syntax in your code can resolve to the +other method. For example, with +`rpc Into(stream NameRequest) returns (NameResponse)`, +`client.into(requests)` resolves to `Into::into`, which takes no +request, and does not compile. Name the trait or the client type in +the call instead: + +```rust +// A handler on a service implementation. +NamesService::into(&svc, ctx, requests).await?; +// A handler on an `Arc` that holds one: `&*` passes the service, not the `Arc`. +NamesService::into(&*svc, ctx, requests).await?; +// A client method. +NamesServiceClient::into(&client, requests).await?; +// Cloning a client whose service has an RPC named `Clone`. +let copy = Clone::clone(&client); +``` + +An RPC named `New`, `Config`, or `ConfigMut` does not compile wherever +the generated client is compiled, because the client already has +methods with those names. Rename the RPC. + ### Handler signatures Unary handlers take a read-only `RequestContext` plus a borrowed @@ -1966,6 +1991,9 @@ see [Health checking](#health-checking) and Enable the `client` feature for HTTP client support with connection pooling. +An RPC named like a method that the client already has needs a +qualified call; see [Implementing servers](#implementing-servers). + ### HttpClient `HttpClient` is the standard transport built on hyper. Construct one diff --git a/examples/eliza/src/generated/connect/connectrpc.eliza.v1.eliza.__connect.rs b/examples/eliza/src/generated/connect/connectrpc.eliza.v1.eliza.__connect.rs index 3e61eb55..a909c53f 100644 --- a/examples/eliza/src/generated/connect/connectrpc.eliza.v1.eliza.__connect.rs +++ b/examples/eliza/src/generated/connect/connectrpc.eliza.v1.eliza.__connect.rs @@ -321,7 +321,7 @@ impl ElizaServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::eliza::v1::SayRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.say(ctx, sreq) + ElizaService::say(&*svc, ctx, sreq) .await? .encode::< crate::proto::connectrpc::eliza::v1::SayResponse, @@ -346,7 +346,7 @@ impl ElizaServiceExt for S { let req = ::connectrpc::dispatcher::codegen::into_stream_messages::< crate::proto::connectrpc::eliza::v1::ConverseRequest, >(req); - svc.converse(ctx, req).await + ElizaService::converse(&*svc, ctx, req).await } } }), @@ -374,7 +374,7 @@ impl ElizaServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::eliza::v1::IntroduceRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.introduce(ctx, sreq).await + ElizaService::introduce(&*svc, ctx, sreq).await } } }), @@ -482,7 +482,7 @@ impl ::connectrpc::Dispatcher for ElizaServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::eliza::v1::SayRequest, >::from_parts(&req, &body); - svc.say(ctx, req) + ElizaService::say(&*svc, ctx, req) .await? .encode::< crate::proto::connectrpc::eliza::v1::SayResponse, @@ -519,7 +519,7 @@ impl ::connectrpc::Dispatcher for ElizaServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::connectrpc::eliza::v1::IntroduceRequest, >::from_parts(&req, &body); - let resp = svc.introduce(ctx, req).await?; + let resp = ElizaService::introduce(&*svc, ctx, req).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< @@ -566,7 +566,7 @@ impl ::connectrpc::Dispatcher for ElizaServiceServer { let req_stream = ::connectrpc::dispatcher::codegen::decode_message_request_stream::< crate::proto::connectrpc::eliza::v1::ConverseRequest, >(requests, format, ctx.decode_options().clone()); - let resp = svc.converse(ctx, req_stream).await?; + let resp = ElizaService::converse(&*svc, ctx, req_stream).await?; Ok( resp .map_body(|s| ::connectrpc::dispatcher::codegen::encode_response_stream::< diff --git a/examples/multiservice/src/generated/connect/anthropic.connectrpc.greet.v1.greet.__connect.rs b/examples/multiservice/src/generated/connect/anthropic.connectrpc.greet.v1.greet.__connect.rs index 98b6a99b..e75111b4 100644 --- a/examples/multiservice/src/generated/connect/anthropic.connectrpc.greet.v1.greet.__connect.rs +++ b/examples/multiservice/src/generated/connect/anthropic.connectrpc.greet.v1.greet.__connect.rs @@ -186,7 +186,7 @@ impl GreetServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::greet::v1::GreetRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.greet(ctx, sreq) + GreetService::greet(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::greet::v1::GreetResponse, @@ -287,7 +287,7 @@ impl ::connectrpc::Dispatcher for GreetServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::greet::v1::GreetRequest, >::from_parts(&req, &body); - svc.greet(ctx, req) + GreetService::greet(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::greet::v1::GreetResponse, diff --git a/examples/multiservice/src/generated/connect/anthropic.connectrpc.math.v1.math.__connect.rs b/examples/multiservice/src/generated/connect/anthropic.connectrpc.math.v1.math.__connect.rs index ed495904..e057dcdf 100644 --- a/examples/multiservice/src/generated/connect/anthropic.connectrpc.math.v1.math.__connect.rs +++ b/examples/multiservice/src/generated/connect/anthropic.connectrpc.math.v1.math.__connect.rs @@ -177,7 +177,7 @@ impl MathServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::math::v1::AddRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.add(ctx, sreq) + MathService::add(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::math::v1::AddResponse, @@ -278,7 +278,7 @@ impl ::connectrpc::Dispatcher for MathServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::math::v1::AddRequest, >::from_parts(&req, &body); - svc.add(ctx, req) + MathService::add(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::math::v1::AddResponse, diff --git a/examples/multiservice/src/generated/connect/anthropic.connectrpc.wkt.v1.wkt.__connect.rs b/examples/multiservice/src/generated/connect/anthropic.connectrpc.wkt.v1.wkt.__connect.rs index ee582445..20688da1 100644 --- a/examples/multiservice/src/generated/connect/anthropic.connectrpc.wkt.v1.wkt.__connect.rs +++ b/examples/multiservice/src/generated/connect/anthropic.connectrpc.wkt.v1.wkt.__connect.rs @@ -386,7 +386,7 @@ impl WellKnownTypesServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::CreateEventRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.create_event(ctx, sreq) + WellKnownTypesService::create_event(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::CreateEventResponse, @@ -415,7 +415,7 @@ impl WellKnownTypesServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::CalculateDurationRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.calculate_duration(ctx, sreq) + WellKnownTypesService::calculate_duration(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::CalculateDurationResponse, @@ -444,7 +444,7 @@ impl WellKnownTypesServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::ProcessMetadataRequest, >::from_parts(req.reborrow(), req.bytes()); - svc.process_metadata(ctx, sreq) + WellKnownTypesService::process_metadata(&*svc, ctx, sreq) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::ProcessMetadataResponse, @@ -473,7 +473,7 @@ impl WellKnownTypesServiceExt for S { let sreq = ::connectrpc::ServiceRequest::< ::buffa_types::google::protobuf::Empty, >::from_parts(req.reborrow(), req.bytes()); - svc.heartbeat(ctx, sreq) + WellKnownTypesService::heartbeat(&*svc, ctx, sreq) .await? .encode::< ::buffa_types::google::protobuf::Timestamp, @@ -596,7 +596,7 @@ for WellKnownTypesServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::CreateEventRequest, >::from_parts(&req, &body); - svc.create_event(ctx, req) + WellKnownTypesService::create_event(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::CreateEventResponse, @@ -618,7 +618,7 @@ for WellKnownTypesServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::CalculateDurationRequest, >::from_parts(&req, &body); - svc.calculate_duration(ctx, req) + WellKnownTypesService::calculate_duration(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::CalculateDurationResponse, @@ -640,7 +640,7 @@ for WellKnownTypesServiceServer { let req = ::connectrpc::ServiceRequest::< crate::proto::anthropic::connectrpc::wkt::v1::ProcessMetadataRequest, >::from_parts(&req, &body); - svc.process_metadata(ctx, req) + WellKnownTypesService::process_metadata(&*svc, ctx, req) .await? .encode::< crate::proto::anthropic::connectrpc::wkt::v1::ProcessMetadataResponse, @@ -662,7 +662,7 @@ for WellKnownTypesServiceServer { let req = ::connectrpc::ServiceRequest::< ::buffa_types::google::protobuf::Empty, >::from_parts(&req, &body); - svc.heartbeat(ctx, req) + WellKnownTypesService::heartbeat(&*svc, ctx, req) .await? .encode::<::buffa_types::google::protobuf::Timestamp>(format) }) diff --git a/tests/streaming/build.rs b/tests/streaming/build.rs index 40cb28dd..c6f6d43f 100644 --- a/tests/streaming/build.rs +++ b/tests/streaming/build.rs @@ -1,6 +1,6 @@ fn main() { connectrpc_build::Config::new() - .files(&["proto/echo.proto"]) + .files(&["proto/echo.proto", "proto/names.proto"]) .includes(&["proto/"]) .include_file("_connectrpc.rs") .compile() diff --git a/tests/streaming/proto/names.proto b/tests/streaming/proto/names.proto new file mode 100644 index 00000000..14fd0dd9 --- /dev/null +++ b/tests/streaming/proto/names.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package test.names.v1; + +// A service whose RPC names are also methods of the `Arc` that holds the +// service implementation: the generated `register`, and `clone`, `into` and +// `drop` from traits in the prelude. It has one RPC of each of the four types. +service NamesService { + rpc Register(NameRequest) returns (NameResponse); + rpc Clone(NameRequest) returns (stream NameResponse); + rpc Into(stream NameRequest) returns (NameResponse); + rpc Drop(stream NameRequest) returns (stream NameResponse); +} + +message NameRequest { + string name = 1; +} + +message NameResponse { + string name = 1; +} diff --git a/tests/streaming/src/lib.rs b/tests/streaming/src/lib.rs index bef6ff35..b03678eb 100644 --- a/tests/streaming/src/lib.rs +++ b/tests/streaming/src/lib.rs @@ -3,6 +3,9 @@ pub mod proto { } pub use proto::test::echo::v1::*; +#[cfg(test)] +mod names; + #[cfg(test)] mod tests { diff --git a/tests/streaming/src/names.rs b/tests/streaming/src/names.rs new file mode 100644 index 00000000..de13fc7d --- /dev/null +++ b/tests/streaming/src/names.rs @@ -0,0 +1,186 @@ +//! RPCs named after methods that an `Arc` already has (issue #309). +//! +//! That the generated code for `NamesService` compiles is the main assertion. +//! Each RPC is also called through both server paths, which shows that the +//! call reaches the handler and not the `Arc`'s method of the same name. + +use std::sync::Arc; + +use buffa::Message; +use buffa::view::HasMessageView; +use connectrpc::client::{ClientConfig, HttpClient}; +use connectrpc::{ + CodecFormat, ConnectError, ConnectRpcService, Encodable, RequestContext, Response, Router, + ServiceRequest, ServiceResult, ServiceStream, StreamMessage, +}; +use futures::StreamExt; +use tokio::net::TcpListener; + +use crate::proto::test::names::v1::{ + NameRequest, NameResponse, NamesService, NamesServiceClient, NamesServiceExt, + NamesServiceServer, +}; + +fn named(name: String) -> NameResponse { + NameResponse { + name, + ..Default::default() + } +} + +fn one(name: String) -> ServiceResult> { + Response::stream_ok(futures::stream::iter([Ok::<_, ConnectError>(named(name))])) +} + +async fn count( + mut requests: ServiceStream>, +) -> Result { + let mut count = 0; + while let Some(request) = requests.next().await { + request?; + count += 1; + } + Ok(count) +} + +struct Names; + +impl NamesService for Names { + async fn register( + &self, + _ctx: RequestContext, + request: ServiceRequest<'_, NameRequest>, + ) -> ServiceResult { + Response::ok(named(format!("register {}", request.name))) + } + + async fn clone( + &self, + _ctx: RequestContext, + request: ServiceRequest<'_, NameRequest>, + ) -> ServiceResult> { + one(format!("clone {}", request.name)) + } + + async fn into( + &self, + _ctx: RequestContext, + requests: ServiceStream>, + ) -> ServiceResult { + Response::ok(named(format!("into {}", count(requests).await?))) + } + + async fn drop( + &self, + _ctx: RequestContext, + requests: ServiceStream>, + ) -> ServiceResult> { + one(format!("drop {}", count(requests).await?)) + } +} + +async fn serve(app: axum::Router) -> std::net::SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + addr +} + +fn ada() -> NameRequest { + NameRequest { + name: "ada".into(), + ..Default::default() + } +} + +/// Calls each RPC once and returns the four responses. +async fn call_each(addr: std::net::SocketAddr) -> [String; 4] { + let config = ClientConfig::new(format!("http://{addr}").parse().unwrap()); + let client = NamesServiceClient::new(HttpClient::plaintext(), config); + // `client.clone()` would be the RPC, which takes a request. + let client = Clone::clone(&client); + + let register = client.register(ada()).await.unwrap().into_owned().name; + + let mut stream = client.clone(ada()).await.unwrap(); + let clone = stream + .message() + .await + .unwrap() + .unwrap() + .view() + .name + .to_owned(); + + let requests = futures::stream::iter([ada(), ada()]); + // `client.into(requests)` would resolve to `Into::into`, which takes the + // client by value and so matches before the generated `&self` method. + let into = NamesServiceClient::into(&client, requests) + .await + .unwrap() + .into_owned() + .name; + + let mut stream = client.drop().await.unwrap(); + stream.send(ada()).await.unwrap(); + stream.close_send(); + let drop = stream + .message() + .await + .unwrap() + .unwrap() + .view() + .name + .to_owned(); + + [register, clone, into, drop] +} + +const EXPECTED: [&str; 4] = ["register ada", "clone ada", "into 2", "drop 1"]; + +#[tokio::test] +async fn rpcs_named_like_arc_methods_through_the_router() { + // On an `Arc`, `register` is the extension method that takes a `Router`. + let router = Arc::new(Names).register(Router::new()); + let addr = serve(router.into_axum_router()).await; + assert_eq!(call_each(addr).await, EXPECTED); +} + +#[tokio::test] +async fn rpcs_named_like_arc_methods_through_the_dispatcher() { + let service = ConnectRpcService::new(NamesServiceServer::new(Names)); + let addr = serve(axum::Router::new().fallback_service(service)).await; + assert_eq!(call_each(addr).await, EXPECTED); +} + +#[tokio::test] +async fn rpcs_named_like_arc_methods_through_add_service() { + let router = Router::new().add_service(Arc::new(Names)); + let addr = serve(router.into_axum_router()).await; + assert_eq!(call_each(addr).await, EXPECTED); +} + +/// Encodes the response of a unary handler and returns its name. +fn name_of(response: &Response>) -> String { + let bytes = Encodable::encode(&response.body, CodecFormat::Proto).unwrap(); + NameResponse::decode_from_slice(&bytes).unwrap().name +} + +#[tokio::test] +async fn handler_called_directly() { + let body = bytes::Bytes::from(ada().encode_to_vec()); + let view = NameRequest::decode_view(&body).unwrap(); + let request = || ServiceRequest::::from_parts(&view, &body); + let ctx = || RequestContext::new(http::HeaderMap::new()); + + let names = Names; + let response = NamesService::register(&names, ctx(), request()).await; + assert_eq!(name_of(&response.unwrap()), "register ada"); + + // `names.register(ctx, request)` would be the extension method. + let names = Arc::new(names); + let response = NamesService::register(&*names, ctx(), request()).await; + assert_eq!(name_of(&response.unwrap()), "register ada"); +}