feat: add protoc-gen-pony plugin#47
Merged
yordis merged 11 commits intoTrogonStack:mainfrom Apr 29, 2026
Merged
Conversation
PR SummaryMedium Risk Overview Updates release/build automation to ship the new binary: adds Reviewed by Cursor Bugbot for commit 0b74665. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds a new protoc plugin Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (protoc/buf)
participant Plugin as protoc-gen-pony
participant Protogen as protogen
participant FS as File Output
Client->>Plugin: start plugin, send CodeGeneratorRequest (stdin)
Plugin->>Plugin: injectGoImportStubs(req)
Plugin->>Protogen: New(req) via protogen.Options (ParamFunc)
Protogen->>Plugin: provide Files to generate
Plugin->>Plugin: generateFile(...) per input -> emit .pony content
Plugin->>FS: add files to CodeGeneratorResponse
Plugin->>Client: write CodeGeneratorResponse to stdout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 26 minutes and 49 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cmd/protoc-gen-pony/main_test.go (1)
161-193: Consider widening unsupported-shape tests beyond repeated fields.
Given v1 explicitly stubsoptional,oneof,map, embedded message, and enum cases, adding assertions for those here would harden regressions around TODO emission and constructor exclusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/protoc-gen-pony/main_test.go` around lines 161 - 193, TestUnsupportedShapesEmitTodo currently only asserts behavior for repeated fields; expand the test (TestUnsupportedShapesEmitTodo) to include proto examples for the other unsupported shapes (optional, oneof, map, embedded message, enum) by extending the FileDescriptorProto passed into runPlugin and add assertions that out contains "TODO protoc-gen-pony: field <name>" for each added unsupported field and that none of those field names appear in the constructor signature (similar to the existing tags checks), ensuring both TODO emission and constructor exclusion are validated for optional, oneof, map, embedded message, and enum cases.Taskfile.yml (1)
61-85: Optional: de-duplicate plugin build commands in one place.
build-pluginandbuild-plugin-ponynow duplicate the same pony build step. Consider makingbuild-plugindepend on per-plugin tasks to keep maintenance centralized.♻️ Example refactor
build-plugin: desc: Build all protoc plugin binaries + deps: [build-plugin-go, build-plugin-elixir, build-plugin-pony] cmds: - echo "Building protoc plugin binaries..." - - go build -o protoc-gen-connect-go-servicestruct ./cmd/protoc-gen-connect-go-servicestruct - - go build -o protoc-gen-elixir-grpc ./cmd/protoc-gen-elixir-grpc - - go build -o protoc-gen-pony ./cmd/protoc-gen-pony🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Taskfile.yml` around lines 61 - 85, The top-level build-plugin task duplicates per-plugin build steps (notably the protoc-gen-pony build also appears in build-plugin-pony); change build-plugin to depend on the individual tasks instead of inlining commands. Specifically, replace the repeated go build lines in the build-plugin task with a dependency list referencing build-plugin-go, build-plugin-elixir, and build-plugin-pony (leave each per-plugin task—build-plugin-go, build-plugin-elixir, build-plugin-pony—intact so their cmds own the go build invocations). This centralizes maintenance and removes the duplicated pony build step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/protoc-gen-pony/generate.go`:
- Around line 184-187: The empty explicit-presence branch in isSupported leaves
fields with HasPresence() && !HasOptionalKeyword() incorrectly treated as
supported; change that branch to explicitly mark them as not supported/deferred
(e.g., return false or otherwise indicate unsupported) so explicit-presence
fields are deferred to presence codegen; update the branch that checks
field.Desc.HasPresence() && !field.Desc.HasOptionalKeyword() in the isSupported
function (and ensure callers still behave when isSupported returns false).
In `@cmd/protoc-gen-pony/main.go`:
- Around line 90-95: The plugin is advertising edition support by setting
plugin.SupportedFeatures to include
CodeGeneratorResponse_FEATURE_SUPPORTS_EDITIONS and setting
plugin.SupportedEditionsMinimum/Maximum, which is incorrect until full
proto2/editions semantics are implemented; remove the FEATURE_SUPPORTS_EDITIONS
bit and the SupportedEditionsMinimum/Maximum assignments (or if you must
advertise, set both Minimum and Maximum to the single edition your generator
fully supports and ensure semantic behavior matches that edition) so protoc
won't assume broader edition/proto2 semantics that you don't implement; update
the code touching plugin.SupportedFeatures, plugin.SupportedEditionsMinimum, and
plugin.SupportedEditionsMaximum accordingly.
---
Nitpick comments:
In `@cmd/protoc-gen-pony/main_test.go`:
- Around line 161-193: TestUnsupportedShapesEmitTodo currently only asserts
behavior for repeated fields; expand the test (TestUnsupportedShapesEmitTodo) to
include proto examples for the other unsupported shapes (optional, oneof, map,
embedded message, enum) by extending the FileDescriptorProto passed into
runPlugin and add assertions that out contains "TODO protoc-gen-pony: field
<name>" for each added unsupported field and that none of those field names
appear in the constructor signature (similar to the existing tags checks),
ensuring both TODO emission and constructor exclusion are validated for
optional, oneof, map, embedded message, and enum cases.
In `@Taskfile.yml`:
- Around line 61-85: The top-level build-plugin task duplicates per-plugin build
steps (notably the protoc-gen-pony build also appears in build-plugin-pony);
change build-plugin to depend on the individual tasks instead of inlining
commands. Specifically, replace the repeated go build lines in the build-plugin
task with a dependency list referencing build-plugin-go, build-plugin-elixir,
and build-plugin-pony (leave each per-plugin task—build-plugin-go,
build-plugin-elixir, build-plugin-pony—intact so their cmds own the go build
invocations). This centralizes maintenance and removes the duplicated pony build
step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08b6f7c8-967e-409a-aed7-42fac60aca44
📒 Files selected for processing (6)
Taskfile.ymlcmd/protoc-gen-pony/CHANGELOG.mdcmd/protoc-gen-pony/README.mdcmd/protoc-gen-pony/generate.gocmd/protoc-gen-pony/main.gocmd/protoc-gen-pony/main_test.go
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
enilsen16
force-pushed
the
feat/protoc-gen-pony
branch
from
d14b30d to
fde4b64
Compare
yordis
reviewed
Apr 28, 2026
enilsen16
force-pushed
the
feat/protoc-gen-pony
branch
from
ef33c5e to
1aa4d64
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/protoc-gen-pony/main_test.go (1)
15-46: Add a targeted regression test for user-parameter preservation/ordering.Line 29 exercises stub injection, but there isn’t a dedicated assertion for the PR contract that existing params remain after injected stubs. Pinning that behavior will prevent accidental regressions in
injectGoImportStubs.✅ Suggested test addition
+func TestInjectGoImportStubs_PreservesExistingParamsOrder(t *testing.T) { + t.Parallel() + req := &pluginpb.CodeGeneratorRequest{ + ProtoFile: []*descriptorpb.FileDescriptorProto{ + {Name: proto.String("user.proto")}, + }, + Parameter: proto.String("Muser.proto=custom/path,foo=bar"), + } + + injectGoImportStubs(req) + + got := req.GetParameter() + assert.Contains(t, got, "Muser.proto="+goImportStub) + assert.True( + t, + strings.HasSuffix(got, "Muser.proto=custom/path,foo=bar"), + "existing params should be appended after injected stubs", + ) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/protoc-gen-pony/main_test.go` around lines 15 - 46, Add a focused regression test that asserts injectGoImportStubs does not drop or reorder existing CodeGeneratorRequest parameter entries: reuse runPlugin to build a request with pluginpb.CodeGeneratorRequest.Parameters pre-populated with a known comma-separated list (including edge cases like empty values or duplicates), call injectGoImportStubs(req) (or runPlugin which calls it) and then assert the resulting plugin.Response or the modified req still contains the original parameters in the same order with the injected stubs appended (or otherwise documented behavior) — reference runPlugin and injectGoImportStubs to locate where to insert this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/protoc-gen-pony/main_test.go`:
- Around line 15-46: Add a focused regression test that asserts
injectGoImportStubs does not drop or reorder existing CodeGeneratorRequest
parameter entries: reuse runPlugin to build a request with
pluginpb.CodeGeneratorRequest.Parameters pre-populated with a known
comma-separated list (including edge cases like empty values or duplicates),
call injectGoImportStubs(req) (or runPlugin which calls it) and then assert the
resulting plugin.Response or the modified req still contains the original
parameters in the same order with the injected stubs appended (or otherwise
documented behavior) — reference runPlugin and injectGoImportStubs to locate
where to insert this assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e8a0315-9f3e-42c7-b44b-09cc8d936daf
📒 Files selected for processing (8)
.github/.release-please-config.json.github/.release-please-manifest.json.github/goreleaser.ymlTaskfile.ymlcmd/protoc-gen-pony/README.mdcmd/protoc-gen-pony/generate.gocmd/protoc-gen-pony/main.gocmd/protoc-gen-pony/main_test.go
✅ Files skipped from review due to trivial changes (3)
- .github/.release-please-config.json
- .github/.release-please-manifest.json
- cmd/protoc-gen-pony/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Taskfile.yml
- cmd/protoc-gen-pony/generate.go
yordis
reviewed
Apr 28, 2026
Generates Pony source code (class val records + sister Codec primitives) from .proto files. Calls into the Pony `protobuf` runtime library at straw-hat-team/trogonai.com/mrmeeseeks/protobuf for WireReader/ WireWriter/Tag/Scalar/WireType/WireError. Built on `google.golang.org/protobuf/compiler/protogen` for descriptor traversal. Pre-injects M-stub mappings so users targeting Pony don't have to set go_package or M-params just to satisfy protogen's Go-import requirement. v1 scope: singular implicit-presence proto3 scalars (bool, int32/64, uint32/64, sint32/64, fixed32/64, sfixed32/64, float, double, string, bytes). Repeated, optional explicit presence, oneofs, maps, embedded messages, and enums emit a `// TODO protoc-gen-pony` placeholder until the corresponding codegen lands. Services (gRPC) are out of scope. End-to-end smoke verified: protoc --pony_out=... produces .pony files that compile cleanly against the runtime and round-trip a User message through encode→bytes→decode with field values preserved. 9 tests pass, all running in parallel via t.Parallel(). Signed-off-by: Erik Nilsen <enilsen16@live.com>
Five parallel switches over protoreflect.Kind (ponyType / ponyDefault / ponyWireType / ponyReadExpr / ponyWriteCall / presenceCheck) collapse into a single scalarSpecs map. Helpers become one-line lookups; adding a Kind now requires editing one entry instead of risking a missed update across five tables. Also: - Fix isSupported's empty-if branch for editions explicit-presence fields; they now correctly fall into the unsupported (TODO) bucket. - Drop classifyFields's unused `unsupported` return and the dead `_ = unsupported` at the call site. emitClass now walks msg.Fields directly, branching on isSupported per field. - Inline fieldName (one-liner placeholder) into its 8 call sites. - Wire test runPlugin through injectGoImportStubs so the production workaround gets test coverage and the test stops re-implementing it. - Add a `field()` test fixture builder; collapses 6-line FieldDescriptor literals to one-liners. - Trim a duplicate godoc and a WHAT-comment. Net: -147 lines across the three files. End-to-end protoc smoke still generates Pony that compiles against the runtime and round-trips a User message correctly. 9 tests pass with -race. Signed-off-by: Erik Nilsen <enilsen16@live.com>
The runtime extracted out of straw-hat-team/trogonai.com/mrmeeseeks into its own repo. README and main.go godoc now reference the new location. No code change. Signed-off-by: Erik Nilsen <enilsen16@live.com>
- main.go: drop FEATURE_SUPPORTS_EDITIONS + FEATURE_PROTO3_OPTIONAL advertisement and the EDITION_PROTO2..EDITION_2024 range. The v1 generator emits TODO comments for explicit-presence/oneof/map/ embedded/enum fields rather than handling them, so advertising the feature flags risks protoc passing inputs we'd silently miscompile. When a future PR ships real codegen for those shapes, flip the flags back on. - cmd/protoc-gen-pony/CHANGELOG.md: deleted. release-please owns the changelog generation; the hand-written file conflicts with that flow. - .github/.release-please-config.json + .release-please-manifest.json: wire protoc-gen-pony in alongside the connect-go and elixir-grpc plugins. Manifest entry starts at 0.0.0 so the first feat: commit bumps to 0.1.0. - .github/goreleaser.yml: add a builds: stanza for protoc-gen-pony so tagged releases produce a binary across linux/darwin/windows × amd64/arm64. - Taskfile.yml: build-plugin now uses deps: [build-plugin-go, build-plugin-elixir, build-plugin-pony] instead of inlining the same go build commands. - main_test.go: TestUnsupportedShapesEmitTodo now exercises every unsupported shape — repeated, proto3 optional, oneof, embedded message, enum, and map — asserting both the TODO comment emission and that none of the unsupported field names appear in the constructor signature. Signed-off-by: Erik Nilsen <enilsen16@live.com>
…an up review nits - .github/workflows/cd.yml: add protoc-gen-pony to the validation case statement. Without this, a protoc-gen-pony@v0.1.0 tag would fail the "Valid component" gate and abort the release. Caught by simplify pass. - cmd/protoc-gen-pony/main.go: trim the no-features-advertised comment from 6 lines to 4. Speculation about future PR re-enabling the flags is recoverable from git history; the load-bearing WHY (silent miscompile risk) stays. - cmd/protoc-gen-pony/main_test.go: replace assert.False(strings.Contains(...)) with assert.NotContains, and drop the now-unused strings import. Drop a few WHAT-comments that restated the line below them; keep the protobuf-internals comments (synthetic-oneof ordering, optional encoding, MapEntry shape) — those document non-obvious descriptor semantics future readers need. Signed-off-by: Erik Nilsen <enilsen16@live.com>
…tion Cover the "user-provided params still win" contract — existing parameter entries (including empty values and duplicates) must appear verbatim after the injected M-stubs, since protogen's later-wins semantics depend on it. Signed-off-by: Erik Nilsen <enilsen16@live.com>
…ional, cross-dir refs - Enums: primitive per value + type alias + FromValue dispatcher - Singular message fields: (Child val | None) type, sub-codec decode/encode - Repeated scalar/enum fields: packed wire format, Array[T] trn → val - Repeated message fields: per-entry len-delim, same trn accumulator pattern - proto3 optional scalars/enums: (T | None) type, match-on-None encode (explicit presence) - Cross-file same-directory refs: path.Dir equality instead of exact path match - emitPackedEncode helper: eliminates 4 near-identical packed encode blocks - ponyEnumFromValueName helper: centralises the FromValue name derivation - ponyMessageClassName/enumNamePrefix: O(n) append+reverse instead of O(n²) prepend - Manifest: bump protoc-gen-pony to 0.0.1 per Yordis review comment Signed-off-by: Erik Nilsen <enilsen16@live.com>
enilsen16
force-pushed
the
feat/protoc-gen-pony
branch
from
cd3db2a to
fc84e7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/protoc-gen-pony/generate.go`:
- Around line 147-148: The decoder currently only generates a WireLenDelim arm
for repeated fields; update the code that emits the match arm (the call site
using ctx.fieldPonyWireType(field) which produces the line starting with `| (`,
num, `, `, wt, `) =>`) so that for packable repeated scalar/enum fields you also
accept the element wire type (the scalar element wt) in addition to WireLenDelim
— either by emitting two pattern alternatives (`(num, WireLenDelim)` and `(num,
<elementWireType>)`) or a combined pattern — and in the element-wire-type branch
decode a single element and append it to the slice (instead of treating it as
length-delim). Apply the same change to the other similar generation sites
referenced (the blocks around 164-191 and 488-492) so unpacked encodings for
repeated numeric/enum fields are handled.
- Around line 178-191: The code treats all repeated fields as packed (using
reader.read_len_delim and a WireReader) regardless of type; strings and bytes
must be handled as unpacked repeated values. Update the generation in the case
handling repeated fields (the case under field.Desc.IsList() that uses
scalarSpecs, spec.readExpr, reader.read_len_delim(), WireReader, readExpr and
spec.ponyType and the corresponding serialization branches) to only emit packed
len-delimited handling for types that are packable (numeric/boolean/enum). For
non-packable kinds (string, bytes, messages), emit the normal per-item
read/write loop that calls the scalar/method readExpr/writeExpr for each
occurrence rather than wrapping in read_len_delim/packed form; determine
packability via the field descriptor kind (e.g., check field.Desc.Kind() against
the set of packable kinds or use a helper like field.Desc.IsPackable()) and
apply the same change to the other repeated-field sites noted (around the other
branches at the same file).
- Around line 29-47: The current generateFile function silently drops services
because it returns early when there are no messages/enums; change the logic so
service-only or mixed files are not lost: do not return early just because
messages/enums are empty — always create the output file (outPath := ...; g :=
plugin.NewGeneratedFile(...)) when file.Services is non-empty; for unsupported
services either fail generation via plugin.Errorf("unsupported service %s in
%s", service.Desc.FullName(), file.Desc.Path()) or emit explicit TODO comments
into the generated file using the generated file writer (ctx.g.P) listing each
service and its RPCs (use service.Desc.FullName()/method.Desc.Name() to identify
them), while leaving existing behavior for messages/enums
(collectAndEmitMessages, emitEnum) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 327d1213-50c7-4df6-b8fe-7043d7487653
📒 Files selected for processing (9)
.github/.release-please-config.json.github/.release-please-manifest.json.github/goreleaser.yml.github/workflows/cd.ymlTaskfile.ymlcmd/protoc-gen-pony/README.mdcmd/protoc-gen-pony/generate.gocmd/protoc-gen-pony/main.gocmd/protoc-gen-pony/main_test.go
✅ Files skipped from review due to trivial changes (4)
- .github/.release-please-config.json
- .github/workflows/cd.yml
- cmd/protoc-gen-pony/README.md
- .github/.release-please-manifest.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/goreleaser.yml
- Taskfile.yml
…enum round-trip - Services now included in empty-file early return; emit TODO comment per service - Repeated string/bytes get per-element WireLenDelim arms (never packable) - Packable repeated (numeric/enum) emit both packed (WireLenDelim) and unpacked (element wire type) decode arms per proto3 decoder spec - Add StatusRaw class val for unknown enum values; FromValue preserves raw I32 instead of collapsing to zero, fixing encode round-trip for unknown values - Remove unused fieldPonyWireType helper Signed-off-by: Erik Nilsen <enilsen16@live.com>
yordis
approved these changes
Apr 29, 2026
…+ property tests - Real oneofs: emits wrapper class per member (ZooKindTypeA) + union type alias (ZooKind); whole oneof stays TODO if any member is WKT/group - Cross-dir refs: collects dep dirs across all nested messages/oneofs, computes relative paths via protoRelDir, emits sorted use directives - Property tests: protoRelDir inverse round-trip + snakeToPascal never contains underscore (testing/quick, no new dependency) - Simplify: screamingToPascal delegates to snakeToPascal; isSupported delegates kind-check to isSupportedOneofMember; crossDirUseDirectives caches depDir→relDir to avoid recomputing protoRelDir per field Signed-off-by: Erik Nilsen <enilsen16@live.com>
enilsen16
force-pushed
the
feat/protoc-gen-pony
branch
from
88dd803 to
95ca568
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/protoc-gen-pony/main_test.go (1)
435-439: Update the stale comment aboveTestOptionalScalarField.This comment describes
injectGoImportStubsparameter-order semantics, but the adjacent test validates optional scalar codegen. Please retitle/reword it to match the test below.✏️ Suggested comment fix
-// "User-provided params still win" (main.go:50) relies on existing entries -// appearing verbatim after the injected stubs — protogen's later-wins -// semantics depend on it. Lock in: nothing dropped or reordered, including -// empty values and duplicates. +// TestOptionalScalarField verifies proto3 explicit optional scalar fields are +// generated as `(T | None)` and encoded using presence-based `match`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/protoc-gen-pony/main_test.go` around lines 435 - 439, The existing comment above TestOptionalScalarField is stale (it talks about injectGoImportStubs parameter-order semantics) — update it to describe what the test actually verifies: that generating code for optional scalar fields produces the expected output (including presence/absence of wrapper types, nil handling, and any helper methods) and does not conflate with import stub ordering; locate the comment directly above the TestOptionalScalarField function and replace the text to clearly summarize the optional scalar field codegen expectations, mentioning TestOptionalScalarField and removing any reference to injectGoImportStubs parameter-order behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/protoc-gen-pony/main_test.go`:
- Around line 435-439: The existing comment above TestOptionalScalarField is
stale (it talks about injectGoImportStubs parameter-order semantics) — update it
to describe what the test actually verifies: that generating code for optional
scalar fields produces the expected output (including presence/absence of
wrapper types, nil handling, and any helper methods) and does not conflate with
import stub ordering; locate the comment directly above the
TestOptionalScalarField function and replace the text to clearly summarize the
optional scalar field codegen expectations, mentioning TestOptionalScalarField
and removing any reference to injectGoImportStubs parameter-order behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c973c51f-4062-4f33-a6e6-3354eabaaa91
📒 Files selected for processing (9)
.github/.release-please-config.json.github/.release-please-manifest.json.github/goreleaser.yml.github/workflows/cd.ymlTaskfile.ymlcmd/protoc-gen-pony/README.mdcmd/protoc-gen-pony/generate.gocmd/protoc-gen-pony/main.gocmd/protoc-gen-pony/main_test.go
✅ Files skipped from review due to trivial changes (6)
- .github/.release-please-config.json
- .github/.release-please-manifest.json
- .github/workflows/cd.yml
- .github/goreleaser.yml
- cmd/protoc-gen-pony/README.md
- cmd/protoc-gen-pony/generate.go
🚧 Files skipped from review as they are similar to previous changes (1)
- Taskfile.yml
- isSupportedMapField: scalar and non-WKT enum values supported; message values stay TODO (no proto3 zero-value default available) - Decode: Map[K, V] trn accumulator, per-entry sub-reader (entry_sub), key field 1 + value field 2 arms, skip-unknown, assign via (k) = v - Encode: pairs() loop, always writes both key and value (no zero elision inside map entries per proto3 spec) - use "collections" emitted when any supported map field is present; sorted together with cross-dir relative use directives - crossDirUseDirectives: handles cross-dir enum value deps in map fields - 4 new tests (37 total): ClassAndCodec, UseCollections, MessageValueTodo, EnumValue Signed-off-by: Erik Nilsen <enilsen16@live.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 92c7ca2. Configure here.
Well-known types: - Narrow isWKT blocklist to only the 4 circular/JSON-only files (struct, type, api, descriptor). Timestamp, Duration, Any, wrappers, FieldMask, Empty, etc. now generate as regular proto3 messages. - Update TestWKTRefEmitsTodo to use struct.proto (still blocked); add TestWKT_TimestampGenerates to confirm Timestamp generates. - Update TestOneofUnsupportedWhenMemberIsWKT to use struct.proto Value. map<K, message> values: - Add fun default(): ClassName val => ClassName to every XxxCodec — map message values initialise to codec.default() when the value field is absent on the wire (proto3 zero semantics). - isSupportedMapField: accept non-WKT message values (was always TODO). - mapValuePonyType/mapValueDefault: handle MessageKind (Child val, ChildCodec.default()). - emitMapDecodeArm: sub-codec decode arm for message values. - emitMapEncodeField: vsub WireWriter + codec encode for message values. - crossDirUseDirectives: also collect message-valued map cross-dir deps. - Rename TestMapField_MessageValueTodo → TestMapField_MessageValue (now a positive test). Release / docs: - Add // Requires protobuf-pony runtime >= 0.1.0 to generated header. - CHANGELOG.md created with full feature coverage list. - README.md: replace outdated coverage stub with accurate supported / known-limitations sections. Signed-off-by: Erik Nilsen <enilsen16@live.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
protoc-gen-ponyplugin that generates Pony source —class valrecords + sisterCodecprimitives — from.protofiles. Generated code calls into a Ponyprotobufruntime library atstraw-hat-team/trogonai.com/mrmeeseeks/protobuf(which providesWireReader/WireWriter/Tag/Scalar/WireType/WireError).Built on
google.golang.org/protobuf/compiler/protogenfor descriptor traversal, mirroring the Connect-Go plugin's framework choice. Pre-injectsM-stub mappings insidemain()so users targeting Pony don't need to setgo_packageor--Mparams just to satisfy protogen's Go-import requirement — user-supplied params still win because they appear later in the parameter string.v1 scope
bool,int32/64,uint32/64,sint32/64,fixed32/64,sfixed32/64,float,double,string,bytes_(Outer.Inner→Outer_Inner)class val Foo+primitive FooCodecpair withdecode(WireReader ref): (Foo val | WireError)andencode(WireWriter ref, Foo val)Out of scope for v1 (each surfaces as a
// TODO protoc-gen-ponyplaceholder in generated output): repeated,optionalexplicit presence, oneofs, maps, embedded messages, enums, services/gRPC.Test plan
task test(orgo test ./...) — all 9 plugin tests pass, all running in parallel viat.Parallel()task build-plugin-pony— produces aprotoc-gen-ponybinaryprotocinvocation:protoc --pony_out=out user.protoproduces a.ponyfile with the expected shapeuser.ponyplaced alongside the runtime, compiled withponyc, ran aUser(42, "alice", true)round-trip through encode → bytes → decode and recovered all field values// TODOplaceholders to confirm the v1 cut and follow-up scope match expectations