|
| 1 | +--- |
| 2 | +name: strict-typing-luau |
| 3 | +description: Convert a Nevermore Luau file from --!nonstrict (or untyped/--!nocheck) to --!strict, adding the project's explicit type annotations and fixing every type error the checker reports. Use this whenever the user asks to "strictly type", "add types to", "make strict", "type-annotate", "convert to --!strict", or clean up the typing on a .lua/.luau file in this repo — including when they just select a file with a `--!nonstrict` header and say "type this" or point you at a legacy module. Also use it when a strict file is throwing luau-lsp type errors and the user wants them resolved following Nevermore conventions. |
| 4 | +--- |
| 5 | + |
| 6 | +# Strictly typing Luau files |
| 7 | + |
| 8 | +Convert a file to `--!strict` and make it pass the type checker cleanly. This is mostly |
| 9 | +**mechanical pattern application** — move fast and correctly. The checker is your fast |
| 10 | +feedback loop; lean on it instead of guessing. |
| 11 | + |
| 12 | +## Triage first — match effort to the file |
| 13 | + |
| 14 | +- **Plain util / data module** (`local X = {}` of functions, or a `return {...}` table; no |
| 15 | + `setmetatable`): flip the header, give every function typed params/returns, run single-file |
| 16 | + analyze. Usually no `export type` block needed. |
| 17 | +- **Class** (`setmetatable({}, ...)` + constructor + methods): the real work — needs the |
| 18 | + `export type` block. Follow the patterns below and `references/conventions.md`. |
| 19 | +- **Already `--!strict` but erroring**: skip conversion; just resolve the reported errors. |
| 20 | + |
| 21 | +## Why it's not find-and-replace |
| 22 | + |
| 23 | +Luau can't infer fields through `setmetatable`, so strict mode turns that blind spot into |
| 24 | +errors. The fix is an explicit `export type` block enumerating every `self` field, plus |
| 25 | +dot-syntax methods that name `self`. Flipping the header without these just produces a wall |
| 26 | +of errors — supplying the types the checker can't infer *is* the job. |
| 27 | + |
| 28 | +## The verification loop (fast inner loop) |
| 29 | + |
| 30 | +`luau-lsp analyze` is the same engine as `lint:luau`, pointed at one file (~2.5s). One-time |
| 31 | +setup if `sourcemap.json` / `globalTypes.d.lua` are missing at repo root: `npm run prelint:luau`. |
| 32 | + |
| 33 | +```bash |
| 34 | +luau-lsp analyze --sourcemap=sourcemap.json --base-luaurc=.luaurc \ |
| 35 | + --defs=globalTypes.d.lua --flag:LuauSolverV2=false --ignore='**/node_modules/**' \ |
| 36 | + src/<package>/src/<Realm>/<File>.lua |
| 37 | +``` |
| 38 | + |
| 39 | +Clean = only the `[INFO] Loading...` line. `LuauSolverV2=false` is required (repo pins the old |
| 40 | +solver). Iterate until clean, then run `npm run lint:luau` **once** as the final gate — single-file |
| 41 | +analyze can't see files that depend on *yours*, and tightening a type ripples to subclasses and |
| 42 | +callers. Triage new downstream errors: pre-existing → leave & flag; your type is genuinely too |
| 43 | +tight for the real contract → loosen *your* type (often `T?` not `T`); a small obvious follow-on |
| 44 | +→ fix it. |
| 45 | + |
| 46 | +## Core patterns |
| 47 | + |
| 48 | +**Class with a parent (the common case):** |
| 49 | + |
| 50 | +```lua |
| 51 | +--!strict |
| 52 | +local require = require(script.Parent.loader).load(script) |
| 53 | + |
| 54 | +local BaseObject = require("BaseObject") |
| 55 | + |
| 56 | +local MyClass = setmetatable({}, BaseObject) |
| 57 | +MyClass.ClassName = "MyClass" |
| 58 | +MyClass.__index = MyClass |
| 59 | + |
| 60 | +export type MyClass = typeof(setmetatable( |
| 61 | + {} :: { |
| 62 | + _serviceBag: ServiceBag.ServiceBag, -- EVERY self field, with its type |
| 63 | + _enabled: ValueObject.ValueObject<boolean>, |
| 64 | + }, |
| 65 | + {} :: typeof({ __index = MyClass }) |
| 66 | +)) & BaseObject.BaseObject -- intersection pulls in inherited _maid, _obj, etc. |
| 67 | + |
| 68 | +function MyClass.new(serviceBag: ServiceBag.ServiceBag): MyClass |
| 69 | + local self: MyClass = setmetatable(BaseObject.new() :: any, MyClass) |
| 70 | + self._serviceBag = assert(serviceBag, "No serviceBag") |
| 71 | + return self |
| 72 | +end |
| 73 | +``` |
| 74 | + |
| 75 | +**Methods use dot syntax with explicit `self`** (colon syntax loses the `self` type in strict |
| 76 | +mode). Callers still write `obj:Method()`; only the definition changes: |
| 77 | + |
| 78 | +```lua |
| 79 | +function MyClass.GetEnabled(self: MyClass): boolean |
| 80 | + return self._enabled.Value |
| 81 | +end |
| 82 | +``` |
| 83 | + |
| 84 | +## The export type rule — always `typeof(setmetatable(...))`, never hand-list methods |
| 85 | + |
| 86 | +There is **one** way to write a class's export type, and it holds for generic, inherited, and |
| 87 | +dynamic-`__index` classes alike: |
| 88 | + |
| 89 | +```lua |
| 90 | +export type MyClass = typeof(setmetatable( |
| 91 | + {} :: { ...only the instance FIELDS... }, |
| 92 | + {} :: typeof({ __index = MyClass }) |
| 93 | +)) [& Parent.Parent] |
| 94 | +``` |
| 95 | + |
| 96 | +Methods come from `typeof({ __index = MyClass })` — never hand-write `Method: (self, ...) -> ...` |
| 97 | +records (they drift and cost enormous churn on big classes). Inheritance: list only the child's |
| 98 | +*own new* fields; inherited ones arrive via `& Parent.Parent`. Generics: keep `<T>` load-bearing |
| 99 | +by putting it in a field (e.g. a virtual `Value: T`); never collapse `<T>` to `any`. The one tax: |
| 100 | +a metatable'd type needs `self :: any` for dynamic self-access (`rawget(self :: any, k)`). |
| 101 | + |
| 102 | +## When `:: any` is acceptable |
| 103 | + |
| 104 | +Confined to boundaries the checker genuinely can't model: |
| 105 | + |
| 106 | +- `setmetatable(ParentClass.new(...) :: any, MyClass)` — the metatable transform |
| 107 | +- `Binder.new("Tag", MyClass :: any) :: Binder.Binder<MyClass>` — binder registration |
| 108 | +- `local t: any = require("t")` — `t` (and the rare library like it) isn't strict-friendly |
| 109 | +- **Rx / reactive chains** (`:Pipe({...})`, `switchMap`/`map` closures, `RxSignal`, `Signal.new()`) |
| 110 | + — the #1 time-sink. Don't try to thread types through them; cast to `any` on the *first* analyze |
| 111 | + error and move on, keeping the public return type precise. See `references/rx.md`. |
| 112 | + |
| 113 | +**Cast the chain body, never the public return type.** Type every public method's return precisely |
| 114 | +(`Promise.Promise<T>`, `Observable.Observable<T>`); only the internal *expression* producing it may |
| 115 | +be `any`. Casting the return type itself is the single biggest source of avoidable looseness — a |
| 116 | +`GetX(self): any` leaks `any` to every caller. See the Rx/Promise examples in `references/conventions.md`. |
| 117 | + |
| 118 | +Reaching for `:: any` inside a method body means the `export type` block is wrong — fix it there. |
| 119 | +A call-site `(x :: any):Method()` to dodge a *signature mismatch* is a smell: usually the upstream |
| 120 | +signature is too strict (a param that should be optional) — fix it upstream if in scope, else flag |
| 121 | +it. Don't bury a real type bug under a cast. |
| 122 | + |
| 123 | +## Hard cases — try the precise type first, loosen the narrowest spot only |
| 124 | + |
| 125 | +Concrete metatable types (`Signal.Signal<T>`, `ValueObject.ValueObject<T>`, `Maid.Maid`) import |
| 126 | +and check cleanly almost always — write them out. Only these justify deviating: |
| 127 | + |
| 128 | +1. **A type you cannot import** (sibling is still nonstrict, exports no type) → write a **precise |
| 129 | + structural interface** of the exact surface you use (real signatures, real field types) — this |
| 130 | + is typing by another means, not a loosening. Better, if cheap: add the `export type` upstream. |
| 131 | +2. **A heavy cyclic generic** the old solver can't hold (a deep class wrapped across many |
| 132 | + members) → type each occurrence as `any` with JUST the intended type in a trailing comment: |
| 133 | + `_indexObservers: any, -- ObservableSubscriptionTable<T?>`. Find the one culprit type and sweep |
| 134 | + **all** its occurrences in one pass (fields, params, **returns**), don't bisect. |
| 135 | +3. **Require cycle** between a class and its definition/factory → hoist shared shapes into a |
| 136 | + types-only `<Package>Types.lua` (requires nothing at runtime, so importing it can't recreate the |
| 137 | + cycle). Keep the public API precise; only the internal back-reference may give. |
| 138 | + |
| 139 | +Time-box it: if a file won't go clean within ~2 iterations on the *same* cyclic/"too complex" |
| 140 | +error, take the escape for that member and move on. The public API must stay precise; internal |
| 141 | +plumbing can absorb imprecision. |
| 142 | + |
| 143 | +## Comment discipline |
| 144 | + |
| 145 | +**Default: write loose `any`s BARE — no explanatory comment.** Write `_cmdrService: any,` — NOT |
| 146 | +`_cmdrService: any, -- CmdrServiceClient (nonstrict, no exported type)`. This holds for *every* `any` |
| 147 | +whose source module simply exports no type yet (the common case). The comment reads as "blessed, |
| 148 | +intentional" and discourages the next person from tightening it — and we *want* these `any`s gone. A |
| 149 | +bare `any` is honest debt; a commented one looks finished. Don't explain yourself. |
| 150 | + |
| 151 | +Only two exceptions earn a comment: |
| 152 | +- **A forced cyclic-complexity `any`** (Hard case #2) records its intended type so it's recoverable: |
| 153 | + `_indexObservers: any, -- ObservableSubscriptionTable<T?>`. The comment is *information*, not an apology. |
| 154 | +- **A non-obvious deliberate structure** (e.g. a structural type dodging a require cycle) gets a line |
| 155 | + so no one "fixes" it back into the cycle. |
| 156 | + |
| 157 | +If you're tempted to comment an `any` for any other reason — don't. |
| 158 | + |
| 159 | +## Report your hand-offs |
| 160 | + |
| 161 | +End with a short **"Hand-offs"** list: every spot where you took an escape instead of a precise |
| 162 | +type — file, member, the `any`/structural fallback, and the intended type. Skip routine sanctioned |
| 163 | +boundaries (metatable casts, `t: any`, binder registration). This is the deliverable that lets the |
| 164 | +user tighten later; a run that loosened things silently is not done. |
| 165 | + |
| 166 | +## Pitfalls |
| 167 | + |
| 168 | +- **`--!strict` must be the file's FIRST line** — before the `--[=[ ]=]` docstring. Luau only honors |
| 169 | + a mode directive at the very top; placed after the docstring it's silently ignored and the file is |
| 170 | + never strict-checked (analyze looks "clean" but isn't). |
| 171 | +- Don't leave `--!nonstrict`/`--!nocheck` "for safety" — the point is `--!strict`. |
| 172 | +- Don't run repo-wide `lint:luau` on every edit — single-file analyze is the inner loop, `lint:luau` |
| 173 | + is the final gate. |
| 174 | +- Don't invent fields — the `export type` block must list exactly what `self` gets. |
| 175 | +- Keep legacy/deprecation docstrings; typing doesn't change deprecation status. |
| 176 | + |
| 177 | +## Tooling |
| 178 | + |
| 179 | +- `references/conventions.md` — full canonical examples + a common-error → fix table. |
| 180 | + `references/rx.md` — the Rx/reactive escape in detail. |
| 181 | +- **Converting a whole package?** Don't eyeball the order. Run the planner for a |
| 182 | + dependency-ordered, model-routed conversion plan — which files, in what order, and which form |
| 183 | + cyclic clusters to convert together: |
| 184 | + |
| 185 | + ```bash |
| 186 | + # from the repo root (the script cd's to the repo top itself, so CWD doesn't matter): |
| 187 | + .claude/skills/strict-typing-luau/evals/lib/run.sh plan src/<package> # e.g. src/settings |
| 188 | + ``` |
| 189 | + |
| 190 | + It defaults to **real mode** (targets the files that aren't `--!strict` yet); convert leaves-first |
| 191 | + in the order it prints. (`--eval-gold` switches it to the harness's gold-bearing view.) |
| 192 | +- **Parallelize at scale.** Files in the same dependency layer of the plan are independent — they |
| 193 | + don't require each other, so they can convert concurrently. When a package has **more than ~3 |
| 194 | + files to convert**, fan out **one sub-agent per file within a layer** (launch them in a single |
| 195 | + message so they run in parallel), wait for the layer, then move to the next. Each sub-agent |
| 196 | + converts ONE file against its already-converted deps. This makes wall-clock ≈ the slowest file per |
| 197 | + layer instead of the sum, and keeps each context small. At ≤3 files, just convert sequentially — |
| 198 | + the fan-out overhead isn't worth it. |
| 199 | +- That same **`run.sh`** with no arguments lists every command. `plan` is the one generally useful |
| 200 | + for real conversions; the rest (`gold`, `convert`, `place`/`score`/`restore`, `triggers`) *evaluate* |
| 201 | + the skill — see `evals/README.md` for those workflows. |
0 commit comments