typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Reapply Program diagnostic func update, CheckerPool cleanup

Open jakebailey opened this issue 2 weeks ago • 0 comments

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: https://github.com/microsoft/typescript-go/pull/2283#issuecomment-3635213002
  • 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.

jakebailey avatar Dec 10 '25 05:12 jakebailey