Skip to content

feat(csharp): add contract model and C# generator - #131

Open
elzalem wants to merge 13 commits into
mainfrom
split/csharp-generator
Open

feat(csharp): add contract model and C# generator#131
elzalem wants to merge 13 commits into
mainfrom
split/csharp-generator

Conversation

@elzalem

@elzalem elzalem commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

This refresh rebuilds the C# generator on current main and substantially
expands it from contract stubs into production-oriented DTO and HTTP client
generation.

It adds:

  • a generator-neutral internal/contractmodel
  • protoc-gen-csharp-http
  • annotated routes, verbs, path/query parameters, and headers
  • injectable HttpClient, client/call options, cancellation, and typed errors
  • Newtonsoft.Json and System.Text.Json output
  • Sebuf wire annotations, cross-package contracts, and nullable-clean output
  • unit, golden, generated-compilation, and generated-runtime tests
  • dedicated documentation and a complete C# generation example

Review follow-up

This update addresses every item from the maintainer review:

  1. Nested naming: nested symbols are flattened idiomatically without __
    (WidgetDetails, DeepNestLevel1Level2) and covered directly.
  2. Well-known types: Any, Duration, Empty, FieldMask, ListValue,
    Struct, Timestamp, Value, and all scalar wrappers are modeled and
    mapped.
  3. optional / oneof: presence, oneof grouping/variants,
    discriminators, and flatten behavior are represented and tested.
  4. No raw protogen leakage: the public contract model exposes its own
    source, contract, and service types; protogen is construction input only.
  5. Direct contract-model tests: rich model, nesting, maps, WKT,
    cross-file resolution, services, and annotations have focused unit tests.
  6. Sebuf annotations: query, unwrap, int64/enum/bytes encodings, nullable,
    empty behavior, timestamp format, flatten/prefix, and oneof configuration
    flow through the model and generated JSON behavior.
  7. ALL_CAPS casing: STATE_UNSPECIFIED now becomes StateUnspecified,
    with direct regression coverage.
  8. Enums: generated output uses real C# enums with numeric values and
    explicit wire mappings.
  9. C# annotation behavior: root unwrap, nested unwrap, flatten, oneof
    normalization, nullable/empty semantics, timestamps, enums, bytes, and
    int64 formatting are covered under both JSON libraries.
  10. Useful service contracts: generated metadata includes base paths,
    verbs, paths, and request/response types; generated clients implement
    annotated bindings, headers, options, cancellation, and typed errors.
    SSE is rejected explicitly with NotSupportedException rather than
    silently generating incorrect behavior.
  11. Test breadth: fixtures cover optional fields, oneofs, maps, multiple
    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.
  12. Shared diff helper: C# and Python golden tests now use
    internal/testutil.DiffStrings instead of duplicating the helper.

The requested documentation and example are included:

  • README.md and CLAUDE.md list the generator
  • docs/csharp-generation.md documents capabilities, options, wire behavior,
    errors, testing, and limitations
  • examples/csharp-contracts-demo generates both JSON-library variants
  • Makefile command discovery already includes every cmd/* generator

Verification

  • env DOTNET_ROOT=/opt/homebrew/opt/dotnet@8/libexec ... ./scripts/run_tests.sh --verbose
    — complete race and coverage suite passed
  • all repository golden tests passed
  • generated Newtonsoft.Json and System.Text.Json contracts compiled
  • generated clients and wire-format normalization executed under .NET 8
  • go vet ./... passed
  • make build passed
  • buf lint proto passed
  • C# example generation passed

@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown

🔍 CI Pipeline Status

Lint: success
Test: success
Coverage: success
Build: success
Integration: success


📊 Coverage Report: Available in checks above
🔗 Artifacts: Test results and coverage reports uploaded

@codecov

codecov Bot commented Mar 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.66102% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 22.62%. Comparing base (b95aec5) to head (a09605a).

Files with missing lines Patch % Lines
internal/contractmodel/model.go 79.66% 65 Missing and 7 partials ⚠️
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     
Flag Coverage Δ
unittests 22.62% <79.66%> (+10.87%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SebastienMelki SebastienMelki left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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:

  • optional fields
  • oneof fields
  • 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:

  1. Documentation: Both top-level README/CLAUDE.md updates and generator-specific documentation explaining the C# generator's capabilities, options, and limitations.

  2. Examples: An example in the examples/ directory that demonstrates all supported use cases — covering the various proto features and annotations the generator handles (similar to examples/ts-client-demo/ or examples/ts-fullstack-demo/).

  3. 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 csharpgen functions (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
  4. Makefile integration: The new plugin should be discovered by the Makefile automatically (it likely already is via cmd/ auto-discovery, but verify).

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.

2 participants