feat(csharp): add contract model and C# generator - #131
Conversation
🔍 CI Pipeline Status✅ Lint: success 📊 Coverage Report: Available in checks above |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
===========================================
+ Coverage 11.75% 22.62% +10.87%
===========================================
Files 65 67 +2
Lines 11121 13706 +2585
===========================================
+ Hits 1307 3101 +1794
- Misses 9780 10491 +711
- Partials 34 114 +80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SebastienMelki
left a comment
There was a problem hiding this comment.
Code Review — PR #131
@elzalem thanks for the contribution! The shared contractmodel abstraction and C# generator are a solid start. I've gone through the code in detail and have feedback across several areas.
internal/contractmodel/model.go
1. Nested message naming uses __ separator — leaks into generated output
walkMessageSymbols joins nested message names with __ (e.g., Item__Item_Details). This produces ugly class/type names in every consumer (C#: Item__Item_Details, Python: Item__Item_Details). In C# specifically, nested classes (Item.Details) or flattened names (ItemDetails) would be far more idiomatic. Since this is the shared model, the naming strategy here cascades to all downstream generators — it should be thought through carefully.
2. Limited well-known type handling
Only google.protobuf.Struct and google.protobuf.Timestamp are handled. Missing: Any, Duration, Value, ListValue, Empty, FieldMask, wrappers (StringValue, Int32Value, etc.). These are commonly used in real protos and will produce incorrect output or object/Any fallbacks silently.
3. No handling for optional fields or oneof
The Field struct has no concept of optionality (proto3 optional keyword) or oneof grouping. These are semantically important — optional produces hasField semantics, and oneof means mutually exclusive fields. Both affect generated code shape.
4. Package exposes Files []*protogen.File
This leaks protogen types into the abstraction layer. If the goal of contractmodel is to provide a clean, generator-agnostic model, it should not expose raw protogen types. Consumers should work purely through the model's own types.
5. No unit tests for contractmodel
This package is the foundation for all new generators but has zero unit tests of its own. It's only tested indirectly via golden tests that exercise one simple proto. Functions like Packages(), buildSymbols(), resolveType(), walkMessageSymbols() need direct unit tests with targeted edge cases (deeply nested messages, map<K, Enum>, cross-file imports, etc.).
6. sebuf annotation support missing
The existing generators support unwrap, int64_encoding, enum_encoding, nullable, empty_behavior, timestamp_format, bytes_encoding, oneof_config, flatten, etc. The contract model doesn't carry any of this annotation metadata, so consumers can't generate code that respects these annotations. This is a significant gap for a shared model in this project.
internal/csharpgen/generator.go
7. pascalCase() doesn't properly handle ALL_CAPS proto names
pascalCase("STATE_UNSPECIFIED") produces STATEUNSPECIFIED (each segment keeps its original casing after the first letter). The golden file confirms this. The expected C# convention is StateUnspecified. Fix: lowercase each segment before capitalizing the first letter.
8. Enums generated as static class with string constants
C# has proper enum types. Generating public static class with public const string is non-idiomatic and loses type safety. C# developers expect real enums (or at least smart enums). The current approach makes it impossible to use enums in switch statements, get compiler exhaustiveness checks, etc.
9. No handling of sebuf annotations
The generator doesn't read or act on any of the project's custom annotations (int64_encoding, enum_encoding, nullable, timestamp_format, bytes_encoding, oneof_config, flatten, etc.). For a generator in this project, these are core requirements — without them the generated C# code won't correctly serialize/deserialize messages that use these annotations.
10. ServiceContracts is barely useful
The generated ServiceContracts class only emits service name constants. No methods, no route info, no HTTP verb/path configuration, no request/response type associations. For an HTTP client generator (protoc-gen-csharp-http), this should at minimum include HTTP method + path + types per RPC.
internal/csharpgen/golden_test.go
11. Single minimal test proto
The test proto (contracts.proto) only covers: basic scalar fields, repeated string, google.protobuf.Struct, nested enums, nested messages, one service with one RPC. Missing coverage:
optionalfieldsoneoffields- Maps with non-string keys, enum values, message values
- Multiple services
- Well-known types (Timestamp, Duration, Any, etc.)
- Deeply nested messages (3+ levels)
- Cross-file imports
- sebuf annotations (unwrap, int64_encoding, etc.)
- Empty messages
- Streaming RPCs (should be skipped or handled gracefully)
12. diffStrings() is duplicated
This helper is copy-pasted between csharpgen and pyclientgen test files. Should be in a shared test utility.
Missing Requirements
Beyond the code issues above, this PR needs several additions before it's ready to merge:
-
Documentation: Both top-level README/CLAUDE.md updates and generator-specific documentation explaining the C# generator's capabilities, options, and limitations.
-
Examples: An example in the
examples/directory that demonstrates all supported use cases — covering the various proto features and annotations the generator handles (similar toexamples/ts-client-demo/orexamples/ts-fullstack-demo/). -
Extensive testing: The current single golden test with one basic proto is insufficient. We need:
- Unit tests for
contractmodel(edge cases for type resolution, nested message naming, map handling, well-known types) - Unit tests for
csharpgenfunctions (pascalCase,csharpType,csharpScalar,jsonAttribute) - Multiple golden test protos covering all supported type combinations and edge cases
- Tests for both Newtonsoft and System.Text.Json output paths
- Unit tests for
-
Makefile integration: The new plugin should be discovered by the Makefile automatically (it likely already is via cmd/ auto-discovery, but verify).
a1b8a29 to
a9bf1ab
Compare
Summary
This refresh rebuilds the C# generator on current
mainand substantiallyexpands it from contract stubs into production-oriented DTO and HTTP client
generation.
It adds:
internal/contractmodelprotoc-gen-csharp-httpHttpClient, client/call options, cancellation, and typed errorsReview follow-up
This update addresses every item from the maintainer review:
__(
WidgetDetails,DeepNestLevel1Level2) and covered directly.Any,Duration,Empty,FieldMask,ListValue,Struct,Timestamp,Value, and all scalar wrappers are modeled andmapped.
optional/oneof: presence, oneof grouping/variants,discriminators, and flatten behavior are represented and tested.
source, contract, and service types;
protogenis construction input only.cross-file resolution, services, and annotations have focused unit tests.
empty behavior, timestamp format, flatten/prefix, and oneof configuration
flow through the model and generated JSON behavior.
STATE_UNSPECIFIEDnow becomesStateUnspecified,with direct regression coverage.
explicit wire mappings.
normalization, nullable/empty semantics, timestamps, enums, bytes, and
int64 formatting are covered under both JSON libraries.
verbs, paths, and request/response types; generated clients implement
annotated bindings, headers, options, cancellation, and typed errors.
SSE is rejected explicitly with
NotSupportedExceptionrather thansilently generating incorrect behavior.
services, WKT, deep nesting, cross-file packages, annotations, empty
messages, typed errors, and streaming behavior. Both JSON backends are
golden-tested, compiled with nullable warnings as errors, and executed
through an injected HTTP handler.
internal/testutil.DiffStringsinstead of duplicating the helper.The requested documentation and example are included:
README.mdandCLAUDE.mdlist the generatordocs/csharp-generation.mddocuments capabilities, options, wire behavior,errors, testing, and limitations
examples/csharp-contracts-demogenerates both JSON-library variantscmd/*generatorVerification
env DOTNET_ROOT=/opt/homebrew/opt/dotnet@8/libexec ... ./scripts/run_tests.sh --verbose— complete race and coverage suite passed
go vet ./...passedmake buildpassedbuf lint protopassed