Skip to content

Comments

feat: add protoc-gen-connect-go-service-struct plugin#4

Merged
yordis merged 1 commit intomainfrom
connect-struct-service
Sep 17, 2025
Merged

feat: add protoc-gen-connect-go-service-struct plugin#4
yordis merged 1 commit intomainfrom
connect-struct-service

Conversation

@yordis
Copy link
Member

@yordis yordis commented Sep 17, 2025

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Sep 17, 2025

Warning

Rate limit exceeded

@yordis has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 29 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a6b8d1 and 30b6c46.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • .gitignore (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/README.md (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/main.go (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/main_test.go (1 hunks)
  • go.mod (1 hunks)

Walkthrough

Introduces a new protoc plugin (protoc-gen-connect-go-servicestruct) that generates Go service structs and handler function types for Connect RPC services, adds its README, comprehensive tests, adjusts module dependencies, and updates .gitignore to ignore the plugin’s generated build artifact.

Changes

Cohort / File(s) Summary
Git ignore config
./.gitignore
Added ignore entry for generated build file: protoc-gen-connect-go-servicestruct, with header comment.
Plugin documentation
cmd/protoc-gen-connect-go-servicestruct/README.md
New README describing plugin purpose, installation, usage with protoc/buf, generation behavior, and example wiring.
Protoc plugin implementation
cmd/protoc-gen-connect-go-servicestruct/main.go
New generator: groups files by Go import path; emits “.servicestruct.connect.go”; generates service structs, per-RPC handler func types, server wrapper methods; handles deprecation/comments; supports --version and help.
Plugin tests
cmd/protoc-gen-connect-go-servicestruct/main_test.go
New tests covering version output, generation for unary/streaming RPCs, no-service case, and multiple services with same method names; helper subprocess runner.
Module dependencies
./go.mod
Added testify; updated/added indirect deps (go-cmp v0.6.0, x/net, yaml.v3, spew, difflib); reorganized require blocks.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant protoc as protoc/buf
  participant plugin as protoc-gen-connect-go-servicestruct
  participant protogen as protogen API
  participant fs as File System

  protoc->>plugin: CodeGeneratorRequest (files, params)
  plugin->>protogen: Parse request, analyze services
  loop per Go import path group
    plugin->>plugin: Generate service structs & handler types
    plugin->>fs: Emit *.servicestruct.connect.go (in-memory response)
  end
  plugin-->>protoc: CodeGeneratorResponse (generated files)
Loading
sequenceDiagram
  autonumber
  participant App as Application Server
  participant S as <ServiceName>Struct
  participant H as <MethodName>Func (handler)
  participant Conn as connect runtime

  App->>S: Register S with Connect mux
  Conn->>S: Invoke RPC (unary/streaming)
  alt Unary
    S->>H: ctx, *connect.Request[In]
    H-->>S: *connect.Response[Out], error
  else Client stream
    S->>H: ctx, *connect.ClientStream[In]
    H-->>S: *connect.Response[Out], error
  else Server stream
    S->>H: ctx, *connect.Request[In], *connect.ServerStream[Out]
    H-->>S: error
  else Bidi stream
    S->>H: ctx, *connect.BidiStream[In,Out]
    H-->>S: error
  end
  S-->>Conn: Forward result/error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I nibbled on proto and handler vine,
Wove structs and streams in tidy line.
With tests that hop through unary glades,
And bidis where the buffer wades.
Ignore the crumbs, deps neatly stacked—
My paws compiled a service pack! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning No pull request description was provided by the author, so the PR does not describe any part of the changeset and therefore fails this check. The absence of an author-written description forces reviewers to infer intent solely from the diff. Ask the author to add a brief description summarizing the plugin's purpose, the main files/dirs added (cmd/protoc-gen-connect-go-servicestruct, README, tests, go.mod), any dependency or compatibility notes, and a short usage example or link to documentation to aid reviewers and release-note authors.
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately describes the primary change: adding a protoc plugin to generate Connect service structs. It directly matches the changeset which introduces a new cmd/protoc-gen-connect-go-servicestruct tool (plus README, tests, and go.mod updates). The phrasing is concise and specific enough for repository history and review.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yordis yordis force-pushed the connect-struct-service branch 2 times, most recently from a469054 to d9a36da Compare September 17, 2025 16:29
@yordis yordis changed the title connect struct service feat: add protoc-gen-connect-go-service-struct plugin Sep 17, 2025
@yordis yordis force-pushed the connect-struct-service branch 6 times, most recently from 7523022 to 5271042 Compare September 17, 2025 21:47
@yordis yordis marked this pull request as ready for review September 17, 2025 21:47
@yordis yordis force-pushed the connect-struct-service branch from 5271042 to 3a6b8d1 Compare September 17, 2025 21:48
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
go.mod (1)

3-3: Invalid go directive: use major.minor only

go 1.24.2 is not valid in go.mod. The directive must be go 1.24 (patch versions are not allowed).

Apply this diff:

-go 1.24.2
+go 1.24

If you need to pin a specific toolchain, add a separate toolchain directive:

toolchain go1.24.2
🧹 Nitpick comments (5)
cmd/protoc-gen-connect-go-servicestruct/README.md (1)

1-110: Solid README; add prerequisites section for clarity

Consider a short “Requirements” block listing protoc, protoc-gen-go, and protoc-gen-connect-go with minimum versions, plus the Go version (e.g., Go 1.24+). Helps users avoid setup surprises.

cmd/protoc-gen-connect-go-servicestruct/main_test.go (1)

308-323: Minor: exitCode checks are redundant after require.NoError

require.NoError(cmd.Run()) already guarantees exitCode == 0. You can drop the extra equality assert, or switch to checking exitCode and use assert.NoError on run to keep both signals.

cmd/protoc-gen-connect-go-servicestruct/main.go (3)

55-61: Decouple --version from connect library version

Printing connect.Version reports the Connect library version, not this plugin’s. Expose a plugin version variable and set it via -ldflags during release builds; drop the unused connect import afterward.

Apply this diff to use a pluggable version:

 func main() {
-	if len(os.Args) == 2 && os.Args[1] == "--version" {
-		if _, err := fmt.Fprintln(os.Stdout, connect.Version); err != nil {
+	if len(os.Args) == 2 && os.Args[1] == "--version" {
+		if _, err := fmt.Fprintln(os.Stdout, version); err != nil {
 			os.Exit(1)
 		}
 		os.Exit(0)
 	}

Add this right after the const block:

+// version is set at build time: -ldflags "-X main.version=$(git describe --tags --always --dirty)"
+var version = "dev"

And remove the now‑unused import:

-import (
-	"bytes"
-	"fmt"
-	"os"
-	"strings"
-	"unicode/utf8"
-
-	connect "connectrpc.com/connect"
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"strings"
+	"unicode/utf8"

80-92: Stabilize output ordering for deterministic builds

Map iteration over packageGroups is random; consider sorting groups and files to reduce churn in generated diffs and make tests more robust.

Example patch (also add sort to imports):

 import (
   "bytes"
   "fmt"
   "os"
   "strings"
   "unicode/utf8"
+  "sort"
@@
- for _, files := range packageGroups {
-   generatePackage(plugin, files)
- }
+ // stable order of packages
+ pkgKeys := make([]string, 0, len(packageGroups))
+ for k := range packageGroups {
+   pkgKeys = append(pkgKeys, k)
+ }
+ sort.Strings(pkgKeys)
+ for _, k := range pkgKeys {
+   files := packageGroups[k]
+   sort.Slice(files, func(i, j int) bool { return files[i].Desc.Path() < files[j].Desc.Path() })
+   generatePackage(plugin, files)
+ }

179-191: Optional: guard nil handlers to improve UX

Calling a nil *Func field will panic at runtime. Consider defaulting to a stub that returns an unimplemented error, or check for nil and return a clear error.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c73622a and 3a6b8d1.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • .gitignore (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/README.md (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/main.go (1 hunks)
  • cmd/protoc-gen-connect-go-servicestruct/main_test.go (1 hunks)
  • go.mod (1 hunks)
🔇 Additional comments (2)
.gitignore (1)

33-35: Ignore rule for plugin binary looks good

Covers both root and nested outputs when running go build from different working dirs.

cmd/protoc-gen-connect-go-servicestruct/main_test.go (1)

25-37: Nice parallelized test scaffolding

Good use of table-ish subtests and compiler version metadata to mimic protoc. Assertions read cleanly.

@yordis yordis force-pushed the connect-struct-service branch 3 times, most recently from 69cab85 to 1a712f4 Compare September 17, 2025 22:15
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis force-pushed the connect-struct-service branch from 1a712f4 to 30b6c46 Compare September 17, 2025 22:16
@yordis yordis merged commit 7b38baa into main Sep 17, 2025
3 checks passed
@yordis yordis deleted the connect-struct-service branch September 17, 2025 22:19
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.

1 participant