ReviewOS

stacks/stx

typecheck: --lib silently suppresses every diagnostic (reports 0 errors on a deliberate type error)

#1906
Closed glennmichael123 opened this 11 days ago ·
11 days ago

v0.2.171. Passing --lib turns the checker off. It still reports the same block and expression counts, and exits 0, so it looks like a clean run.

Repro

Put an unambiguous type error in a <script server> block:

<script server>
const __sabotage: number = "definitely a string"
</script>
$ stx typecheck 'resources/views/pricing.stx'
  pricing.stx:13:7  error TS2322  Type 'string' is not assignable to type 'number'.  [<script server>]
  Checked 2 script block(s) and 10 template expression(s) in 1 file(s) — 4 error(s), 0 warning(s)

$ stx typecheck 'resources/views/pricing.stx' --lib types/session.d.ts
  Checked 2 script block(s) and 10 template expression(s) in 1 file(s) — 0 error(s), 0 warning(s)

Same file, same counts, exit 0. Every diagnostic is gone — not just ones relating to the lib.

Why this is worse than it looks

--lib is exactly the flag you reach for when the checker cannot see your app's types. On a real app (39 .stx files) the unwired run reported 220 errors, of which ~89 were TS2304 Cannot find name / TS2307 Cannot find module for the app's own SessionStore, AuthResponse and friends. The obvious fix is to point --lib at types/*.d.ts — and that turns 220 into 0.

The natural conclusion is "wired up correctly, codebase is clean." It is not; the checker stopped reporting. I nearly recommended exactly that as a CI gate.

Likely cause

This is the failure mode typecheck.ts already documents in its own comments — a syntax error in any file added to the program makes tsc abort and report nothing about user code, because skipLibCheck only suppresses semantic diagnostics:

stx.d.ts types them richly, but including it drags in the package's built declarations, and dist/composables.d.ts is currently syntactically invalid … tsc aborts on a SYNTAX error in a lib file … so the checker reported nothing at all about the user's code.

So --lib is presumably appending to the same files array and hitting the same class of abort — or the ambient globals file and the lib are colliding. Either way the symptom is identical: tsc exits without diagnostics and the parser sees no TSC_LINE_RE matches, which reads as success.

Suggested fix

  • Fail loudly instead of silently. If tsc produces no parseable diagnostic lines and exited non-zero, that is an abort, not a clean run — surface the raw output rather than printing 0 error(s).
  • Ideally check each --lib parses before adding it, and name the one that broke.

The distinction matters: "0 errors" and "the compiler refused to run" should never render the same way, especially for something intended as a CI gate.

Related: #1905 (codemod false positives, found in the same sweep).

11 days ago

Fixed in 1911fbc404. Your repro reports the deliberate type error again, with --lib and without.

Two defects, and the second is what made it silent.

The lib was never loaded. A relative --lib types/session.d.ts was resolved against the generated tsconfig, which lives in a temp directory — so tsc looked for .stx/typecheck/types/session.d.ts, found nothing, and aborted with TS6053. The most natural way to write the flag was the broken one. Paths now resolve against the caller's cwd.

tsc's exit code was never read. Your diagnosis was right, and it was the second half. An abort produces no line the parser recognises, and zero parsed diagnostics was reported as zero errors — the one outcome that cannot be told apart from success. TypecheckResult now carries a failure, and the CLI prints it and exits 2:

  Type-check did not run.

  a file passed with --lib has errors of its own, so nothing in your code was checked.
  A syntax error in an ambient declaration file does this — skipLibCheck suppresses
  SEMANTIC diagnostics only. tsc said:
  …

Getting the detection right took two attempts, and the first was wrong in an instructive way

My first condition was "tsc exited non-zero and no diagnostics survived filtering". That fires on a genuinely clean page: tsc can legitimately report errors in files this checker does not own — it did, in this repo's own packages/stx/src — all of which are discarded, and calling that "did not run" would make the checker cry wolf. Its own test caught it.

So it is two specific shapes instead:

  • tsc printed nothing positioned and exited non-zero (the missing lib; TS6053 carries no position), or
  • tsc printed diagnostics in a file we added while nothing of the user's survived (a broken lib taking the run down with it).

Both are pinned, along with the two negatives: a clean run and a run with real errors must not report failure.

The "I nearly recommended exactly that as a CI gate" line is the reason this went ahead of the other two in the batch. A gate that reports green because it did not run is worse than no gate.

Sign in to comment on this issue.