Move unreachable checks to checker, allowing more AST reuse

[jakebailey]

They say it's not normal to have a "white whale", but this is one of mine...

---

Currently, we define these "source affecting compiler options":

type SourceFileAffectingCompilerOptions struct {
	AllowUnreachableCode     Tristate
	AllowUnusedLabels        Tristate
	BindInStrictMode         bool
	ShouldPreserveConstEnums bool
}

// ...
options.sourceFileAffectingCompilerOptions = SourceFileAffectingCompilerOptions{
	AllowUnreachableCode:     options.AllowUnreachableCode,
	AllowUnusedLabels:        options.AllowUnusedLabels,
	BindInStrictMode:         options.AlwaysStrict.IsTrue() || options.Strict.IsTrue(),
	ShouldPreserveConstEnums: options.ShouldPreserveConstEnums(),
}
// ...

This is a part of the input/cache key for ASTs. A difference in any of these options between projects means we cannot reuse the ASTs and therefore reparse files, which can have a memory impact for the language server, build mode, even our test suite. (Declaration files are exempt from this as none of these options matter for them; since #1158.)

We're already planning on making all files strict (microsoft/TypeScript#62213) meaning BindInStrictMode will go away, so that leaves three other options.

It turns out that all three of them have to do with unreachable code detection, including ShouldPreserveConstEnums.

After some futzing, I was able to move all of the checks the binder previously did into the checker.

The binder still does its usual unreachableFlow stuff, but also sets NodeFlagUnreachable on statements/declarations to be reported by the checker.

These diagnostics are still presented as suggestion diagnostics when needed, and appear to work for me, including in JS files with/without // @ts-check.

---

If we go through with the strict mode change, there will be no more source file affecting compiler options, leaving just this as the input to parse/bind (which are a pure function in Corsa):

type SourceFileParseOptions struct {
	FileName                       string
	Path                           tspath.Path
	ExternalModuleIndicatorOptions ExternalModuleIndicatorOptions
	JSDocParsingMode               JSDocParsingMode
}

Plus the text itself, and the ScriptKind.

JSDocParsingMode is stable, and ExternalModuleIndicatorOptions is effectively derived from the path, so this would be as minimal as it can get!

[Copilot]

Pull Request Overview

This PR refactors unreachable code and unused label detection by moving these checks from the binder to the checker, enabling better AST reuse between projects with different allowUnreachableCode, allowUnusedLabels, and preserveConstEnums settings. The binder now only sets flow nodes and tracks label usage, while the checker performs all diagnostic reporting.

Key changes:

• Removed unreachable/unused label options from SourceFileAffectingCompilerOptions
• Moved unreachable code detection logic from binder to checker
• Added IsReferenced field to LabeledStatement for the checker to use
• Updated test baselines to reflect changes in error reporting behavior

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/core/compileroptions.go	Removed AllowUnreachableCode, AllowUnusedLabels, and ShouldPreserveConstEnums from source file affecting options
internal/binder/binder.go	Removed unreachable code/label checking logic; now only sets flow nodes and tracks label references
internal/ast/ast.go	Added IsReferenced boolean field to LabeledStatement
internal/checker/checker.go	Added unreachable code and unused label detection logic from binder, with new reportedUnreachableStatements tracking
testdata/baselines/reference/submodule/conformance/neverReturningFunctions1.errors.txt	Updated baseline showing reduced error count and changed error ranges
testdata/baselines/reference/submodule/conformance/neverReturningFunctions1.errors.txt.diff	Diff of baseline changes

[jakebailey]

Alternatively, Statements() could return nil. I am unsure if that's a great idea.

[jakebailey]

In porting this back to Strada, I've found other bugs in this that are being silenced by the main test for unreachable code being all unreachable itself. Will figure that one out.

[jakebailey]

Okay, seems to be working as expected now. I forgot about the var x; carve-out.

I also made microsoft/TypeScript#62751 which helped me find that bug. Will be interested in those results.

[ahejlsberg]

This will be a nice improvement!

[jakebailey]

I'm still investigating these breaks: microsoft/TypeScript#62751 (comment)

[ahejlsberg]

One thing to keep in mind is that if we ever do region based semantic diagnostics (like we do in Strada), the logic in this PR may get the merged error spans wrong. We'd have to add some sort of backtracking do determine where a merged span starts.

[jakebailey]

Agh, yeah. I have this as a PR ported to Strada so I'm going to have to see if that's majorly broken ☹️

[jakebailey]

Alternatively, I could just issue errors at every unreachable statement, but that's probably too chatty...

[jakebailey]

One thing to keep in mind is that if we ever do region based semantic diagnostics (like we do in Strada), the logic in this PR may get the merged error spans wrong. We'd have to add some sort of backtracking do determine where a merged span starts.

I've added that code, commented out for now until we do region diags.

[jakebailey]

Not in this PR is the modification to the options declarations to not declare these as source affecting, but that code is not even used at the moment so I'm going to skip it for now.