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.
2026-02-26 07:27:30 -05:00
parent f7a4f56ce4
commit 8051436f57
8 changed files with 275 additions and 5 deletions
--- a/docs/standards/dotnet-standards.md
+++ 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 (`<Nullable>enable</Nullable>`)
+- **Implicit usings**: Enabled (`<ImplicitUsings>enable</ImplicitUsings>`)
+
+## 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<byte>` and `ReadOnlyMemory<byte>` 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<InvalidOperationException>(() => subject.DoSomething());
+await Should.ThrowAsync<TimeoutException>(async () => await subject.DoSomethingAsync());
+```
+
+### NSubstitute Conventions
+
+```csharp
+// Create substitutes
+var logger = Substitute.For<ILogger<MyService>>();
+var repository = Substitute.For<IRepository>();
+
+// Configure returns
+repository.GetByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
+    .Returns(new Entity { Id = 1 });
+
+// Verify calls
+logger.Received(1).Log(
+    Arg.Is(LogLevel.Warning),
+    Arg.Any<EventId>(),
+    Arg.Any<object>(),
+    Arg.Any<Exception?>(),
+    Arg.Any<Func<object, Exception?, string>>());
+```
+
+### 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<T>` via constructor injection:
+
+```csharp
+public class ConnectionHandler
+{
+    private readonly ILogger<ConnectionHandler> _logger;
+
+    public ConnectionHandler(ILogger<ConnectionHandler> 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<T>` / `IOptionsMonitor<T>` 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<T>` 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<T>` / `Memory<T>` over arrays for buffer operations
+- Use `ArrayPool<T>.Shared` for temporary buffers
+- Use `ObjectPool<T>` 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)