Skip to content

Commit 1d138ea

Browse files
jakebaileyjakebailey
authored
Revert "Make Program diagnostic API clearer (#2197)" (#2250)
1 parent f16a4b7 commit 1d138ea

File tree

7 files changed

+162
-73
lines changed

7 files changed

+162
-73
lines changed

‎internal/ast/diagnostic.go‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,11 @@ func getDiagnosticPath(d *Diagnostic) string {
195195
}
196196

197197
func EqualDiagnostics(d1, d2 *Diagnostic) bool {
198-
if d1 == d2 {
199-
return true
200-
}
201198
return EqualDiagnosticsNoRelatedInfo(d1, d2) &&
202199
slices.EqualFunc(d1.RelatedInformation(), d2.RelatedInformation(), EqualDiagnostics)
203200
}
204201

205202
func EqualDiagnosticsNoRelatedInfo(d1, d2 *Diagnostic) bool {
206-
if d1 == d2 {
207-
return true
208-
}
209203
return getDiagnosticPath(d1) == getDiagnosticPath(d2) &&
210204
d1.Loc() == d2.Loc() &&
211205
d1.Code() == d2.Code() &&
@@ -214,9 +208,6 @@ func EqualDiagnosticsNoRelatedInfo(d1, d2 *Diagnostic) bool {
214208
}
215209

216210
func equalMessageChain(c1, c2 *Diagnostic) bool {
217-
if c1 == c2 {
218-
return true
219-
}
220211
return c1.Code() == c2.Code() &&
221212
slices.Equal(c1.MessageArgs(), c2.MessageArgs()) &&
222213
slices.EqualFunc(c1.MessageChain(), c2.MessageChain(), equalMessageChain)
@@ -267,9 +258,6 @@ func compareRelatedInfo(r1, r2 []*Diagnostic) int {
267258
}
268259

269260
func CompareDiagnostics(d1, d2 *Diagnostic) int {
270-
if d1 == d2 {
271-
return 0
272-
}
273261
c := strings.Compare(getDiagnosticPath(d1), getDiagnosticPath(d2))
274262
if c != 0 {
275263
return c

‎internal/checker/checker_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,25 @@ foo.bar;`
6161
}
6262
}
6363

64+
func TestCheckSrcCompiler(t *testing.T) {
65+
t.Parallel()
66+
67+
repo.SkipIfNoTypeScriptSubmodule(t)
68+
fs := osvfs.FS()
69+
fs = bundled.WrapFS(fs)
70+
71+
rootPath := tspath.CombinePaths(tspath.NormalizeSlashes(repo.TypeScriptSubmodulePath), "src", "compiler")
72+
73+
host := compiler.NewCompilerHost(rootPath, fs, bundled.LibPath(), nil, nil)
74+
parsed, errors := tsoptions.GetParsedCommandLineOfConfigFile(tspath.CombinePaths(rootPath, "tsconfig.json"), &core.CompilerOptions{}, nil, host, nil)
75+
assert.Equal(t, len(errors), 0, "Expected no errors in parsed command line")
76+
p := compiler.NewProgram(compiler.ProgramOptions{
77+
Config: parsed,
78+
Host: host,
79+
})
80+
p.CheckSourceFiles(t.Context(), nil)
81+
}
82+
6483
func BenchmarkNewChecker(b *testing.B) {
6584
repo.SkipIfNoTypeScriptSubmodule(b)
6685
fs := osvfs.FS()

‎internal/compiler/checkerpool.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type CheckerPool interface {
1616
GetChecker(ctx context.Context) (*checker.Checker, func())
1717
GetCheckerForFile(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
1818
GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
19+
ForEachCheckerParallel(ctx context.Context, cb func(idx int, c *checker.Checker))
1920
Files(checker *checker.Checker) iter.Seq[*ast.SourceFile]
2021
}
2122

@@ -97,7 +98,7 @@ func (p *checkerPool) createCheckers() {
9798

9899
// Runs `cb` for each checker in the pool concurrently, locking and unlocking checker mutexes as it goes,
99100
// making it safe to call `ForEachCheckerParallel` from many threads simultaneously.
100-
func (p *checkerPool) ForEachCheckerParallel(cb func(idx int, c *checker.Checker)) {
101+
func (p *checkerPool) ForEachCheckerParallel(ctx context.Context, cb func(idx int, c *checker.Checker)) {
101102
p.createCheckers()
102103
wg := core.NewWorkGroup(p.program.SingleThreaded())
103104
for idx, checker := range p.checkers {

‎internal/compiler/program.go‎

Lines changed: 115 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -371,15 +371,23 @@ func (p *Program) BindSourceFiles() {
371371
wg.RunAndWait()
372372
}
373373

374+
func (p *Program) CheckSourceFiles(ctx context.Context, files []*ast.SourceFile) {
375+
p.checkerPool.ForEachCheckerParallel(ctx, func(_ int, checker *checker.Checker) {
376+
for file := range p.checkerPool.Files(checker) {
377+
if files == nil || slices.Contains(files, file) {
378+
checker.CheckSourceFile(ctx, file)
379+
}
380+
}
381+
})
382+
}
383+
374384
// Return the type checker associated with the program.
375385
func (p *Program) GetTypeChecker(ctx context.Context) (*checker.Checker, func()) {
376386
return p.checkerPool.GetChecker(ctx)
377387
}
378388

379-
func (p *Program) ForEachCheckerParallel(cb func(idx int, c *checker.Checker)) {
380-
if pool, ok := p.checkerPool.(*checkerPool); ok {
381-
pool.ForEachCheckerParallel(cb)
382-
}
389+
func (p *Program) ForEachCheckerParallel(ctx context.Context, cb func(idx int, c *checker.Checker)) {
390+
p.checkerPool.ForEachCheckerParallel(ctx, cb)
383391
}
384392

385393
// Return a checker for the given file. We may have multiple checkers in concurrent scenarios and this
@@ -417,59 +425,39 @@ func (p *Program) GetResolvedModules() map[tspath.Path]module.ModeAwareCache[*mo
417425
return p.resolvedModules
418426
}
419427

420-
// collectDiagnostics collects diagnostics from a single file or all files.
421-
// If sourceFile is non-nil, returns diagnostics for just that file.
422-
// If sourceFile is nil, returns diagnostics for all files in the program.
423-
func (p *Program) collectDiagnostics(ctx context.Context, sourceFile *ast.SourceFile, collect func(context.Context, *ast.SourceFile) []*ast.Diagnostic) []*ast.Diagnostic {
424-
var result []*ast.Diagnostic
425-
if sourceFile != nil {
426-
result = collect(ctx, sourceFile)
427-
} else {
428-
for _, file := range p.files {
429-
result = append(result, collect(ctx, file)...)
430-
}
431-
}
432-
return SortAndDeduplicateDiagnostics(result)
433-
}
434-
435428
func (p *Program) GetSyntacticDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
436-
return p.collectDiagnostics(ctx, sourceFile, func(_ context.Context, file *ast.SourceFile) []*ast.Diagnostic {
437-
return core.Concatenate(file.Diagnostics(), file.JSDiagnostics())
438-
})
429+
return p.getDiagnosticsHelper(ctx, sourceFile, false /*ensureBound*/, false /*ensureChecked*/, p.getSyntacticDiagnosticsForFile)
439430
}
440431

441432
func (p *Program) GetBindDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
442-
if sourceFile != nil {
443-
binder.BindSourceFile(sourceFile)
444-
} else {
445-
p.BindSourceFiles()
446-
}
447-
return p.collectDiagnostics(ctx, sourceFile, func(_ context.Context, file *ast.SourceFile) []*ast.Diagnostic {
448-
return file.BindDiagnostics()
449-
})
433+
return p.getDiagnosticsHelper(ctx, sourceFile, true /*ensureBound*/, false /*ensureChecked*/, p.getBindDiagnosticsForFile)
450434
}
451435

452436
func (p *Program) GetSemanticDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
453-
return p.collectDiagnostics(ctx, sourceFile, p.getSemanticDiagnosticsForFile)
437+
return p.getDiagnosticsHelper(ctx, sourceFile, true /*ensureBound*/, true /*ensureChecked*/, p.getSemanticDiagnosticsForFile)
454438
}
455439

456-
func (p *Program) GetSemanticDiagnosticsWithoutNoEmitFiltering(ctx context.Context, sourceFiles []*ast.SourceFile) map[*ast.SourceFile][]*ast.Diagnostic {
440+
func (p *Program) GetSemanticDiagnosticsNoFilter(ctx context.Context, sourceFiles []*ast.SourceFile) map[*ast.SourceFile][]*ast.Diagnostic {
441+
p.BindSourceFiles()
442+
p.CheckSourceFiles(ctx, sourceFiles)
443+
if ctx.Err() != nil {
444+
return nil
445+
}
457446
result := make(map[*ast.SourceFile][]*ast.Diagnostic, len(sourceFiles))
458447
for _, file := range sourceFiles {
459-
result[file] = SortAndDeduplicateDiagnostics(p.getBindAndCheckDiagnosticsForFile(ctx, file))
448+
result[file] = SortAndDeduplicateDiagnostics(p.getSemanticDiagnosticsForFileNotFilter(ctx, file))
460449
}
461450
return result
462451
}
463452

464453
func (p *Program) GetSuggestionDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
465-
return p.collectDiagnostics(ctx, sourceFile, p.getSuggestionDiagnosticsForFile)
454+
return p.getDiagnosticsHelper(ctx, sourceFile, true /*ensureBound*/, true /*ensureChecked*/, p.getSuggestionDiagnosticsForFile)
466455
}
467456

468457
func (p *Program) GetProgramDiagnostics() []*ast.Diagnostic {
469-
return SortAndDeduplicateDiagnostics(core.Concatenate(
458+
return SortAndDeduplicateDiagnostics(slices.Concat(
470459
p.programDiagnostics,
471-
p.includeProcessor.getDiagnostics(p).GetGlobalDiagnostics(),
472-
))
460+
p.includeProcessor.getDiagnostics(p).GetGlobalDiagnostics()))
473461
}
474462

475463
func (p *Program) GetIncludeProcessorDiagnostics(sourceFile *ast.SourceFile) []*ast.Diagnostic {
@@ -998,26 +986,40 @@ func (p *Program) GetGlobalDiagnostics(ctx context.Context) []*ast.Diagnostic {
998986
}
999987

1000988
globalDiagnostics := make([][]*ast.Diagnostic, p.checkerPool.Count())
1001-
p.ForEachCheckerParallel(func(idx int, checker *checker.Checker) {
989+
p.checkerPool.ForEachCheckerParallel(ctx, func(idx int, checker *checker.Checker) {
1002990
globalDiagnostics[idx] = checker.GetGlobalDiagnostics()
1003991
})
1004992

1005993
return SortAndDeduplicateDiagnostics(slices.Concat(globalDiagnostics...))
1006994
}
1007995

1008996
func (p *Program) GetDeclarationDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
1009-
return p.collectDiagnostics(ctx, sourceFile, p.getDeclarationDiagnosticsForFile)
997+
return p.getDiagnosticsHelper(ctx, sourceFile, true /*ensureBound*/, true /*ensureChecked*/, p.getDeclarationDiagnosticsForFile)
1010998
}
1011999

10121000
func (p *Program) GetOptionsDiagnostics(ctx context.Context) []*ast.Diagnostic {
1013-
return SortAndDeduplicateDiagnostics(core.Concatenate(p.GetGlobalDiagnostics(ctx), p.getOptionsDiagnosticsOfConfigFile()))
1001+
return SortAndDeduplicateDiagnostics(append(p.GetGlobalDiagnostics(ctx), p.getOptionsDiagnosticsOfConfigFile()...))
10141002
}
10151003

10161004
func (p *Program) getOptionsDiagnosticsOfConfigFile() []*ast.Diagnostic {
1005+
// todo update p.configParsingDiagnostics when updateAndGetProgramDiagnostics is implemented
10171006
if p.Options() == nil || p.Options().ConfigFilePath == "" {
10181007
return nil
10191008
}
1020-
return p.GetConfigFileParsingDiagnostics()
1009+
return p.GetConfigFileParsingDiagnostics() // TODO: actually call getDiagnosticsHelper on config path
1010+
}
1011+
1012+
func (p *Program) getSyntacticDiagnosticsForFile(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
1013+
return core.Concatenate(sourceFile.Diagnostics(), sourceFile.JSDiagnostics())
1014+
}
1015+
1016+
func (p *Program) getBindDiagnosticsForFile(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
1017+
// TODO: restore this; tsgo's main depends on this function binding all files for timing.
1018+
// if checker.SkipTypeChecking(sourceFile, p.compilerOptions) {
1019+
// return nil
1020+
// }
1021+
1022+
return sourceFile.BindDiagnostics()
10211023
}
10221024

10231025
func FilterNoEmitSemanticDiagnostics(diagnostics []*ast.Diagnostic, options *core.CompilerOptions) []*ast.Diagnostic {
@@ -1030,26 +1032,40 @@ func FilterNoEmitSemanticDiagnostics(diagnostics []*ast.Diagnostic, options *cor
10301032
}
10311033

10321034
func (p *Program) getSemanticDiagnosticsForFile(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
1033-
return core.Concatenate(
1034-
FilterNoEmitSemanticDiagnostics(p.getBindAndCheckDiagnosticsForFile(ctx, sourceFile), p.Options()),
1035+
return slices.Concat(
1036+
FilterNoEmitSemanticDiagnostics(p.getSemanticDiagnosticsForFileNotFilter(ctx, sourceFile), p.Options()),
10351037
p.GetIncludeProcessorDiagnostics(sourceFile),
10361038
)
10371039
}
10381040

1039-
// getBindAndCheckDiagnosticsForFile gets semantic diagnostics for a single file,
1040-
// including bind diagnostics, checker diagnostics, and handling of @ts-ignore/@ts-expect-error directives.
1041-
func (p *Program) getBindAndCheckDiagnosticsForFile(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
1041+
func (p *Program) getSemanticDiagnosticsForFileNotFilter(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic {
10421042
compilerOptions := p.Options()
10431043
if checker.SkipTypeChecking(sourceFile, compilerOptions, p, false) {
10441044
return nil
10451045
}
10461046

1047-
fileChecker, done := p.checkerPool.GetCheckerForFile(ctx, sourceFile)
1048-
defer done()
1049-
1050-
// Getting a checker will force a bind, so this will be populated.
1047+
var fileChecker *checker.Checker
1048+
var done func()
1049+
if sourceFile != nil {
1050+
fileChecker, done = p.checkerPool.GetCheckerForFile(ctx, sourceFile)
1051+
defer done()
1052+
}
10511053
diags := slices.Clip(sourceFile.BindDiagnostics())
1052-
diags = append(diags, fileChecker.GetDiagnostics(ctx, sourceFile)...)
1054+
// Ask for diags from all checkers; checking one file may add diagnostics to other files.
1055+
// These are deduplicated later.
1056+
checkerDiags := make([][]*ast.Diagnostic, p.checkerPool.Count())
1057+
p.checkerPool.ForEachCheckerParallel(ctx, func(idx int, checker *checker.Checker) {
1058+
if sourceFile == nil || checker == fileChecker {
1059+
checkerDiags[idx] = checker.GetDiagnostics(ctx, sourceFile)
1060+
}
1061+
})
1062+
if ctx.Err() != nil {
1063+
return nil
1064+
}
1065+
1066+
diags = append(diags, slices.Concat(checkerDiags...)...)
1067+
1068+
// !!! This should be rewritten to work like getBindAndCheckDiagnosticsForFileNoCache.
10531069

10541070
isPlainJS := ast.IsPlainJSFile(sourceFile, compilerOptions.CheckJs)
10551071
if isPlainJS {
@@ -1131,12 +1147,28 @@ func (p *Program) getSuggestionDiagnosticsForFile(ctx context.Context, sourceFil
11311147
return nil
11321148
}
11331149

1134-
fileChecker, done := p.checkerPool.GetCheckerForFile(ctx, sourceFile)
1135-
defer done()
1150+
var fileChecker *checker.Checker
1151+
var done func()
1152+
if sourceFile != nil {
1153+
fileChecker, done = p.checkerPool.GetCheckerForFile(ctx, sourceFile)
1154+
defer done()
1155+
}
11361156

1137-
// Getting a checker will force a bind, so this will be populated.
11381157
diags := slices.Clip(sourceFile.BindSuggestionDiagnostics)
1139-
diags = append(diags, fileChecker.GetSuggestionDiagnostics(ctx, sourceFile)...)
1158+
1159+
checkerDiags := make([][]*ast.Diagnostic, p.checkerPool.Count())
1160+
p.checkerPool.ForEachCheckerParallel(ctx, func(idx int, checker *checker.Checker) {
1161+
if sourceFile == nil || checker == fileChecker {
1162+
checkerDiags[idx] = checker.GetSuggestionDiagnostics(ctx, sourceFile)
1163+
} else {
1164+
// !!! is there any case where suggestion diagnostics are produced in other checkers?
1165+
}
1166+
})
1167+
if ctx.Err() != nil {
1168+
return nil
1169+
}
1170+
1171+
diags = append(diags, slices.Concat(checkerDiags...)...)
11401172

11411173
return diags
11421174
}
@@ -1189,6 +1221,29 @@ func compactAndMergeRelatedInfos(diagnostics []*ast.Diagnostic) []*ast.Diagnosti
11891221
return diagnostics[:j]
11901222
}
11911223

1224+
func (p *Program) getDiagnosticsHelper(ctx context.Context, sourceFile *ast.SourceFile, ensureBound bool, ensureChecked bool, getDiagnostics func(context.Context, *ast.SourceFile) []*ast.Diagnostic) []*ast.Diagnostic {
1225+
if sourceFile != nil {
1226+
if ensureBound {
1227+
binder.BindSourceFile(sourceFile)
1228+
}
1229+
return SortAndDeduplicateDiagnostics(getDiagnostics(ctx, sourceFile))
1230+
}
1231+
if ensureBound {
1232+
p.BindSourceFiles()
1233+
}
1234+
if ensureChecked {
1235+
p.CheckSourceFiles(ctx, nil)
1236+
if ctx.Err() != nil {
1237+
return nil
1238+
}
1239+
}
1240+
var result []*ast.Diagnostic
1241+
for _, file := range p.files {
1242+
result = append(result, getDiagnostics(ctx, file)...)
1243+
}
1244+
return SortAndDeduplicateDiagnostics(result)
1245+
}
1246+
11921247
func (p *Program) LineCount() int {
11931248
var count int
11941249
for _, file := range p.files {
@@ -1212,23 +1267,23 @@ func (p *Program) SymbolCount() int {
12121267
}
12131268
var val atomic.Uint32
12141269
val.Store(uint32(count))
1215-
p.ForEachCheckerParallel(func(_ int, c *checker.Checker) {
1270+
p.checkerPool.ForEachCheckerParallel(context.Background(), func(idx int, c *checker.Checker) {
12161271
val.Add(c.SymbolCount)
12171272
})
12181273
return int(val.Load())
12191274
}
12201275

12211276
func (p *Program) TypeCount() int {
12221277
var val atomic.Uint32
1223-
p.ForEachCheckerParallel(func(_ int, c *checker.Checker) {
1278+
p.checkerPool.ForEachCheckerParallel(context.Background(), func(idx int, c *checker.Checker) {
12241279
val.Add(c.TypeCount)
12251280
})
12261281
return int(val.Load())
12271282
}
12281283

12291284
func (p *Program) InstantiationCount() int {
12301285
var val atomic.Uint32
1231-
p.ForEachCheckerParallel(func(_ int, c *checker.Checker) {
1286+
p.checkerPool.ForEachCheckerParallel(context.Background(), func(idx int, c *checker.Checker) {
12321287
val.Add(c.TotalInstantiationCount)
12331288
})
12341289
return int(val.Load())
@@ -1323,6 +1378,8 @@ type SourceMapEmitResult struct {
13231378
}
13241379

13251380
func (p *Program) Emit(ctx context.Context, options EmitOptions) *EmitResult {
1381+
// !!! performance measurement
1382+
p.BindSourceFiles()
13261383
if options.EmitOnly != EmitOnlyForcedDts {
13271384
result := HandleNoEmitOnError(
13281385
ctx,

‎internal/execute/incremental/program.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func (p *Program) collectSemanticDiagnosticsOfAffectedFiles(ctx context.Context,
262262
}
263263

264264
// Get their diagnostics and cache them
265-
diagnosticsPerFile := p.program.GetSemanticDiagnosticsWithoutNoEmitFiltering(ctx, affectedFiles)
265+
diagnosticsPerFile := p.program.GetSemanticDiagnosticsNoFilter(ctx, affectedFiles)
266266
// commit changes if no err
267267
if ctx.Err() != nil {
268268
return

0 commit comments

Comments
 (0)