Restore tsgo exit code for noEmit type errors#4407
Conversation
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
|
@copilot this seems totally random and hacky and not at all related to what the old compiler did, and the test case is not good as it's not a tsc test rethink this entirely |
Port tsc's handleNoEmitOptions in Program.Emit: a whole-program noEmit is not skipped (only a single-file emit is), so a program with diagnostics reports DiagnosticsPresent_OutputsGenerated, matching tsc and the process exit code 2 that older tsgo releases produced. Revert the earlier cmd/tsgo exit-code mapping hack and its test; add a tsc command-line regression test instead. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Rethought in cc8ea69. The root cause is the |
| ExitStatus:: DiagnosticsPresent_OutputsGenerated | ||
| Output:: | ||
| [96ma.ts[0m:[93m1[0m:[93m7[0m - [91merror[0m[90m TS2322: [0mType 'string' is not assignable to type 'number'. | ||
|
|
||
| [7m1[0m const a: number = "hello" | ||
| [7m [0m [91m ~[0m | ||
|
|
||
|
|
||
| Found 1 error in a.ts[90m:1[0m | ||
|
|
||
|
|
There was a problem hiding this comment.
@copilot Isn't this strange? What files were generated?
There was a problem hiding this comment.
It's strange, but it matches tsc: no files are generated. DiagnosticsPresent_OutputsGenerated is just a misnomer here.
For a whole-program --noEmit, tsc's handleNoEmitOptions (with sourceFile === undefined) returns program.emitBuildInfo(...). In a non-incremental program there's no buildInfo path, so emitFiles writes nothing and emitBuildInfo bails out early without setting emitSkipped, which is initialized to false. So the result is emitSkipped: false with zero emitted files, and emitFilesAndReportErrorsAndGetExitStatus falls into the diagnostics.length > 0 branch -> DiagnosticsPresent_OutputsGenerated (exit code 2).
The corresponding upstream baseline shows the same thing: no *new* files, yet exitCode:: ExitStatus.DiagnosticsPresent_OutputsGenerated (_submodules/TypeScript/tests/baselines/reference/tsc/noEmit/multiFile/semantic-errors.js:54). The single-file case is the only one tsc treats as skipped (emitSkippedWithNoDiagnostics), which is why the port returns EmitSkipped: options.TargetSourceFile != nil.
There was a problem hiding this comment.
Pull request overview
Restores tsgo’s exit-code behavior for --noEmit runs with diagnostics by aligning Program.Emit’s EmitSkipped semantics with upstream tsc (whole-program noEmit should not count as “emit skipped”, preserving exit status DiagnosticsPresent_OutputsGenerated).
Changes:
- Short-circuit
Program.EmitfornoEmitand setEmitSkippedbased on whether a target source file was specified. - Add a command-line regression test scenario for
--noEmitwith a type error. - Update
tscbaseline outputs (notablyExitStatusand trace emit spans) to match the corrected behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/compiler/program.go | Implements noEmit short-circuit in Program.Emit, setting EmitSkipped to match tsc’s single-file vs whole-program behavior. |
| internal/execute/tsctests/tsc_test.go | Adds a new command-line test case covering --noEmit with a type error. |
| testdata/baselines/reference/tsc/commandLine/noEmit-with-type-error.js | New baseline validating exit status DiagnosticsPresent_OutputsGenerated for tsgo --noEmit index.ts with a type error. |
| testdata/baselines/reference/tsc/noEmit/syntax-errors.js | Updates expected ExitStatus for --noEmit syntax-error scenarios to DiagnosticsPresent_OutputsGenerated. |
| testdata/baselines/reference/tsc/noEmit/semantic-errors.js | Updates expected ExitStatus for --noEmit semantic-error scenarios to DiagnosticsPresent_OutputsGenerated. |
| testdata/baselines/reference/tsc/noEmit/dts-errors.js | Updates expected ExitStatus for --noEmit declaration-related error scenarios to DiagnosticsPresent_OutputsGenerated. |
| testdata/baselines/reference/tsc/generateTrace/generateTrace-with-multiple-files-and-complex-types.js | Updates expected ExitStatus and trace output to reflect the corrected noEmit/emit-skipped behavior. |
| testdata/baselines/reference/tsc/generateTrace/generateTrace-generates-types-file.js | Updates trace output to reflect the corrected noEmit/emit-skipped behavior. |
tsgo --noEmitstarted returning process exit code1for type errors after the internalExitStatusvalues changed.tscand previoustsgoreleases return2for the same diagnostic path.Analysis
The exit code derives from the
ExitStatus:DiagnosticsPresent_OutputsSkipped(exit1) vsDiagnosticsPresent_OutputsGenerated(exit2). The status logic ininternal/execute/tsc/emit.goalready mirrors tsc (emitSkipped && diagnostics-> OutputsSkipped, else OutputsGenerated), so the bug was the wrongEmitSkippedvalue. tsgo's per-file emitters setEmitSkipped = truefornoEmit, but tsc'shandleNoEmitOptionsonly treats a single-file noEmit as skipped; a whole-program noEmit reportsemitSkipped: false.Fix
Port
handleNoEmitOptionsintoProgram.Emit: short-circuitnoEmitbefore the per-file emitters and returnEmitSkipped: options.TargetSourceFile != nil(false for a whole-program emit, true for a single file). This restoresDiagnosticsPresent_OutputsGeneratedfortsgo --noEmitand converges thetsc/noEmitbaselines with upstream.The change is scoped to the non-incremental path that the reported scenario uses; the shared incremental path (also used by build mode, whose status is derived differently upstream) is left unchanged.
Regression coverage
Reverted the earlier
cmd/tsgoexit-code mapping hack and its non-tsc test, and added a tsc command-line regression test (noEmit with type error).