|
| 1 | +--- |
| 2 | +applyTo: |
| 3 | + - "src/Compiler/**/*.{fs,fsi}" |
| 4 | + - "vsintegration/src/**/*.{fs,fsi}" |
| 5 | + - "tests/FSharp.Compiler.ComponentTests/**/*.fs" |
| 6 | + - "tests/FSharp.Compiler.Service.Tests/**/*.fs" |
| 7 | + - "vsintegration/tests/**/*.fs" |
| 8 | +--- |
| 9 | + |
| 10 | +# Code Style: No Bloat |
| 11 | + |
| 12 | +Reviewers read code, not prose. Add bytes only when they pay for themselves. |
| 13 | + |
| 14 | +## Comments |
| 15 | + |
| 16 | +Good names **always** beat comments. Before writing a comment, ask: *can I rename a value, extract a function, or use an active pattern so the comment becomes unnecessary?* If yes, do that instead. |
| 17 | + |
| 18 | +- **Do not** restate what the code says (variable name, type name, attribute name, function signature). |
| 19 | +- **Do not** narrate the algorithm step-by-step. The diff is the algorithm. |
| 20 | +- **Do not** justify design decisions inline ("we chose X over Y because…"). Put rationale in the commit message or PR body. |
| 21 | +- **Do not** leave war-story comments ("previously we did Z, but…", "counter-example: …"). The history is in `git log`. |
| 22 | +- **Do not** write multi-line `///` doc comments for internal helpers whose body is one expression. |
| 23 | + |
| 24 | +Acceptable comments answer **why**, not **what**, and only when the *why* is non-obvious and cannot be expressed by renaming: |
| 25 | +- Workarounds for compiler/runtime bugs (link the bug). |
| 26 | +- Performance constraints invisible from the code shape ("inner loop runs 50M times per typecheck"). |
| 27 | +- Cross-file invariants the code itself can't enforce. |
| 28 | + |
| 29 | +If you are tempted to write `// This is intentional`, change the code so the intent is structural, not decorative. |
| 30 | + |
| 31 | +## Code shape |
| 32 | + |
| 33 | +- Compact, idiomatic F#: pattern matching over `if`/`elif` ladders; active patterns where they remove duplication. |
| 34 | +- Low cyclomatic complexity per function. Extract helpers — even one-line ones — when a name clarifies a step. |
| 35 | +- Prefer module-level `let` over big bodies with nested locals. |
| 36 | +- New file > bloating an existing 5000-line file when adding a self-contained concept. |
| 37 | + |
| 38 | +## Test code |
| 39 | + |
| 40 | +Tests get a touch more leeway for explanation, but the same rules largely apply: |
| 41 | + |
| 42 | +- One parametrized test (`[<Theory>]`, `[<InlineData>]`, or a `for/yield` over inputs) > five copy-pasted tests. |
| 43 | +- Module-level constants for shared paths (`Path.Combine` for OS neutrality), shared source strings, and shared expected outputs. |
| 44 | +- Helpers like `parseAndCheck code` over reinventing the setup per test. |
| 45 | +- Don't reinvent an entire `.fs` file inside each test when one shared module-level binding will do. |
| 46 | + |
| 47 | +## PR scope |
| 48 | + |
| 49 | +**Not paid by LOC.** Large PRs are typically shitty PRs. If the diff has 1000+ lines, split it. |
| 50 | +- Cleanup commits separate from feature commits. |
| 51 | +- No "phase tag" / "transitional measure" / "follow-up" comments left behind — either do it now in a follow-up commit, or file an issue. Don't leave breadcrumbs in the code. |
| 52 | + |
| 53 | +## When in doubt |
| 54 | + |
| 55 | +Delete the comment, rename the value, and re-read. If the code is still unclear, *that* is what needs fixing — not the comment. |
0 commit comments