--- description: C# naming, layout, and primary constructors; Microsoft conventions and .NET idioms. globs: "**/*.cs" alwaysApply: true --- # C# style (Neon Sprawl) Follow Microsoft’s **[C# Coding Conventions](https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/coding-conventions)** and **[C# identifier rules](https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names)**. Prefer clarity and consistency with existing server code. ## Naming - **Types** (classes, structs, records, interfaces, enums, delegates): `PascalCase`. - **Interfaces:** prefix with `I` (e.g. `IPlayerSession`). - **Methods, properties, events, public fields:** `PascalCase`. - **Parameters, local variables:** `camelCase`. - **Private instance fields:** `camelCase` (no leading underscore), unless an existing file consistently does otherwise—then match the file. If a parameter or local shadows a field, use `this.` or rename for clarity. - **Static fields:** `camelCase` for private/internal static fields; `PascalCase` for `public static` members (including `readonly`/constants) per Microsoft guidance; stay consistent within a project. - **Async methods:** suffix with `Async` (e.g. `LoadProfileAsync`). ## Primary constructors - Prefer **primary constructor** syntax for classes, structs, and records when it fits: dependency injection, `IClassFixture<>` test classes, small services, and any type that mainly captures parameters into fields or base calls. - **Skip** primary constructors when they hurt clarity: heavy logic in the body that belongs in a conventional constructor, `this` references before the implicit constructor runs in odd ways, or a file already uses a consistent legacy style—then match the file. ```csharp // Prefer public sealed class OrderService(IOrderStore store, ILogger log) { public Task GetAsync(Guid id) => store.FindAsync(id); } // Avoid when you would only reassign into mutable fields with non-trivial validation—use an explicit constructor instead. ``` ## Layout and syntax - **Braces:** opening brace `{` on a **new line** for types and members (Allman-style), per common Microsoft examples. - **Braces for every block:** never omit `{ }` on `if`, `else`, `for`, `foreach`, `while`, `do`, or `using` when the language allows a single statement without braces—**always** use a braced block, even for one line. This avoids accidental logic changes when editing. Expression-bodied members and expression lambdas (`x => x.Id`) stay valid when the whole body is a single expression. ```csharp // Prefer if (string.IsNullOrEmpty(key)) { return false; } // Avoid if (string.IsNullOrEmpty(key)) return false; ``` - **Indentation:** **4 spaces** per level; no tabs unless the file already uses tabs—never mix. - **Trailing whitespace:** do not leave spaces or tabs at the end of a line, including blank lines between statements (RCS1037). - **`var`:** use when the type is obvious from the right-hand side; use explicit types when it improves readability or for literals where the type matters. - **File-scoped namespaces** (`namespace X;`) for new single-namespace files when the SDK/version allows. - **Empty marker types** (collection fixtures, `[CollectionDefinition]`, marker interfaces): omit `{ }` and end with `;` when the type has no members (RCS1251). - **Pattern matching / nullability:** prefer modern C# features where they simplify code; honor **nullable reference type** annotations when the project enables them. ## Collection expressions (IDE0300 / IDE0305) - Prefer **`["a", "b"]`** over **`new[] { "a", "b" }`** for inline string (and other) array/list literals. - Prefer **collection spread** (`[.. source]`) over **`.ToArray()`** / **`.ToList()`** when materializing a sequence into a new array or list for return values, record `with` copies, or local snapshots. - Give an **explicit target type** when the compiler cannot infer one (CS9176), e.g. `string[] ids = [.. query.Select(static x => x.Id)];`. - In **assertions**, avoid passing a bare spread directly to APIs with span overloads (e.g. `Assert.Equal`) — assign to a typed local first, or use a **`private static readonly`** field when the same literal is reused (CA1861): ```csharp // Prefer return [.. query]; GrantedItems = [.. row.GrantedItems], new SkillDefRow("salvage", "gather", "Salvage", ["activity", "mission_reward"]), // When type inference fails string[] expectedOrder = [.. registry.GetDefinitionsInIdOrder().Select(static d => d.Id)]; string[] actualOrder = [.. body.Quests.Select(static row => row.QuestId)]; Assert.Equal(expectedOrder, actualOrder); // Reused expected literals in tests private static readonly string[] ExpectedSkillIds = ["intrusion", "refine", "salvage"]; Assert.Equal(ExpectedSkillIds, ids); // Avoid (ambiguous Assert overloads; unnecessary allocation helper) Assert.Equal(expectedOrder, [.. body.Quests.Select(static row => row.QuestId)]); return query.ToArray(); new[] { "activity", "mission_reward" } ``` ## Target-typed `new` (IDE0090) - When the return type or variable type is already explicit, prefer **`new()`** over repeating the type name in object initializers (common in test request helpers): ```csharp private static SkillProgressionGrantRequest Grant(...) => new() { SchemaVersion = schemaVersion, SkillId = skillId, }; ``` ## Async pass-through (CDT1003) - Do not mark a method **`async`** when it only **`await`**s and returns a single **`Task`** — return that task directly with an expression body or **`return`** statement. ## `using` / `await using` (IDE0063) - When a **`using` or `await using` block contains exactly one statement**, prefer the declaration form without braces: `await using var conn = …;` then the single statement on following lines **only if** it is still logically one resource scope (typical for `NpgsqlConnection` + one command in integration-test reset helpers). - **Keep braced `using (var …) { … }`** when the block has **multiple statements** (Arrange + Act + Assert inside one scope, seed loops, or any sequence that shares locals). - **Do not flatten** multi-statement blocks into `using var` at method scope when inner scopes reuse names like `store`, `registry`, or `scope` — that causes **CS0136** name clashes. Use distinct names (`seedStore`, `actScope`, `readStore`) or keep the braced block. ## Local `const` (RCS1118) - When a local is initialized from a **compile-time constant** (string literal, raw string literal, numeric literal, or `public const` field), declare it **`const`** instead of **`var`**. ```csharp const string questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; const string fixtureJson = """ { "schemaVersion": 1 } """; ``` ## Inline declarations (IDE0018 / IDE0059) - Prefer **inline initialization** over declare-then-assign when the analyzer suggests it, e.g. `var outcome = Operation(...)` and `TryGet(..., out var snapshot)` instead of separate upfront declarations. - Do **not** pre-initialize locals with **`default`**, **`null`**, or **`false`** when the variable is assigned in the next block before its first read (IDE0059) — declare without an initializer instead. ```csharp // Prefer — single disposable, single follow-on statement await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); // Prefer — multiple statements in one scope using (var firstScope = Factory.Services.CreateScope()) { var store = firstScope.ServiceProvider.GetRequiredService(); Assert.True(store.TryAppend(row)); } // Avoid — flattens to method scope and collides with a later `var store` using var firstScope = Factory.Services.CreateScope(); var store = firstScope.ServiceProvider.GetRequiredService(); Assert.True(store.TryAppend(row)); // … later … var store = secondScope.ServiceProvider.GetRequiredService(); // CS0136 ``` ## Postgres schema gates (IDE0330) - In `*Bootstrap.cs` types, use **`System.Threading.Lock`** (not `object`) for static schema-initialization gates: `private static readonly Lock SchemaGate = new();`. ## Concrete return types (CA1859) - When a private helper always returns a materialized array or list, prefer the **concrete type** as the return type (e.g. `List`, `T[]`, `Dictionary<,>`) instead of `IReadOnlyList` / `IReadOnlyDictionary<,>` when callers only need the materialized value and analyzers suggest it. - Same rule applies to **`BuildDefinitionsInIdOrder`**, **`MapSlots`**, **`ReadBindings`**, and similar helpers that always construct a new collection. ## Logging guards (CA1873) - Before **`LogInformation`**, **`LogDebug`**, or other structured logs whose arguments allocate (interpolated strings, exceptions, collection counts), guard with **`logger.IsEnabled(LogLevel.…)`** (or **`logger?.IsEnabled(...) is true`** when the logger is nullable). ```csharp if (logger.IsEnabled(LogLevel.Information)) { logger.LogInformation("Loaded catalog from {Path} with {Count} rows.", path, rows.Count); } ``` ## SQL parameter names (RCS1015) - When an ADO.NET / Npgsql parameter name matches a local variable, prefer **`nameof(variable)`** over a string literal: **`AddWithValue(nameof(xp), xp)`**. ## Arithmetic clarity (RCS1123) - In mixed products inside calls (e.g. **`Math.Sqrt((dx * dx) + (dz * dz))`**), parenthesize each product so precedence is explicit. - Prefer **expression-bodied** members only when they stay one clear idea. - **LINQ:** favor readability over micro-chains; break complex queries across lines. - **Exception handling:** catch specific exceptions; avoid empty `catch`; log or rethrow with context when appropriate. ## `Program.cs` and minimal APIs - Top-level statements and minimal APIs are fine for small apps; extract registration/build logic into extension methods or dedicated types when the file grows. ## Documentation - Use **`///` XML doc comments** on public APIs (types and members) when behavior or contracts are not obvious. - **Primary-constructor records:** include a **``** on the type. Document positional parameters with **``** inside the summary — **never** **`/// `** on **`record struct`** or **`record class`** primary constructors (Roslynator **RCS1263** false positive; [roslynator#1730](https://github.com/dotnet/roslynator/issues/1730)). - **Property / field docs:** every **`///`** block must include a **``** — do not ship **``**-only comments (RCS1139). Put extra detail in **``** after the summary when needed. ```csharp /// Deny reason when is false. /// Stable snake_case values; see README. public string? ReasonCode { get; init; } ``` ## Null checks (IDE0270) - Prefer **`?? throw`** over `if (x is null) { throw … }` when assigning a non-null local from a nullable expression. ```csharp var root = FindDockerComposeRoot() ?? throw new InvalidOperationException("…"); ``` ## Test project layout (`*.Tests`) - **Mirror the server project:** place test types under the same relative path as the production code they exercise (e.g. `NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` → `NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs`). - Use a namespace that matches the folder tree under the test assembly root, e.g. `NeonSprawl.Server.Tests.Game.PositionState` for files in `Game/PositionState/`. ## Test method naming convention - Use **`MethodName_ShouldExpectedOutcome_WhenScenario`** (three segments, **`PascalCase`** inside each segment, separated by **underscores**). - **`MethodName`:** the behavior or entry under test (SUT method, HTTP operation, or short feature verb)—e.g. `GetPosition`, `PostMove`, `TryApplyMoveTarget`. - **`ShouldExpectedOutcome`:** the outcome the test proves—e.g. `ShouldReturnNotFound`, `ShouldPersistTargetAndIncrementSequence`. - **`WhenScenario`:** the condition or inputs that trigger it—e.g. `WhenPlayerIsUnknown`, `WhenDevPlayerPostsValidMove`. - Omit redundant words when the scenario is already obvious; keep names readable in test runners and failure output. ```csharp // Examples public async Task GetPosition_ShouldReturnNotFound_WhenPlayerIsUnknown() { … } public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() { … } ``` ## Unit and integration tests (Arrange, Act, Assert) **Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (e.g. comments present but body read still in Act). Agents and humans must treat this as **merge-ready** layout, not a suggestion. ### Required layout 1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent. **No blank line is required** immediately after those comments; the first statement of each phase may follow on the next line. 2. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable). 3. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls. 4. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert. 5. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method. ### Authoring defaults - Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`**. ### Example (minimal HTTP integration) ```csharp [Fact] public async Task PostExample_ShouldReturnOk_WhenBodyValid() { // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); var request = new { schemaVersion = 1 }; // Act var response = await client.PostAsJsonAsync("/game/example", request); // Assert var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); } ``` ## Tooling - **`Directory.Build.props`**: **`TreatWarningsAsErrors`** — any compiler or analyzer **warning** fails **`dotnet build`**. - **`.editorconfig`**: **`IDE0005`** / **`CS8019`** (unnecessary usings) at **warning** severity — caught by **`dotnet format`**, not duplicated in per-file usings when covered by **`GlobalUsings.cs`** or implicit usings. - Before commit on touched **`server/**/*.cs`**: pre-commit runs **`scripts/verify-dotnet-format.sh`**; CI runs the same after restore. - Fix redundant usings locally: `dotnet format server/NeonSprawl.Server/NeonSprawl.Server.csproj --severity warn` (and Tests project when applicable). - Prefer fixes that satisfy built-in **.NET analyzers** / `dotnet format` rather than fighting IDE warnings without reason. - When adding **`server/NeonSprawl.Server.Tests/**/*.cs`**, read **`GlobalUsings.cs`** first — do not copy sibling **`using`** blocks blindly; only import namespaces used in that file.