Audit: inline /** @type {T} */ (v) casts - #1586
Merged
sergey-shandar merged 9 commits intoAug 15, 2026
Merged
Conversation
Enumerates all 357 non-`const` inline `@type` casts under `fjs/` and records, per site, whether AGENTS.md's alternatives apply: an annotated `const` declaration, `@satisfies`, or `assert`/`assertNotNullish`. Each site was probed against a clean `npx tsc` baseline twice — once with the cast deleted, once with `@type` rewritten to `@satisfies` — so the verdicts are measured rather than guessed. 181 casts are redundant outright, 27 only pin a type (so they can check instead of override), 72 have a runtime check available, and 77 are load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 2be550f | Commit Preview URL Branch Preview URL |
Aug 15 2026, 03:47 PM |
… audit Two issues the cast audit surfaced but that are not about casts: `tsconfig-strict-flags.md` records what each commented-out checking flag costs today, measured one flag at a time against a clean tree. Four are free right now; `noUncheckedIndexedAccess` is the one with design value, since it turns silent index access into the `assertNotNullish` obligation AGENTS.md prefers. `eslint.md` covers what no `tsc` flag can express: banning inline `@type` casts, rejecting misspelled JSDoc tags (which compile clean today, the one place JSDoc is weaker than authored TypeScript), and the type-predicate rule. Weighs ESLint against an `fjs lint` subcommand, since all three rules are syntactic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
Works out the `fjs` route from eslint.md: the three rules that matter are
syntactic, so a token-stream scan is enough, and the tokenizer already keeps a
JSDoc block's leading `*` so `/** … */` is distinguishable without parsing.
A prototype of the inline-cast rule over all 260 `.mjs` files reproduces 221 of
221 `@type {const}` sites and 353 of 357 inline casts found by the audit — but
also 24,166 error tokens across 251 files. The tokenizer accepts neither
single-quoted strings nor template literals, the two forms the repository is
written in. That also explains the misses: `'/*'` inside a single-quoted string
opens a phantom block comment that swallows the following cast, which is why
three of the four missed sites are in the tokenizer's own module.
Completing the tokenizer is therefore step one, and worth doing on its own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
Matching `@type\s*\{` against comment text is a hack. A JSDoc block is a
language — a tag sequence, brace-delimited type expressions in a TypeScript
subset, inline `{@link}`, free text — and it belongs in the transducer stack
`fjs/bnf/todo/layered-parser.md` already describes, as one more layer above the
JS tokenizer. Even the inline-cast rule needs it: exempting `@type {const}`
means knowing the type body *is* the identifier `const`, not that it contains
the word.
Sizes the subset from what the tree actually writes: 3772 type bodies, led by
function types (1318), generics (1128), `readonly` (492) and array types (257),
with a small but real tail of conditional types, `infer`, `import('…')` and
intersections that a stunted subset would miss.
The unknown-tag rule already has two live targets, both silently ignored by
TypeScript: `@result` on `fromArrayLike` where `@returns` was meant, so its
declared return type does not exist; and `@remark` for `@remarks`.
Renames fjs-lint.md to jsdoc-parser.md, since the parser is the substance and
the lint rules are the smallest useful thing to build on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
…grammar The previous revision proposed a grammar for a subset of TypeScript's type expressions. That is the superset FunctionalScript exists to avoid, rebuilt in the repository's own BNF. Under `/*: expr */` there is no type grammar at all: a type is a value from `fjs/types/rtti`, an annotation is an ordinary expression, and the existing parser already reads it. The tokenizer keeps a block comment's body verbatim, so `': myType '`, `'* @type {X} '` and `' plain '` differ in their first character — which is what lets the two annotation forms coexist while the tree migrates. Records what is already built: `validate`/`parse`, the canonical `data` form with `subset` (assignability as a decidable operation), and `rtti/ts`, which is already a printer from schemas to canonical TypeScript aliases — the `.d.ts` generator half. And the open questions: compile-time staging of annotation expressions; inferring an RTTI for a non-literal right-hand side so `subset` has two operands; and function types, which `Type` has no case for even though 1318 of the tree's 3772 JSDoc type bodies are function types. Renames jsdoc-parser.md accordingly and marks the lint rules transitional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
`/*: type */` + RTTI is a direction, not a plan: it needs the compiler's own parser to see the annotation and the compiler to evaluate a module at compile time (`fjs/fsc/todo/47.md`). Dropped to P5 and marked gated, with its near-term proposal steps rewritten as a sketch and the dependencies made explicit. Until it arrives, TypeScript and the standard toolchain are the checker. Adds `todo/strict-static-analysis.md` as the umbrella. The asymmetry it names: CI holds Rust to `clippy -- -D warnings` and `fmt --check` across nine targets, while the JavaScript half runs `tsc` and the test suites and nothing else — no lint, no formatting check, no unused-code check, no package-correctness check. Surveys what each candidate catches that the others do not, and notes that `deno.json` already configures `fmt` and both Deno and Bun are already on the runners, so two of the checks cost no setup at all. Also records that CI is generated from `fjs/ci/`, so a new check is a change there, not to the YAML. Reframes eslint.md accordingly: ESLint is the near-term answer because it is the only route to type-aware rules, and `fjs lint` waits with the compiler. Adds the tokenizer gap as its own module-scoped issue: it accepts neither single-quoted strings nor template literals, the two forms this repository is written in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
`spec/todo/2460-js-string-literals.md` already covers single-quoted strings, and covers them better: JSON string syntax across JSON ⊂ DJS ⊂ FS is a design decision with a stated rationale, not an omission, and the documented workaround is to normalize `'x'` to `"x"` rather than to extend the grammar. The new issue also had the framing backwards — the tokenizer rejecting this repository's `.mjs` sources is those sources not yet being FunctionalScript, which migrate-typescript-to-mjs.md stage 3 covers, not a tokenizer defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
`2460-js-string-literals` excludes them explicitly — a substitution embeds an expression, so this is FJS-level sugar (§3.4) rather than lexical sugar at the DJS level — but nothing tracked them afterwards. Records the semantics as concatenation, and three questions to settle before implementing: whether substitutions follow ECMAScript's implicit `ToString` (which is the coercion FunctionalScript avoids elsewhere) or must already be strings; whether tagged templates are in scope at all; and how a substitution-free template stays distinguishable from the JSON string it denotes, which content addressing needs. Notes the migration cost for the same reason 2460 does: 202 of the 260 `.mjs` files use template literals, with 494 substitutions between them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
It is a language feature the `fjs` parser does not recognize yet, which is what spec/todo is for — so it drops the todo/ issue header for spec-draft style, sits next to 3370-type-inference in §3.3, and is registered in the spec README. Placing it there also puts the contrast with the TC39 Type Annotations proposal (§4.1) in view: that one is erasable syntax with no checker, this one is a comment plus a checker that is an ordinary library. Two of its open questions turned out to have homes already. Function types are `fjs/types/rtti/todo/668-rtti-function-types.md`, which independently reached the same conclusion — a function can be checked as callable, but its contract is only observable when called — so the question left here is narrower: what an annotation on a function should mean. Inference is 3370, now cross-linked both ways. Nominal types genuinely have no issue, and that is now stated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
o2alexanderfedin
approved these changes
Aug 15, 2026
o2alexanderfedin
left a comment
Contributor
There was a problem hiding this comment.
Re-derived the audit mechanically rather than sampling it, and it holds.
- Table integrity: a balanced-brace scan of
fjs/finds exactly 357 non-constinline@typecasts and exactly 221@type {const}casts — both cited figures. All 357 rows match a real cast by file, line and type text (no unmatched rows, no orphans). Verdict tallies 181/21/6/72/77 sum to 357 and agree with the summary table and the 208/58% claim. - "remove" bucket proven: deleting all 172 redundant casts outside the two rtti visitors leaves
npx tsc(7.0.2) clean; the 9 rtti sites are each individually redundant and the group removal trips exactlyTS2589, as documented. - Sub-findings proven: all 37 tokenizer casts delete clean; switching the 8
Dirguards toinstanceof Arraylets all 8 casts delete withtscgreen; 22+10+8 matches the claimed 40-of-72 MCP/CAS split; exactly one pre-existing@satisfies. - Spot-checks: ~20 sites probed both ways —
@satisfiescandidates pass as@satisfiesbut fail deletion;assert/keepsites fail both with the documented codes (TS7022 atdescent:199, TS2589 atprotocol/mcp:167, TS2322 for the literal-range sites). tsconfig-strict-flags.md: every measured number reproduces exactly (0/0/0/0, 8, 31, 202, 209 = 130 TS6196 + 79 TS6133).@templatecount 169 exact; template-literal counts exact;deno.jsonfmt settings quoted correctly.
npx tsc clean; npm test → 2797 pass / 0 fail. All relative links in the 9 changed docs resolve, and the spec/README.md renumbering is consistent with the NNNN-slug.md convention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR documents a comprehensive audit of inline JSDoc type casts (
/** @type {T} */ (expr)) across thefjs/codebase, as called for in AGENTS.md.Summary
A mechanical enumeration and analysis of all 357 inline
@typecasts (excluding 221@type {const}casts which are out of scope) was performed against TypeScript 7.0.2 to determine which casts are:@satisfiesor annotated declarations insteadassertNotNullish,assert(typeof ...), etc.)Key Findings
@satisfiesto check instead of overrideconstdeclarationsassertNotNullish,assert(...), etc.)anybridges, generic erasure, nominal branding, TS2589 depth limits, or genuine API mismatchesResult: 208 of 357 casts (58%) need no cast and no replacement machinery—181 simply delete, 27 become checks instead of overrides. Another 72 have real runtime checks available. Only 77 are genuinely necessary.
Notable Sub-findings
fjs/js/tokenizer/module.f.mjs: 37 of 39 casts are redundant; the_CreateToToken<…>casts are already contextually typed throughcreate(def)(…).Array.isArraynarrowing issue: EightDircasts sit after guards usingArray.isArray, which never narrowsreadonly T[]out of a union. Swapping toinstanceof Arraymakes the existing checks narrow correctly and deletes all eight casts.unknownvalues: 40 of the 72assertcandidates are unchecked claims about JSON-RPC response shapes. A small set ofassert-based accessors or rttivalidatewould replace all of them.fjs/asn.1/module.f.mjs:68andfjs/types/function/compare/module.f.mjs:12,17narrow arithmetic results to literal unions;assert(i === 0 || i === 1 || i === 2)narrows identically and checks.Proposed Implementation Order
Array.isArray→instanceof Arrayat the eightDirsites and delete those casts.@satisfies/ annotated declarations.assert-based accessors for the MCP/CAS proofs and convert the 72assertcandidates.Deliverable
This PR adds
todo/inline-type-casts.md, a detailed audit document with:https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih