Fix function/constructor/enum checks that relied on walking the "first declaration"#2788
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Go checker to avoid “check only on first declaration” patterns that break under concurrent checking, ensuring function/constructor/enum validation runs exactly once per symbol per checker (similar to the approach taken in #2134). It also re-enables the previously skipped concurrent-checking test case by accepting new baselines.
Changes:
- Replace “first declaration” gating for function/constructor checks with a per-checker
ValueSymbolLinksguard (functionOrConstructorChecked). - Replace “first declaration” gating for enum checks with a per-checker
DeclaredTypeLinksguard (enumChecked). - Unskip
controlFlowFunctionLikeCircular1.tsand add/update baselines accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/checker/checker.go | Removes first-declaration gating and introduces per-checker “checked once” guards for function/constructor symbols and enums. |
| internal/checker/types.go | Extends symbol link structs with new boolean guards used by the checker. |
| internal/testrunner/compiler_runner.go | Removes controlFlowFunctionLikeCircular1.ts from the concurrent-checking skip list. |
| testdata/baselines/reference/submodule/conformance/templateInsideCallback.errors.txt | Updates expected tsgo diagnostics output for the conformance/jsdoc test. |
| testdata/baselines/reference/submodule/conformance/templateInsideCallback.errors.txt.diff | Updates the tracked diff vs the TS submodule baseline (shows remaining divergence). |
| testdata/baselines/reference/submodule/compiler/controlFlowFunctionLikeCircular1.* | Adds/updates baselines now that the test is no longer skipped. |
| templateInsideCallback.js(23,5): error TS8039: A JSDoc '@template' tag may not follow a '@typedef', '@callback', or '@overload' tag | ||
| +templateInsideCallback.js(29,5): error TS2394: This overload signature is not compatible with its implementation signature. | ||
| +templateInsideCallback.js(29,5): error TS7012: This overload implicitly returns the type 'any' because it lacks a return type annotation. |
There was a problem hiding this comment.
The new TS2394 entry is marked with a leading '+' in this .diff baseline, which indicates tsgo is now emitting an extra diagnostic that the TypeScript submodule baseline does not. Please double-check upstream behavior for this test; if TS doesn’t report TS2394 here, this likely represents a regression (and the fix should avoid emitting it). If the mismatch is intentional/acceptable, consider moving this diff to the submoduleAccepted baselines so we’re explicitly tracking it as an accepted divergence.
controlFlowFunctionLikeCircular1was failing in concurrent checking for the same reasons we needed #2134.This PR applies the same fix to the other places where we checked the "first declaration" of something in order to ensure it was only checked once.
Not 100% sure which links struct to use, so I let copilot choose. 😅