|
| 1 | +--- |
| 2 | +name: style-review |
| 3 | +description: Review changed or staged C# code against the docs-builder coding standards and flag violations. Use when the user asks to review code style, check for standards violations, or audit a diff before committing. |
| 4 | +--- |
| 5 | + |
| 6 | +# Style Review Skill |
| 7 | + |
| 8 | +Reviews C# changes for violations that `dotnet format` and `.editorconfig` cannot catch — behavioral patterns, naming conventions, and architectural rules. |
| 9 | + |
| 10 | +## What this skill does NOT check |
| 11 | + |
| 12 | +`.editorconfig` + `dotnet format` already enforce (mechanically): |
| 13 | +- `var` everywhere, no explicit types |
| 14 | +- No `this.` qualifier |
| 15 | +- Language keywords (`string` not `String`) |
| 16 | +- Expression bodies |
| 17 | +- Allman braces, newlines before `{` |
| 18 | +- Null propagation (`?.`, `??`) |
| 19 | +- Pattern matching over `is`/`as` casts |
| 20 | +- Inlined variable declarations (`out var x`) |
| 21 | +- Throw expressions, conditional delegate calls |
| 22 | +- Modifier order |
| 23 | +- File-scoped namespaces |
| 24 | +- Object/collection initializers |
| 25 | +- `[]` instead of `new List<T>()` / `Array.Empty<T>()` |
| 26 | + |
| 27 | +Run `dotnet format` (or `/lint`) to fix those. Only continue with this skill for the behavioral checks below. |
| 28 | + |
| 29 | +## Steps |
| 30 | + |
| 31 | +### 1. Get the diff |
| 32 | + |
| 33 | +```bash |
| 34 | +git diff # unstaged changes |
| 35 | +git diff --staged # staged changes |
| 36 | +git diff main...HEAD # full branch |
| 37 | +``` |
| 38 | + |
| 39 | +### 2. Check behavioral rules |
| 40 | + |
| 41 | +**Async correctness** (error-level): |
| 42 | +- Public async methods must end in `Async`; private async methods must NOT |
| 43 | +- Never `.Result` or `.Wait()` on an async call |
| 44 | +- Library code must use `.ConfigureAwait(false)` on awaits |
| 45 | +- All async methods should accept `CancellationToken ct = default` |
| 46 | + |
| 47 | +**Method design** (error-level): |
| 48 | +- Max 4 parameters — flag anything over that without a record/options object |
| 49 | +- Boolean parameters must use named argument syntax at every call site |
| 50 | +- Cyclomatic complexity > 7 branches in one method — flag and suggest extraction |
| 51 | + |
| 52 | +**Structural** (error-level): |
| 53 | +- No `#region` directives |
| 54 | +- Class member order violated: fields → constructors → properties → methods (grouped by function, not visibility) |
| 55 | + |
| 56 | +**Return values** (error-level): |
| 57 | +- Methods returning collections must never return `null` — must return `[]` |
| 58 | +- Lookups should use the TryGet pattern, not null returns |
| 59 | + |
| 60 | +**Testing** (style-level, in `tests/` and `tests-integration/`): |
| 61 | +- New test projects should use TUnit + AwesomeAssertions, not xUnit directly |
| 62 | +- Test method naming: `MethodName_Scenario_Expected` |
| 63 | +- No assertions without fluent style (`Should().Be(...)` not `Assert.Equal(...)`) |
| 64 | + |
| 65 | +**Comments** (style-level): |
| 66 | +- Comments that describe *what* the code does (not *why*) — flag for removal |
| 67 | +- Multi-paragraph docstrings on non-public members |
| 68 | +- `#region` (already error-level above) |
| 69 | + |
| 70 | +### 3. Report |
| 71 | + |
| 72 | +Format violations as a numbered list with severity: |
| 73 | + |
| 74 | +``` |
| 75 | +ERROR: |
| 76 | +1. src/Elastic.Markdown/Foo.cs:42 — public async method `GetData` missing `Async` suffix |
| 77 | +2. src/Elastic.Markdown/Foo.cs:67 — `.Result` blocks on async call; use `await` |
| 78 | +
|
| 79 | +STYLE: |
| 80 | +3. tests/Elastic.Markdown.Tests/Bar.cs:15 — test name `TestGetUser` should follow `GetUser_ValidId_ReturnsUser` pattern |
| 81 | +``` |
| 82 | + |
| 83 | +If no violations: "No behavioral style violations found." |
| 84 | + |
| 85 | +### 4. Do NOT auto-fix |
| 86 | + |
| 87 | +Report only. For formatting issues, run `/lint`. For logic violations (blocking async, null collections), the developer must fix manually. |
0 commit comments