From 8051436f57fee726a9b955aa6fdb540259f8c76a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 26 Feb 2026 07:27:30 -0500 Subject: [PATCH] docs: add .NET coding standards and reference from phase docs Establish project-wide rules for testing (xUnit 3 / Shouldly / NSubstitute), logging (Microsoft.Extensions.Logging + Serilog + LogContext), and general C# conventions. Referenced from CLAUDE.md and phases 4-7. --- CLAUDE.md | 9 +- docs/plans/phases/phase-4-dotnet-design.md | 3 +- .../phases/phase-5-mapping-verification.md | 1 + docs/plans/phases/phase-6-porting.md | 1 + .../phases/phase-7-porting-verification.md | 2 +- docs/standards/dotnet-standards.md | 230 ++++++++++++++++++ reports/current.md | 2 +- reports/report_f7a4f56.md | 32 +++ 8 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 docs/standards/dotnet-standards.md create mode 100644 reports/report_f7a4f56.md diff --git a/CLAUDE.md b/CLAUDE.md index 1955c69..31ce8b4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,9 +50,14 @@ dotnet run --project tools/NatsNet.PortTracker -- --db porting.db - **Phase 6: Initial Porting** - `docs/plans/phases/phase-6-porting.md` - **Phase 7: Porting Verification** - `docs/plans/phases/phase-7-porting-verification.md` -## Naming Conventions +## .NET Standards -.NET projects use the `ZB.MOM.NatsNet.Server` prefix. Namespaces follow the `ZB.MOM.NatsNet.Server.[Module]` pattern. +All .NET code must follow the rules in [`docs/standards/dotnet-standards.md`](docs/standards/dotnet-standards.md). Key points: + +- .NET 10, C# latest +- **Testing**: xUnit 3, Shouldly, NSubstitute — do NOT use FluentAssertions or Moq +- **Logging**: `Microsoft.Extensions.Logging` with Serilog provider; use `LogContext.PushProperty` for contextual enrichment +- **Naming**: PascalCase for all public members; `ZB.MOM.NatsNet.Server.[Module]` namespace hierarchy ## Reports diff --git a/docs/plans/phases/phase-4-dotnet-design.md b/docs/plans/phases/phase-4-dotnet-design.md index a652d5a..c94da1f 100644 --- a/docs/plans/phases/phase-4-dotnet-design.md +++ b/docs/plans/phases/phase-4-dotnet-design.md @@ -10,6 +10,7 @@ Every module, feature, and test in the porting database must have either a .NET - Phases 1-3 complete: all Go items in the DB, all libraries mapped - Verify with: `dotnet run --project tools/NatsNet.PortTracker -- report summary --db porting.db` +- Read and follow the [.NET Coding Standards](../../standards/dotnet-standards.md) — all naming, testing, and logging decisions must comply ## Source and Target Locations @@ -149,7 +150,7 @@ dotnet run --project tools/NatsNet.PortTracker -- test map \ --db porting.db ``` -Go test naming (`TestParserValid`) translates to .NET naming (`TryParse_ValidInput_ReturnsTrue`). Each Go `Test*` function maps to one or more `[Fact]` or `[Theory]` methods. Table-driven Go tests often become `[Theory]` with `[InlineData]` or `[MemberData]`. +Go test naming (`TestParserValid`) translates to .NET naming (`TryParse_ValidInput_ReturnsTrue`). Each Go `Test*` function maps to one or more `[Fact]` or `[Theory]` methods. Table-driven Go tests often become `[Theory]` with `[InlineData]` or `[MemberData]`. Use Shouldly for assertions and NSubstitute for mocking — see the [.NET Coding Standards](../../standards/dotnet-standards.md) for details. ### Step 4: Mark N/A items diff --git a/docs/plans/phases/phase-5-mapping-verification.md b/docs/plans/phases/phase-5-mapping-verification.md index bd53993..85bffe6 100644 --- a/docs/plans/phases/phase-5-mapping-verification.md +++ b/docs/plans/phases/phase-5-mapping-verification.md @@ -10,6 +10,7 @@ Confirm zero unmapped items, validate all N/A justifications, enforce naming con - Phase 4 complete: all items have .NET mappings or N/A status - Verify with: `dotnet run --project tools/NatsNet.PortTracker -- report summary --db porting.db` +- Naming verification must check compliance with the [.NET Coding Standards](../../standards/dotnet-standards.md) ## Source and Target Locations diff --git a/docs/plans/phases/phase-6-porting.md b/docs/plans/phases/phase-6-porting.md index f6019d6..bb061e3 100644 --- a/docs/plans/phases/phase-6-porting.md +++ b/docs/plans/phases/phase-6-porting.md @@ -9,6 +9,7 @@ Implement every non-N/A module, feature, and test in the porting database. Work ## Prerequisites - Phase 5 complete: all mappings verified, no collisions, naming validated +- Read and follow the [.NET Coding Standards](../../standards/dotnet-standards.md) — covers testing (xUnit 3 / Shouldly / NSubstitute), logging (Microsoft.Extensions.Logging + Serilog + LogContext), async patterns, and performance guidelines - .NET solution structure created: - `dotnet/src/ZB.MOM.NatsNet.Server/ZB.MOM.NatsNet.Server.csproj` - `dotnet/src/ZB.MOM.NatsNet.Server.Host/ZB.MOM.NatsNet.Server.Host.csproj` diff --git a/docs/plans/phases/phase-7-porting-verification.md b/docs/plans/phases/phase-7-porting-verification.md index 658d49f..b2e6ad2 100644 --- a/docs/plans/phases/phase-7-porting-verification.md +++ b/docs/plans/phases/phase-7-porting-verification.md @@ -9,7 +9,7 @@ Every ported module passes its targeted tests. Every item in the database reache ## Prerequisites - Phase 6 complete: all non-N/A items at `complete` or better -- All tests ported and compilable +- All tests ported and compilable per [.NET Coding Standards](../../standards/dotnet-standards.md) (xUnit 3 / Shouldly / NSubstitute) - Verify readiness: `dotnet run --project tools/NatsNet.PortTracker -- phase check 6 --db porting.db` ## Source and Target Locations diff --git a/docs/standards/dotnet-standards.md b/docs/standards/dotnet-standards.md new file mode 100644 index 0000000..53c8ad8 --- /dev/null +++ b/docs/standards/dotnet-standards.md @@ -0,0 +1,230 @@ +# .NET Coding Standards + +These standards apply to all .NET code in the `dotnet/` directory. All contributors and AI agents must follow these rules. + +## Runtime and Language + +- **Target framework**: .NET 10 +- **Language**: C# (latest stable version) +- **Nullable reference types**: Enabled project-wide (`enable`) +- **Implicit usings**: Enabled (`enable`) + +## General Practices + +- Follow the [Microsoft C# coding conventions](https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/coding-conventions) +- Use `PascalCase` for public members, types, namespaces, and methods +- Use `camelCase` for local variables and parameters +- Prefix private fields with `_` (e.g., `_connectionCount`) +- Prefer `readonly` fields and immutable types where practical +- Use file-scoped namespaces +- Use primary constructors where they simplify the code +- Prefer pattern matching over type-checking casts +- Use `CancellationToken` on all async method signatures +- Use `ReadOnlySpan` and `ReadOnlyMemory` on hot paths to minimize allocations +- Prefer `ValueTask` over `Task` for methods that frequently complete synchronously + +## Forbidden Packages + +Do **NOT** use the following packages anywhere in the solution: + +| Package | Reason | +|---------|--------| +| `FluentAssertions` | Use Shouldly instead | +| `Moq` | Use NSubstitute instead | + +## Unit Testing + +All unit tests live in `dotnet/tests/ZB.MOM.NatsNet.Server.Tests/`. + +### Framework and Libraries + +| Purpose | Package | Version | +|---------|---------|---------| +| Test framework | `xUnit` | 3.x | +| Assertions | `Shouldly` | latest | +| Mocking | `NSubstitute` | latest | +| Benchmarking | `BenchmarkDotNet` | latest (for `Benchmark*` ports) | + +### xUnit 3 Conventions + +- Use `[Fact]` for single-case tests +- Use `[Theory]` with `[InlineData]` or `[MemberData]` for parameterized tests (replaces Go table-driven tests) +- Use `[Collection]` to control test parallelism when tests share resources +- Test classes implement `IAsyncLifetime` when setup/teardown is async +- Do **not** use `[SetUp]` or `[TearDown]` — those are NUnit/MSTest concepts + +### Shouldly Conventions + +```csharp +// Preferred assertion style +result.ShouldBe(expected); +result.ShouldNotBeNull(); +result.ShouldBeGreaterThan(0); +collection.ShouldContain(item); +collection.ShouldBeEmpty(); +Should.Throw(() => subject.DoSomething()); +await Should.ThrowAsync(async () => await subject.DoSomethingAsync()); +``` + +### NSubstitute Conventions + +```csharp +// Create substitutes +var logger = Substitute.For>(); +var repository = Substitute.For(); + +// Configure returns +repository.GetByIdAsync(Arg.Any(), Arg.Any()) + .Returns(new Entity { Id = 1 }); + +// Verify calls +logger.Received(1).Log( + Arg.Is(LogLevel.Warning), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); +``` + +### Test Naming + +``` +[Method]_[Scenario]_[Expected] +``` + +Examples: +- `TryParse_ValidInput_ReturnsTrue` +- `Match_WildcardSubject_ReturnsSubscribers` +- `Connect_InvalidCredentials_ThrowsAuthException` + +### Test Class Naming + +``` +[ClassName]Tests +``` + +Examples: `NatsParserTests`, `SubListTests`, `JetStreamControllerTests` + +## Logging + +Use `Microsoft.Extensions.Logging` with Serilog as the provider. + +### Setup (in `ZB.MOM.NatsNet.Server.Host`) + +```csharp +builder.Host.UseSerilog((context, services, configuration) => + configuration.ReadFrom.Configuration(context.Configuration)); +``` + +### Usage in Services + +Inject `ILogger` via constructor injection: + +```csharp +public class ConnectionHandler +{ + private readonly ILogger _logger; + + public ConnectionHandler(ILogger logger) + { + _logger = logger; + } + + public void HandleConnection(string clientId) + { + using (LogContext.PushProperty("ClientId", clientId)) + { + _logger.LogInformation("Client connected"); + } + } +} +``` + +### Structured Logging Rules + +- **Always use message templates** with named placeholders — never string interpolation: + ```csharp + // Correct + _logger.LogInformation("Client {ClientId} subscribed to {Subject}", clientId, subject); + + // Wrong — loses structured data + _logger.LogInformation($"Client {clientId} subscribed to {subject}"); + ``` +- **Use `LogContext.PushProperty`** to add contextual properties that apply to a scope of operations (e.g., client ID, connection ID, stream name). This enriches all log entries within the `using` block without repeating parameters: + ```csharp + using (LogContext.PushProperty("ConnectionId", connection.Id)) + using (LogContext.PushProperty("RemoteAddress", connection.RemoteEndPoint)) + { + // All log entries here automatically include ConnectionId and RemoteAddress + _logger.LogDebug("Processing command"); + _logger.LogInformation("Subscription created for {Subject}", subject); + } + ``` +- **Use appropriate log levels**: + | Level | Use for | + |-------|---------| + | `Trace` | Wire protocol bytes, parser state transitions | + | `Debug` | Internal state changes, subscription matching details | + | `Information` | Client connects/disconnects, server start/stop, config loaded | + | `Warning` | Slow consumers, approaching limits, recoverable errors | + | `Error` | Failed operations, unhandled protocol errors | + | `Critical` | Server cannot continue, data corruption detected | + +### Serilog Configuration + +Configure via `appsettings.json`: + +```json +{ + "Serilog": { + "Using": ["Serilog.Sinks.Console"], + "MinimumLevel": { + "Default": "Information", + "Override": { + "Microsoft": "Warning", + "System": "Warning" + } + }, + "Enrich": ["FromLogContext"], + "WriteTo": [ + { + "Name": "Console", + "Args": { + "outputTemplate": "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}" + } + } + ] + } +} +``` + +The `"Enrich": ["FromLogContext"]` entry is required for `LogContext.PushProperty` to work. + +## Dependency Injection + +- Register services in `ZB.MOM.NatsNet.Server.Host` using `Microsoft.Extensions.DependencyInjection` +- Prefer constructor injection +- Use `IOptions` / `IOptionsMonitor` for configuration binding +- Register scoped services for per-connection lifetime, singletons for server-wide services + +## Async Patterns + +- All I/O operations must be async (`async`/`await`) +- Use `CancellationToken` propagation consistently +- Use `Channel` for producer-consumer patterns (replaces Go channels) +- Use `Task.WhenAll` / `Task.WhenAny` for concurrent operations (replaces Go `select`) +- Avoid `Task.Run` for CPU-bound work in hot paths — prefer dedicated processing pipelines + +## Performance + +- Use `System.IO.Pipelines` (`PipeReader`/`PipeWriter`) for network I/O +- Prefer `Span` / `Memory` over arrays for buffer operations +- Use `ArrayPool.Shared` for temporary buffers +- Use `ObjectPool` for frequently allocated objects +- Profile before optimizing — do not prematurely optimize + +## Related Documentation + +- [Documentation Rules](../../documentation_rules.md) +- [Phase 4: .NET Solution Design](../plans/phases/phase-4-dotnet-design.md) +- [Phase 6: Porting](../plans/phases/phase-6-porting.md) diff --git a/reports/current.md b/reports/current.md index 25a4a7e..426ec4b 100644 --- a/reports/current.md +++ b/reports/current.md @@ -1,6 +1,6 @@ # NATS .NET Porting Status Report -Generated: 2026-02-26 12:22:00 UTC +Generated: 2026-02-26 12:27:31 UTC ## Modules (12 total) diff --git a/reports/report_f7a4f56.md b/reports/report_f7a4f56.md new file mode 100644 index 0000000..426ec4b --- /dev/null +++ b/reports/report_f7a4f56.md @@ -0,0 +1,32 @@ +# NATS .NET Porting Status Report + +Generated: 2026-02-26 12:27:31 UTC + +## Modules (12 total) + +| Status | Count | +|--------|-------| +| not_started | 12 | + +## Features (3673 total) + +| Status | Count | +|--------|-------| +| not_started | 3673 | + +## Unit Tests (3257 total) + +| Status | Count | +|--------|-------| +| not_started | 3257 | + +## Library Mappings (36 total) + +| Status | Count | +|--------|-------| +| mapped | 36 | + + +## Overall Progress + +**0/6942 items complete (0.0%)**