marco/IsoMmo
Watch
1
0
Fork
You've already forked IsoMmo
 0
Code Issues 42 Pull requests 6 Projects 1 Releases 6 Packages 2 Wiki Activity Actions

Linting & formatting: adopt a modern .NET analyzer/formatter stack #20

New issue
Closed
opened 2026-07-17 11:14:47 +02:00 by marco · 1 comment
marco commented 2026-07-17 11:14:47 +02:00
Owner
Copy link

Goal

Adopt modern, community-standard linting/formatting for the whole .NET solution, enforced consistently (locally + CI). We already run TreatWarningsAsErrors=true via Directory.Build.props — this issue layers real analyzers + a formatter on top so style/quality issues are caught mechanically, not in review.

The modern accepted stack (proposal)

  • .editorconfig at the repo root — the standard, tool-agnostic config; Roslyn analyzers and dotnet format both honor it. Single source of truth for style.
  • Built-in .NET analyzers — EnableNETAnalyzers=true + AnalysisMode (Recommended→All) in Directory.Build.props. Zero extra deps, official.
  • dotnet format — official formatter; CI gate via dotnet format --verify-no-changes. Local just format / just lint.
  • One community analyzer package (pick one, see decisions):
    • Roslynator — huge analyzer + refactoring set, very popular, low noise.
    • StyleCop.Analyzers — the classic style enforcer (ordering, naming, layout) — stricter/nosier.
    • SonarAnalyzer.CSharp — bug/code-smell focused.
    • Meziantou.Analyzer — pragmatic quality rules.
  • Optional: CSharpier — opinionated Prettier-style formatter (removes all formatting debate) vs. staying with dotnet format + .editorconfig.

All of the above are pure .NET/NuGet and cross-platform — no OS lock-in (respects the multi-platform hard requirement).

Hidden complexity / risks

  • We already fail the build on warnings. Turning on a broad analyzer set at high severity will likely surface many findings that instantly become build errors. Needs a phased rollout: enable, triage, fix (or explicitly suppress with justification in .editorconfig), then ratchet severity up — not a big-bang flip.
  • StyleCop is opinionated (member ordering, mandatory XML docs) and can clash with our "comments only for non-obvious constraints" rule — curate the ruleset, don't take defaults.
  • Analyzers add some build time.
  • Keep config in one place (.editorconfig + Directory.Build.props) per our "config in exactly one place" rule.

Scope

  • Root .editorconfig encoding our C# style (file-scoped namespaces, sealed default, collection expressions, naming, etc.).
  • Enable built-in analyzers + chosen community analyzer(s) solution-wide via Directory.Build.props.
  • just format (apply) and just lint (verify) recipes.
  • Phased triage: fix existing findings or suppress-with-reason; land with zero warnings.
  • CI check that fails on unformatted code / analyzer violations (depends on CI existing — see open decisions).
  • Document the workflow in CLAUDE.md.

Definition of Done

Base DoD applies on top (tests green, whole solution builds, zero warnings, multi-platform preserved).

  • dotnet format --verify-no-changes passes on a clean checkout of the whole solution.
  • Analyzers are enabled solution-wide from a single config; a deliberately-introduced violation (e.g. wrong ordering / unused using) fails the build, demonstrated.
  • just lint reproduces the CI check locally; just format fixes formatting.
  • Every suppressed rule has a one-line justification in .editorconfig.

Open decisions

  1. Which community analyzer(s): Roslynator (recommended, low-noise) vs StyleCop vs Sonar vs Meziantou — one or a combo?
  2. CSharpier (opinionated formatter) or stay with dotnet format + .editorconfig?
  3. CI: is there a pipeline to hook into yet, or add a minimal one (GitHub Actions-style / Forgejo Actions) as part of this?
  4. Initial AnalysisMode: Recommended (ease in) vs All (strict from day one, more upfront fixing)?
## Goal Adopt modern, community-standard linting/formatting for the whole .NET solution, enforced consistently (locally + CI). We already run `TreatWarningsAsErrors=true` via `Directory.Build.props` — this issue layers real analyzers + a formatter on top so style/quality issues are caught mechanically, not in review. ## The modern accepted stack (proposal) - **`.editorconfig`** at the repo root — the standard, tool-agnostic config; Roslyn analyzers and `dotnet format` both honor it. Single source of truth for style. - **Built-in .NET analyzers** — `EnableNETAnalyzers=true` + `AnalysisMode` (Recommended→All) in `Directory.Build.props`. Zero extra deps, official. - **`dotnet format`** — official formatter; CI gate via `dotnet format --verify-no-changes`. Local `just format` / `just lint`. - **One community analyzer package** (pick one, see decisions): - **Roslynator** — huge analyzer + refactoring set, very popular, low noise. - **StyleCop.Analyzers** — the classic style enforcer (ordering, naming, layout) — stricter/nosier. - **SonarAnalyzer.CSharp** — bug/code-smell focused. - **Meziantou.Analyzer** — pragmatic quality rules. - Optional: **CSharpier** — opinionated Prettier-style formatter (removes all formatting debate) vs. staying with `dotnet format` + `.editorconfig`. All of the above are pure .NET/NuGet and cross-platform — no OS lock-in (respects the multi-platform hard requirement). ## Hidden complexity / risks - **We already fail the build on warnings.** Turning on a broad analyzer set at high severity will likely surface many findings that instantly become build errors. Needs a **phased rollout**: enable, triage, fix (or explicitly suppress with justification in `.editorconfig`), then ratchet severity up — not a big-bang flip. - **StyleCop is opinionated** (member ordering, mandatory XML docs) and can clash with our "comments only for non-obvious constraints" rule — curate the ruleset, don't take defaults. - Analyzers add some build time. - Keep config in **one place** (`.editorconfig` + `Directory.Build.props`) per our "config in exactly one place" rule. ## Scope - [ ] Root `.editorconfig` encoding our C# style (file-scoped namespaces, `sealed` default, collection expressions, naming, etc.). - [ ] Enable built-in analyzers + chosen community analyzer(s) solution-wide via `Directory.Build.props`. - [ ] `just format` (apply) and `just lint` (verify) recipes. - [ ] Phased triage: fix existing findings or suppress-with-reason; land with zero warnings. - [ ] CI check that fails on unformatted code / analyzer violations (depends on CI existing — see open decisions). - [ ] Document the workflow in CLAUDE.md. ## Definition of Done _Base DoD applies on top (tests green, whole solution builds, zero warnings, multi-platform preserved)._ - [ ] `dotnet format --verify-no-changes` passes on a clean checkout of the whole solution. - [ ] Analyzers are enabled solution-wide from a single config; a deliberately-introduced violation (e.g. wrong ordering / unused using) fails the build, demonstrated. - [ ] `just lint` reproduces the CI check locally; `just format` fixes formatting. - [ ] Every suppressed rule has a one-line justification in `.editorconfig`. ## Open decisions 1. Which community analyzer(s): Roslynator (recommended, low-noise) vs StyleCop vs Sonar vs Meziantou — one or a combo? 2. CSharpier (opinionated formatter) or stay with `dotnet format` + `.editorconfig`? 3. CI: is there a pipeline to hook into yet, or add a minimal one (GitHub Actions-style / Forgejo Actions) as part of this? 4. Initial `AnalysisMode`: Recommended (ease in) vs All (strict from day one, more upfront fixing)?
marco commented 2026-07-17 12:42:39 +02:00
Author
Owner
Copy link

Implemented on branch chore/linting.

Stack: CSharpier (formatter, local dotnet tool) + built-in .NET analyzers (AnalysisMode=Recommended) + Roslynator.Analyzers + SonarAnalyzer.CSharp, all enforced at build under the existing TreatWarningsAsErrors. Tool 'dotnet-ef' (version '10.0.10') was restored. Available commands: dotnet-ef
Tool 'csharpier' (version '1.3.0') was restored. Available commands: csharpier

Restore was successful.
Formatted 127 files in 248ms. (CSharpier) and Tool 'dotnet-ef' (version '10.0.10') was restored. Available commands: dotnet-ef
Tool 'csharpier' (version '1.3.0') was restored. Available commands: csharpier

Restore was successful.
Checked 127 files in 225ms.
Determining projects to restore...
All projects are up-to-date for restore.
IsoMmo.Assets -> /Users/marco/Development/IsoMmo/src/IsoMmo.Assets/bin/Debug/net10.0/IsoMmo.Assets.dll
IsoMmo.Shared -> /Users/marco/Development/IsoMmo/src/IsoMmo.Shared/bin/Debug/net10.0/IsoMmo.Shared.dll
IsoMmo.Shared -> /Users/marco/Development/IsoMmo/src/IsoMmo.Shared/bin/Debug/net8.0/IsoMmo.Shared.dll
IsoMmo.Assets.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Assets.Tests/bin/Debug/net10.0/IsoMmo.Assets.Tests.dll
IsoMmo.Shared.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Shared.Tests/bin/Debug/net10.0/IsoMmo.Shared.Tests.dll
IsoMmo.Client.Core -> /Users/marco/Development/IsoMmo/client/IsoMmo.Client.Core/bin/Debug/net10.0/IsoMmo.Client.Core.dll
IsoMmo.AssetEditor -> /Users/marco/Development/IsoMmo/tools/IsoMmo.AssetEditor/bin/Debug/net10.0/IsoMmo.AssetEditor.dll
IsoMmo.AssetExtractor -> /Users/marco/Development/IsoMmo/tools/IsoMmo.AssetExtractor/bin/Debug/net10.0/IsoMmo.AssetExtractor.dll
IsoMmo.AssetExtractor.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.AssetExtractor.Tests/bin/Debug/net10.0/IsoMmo.AssetExtractor.Tests.dll
IsoMmo.Auth -> /Users/marco/Development/IsoMmo/src/IsoMmo.Auth/bin/Debug/net10.0/IsoMmo.Auth.dll
IsoMmo.Client.Core.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Client.Core.Tests/bin/Debug/net10.0/IsoMmo.Client.Core.Tests.dll
IsoMmo.Client -> /Users/marco/Development/IsoMmo/client/IsoMmo.Client/bin/Debug/net10.0/IsoMmo.Client.dll
IsoMmo.GameServer -> /Users/marco/Development/IsoMmo/src/IsoMmo.GameServer/bin/Debug/net10.0/IsoMmo.GameServer.dll
IsoMmo.Auth.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Auth.Tests/bin/Debug/net10.0/IsoMmo.Auth.Tests.dll
IsoMmo.GameServer.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.GameServer.Tests/bin/Debug/net10.0/IsoMmo.GameServer.Tests.dll

Build succeeded.
0 Warning(s)
0 Error(s)

Time Elapsed 00:00:02.53 (CSharpier check + analyzer build) are the gates; CI hookup deferred to a later issue.

Phased, reasoned policy (root .editorconfig): ~380 initial findings triaged by judgment, not blanket-ignored.

  • Fixed (real issues): AuthApi now uses one shared static HttpClient (no owned disposable, avoids socket exhaustion); WebSocketConnection is IDisposable and disposed via (SemaphoreSlim); GameServerIntegrationTests sealed + GC.SuppressFinalize; removed a dead field; added a missing test assertion; renamed an EF override param.
  • Disabled with a specific reason (documented in .editorconfig): CA1707 (xUnit test-name underscores — tests only), CA1848/CA1873 (LoggerMessage ceremony not warranted at our scale), CA1716 (C#-only), S1075 (dev localhost/JWT defaults — #13), S2245 (Random is gameplay not security), S1118 (top-level Program partial can't be static), S1694 (BaseAi is a deliberate base class).
  • Left as ratchet-later suggestions: CA1305/CA1310, S6966, S3267, S2365, S1244, S1643, S927, S125, S127.
  • EF migrations marked generated_code.

Build green with warnings-as-errors, 161 tests pass. AnalysisMode can be raised (Recommended→All) and suggestions promoted to warnings as we ratchet up.

Implemented on branch chore/linting. **Stack**: CSharpier (formatter, local dotnet tool) + built-in .NET analyzers (AnalysisMode=Recommended) + Roslynator.Analyzers + SonarAnalyzer.CSharp, all enforced at build under the existing TreatWarningsAsErrors. Tool 'dotnet-ef' (version '10.0.10') was restored. Available commands: dotnet-ef Tool 'csharpier' (version '1.3.0') was restored. Available commands: csharpier Restore was successful. Formatted 127 files in 248ms. (CSharpier) and Tool 'dotnet-ef' (version '10.0.10') was restored. Available commands: dotnet-ef Tool 'csharpier' (version '1.3.0') was restored. Available commands: csharpier Restore was successful. Checked 127 files in 225ms. Determining projects to restore... All projects are up-to-date for restore. IsoMmo.Assets -> /Users/marco/Development/IsoMmo/src/IsoMmo.Assets/bin/Debug/net10.0/IsoMmo.Assets.dll IsoMmo.Shared -> /Users/marco/Development/IsoMmo/src/IsoMmo.Shared/bin/Debug/net10.0/IsoMmo.Shared.dll IsoMmo.Shared -> /Users/marco/Development/IsoMmo/src/IsoMmo.Shared/bin/Debug/net8.0/IsoMmo.Shared.dll IsoMmo.Assets.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Assets.Tests/bin/Debug/net10.0/IsoMmo.Assets.Tests.dll IsoMmo.Shared.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Shared.Tests/bin/Debug/net10.0/IsoMmo.Shared.Tests.dll IsoMmo.Client.Core -> /Users/marco/Development/IsoMmo/client/IsoMmo.Client.Core/bin/Debug/net10.0/IsoMmo.Client.Core.dll IsoMmo.AssetEditor -> /Users/marco/Development/IsoMmo/tools/IsoMmo.AssetEditor/bin/Debug/net10.0/IsoMmo.AssetEditor.dll IsoMmo.AssetExtractor -> /Users/marco/Development/IsoMmo/tools/IsoMmo.AssetExtractor/bin/Debug/net10.0/IsoMmo.AssetExtractor.dll IsoMmo.AssetExtractor.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.AssetExtractor.Tests/bin/Debug/net10.0/IsoMmo.AssetExtractor.Tests.dll IsoMmo.Auth -> /Users/marco/Development/IsoMmo/src/IsoMmo.Auth/bin/Debug/net10.0/IsoMmo.Auth.dll IsoMmo.Client.Core.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Client.Core.Tests/bin/Debug/net10.0/IsoMmo.Client.Core.Tests.dll IsoMmo.Client -> /Users/marco/Development/IsoMmo/client/IsoMmo.Client/bin/Debug/net10.0/IsoMmo.Client.dll IsoMmo.GameServer -> /Users/marco/Development/IsoMmo/src/IsoMmo.GameServer/bin/Debug/net10.0/IsoMmo.GameServer.dll IsoMmo.Auth.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.Auth.Tests/bin/Debug/net10.0/IsoMmo.Auth.Tests.dll IsoMmo.GameServer.Tests -> /Users/marco/Development/IsoMmo/tests/IsoMmo.GameServer.Tests/bin/Debug/net10.0/IsoMmo.GameServer.Tests.dll Build succeeded. 0 Warning(s) 0 Error(s) Time Elapsed 00:00:02.53 (CSharpier check + analyzer build) are the gates; CI hookup deferred to a later issue. **Phased, reasoned policy** (root .editorconfig): ~380 initial findings triaged by judgment, not blanket-ignored. - **Fixed (real issues)**: AuthApi now uses one shared static HttpClient (no owned disposable, avoids socket exhaustion); WebSocketConnection is IDisposable and disposed via (SemaphoreSlim); GameServerIntegrationTests sealed + GC.SuppressFinalize; removed a dead field; added a missing test assertion; renamed an EF override param. - **Disabled with a specific reason** (documented in .editorconfig): CA1707 (xUnit test-name underscores — tests only), CA1848/CA1873 (LoggerMessage ceremony not warranted at our scale), CA1716 (C#-only), S1075 (dev localhost/JWT defaults — #13), S2245 (Random is gameplay not security), S1118 (top-level Program partial can't be static), S1694 (BaseAi is a deliberate base class). - **Left as ratchet-later suggestions**: CA1305/CA1310, S6966, S3267, S2365, S1244, S1643, S927, S125, S127. - EF migrations marked generated_code. Build green with warnings-as-errors, 161 tests pass. AnalysisMode can be raised (Recommended→All) and suggestions promoted to warnings as we ratchet up.
marco closed this issue 2026-07-17 12:44:24 +02:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
renovate/opentelemetry-dotnet-monorepo
renovate/sixlabors.imagesharp-4.x
renovate/sonaranalyzer.csharp-10.x
renovate/dotnet-monorepo
renovate/roslynator.analyzers-4.x
fix/blend-knows-a-bank
docs/rendering-open-problems
feat/shaped-cutaway
spike/confine-unwalkable-blend
fix/sprite-depth
design/3d-terrain
feat/p2a-solid-terrain
feat/relief-overlay-occlusion
feat/cliff-face-legibility
renovate/patch-dependencies
chore/sonaranalyzer-10.30
feat/server-compact-worldmap
feat/world-new-biomes
fix/biome-seams
feat/vitals-hud
fix/marsh-creature-parity
feat/biome-props
feat/world-terrain
0.2.1
0.2.0
0.1.5
0.1.4
0.1.2
v0.1.1
v0.1.0
0.1.3
Labels
Clear labels   
alpha:wave-0
Alpha — de-risk, do first

  
alpha:wave-1
Alpha — foundations

  
alpha:wave-2
Alpha — features

  
alpha:wave-3
Alpha — refine & stabilize

  
area:assets
Art, sound, the assets.isoa pack and its tools

  
area:combat
Combat system, weapons, balance

  
area:ecology
Natural nodes, harvesting, spawners

  
area:infra
CI, analyzers, dependencies, performance, ops

  
area:render
Client rendering: terrain mesh, shading, sprites, cutaway

  
area:scripting
Lua content scripting host and API

  
area:ui
Client UI, HUD, input, targeting

  
area:world
World model, map format, editor, topology

  
enhancement
New feature or improvement

  
epic
Milestone-level roadmap epic

  
migration
Platform or framework migration

  
post-alpha
Explicitly out of the alpha scope

  
roadmap
Roadmap tracker

  
tech-debt
Accepted debt to revisit

  
type:bug
Something is wrong and reproducible

  
type:chore
Housekeeping: no behaviour change

  
type:design
Design to agree before any implementation

No labels
alpha:wave-0
alpha:wave-1
alpha:wave-2
alpha:wave-3
area:assets
area:combat
area:ecology
area:infra
area:render
area:scripting
area:ui
area:world
enhancement
epic
migration
post-alpha
roadmap
tech-debt
type:bug
type:chore
type:design
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
marco/IsoMmo#20
No description provided.