Reapply Program diagnostic func update, CheckerPool cleanup

[jakebailey]

This reapplies #2197 after it was reverted in #2250.

This includes a few changes over #2197.

• Actually check concurrently. Oops.
  • Caveat; each file gets its own goroutine. I measure no difference, and checkers are only locked on for the duration of their checks. Not needing to loop over the checkers is IMO a good simplification and means we have the same flow for both the CLI and for the LS.
• Make DiagnosticsCollection concurrency safe, and return copies of diags. See: Fix perfromance when there's lots of error #2283 (comment)
• Remove the code that orchestrates multi-file checking in checkers themselves. The callers do this; nothing was actually asking the checker to check all files. Now there is no exported CheckSourceFile method.
  • This technically also means we don't need to store the file list in the checker anymore. That cleanup is not in this PR. It's sort of inconsequential given we have to give those to the checker anyway for other reasons.
• Move SkipTypeChecking to a Program method, now that the callers are simplified. We were doubly checking this for every file. Oops.
  • IsSourceFromProjectReference goes away on checker.Program.
• Remove Count and Files from CheckerPool (as I did ForEachCheckerParallel before). These are only used in the CLI compilation, and they have been deleted out of the LSP checker pool.

There are further refactors I haven't done that should be possible in the future:

• CheckerPool does not really need GetTypeChecker anymore. The only callers of this are ~mistakenly not asking for the checker for a given file. Thankfully, these are LS users where the request-scoped checker system avoids problems. It's not a hard to fix this but it is somewhat noisy. Might stick in this PR but the ported code is a bit wonky (my fault in many of these cases...).
• In this cleanup, I've discovered that maxCheckers on project.CheckerPool does nothing. Not sure what to make of that but it seems scary.
• And of course, the change that makes us able to use differing checkers for diags and LS features, now that there are way fewer code paths to implement.

[Copilot]

Pull request overview

This PR reapplies diagnostic function updates and checker pool cleanup after a previous revert. The key changes include implementing concurrent diagnostic collection, making DiagnosticsCollection thread-safe, moving SkipTypeChecking from checker utilities to a Program method, and simplifying the checker pool interface by removing methods only needed for CLI compilation.

Key Changes

• Concurrent diagnostic collection with per-file goroutines using the new collectDiagnostics helper
• Thread-safe DiagnosticsCollection with mutex protection and defensive copying of returned slices
• Simplified checker orchestration by removing multi-file checking logic from checkers and moving it to Program callers

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/testrunner/compiler_runner.go	Removed ctx parameter from ForEachCheckerParallel call to match updated signature
internal/project/checkerpool.go	Removed Files, Count, ForEachCheckerParallel methods; simplified GetCheckerForFileExclusive to delegate to GetCheckerForFile
internal/execute/incremental/program.go	Renamed method from GetSemanticDiagnosticsNoFilter to GetSemanticDiagnosticsWithoutNoEmitFiltering for clarity
internal/execute/incremental/affectedfileshandler.go	Updated call site to use Program.SkipTypeChecking instead of checker.SkipTypeChecking
internal/compiler/program.go	Added collectDiagnostics helper for concurrent collection; moved SkipTypeChecking and canIncludeBindAndCheckDiagnostics to Program; simplified diagnostic methods; removed getDiagnosticsHelper
internal/compiler/checkerpool.go	Removed Count, Files, ForEachCheckerParallel from interface; made forEachCheckerParallel private; improved GetCheckerForFileExclusive implementation
internal/checker/utilities.go	Removed SkipTypeChecking and canIncludeBindAndCheckDiagnostics as they moved to Program
internal/checker/checker_test.go	Removed TestCheckSrcCompiler test that relied on removed CheckSourceFiles method
internal/checker/checker.go	Removed IsSourceFromProjectReference from Program interface; removed CheckSourceFile method; simplified getDiagnostics to always operate on a single file
internal/ast/diagnostic.go	Added mutex for thread safety; methods now return defensive copies; added early equality checks in comparison functions for optimization

Comments suppressed due to low confidence (2)

internal/compiler/checkerpool.go:78

• Loop variable i is captured by the closure and will have an unexpected value when the goroutines execute. In Go, loop variables are shared across iterations, so all goroutines may end up using the final value of i.

Fix by capturing the variable explicitly in the loop:

for i := range checkerCount {
	i := i  // Capture loop variable
	wg.Queue(func() {
		p.checkers[i], p.locks[i] = checker.NewChecker(p.program)
	})
}

		for i := range checkerCount {
			wg.Queue(func() {
				p.checkers[i], p.locks[i] = checker.NewChecker(p.program)
			})
		}

internal/compiler/checkerpool.go:100

• Loop variables idx and checker are captured by the closure and will have unexpected values when the goroutines execute. In Go, loop variables are shared across iterations, so all goroutines may end up using the final values of these variables.

Fix by capturing the variables explicitly in the loop:

for idx, checker := range p.checkers {
	idx, checker := idx, checker  // Capture loop variables
	wg.Queue(func() {
		p.locks[idx].Lock()
		defer p.locks[idx].Unlock()
		cb(idx, checker)
	})
}

	for idx, checker := range p.checkers {
		wg.Queue(func() {
			p.locks[idx].Lock()
			defer p.locks[idx].Unlock()
			cb(idx, checker)
		})
	}

[DanielRosenwasser]

While we're here, I wonder if you're open to renaming this filesWithSortedDiagnostics or something

[DanielRosenwasser]

Wow that's funny - so every full test run was checking TypeScript/src/compiler?

[jakebailey]

Yeah but it was quick.

[DanielRosenwasser]

Why the interface and struct implementation if we're always going to cast to a concrete instance in these methods?

[jakebailey]

Because this is also provided by the language server, which doesn't provide these methods.