Updating tests, READMEs, etc. introduces a fair bit of noise in this PR. It may be easiest to first review the changes to the Ping service, then the changes to protoc-gen-connect-go, and finally poke through the rest of the changes.

I'm honestly not sure that this is worthwhile. I do like the shorter names and reduced stuttering, but I'm worried that the extra indirection may be confusing.

so we only generate aliases for messages that are declared in the same file

This effectively introduces a new breaking constraint on Protobuf usage in general - those who were relying on the PACKAGE level for breaking changes, as an example (which would be defensible for a Golang-focused shop), would now need to enforce FILE level breaking changes to get the same guarantees about their Golang code. I'm relatively strongly opposed to any proposal that adds this requirement to Connect's users.

I would like to add a -1 vote on this.

I'm struggling to find much value out of any convenience aliases we might auto generate. In a few cases, I've simply done this in my own code. It might be a bit weird if folks aren't super used to generics, but I don't think it's unbearable and worth the extra effort and potential issues that have been brought up here.

I've never once looked at the type definitions and thought they were overly complicated or needed some simplification to hide the abstraction imo.

Paradoxically, I'm overall in favor of some auto-generated typedefs, as I have found myself wanting them (maybe it's grpc-go muscle memory though). However I have specific issues with this proposal - I rather see longer names if necessary, although when you play out the breaking change game, it might necessitate that ALL types need to be ServiceNameMethodName, which defeats some of the purpose.

Paradoxically, I'm overall in favor of some auto-generated typedefs, as I have found myself wanting them (maybe it's grpc-go muscle memory though). However I have specific issues with this proposal - I rather see longer names if necessary, although when you play out the breaking change game, it might necessitate that ALL types need to be ServiceNameMethodName, which defeats some of the purpose.

ServiceNameMethodName-style aliasing is the only alternative I can think of, and it's (a) painfully verbose, and (b) often much further from the actual name of the base type. Overall it feels like a step backwards, especially if the request/response messages don't use the [Method]Request and [Method]Response style that Buf prefers:

// Today, for a hypothetical ping.v1.Ping procedure that takes a ping.v1.Foo and returns a ping.v1.Bar:
Ping(context.Context, *connect.Request[pingv1.Foo]) (*connect.Response[pingv1.Bar], error)
// As proposed in this PR:
Ping(context.Context, *pingv1connect.Foo) (*pingv1connect.Bar, error)
// With ServiceNameMethodName-style aliases:
Ping(context.Context, *pingv1connect.PingServicePingRequest) (*pingv1connect.PingServicePingResponse, error)

// Regardless of what aliases we choose, requests are constructed like this:
connect.NewRequest(&pingv1.Foo{ ... })

The third option feels bad: it's longer than what we have today and it's very different from the way you'd actually construct a value of that type. Even the second option (what I've PR'ed here) isn't a clear win to me.

Re: restricting to types from the same file, I don't see a good alternative except ServiceNameMethodName-style aliases. Imagine a protobuf structure like this:

// acme/v1/messages.proto
syntax = "proto3";
package acme.v1;

message Foo {
  string name = 1;
}

message Bar {
  string name = 1;
}

// acme/v1/first.proto
syntax = "proto3";
package acme.v1;
import "acme/v1/messages.proto";

service FirstService {
  rpc DoFirst(Foo) returns (Bar) {}
}

// acme/v1/second.proto
syntax = "proto3";
package acme.v1;
import "acme/v1/messages.proto";

service SecondService {
  rpc DoSecond(Bar) returns (Foo) {}
}

When we're generating code for first.proto with protoc, there's no guarantee that we're also generating code for second.proto. (Correct me if I'm wrong about this!) Because our generator can't always see second.proto, we can't know for sure whether it's safe to declare aliases like Foo = connect.Request[acmev1.Foo]. (In this case, it isn't - when we generate for the other service, we'll want Foo to be a response.) I'm not sure how to work around this except to restrict aliasing to messages defined in the same file.

On balance, I agree with Josh and Matt. (I couldn't tell for sure how Ed feels about this.) I don't find the generic types problematic, and generating aliases like this creates as many problems as it solves.

And as a follow-on: if we truly believe that many users simply don't need access to headers/trailers/Spec/etc., IMO the better approach is to generate a separate package, with separate interfaces, that handles wrapping and unwrapping connect.Request and connect.Response automatically.

Peter may remember that long ago - before Connect's public release - the generated code included support for bare structs (without the generic wrappers). Everyone can poke around in that code here (the handler-side magic in that commit is here). A lot has changed since that commit, but we could also resurrect something like that.

I fundamentally think there's no proper way to do this without ServiceNameMethodName naming, which yes is typically worse than it would otherwise be.

An interesting thought might be to add a new lint rule that enforces method name uniqueness within a package, and then say "only generate aliases if you are using this lint rule", but that gets a bit nuts.

And yes, I generally don't love the generic wrappers and was just fine with the context-based approach, but I know I'm in the minority heh. I'm not crazy about having two ways to do things.

One option at least that hasn't been considered, using the proto gen options?

Similar to how we have in vtprotobuf, we have options for defining our pool resources:

--go-vtproto_opt=pool=vitess.io/vitess/go/vt/proto/query.Row

for example. In theory aliases can be supported through options and let a user opt in and then define their own aliases if they choose to rather than us coming up with an auto generated name?

Just another approach to consider, then people can pick their own mappings or just none at all.

A backdrop to this is that I really dislike when protoc plugins start heavily relying on options, but in this case it might be worthwhile. We could generate "context-based" services as an option, or "simple" services where access to headers etc isn't possible.

Context-based services will be difficult, since they require runtime support. We could allow users to opt into simple services where there's no header access possible, but that's problematic too - if that's in the schema, there's one decision enforced for all servers and clients. What if one client wants the simple APIs, but the server and other clients want the more complex APIs?

Options are definitely interesting, though - let's keep that in our back pocket. I'm going to close this PR and link to some of these comments from the associated issue.