Skip to content

Audit: inline /** @type {T} */ (v) casts - #1586

Merged
sergey-shandar merged 9 commits into
mainfrom
claude/cast-type-annotations-refactor-hq8l8o
Aug 15, 2026
Merged

Audit: inline /** @type {T} */ (v) casts#1586
sergey-shandar merged 9 commits into
mainfrom
claude/cast-type-annotations-refactor-hq8l8o

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

This PR documents a comprehensive audit of inline JSDoc type casts (/** @type {T} */ (expr)) across the fjs/ codebase, as called for in AGENTS.md.

Summary

A mechanical enumeration and analysis of all 357 inline @type casts (excluding 221 @type {const} casts which are out of scope) was performed against TypeScript 7.0.2 to determine which casts are:

  • Redundant and can be deleted outright
  • Checking casts that should use @satisfies or annotated declarations instead
  • Assertable via runtime checks (assertNotNullish, assert(typeof ...), etc.)
  • Load-bearing and must remain due to generic erasure, nominal branding, or genuine type/API mismatches

Key Findings

Category Count Action
Remove 181 Redundant casts; delete with no replacement
@Satisfies 21 Expression is assignable; use @satisfies to check instead of override
Declare 6 Hoist to annotated const declarations
Assert 72 Replace with runtime checks (assertNotNullish, assert(...), etc.)
Keep 77 Load-bearing: any bridges, generic erasure, nominal branding, TS2589 depth limits, or genuine API mismatches

Result: 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 through create(def)(…).
  • Array.isArray narrowing issue: Eight Dir casts sit after guards using Array.isArray, which never narrows readonly T[] out of a union. Swapping to instanceof Array makes the existing checks narrow correctly and deletes all eight casts.
  • MCP/CAS proof unknown values: 40 of the 72 assert candidates are unchecked claims about JSON-RPC response shapes. A small set of assert-based accessors or rtti validate would replace all of them.
  • Literal-range casts: fjs/asn.1/module.f.mjs:68 and fjs/types/function/compare/module.f.mjs:12,17 narrow arithmetic results to literal unions; assert(i === 0 || i === 1 || i === 2) narrows identically and checks.

Proposed Implementation Order

  1. Delete the 172 redundant casts outside the two rtti visitors; then take the remaining 9 one at a time (to avoid TS2589 depth limit).
  2. Swap Array.isArrayinstanceof Array at the eight Dir sites and delete those casts.
  3. Convert the 21 + 6 checking casts to @satisfies / annotated declarations.
  4. Introduce assert-based accessors for the MCP/CAS proofs and convert the 72 assert candidates.
  5. For the remaining 77, open follow-up issues per type/API problem rather than rewriting the cast.

Deliverable

This PR adds todo/inline-type-casts.md, a detailed audit document with:

  • Problem statement and methodology
  • Summary findings table
  • Sub-findings worth acting on first
  • Explanation of what genuinely must stay
  • Full table of all 357 sites with verdict and replacement guidance per site

https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih

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
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

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

claude added 8 commits August 15, 2026 14:54
… 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 o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-derived the audit mechanically rather than sampling it, and it holds.

  • Table integrity: a balanced-brace scan of fjs/ finds exactly 357 non-const inline @type casts 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 exactly TS2589, as documented.
  • Sub-findings proven: all 37 tokenizer casts delete clean; switching the 8 Dir guards to instanceof Array lets all 8 casts delete with tsc green; 22+10+8 matches the claimed 40-of-72 MCP/CAS split; exactly one pre-existing @satisfies.
  • Spot-checks: ~20 sites probed both ways — @satisfies candidates pass as @satisfies but fail deletion; assert/keep sites fail both with the documented codes (TS7022 at descent:199, TS2589 at protocol/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). @template count 169 exact; template-literal counts exact; deno.json fmt 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.

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 15, 2026
Merged via the queue into main with commit 28ad3b2 Aug 15, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants