Skip to content

Commit b0fed9f

Browse files
jakebaileyjakebailey
authored
Reapply Program diagnostic func update, CheckerPool cleanup (#2314)
1 parent e93d364 commit b0fed9f

File tree

10 files changed

+176
-277
lines changed

10 files changed

+176
-277
lines changed

‎internal/ast/diagnostic.go‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package ast
22

33
import (
4-
"maps"
54
"slices"
65
"strings"
6+
"sync"
77

88
"github.com/microsoft/typescript-go/internal/collections"
99
"github.com/microsoft/typescript-go/internal/core"
@@ -141,13 +141,20 @@ func NewCompilerDiagnostic(message *diagnostics.Message, args ...any) *Diagnosti
141141
}
142142

143143
type DiagnosticsCollection struct {
144+
mu sync.Mutex
145+
count int
144146
fileDiagnostics map[string][]*Diagnostic
145147
fileDiagnosticsSorted collections.Set[string]
146148
nonFileDiagnostics []*Diagnostic
147149
nonFileDiagnosticsSorted bool
148150
}
149151

150152
func (c *DiagnosticsCollection) Add(diagnostic *Diagnostic) {
153+
c.mu.Lock()
154+
defer c.mu.Unlock()
155+
156+
c.count++
157+
151158
if diagnostic.File() != nil {
152159
fileName := diagnostic.File().FileName()
153160
if c.fileDiagnostics == nil {
@@ -162,11 +169,14 @@ func (c *DiagnosticsCollection) Add(diagnostic *Diagnostic) {
162169
}
163170

164171
func (c *DiagnosticsCollection) Lookup(diagnostic *Diagnostic) *Diagnostic {
172+
c.mu.Lock()
173+
defer c.mu.Unlock()
174+
165175
var diagnostics []*Diagnostic
166176
if diagnostic.File() != nil {
167-
diagnostics = c.GetDiagnosticsForFile(diagnostic.File().FileName())
177+
diagnostics = c.getDiagnosticsForFileLocked(diagnostic.File().FileName())
168178
} else {
169-
diagnostics = c.GetGlobalDiagnostics()
179+
diagnostics = c.getGlobalDiagnosticsLocked()
170180
}
171181
if i, ok := slices.BinarySearchFunc(diagnostics, diagnostic, CompareDiagnostics); ok {
172182
return diagnostics[i]
@@ -175,28 +185,45 @@ func (c *DiagnosticsCollection) Lookup(diagnostic *Diagnostic) *Diagnostic {
175185
}
176186

177187
func (c *DiagnosticsCollection) GetGlobalDiagnostics() []*Diagnostic {
188+
c.mu.Lock()
189+
defer c.mu.Unlock()
190+
191+
return c.getGlobalDiagnosticsLocked()
192+
}
193+
194+
func (c *DiagnosticsCollection) getGlobalDiagnosticsLocked() []*Diagnostic {
178195
if !c.nonFileDiagnosticsSorted {
179196
slices.SortStableFunc(c.nonFileDiagnostics, CompareDiagnostics)
180197
c.nonFileDiagnosticsSorted = true
181198
}
182-
return c.nonFileDiagnostics
199+
return slices.Clone(c.nonFileDiagnostics)
183200
}
184201

185202
func (c *DiagnosticsCollection) GetDiagnosticsForFile(fileName string) []*Diagnostic {
203+
c.mu.Lock()
204+
defer c.mu.Unlock()
205+
206+
return c.getDiagnosticsForFileLocked(fileName)
207+
}
208+
209+
func (c *DiagnosticsCollection) getDiagnosticsForFileLocked(fileName string) []*Diagnostic {
186210
if !c.fileDiagnosticsSorted.Has(fileName) {
187211
slices.SortStableFunc(c.fileDiagnostics[fileName], CompareDiagnostics)
188212
c.fileDiagnosticsSorted.Add(fileName)
189213
}
190-
return c.fileDiagnostics[fileName]
214+
return slices.Clone(c.fileDiagnostics[fileName])
191215
}
192216

193217
func (c *DiagnosticsCollection) GetDiagnostics() []*Diagnostic {
194-
fileNames := slices.Collect(maps.Keys(c.fileDiagnostics))
195-
slices.Sort(fileNames)
196-
diagnostics := slices.Clip(c.nonFileDiagnostics)
197-
for _, fileName := range fileNames {
198-
diagnostics = append(diagnostics, c.fileDiagnostics[fileName]...)
218+
c.mu.Lock()
219+
defer c.mu.Unlock()
220+
221+
diagnostics := make([]*Diagnostic, 0, c.count)
222+
diagnostics = append(diagnostics, c.nonFileDiagnostics...)
223+
for _, diags := range c.fileDiagnostics {
224+
diagnostics = append(diagnostics, diags...)
199225
}
226+
slices.SortFunc(diagnostics, CompareDiagnostics)
200227
return diagnostics
201228
}
202229

@@ -208,11 +235,17 @@ func getDiagnosticPath(d *Diagnostic) string {
208235
}
209236

210237
func EqualDiagnostics(d1, d2 *Diagnostic) bool {
238+
if d1 == d2 {
239+
return true
240+
}
211241
return EqualDiagnosticsNoRelatedInfo(d1, d2) &&
212242
slices.EqualFunc(d1.RelatedInformation(), d2.RelatedInformation(), EqualDiagnostics)
213243
}
214244

215245
func EqualDiagnosticsNoRelatedInfo(d1, d2 *Diagnostic) bool {
246+
if d1 == d2 {
247+
return true
248+
}
216249
return getDiagnosticPath(d1) == getDiagnosticPath(d2) &&
217250
d1.Loc() == d2.Loc() &&
218251
d1.Code() == d2.Code() &&
@@ -221,6 +254,9 @@ func EqualDiagnosticsNoRelatedInfo(d1, d2 *Diagnostic) bool {
221254
}
222255

223256
func equalMessageChain(c1, c2 *Diagnostic) bool {
257+
if c1 == c2 {
258+
return true
259+
}
224260
return c1.Code() == c2.Code() &&
225261
slices.Equal(c1.MessageArgs(), c2.MessageArgs()) &&
226262
slices.EqualFunc(c1.MessageChain(), c2.MessageChain(), equalMessageChain)
@@ -271,6 +307,9 @@ func compareRelatedInfo(r1, r2 []*Diagnostic) int {
271307
}
272308

273309
func CompareDiagnostics(d1, d2 *Diagnostic) int {
310+
if d1 == d2 {
311+
return 0
312+
}
274313
c := strings.Compare(getDiagnosticPath(d1), getDiagnosticPath(d2))
275314
if c != 0 {
276315
return c

‎internal/checker/checker.go‎

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,6 @@ type Program interface {
540540
GetJSXRuntimeImportSpecifier(path tspath.Path) (moduleReference string, specifier *ast.Node)
541541
GetImportHelpersImportSpecifier(path tspath.Path) *ast.Node
542542
SourceFileMayBeEmitted(sourceFile *ast.SourceFile, forceDtsEmit bool) bool
543-
IsSourceFromProjectReference(path tspath.Path) bool
544543
IsSourceFileDefaultLibrary(path tspath.Path) bool
545544
GetProjectReferenceFromOutputDts(path tspath.Path) *tsoptions.SourceOutputAndProjectReference
546545
GetRedirectForResolution(file ast.HasFileName) *tsoptions.ParsedCommandLine
@@ -930,8 +929,6 @@ func NewChecker(program Program) (*Checker, *sync.Mutex) {
930929
c.unionTypes = make(map[string]*Type)
931930
c.unionOfUnionTypes = make(map[UnionOfUnionKey]*Type)
932931
c.intersectionTypes = make(map[string]*Type)
933-
c.diagnostics = ast.DiagnosticsCollection{}
934-
c.suggestionDiagnostics = ast.DiagnosticsCollection{}
935932
c.mergedSymbols = make(map[*ast.Symbol]*ast.Symbol)
936933
c.patternForType = make(map[*Type]*ast.Node)
937934
c.contextFreeTypes = make(map[*ast.Node]*Type)
@@ -2094,13 +2091,6 @@ func (c *Checker) getSymbol(symbols ast.SymbolTable, name string, meaning ast.Sy
20942091
return nil
20952092
}
20962093

2097-
func (c *Checker) CheckSourceFile(ctx context.Context, sourceFile *ast.SourceFile) {
2098-
if SkipTypeChecking(sourceFile, c.compilerOptions, c.program, false) {
2099-
return
2100-
}
2101-
c.checkSourceFile(ctx, sourceFile)
2102-
}
2103-
21042094
func (c *Checker) checkSourceFile(ctx context.Context, sourceFile *ast.SourceFile) {
21052095
c.checkNotCanceled()
21062096
links := c.sourceFileLinks.Get(sourceFile)
@@ -13507,30 +13497,20 @@ func (c *Checker) getDiagnostics(ctx context.Context, sourceFile *ast.SourceFile
1350713497
c.checkNotCanceled()
1350813498
isSuggestionDiagnostics := collection == &c.suggestionDiagnostics
1350913499

13510-
files := c.files
13511-
if sourceFile != nil {
13512-
files = []*ast.SourceFile{sourceFile}
13500+
c.checkSourceFile(ctx, sourceFile)
13501+
if c.wasCanceled {
13502+
return nil
1351313503
}
1351413504

13515-
for _, file := range files {
13516-
c.CheckSourceFile(ctx, file)
13517-
if c.wasCanceled {
13518-
return nil
13519-
}
13520-
13521-
// Check unused identifiers as suggestions if we're collecting suggestion diagnostics
13522-
// and they are not configured as errors
13523-
if isSuggestionDiagnostics && !file.IsDeclarationFile &&
13524-
!(c.compilerOptions.NoUnusedLocals.IsTrue() || c.compilerOptions.NoUnusedParameters.IsTrue()) {
13525-
links := c.sourceFileLinks.Get(file)
13526-
c.checkUnusedIdentifiers(links.identifierCheckNodes)
13527-
}
13505+
// Check unused identifiers as suggestions if we're collecting suggestion diagnostics
13506+
// and they are not configured as errors
13507+
if isSuggestionDiagnostics && !sourceFile.IsDeclarationFile &&
13508+
!(c.compilerOptions.NoUnusedLocals.IsTrue() || c.compilerOptions.NoUnusedParameters.IsTrue()) {
13509+
links := c.sourceFileLinks.Get(sourceFile)
13510+
c.checkUnusedIdentifiers(links.identifierCheckNodes)
1352813511
}
1352913512

13530-
if sourceFile != nil {
13531-
return collection.GetDiagnosticsForFile(sourceFile.FileName())
13532-
}
13533-
return collection.GetDiagnostics()
13513+
return collection.GetDiagnosticsForFile(sourceFile.FileName())
1353413514
}
1353513515

1353613516
func (c *Checker) GetGlobalDiagnostics() []*ast.Diagnostic {

‎internal/checker/checker_test.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,25 +61,6 @@ 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-
8364
func BenchmarkNewChecker(b *testing.B) {
8465
repo.SkipIfNoTypeScriptSubmodule(b)
8566
fs := osvfs.FS()

‎internal/checker/utilities.go‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,34 +1283,6 @@ func forEachYieldExpression(body *ast.Node, visitor func(expr *ast.Node)) {
12831283
traverse(body)
12841284
}
12851285

1286-
func SkipTypeChecking(sourceFile *ast.SourceFile, options *core.CompilerOptions, host Program, ignoreNoCheck bool) bool {
1287-
return (!ignoreNoCheck && options.NoCheck.IsTrue()) ||
1288-
options.SkipLibCheck.IsTrue() && sourceFile.IsDeclarationFile ||
1289-
options.SkipDefaultLibCheck.IsTrue() && host.IsSourceFileDefaultLibrary(sourceFile.Path()) ||
1290-
host.IsSourceFromProjectReference(sourceFile.Path()) ||
1291-
!canIncludeBindAndCheckDiagnostics(sourceFile, options)
1292-
}
1293-
1294-
func canIncludeBindAndCheckDiagnostics(sourceFile *ast.SourceFile, options *core.CompilerOptions) bool {
1295-
if sourceFile.CheckJsDirective != nil && !sourceFile.CheckJsDirective.Enabled {
1296-
return false
1297-
}
1298-
1299-
if sourceFile.ScriptKind == core.ScriptKindTS || sourceFile.ScriptKind == core.ScriptKindTSX || sourceFile.ScriptKind == core.ScriptKindExternal {
1300-
return true
1301-
}
1302-
1303-
isJS := sourceFile.ScriptKind == core.ScriptKindJS || sourceFile.ScriptKind == core.ScriptKindJSX
1304-
isCheckJS := isJS && ast.IsCheckJSEnabledForFile(sourceFile, options)
1305-
isPlainJS := ast.IsPlainJSFile(sourceFile, options.CheckJs)
1306-
1307-
// By default, only type-check .ts, .tsx, Deferred, plain JS, checked JS and External
1308-
// - plain JS: .js files with no // ts-check and checkJs: undefined
1309-
// - check JS: .js files with either // ts-check or checkJs: true
1310-
// - external: files that are added by plugins
1311-
return isPlainJS || isCheckJS || sourceFile.ScriptKind == core.ScriptKindDeferred
1312-
}
1313-
13141286
func getEnclosingContainer(node *ast.Node) *ast.Node {
13151287
return ast.FindAncestor(node.Parent, func(n *ast.Node) bool {
13161288
return binder.GetContainerFlags(n)&binder.ContainerFlagsIsContainer != 0

‎internal/compiler/checkerpool.go‎

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package compiler
22

33
import (
44
"context"
5-
"iter"
65
"slices"
76
"sync"
87

@@ -12,17 +11,13 @@ import (
1211
)
1312

1413
type CheckerPool interface {
15-
Count() int
1614
GetChecker(ctx context.Context) (*checker.Checker, func())
1715
GetCheckerForFile(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
1816
GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
19-
ForEachCheckerParallel(ctx context.Context, cb func(idx int, c *checker.Checker))
20-
Files(checker *checker.Checker) iter.Seq[*ast.SourceFile]
2117
}
2218

2319
type checkerPool struct {
24-
checkerCount int
25-
program *Program
20+
program *Program
2621

2722
createCheckersOnce sync.Once
2823
checkers []*checker.Checker
@@ -43,32 +38,26 @@ func newCheckerPool(program *Program) *checkerPool {
4338
checkerCount = max(min(checkerCount, len(program.files), 256), 1)
4439

4540
pool := &checkerPool{
46-
program: program,
47-
checkerCount: checkerCount,
48-
checkers: make([]*checker.Checker, checkerCount),
49-
locks: make([]*sync.Mutex, checkerCount),
41+
program: program,
42+
checkers: make([]*checker.Checker, checkerCount),
43+
locks: make([]*sync.Mutex, checkerCount),
5044
}
5145

5246
return pool
5347
}
5448

55-
func (p *checkerPool) Count() int {
56-
return p.checkerCount
57-
}
58-
5949
func (p *checkerPool) GetCheckerForFile(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) {
6050
p.createCheckers()
61-
checker := p.fileAssociations[file]
62-
return checker, noop
51+
return p.fileAssociations[file], noop
6352
}
6453

6554
func (p *checkerPool) GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) {
66-
c, done := p.GetCheckerForFile(ctx, file)
55+
p.createCheckers()
56+
c := p.fileAssociations[file]
6757
idx := slices.Index(p.checkers, c)
6858
p.locks[idx].Lock()
6959
return c, sync.OnceFunc(func() {
7060
p.locks[idx].Unlock()
71-
done()
7261
})
7362
}
7463

@@ -80,8 +69,9 @@ func (p *checkerPool) GetChecker(ctx context.Context) (*checker.Checker, func())
8069

8170
func (p *checkerPool) createCheckers() {
8271
p.createCheckersOnce.Do(func() {
72+
checkerCount := len(p.checkers)
8373
wg := core.NewWorkGroup(p.program.SingleThreaded())
84-
for i := range p.checkerCount {
74+
for i := range checkerCount {
8575
wg.Queue(func() {
8676
p.checkers[i], p.locks[i] = checker.NewChecker(p.program)
8777
})
@@ -91,14 +81,14 @@ func (p *checkerPool) createCheckers() {
9181

9282
p.fileAssociations = make(map[*ast.SourceFile]*checker.Checker, len(p.program.files))
9383
for i, file := range p.program.files {
94-
p.fileAssociations[file] = p.checkers[i%p.checkerCount]
84+
p.fileAssociations[file] = p.checkers[i%checkerCount]
9585
}
9686
})
9787
}
9888

9989
// Runs `cb` for each checker in the pool concurrently, locking and unlocking checker mutexes as it goes,
100-
// making it safe to call `ForEachCheckerParallel` from many threads simultaneously.
101-
func (p *checkerPool) ForEachCheckerParallel(ctx context.Context, cb func(idx int, c *checker.Checker)) {
90+
// making it safe to call `forEachCheckerParallel` from many threads simultaneously.
91+
func (p *checkerPool) forEachCheckerParallel(cb func(idx int, c *checker.Checker)) {
10292
p.createCheckers()
10393
wg := core.NewWorkGroup(p.program.SingleThreaded())
10494
for idx, checker := range p.checkers {
@@ -111,17 +101,4 @@ func (p *checkerPool) ForEachCheckerParallel(ctx context.Context, cb func(idx in
111101
wg.RunAndWait()
112102
}
113103

114-
func (p *checkerPool) Files(checker *checker.Checker) iter.Seq[*ast.SourceFile] {
115-
checkerIndex := slices.Index(p.checkers, checker)
116-
return func(yield func(*ast.SourceFile) bool) {
117-
for i, file := range p.program.files {
118-
if i%p.checkerCount == checkerIndex {
119-
if !yield(file) {
120-
return
121-
}
122-
}
123-
}
124-
}
125-
}
126-
127104
func noop() {}

0 commit comments

Comments
 (0)