Match Strada's global diagnostic collection, hide non-diag global errors in editor (#64452)

Author: jakebailey

tsc/internal/checker/checker.go

@@ -14200,6 +14200,7 @@ func (c *Checker) getDiagnostics(ctx context.Context, sourceFile *ast.SourceFile
 
 func (c *Checker) GetGlobalDiagnostics() []*ast.Diagnostic {
 	c.checkNotCanceled()
+	c.produceDeferredDiagnostics()
 	return c.diagnostics.GetGlobalDiagnostics()
 }
 

tsc/internal/compiler/contentmapper_test.go

@@ -91,6 +91,65 @@ func TestContentMapperVirtualExtensionSetsImpliedNodeFormat(t *testing.T) {
 	assert.Equal(t, program.GetSourceFileMetaData(file.Path()).ImpliedNodeFormat, core.ResolutionModeESM)
 }
 
+func TestContentMapperDirectivesPreserveIncrementalGlobals(t *testing.T) {
+	t.Parallel()
+	for _, policy := range []ast.MappedDiagnosticDirectivePolicy{
+		ast.MappedDiagnosticDirectivePolicyIgnore,
+		ast.MappedDiagnosticDirectivePolicyExpect,
+	} {
+		t.Run(core.IfElse(policy == ast.MappedDiagnosticDirectivePolicyExpect, "expect", "ignore"), func(t *testing.T) {
+			t.Parallel()
+			const text = "export function values() { function* generator() { yield 1; } }"
+			program := newContentMapperProgramWithOptions(t, fakeContentMapperHost{
+				transform: func(fileName string, content string) (contentmapper.Result, error) {
+					// The virtual source is unchanged; the directive covers the entire file, including offset zero.
+					return contentmapper.Result{
+						Text:             content,
+						VirtualExtension: ".ts",
+						Mappings: spanmap.New([]spanmap.Segment{{
+							OriginalEnd: core.TextPos(len(content)),
+							VirtualEnd:  core.TextPos(len(content)),
+							Kind:        spanmap.KindVerbatim,
+							Features:    spanmap.FeatureAll,
+						}}),
+						DiagnosticDirectives: []ast.MappedDiagnosticDirective{{
+							VirtualRange:      core.NewTextRange(0, len(content)),
+							OriginalRange:     core.NewTextRange(0, len(content)),
+							Policy:            policy,
+							Source:            "vue",
+							UnusedCode:        2578,
+							UnusedMessageText: "Unused mapped expect directive.",
+						}},
+					}, nil
+				},
+			}, map[string]string{"/src/Component.vue": text}, []string{"/src/Component.vue"}, &core.CompilerOptions{
+				Lib:              []string{"lib.es5.d.ts"},
+				SkipLibCheck:     core.TSTrue,
+				Module:           core.ModuleKindESNext,
+				ModuleResolution: core.ModuleResolutionKindBundler,
+			})
+			assert.Equal(t, len(program.GetGlobalDiagnostics(t.Context())), 0)
+			file := program.GetSourceFile("/src/Component.vue")
+			diags := program.GetSemanticDiagnosticsForIncremental(t.Context(), []*ast.SourceFile{file})[file]
+			var globals, unused int
+			for _, diag := range diags {
+				if diag.File() == nil {
+					assert.Equal(t, diag.Code(), diagnostics.Cannot_find_global_type_0.Code())
+					assert.Equal(t, diag.MessageArgs()[0], "IterableIterator")
+					globals++
+				} else {
+					assert.Equal(t, diag.File(), file)
+					assert.Equal(t, diag.Source(), "vue")
+					assert.Equal(t, diag.Code(), int32(2578))
+					unused++
+				}
+			}
+			assert.Equal(t, globals, 1)
+			assert.Equal(t, unused, core.IfElse(policy == ast.MappedDiagnosticDirectivePolicyExpect, 1, 0))
+		})
+	}
+}
+
 func TestCompositeProjectContentMapperSupplementalRoots(t *testing.T) {
 	t.Parallel()
 	contentMapperHost := fakeContentMapperHost{transform: func(fileName string, content string) (contentmapper.Result, error) {

tsc/internal/compiler/program.go

@@ -799,8 +799,12 @@ func (p *Program) GetSemanticDiagnostics(ctx context.Context, sourceFile *ast.So
 	return p.collectCheckerDiagnostics(ctx, sourceFile, p.getSemanticDiagnosticsWithChecker)
 }
 
-func (p *Program) GetSemanticDiagnosticsWithoutNoEmitFiltering(ctx context.Context, sourceFiles []*ast.SourceFile) map[*ast.SourceFile][]*ast.Diagnostic {
-	allDiags := p.collectCheckerDiagnosticsFromFiles(ctx, sourceFiles, p.getBindAndCheckDiagnosticsWithChecker)
+// GetSemanticDiagnosticsForIncremental includes newly discovered globals in each
+// file's cached diagnostics and leaves noEmit filtering to the builder.
+func (p *Program) GetSemanticDiagnosticsForIncremental(ctx context.Context, sourceFiles []*ast.SourceFile) map[*ast.SourceFile][]*ast.Diagnostic {
+	allDiags := p.collectCheckerDiagnosticsFromFiles(ctx, sourceFiles, func(ctx context.Context, c *checker.Checker, file *ast.SourceFile) []*ast.Diagnostic {
+		return p.getBindAndCheckDiagnosticsWithChecker(ctx, c, file, true /*includeDeferredGlobals*/)
+	})
 	result := make(map[*ast.SourceFile][]*ast.Diagnostic, len(sourceFiles))
 	for i, diags := range allDiags {
 		result[sourceFiles[i]] = filterAndSortDiagnostics(diags)
@@ -1475,24 +1479,42 @@ func FilterNoEmitSemanticDiagnostics(diagnostics []*ast.Diagnostic, options *cor
 
 func (p *Program) getSemanticDiagnosticsWithChecker(ctx context.Context, c *checker.Checker, sourceFile *ast.SourceFile) []*ast.Diagnostic {
 	return core.Concatenate(
-		FilterNoEmitSemanticDiagnostics(p.getBindAndCheckDiagnosticsWithChecker(ctx, c, sourceFile), p.Options()),
+		FilterNoEmitSemanticDiagnostics(p.getBindAndCheckDiagnosticsWithChecker(ctx, c, sourceFile, false /*includeDeferredGlobals*/), p.Options()),
 		p.GetIncludeProcessorDiagnostics(sourceFile),
 	)
 }
 
 // getBindAndCheckDiagnosticsWithChecker gets semantic diagnostics for a single file using a
 // caller-provided checker, including bind diagnostics, checker diagnostics, and handling
 // of @ts-ignore/@ts-expect-error directives.
-func (p *Program) getBindAndCheckDiagnosticsWithChecker(ctx context.Context, fileChecker *checker.Checker, sourceFile *ast.SourceFile) []*ast.Diagnostic {
+func (p *Program) getBindAndCheckDiagnosticsWithChecker(ctx context.Context, fileChecker *checker.Checker, sourceFile *ast.SourceFile, includeDeferredGlobals bool) []*ast.Diagnostic {
 	compilerOptions := p.Options()
 	if p.SkipTypeChecking(sourceFile, false) {
 		return nil
 	}
+	var previousGlobals []*ast.Diagnostic
+	if includeDeferredGlobals {
+		previousGlobals = fileChecker.GetGlobalDiagnostics()
+	}
 
 	// Checker creation forces binding, so bind diagnostics will be populated.
 	diags := slices.Clip(sourceFile.BindDiagnostics())
 	diags = append(diags, fileChecker.GetDiagnostics(ctx, sourceFile)...)
 
+	if includeDeferredGlobals {
+		if fileChecker.WasCanceled() {
+			return nil
+		}
+		currentGlobals := fileChecker.GetGlobalDiagnostics()
+		if len(currentGlobals) > len(previousGlobals) {
+			for _, diagnostic := range currentGlobals {
+				if _, found := slices.BinarySearchFunc(previousGlobals, diagnostic, ast.CompareDiagnostics); !found {
+					diags = append(diags, diagnostic)
+				}
+			}
+		}
+	}
+
 	isPlainJS := ast.IsPlainJSFile(sourceFile, compilerOptions.CheckJs)
 	if isPlainJS {
 		return core.Filter(diags, func(d *ast.Diagnostic) bool {
@@ -1525,7 +1547,7 @@ func applyContentMapperDiagnosticDirectives(sourceFile *ast.SourceFile, diags []
 	}
 	used := make([]bool, len(directives))
 	markUsed := func(diag *ast.Diagnostic) bool {
-		if diag.Source() != "" {
+		if diag.File() != sourceFile || diag.Source() != "" {
 			return false
 		}
 		for i, directive := range directives {
@@ -1568,6 +1590,10 @@ func (p *Program) getDiagnosticsWithPrecedingDirectives(sourceFile *ast.SourceFi
 	filtered := make([]*ast.Diagnostic, 0, len(diags))
 	for _, diagnostic := range diags {
 		ignoreDiagnostic := false
+		if diagnostic.File() != sourceFile {
+			filtered = append(filtered, diagnostic)
+			continue
+		}
 		for line := scanner.ComputeLineOfPosition(lineStarts, diagnostic.Pos()) - 1; line >= 0; line-- {
 			// If line contains a @ts-ignore or @ts-expect-error directive, ignore this diagnostic and change
 			// the directive kind to @ts-ignore to indicate it was used.
@@ -2023,8 +2049,11 @@ func GetDiagnosticsOfAnyProgram(
 
 			if len(allDiagnostics) == configFileParsingDiagnosticsLength {
 				allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics)
-				// Ask for the global diagnostics again (they were empty above); we may have found new during checking, e.g. missing globals.
-				allDiagnostics = append(allDiagnostics, program.GetGlobalDiagnostics(ctx)...)
+				if p, ok := program.(*Program); ok {
+					// Incremental programs cache checking globals with file diagnostics;
+					// a late sweep would also collect incidental signature-generation globals.
+					allDiagnostics = append(allDiagnostics, p.GetGlobalDiagnostics(ctx)...)
+				}
 			}
 
 			if (skipNoEmitCheckForDtsDiagnostics || program.Options().NoEmit.IsTrue()) && program.Options().GetEmitDeclarations() && len(allDiagnostics) == configFileParsingDiagnosticsLength {

tsc/internal/execute/incremental/program.go

@@ -303,7 +303,7 @@ func (p *Program) collectSemanticDiagnosticsOfAffectedFiles(ctx context.Context,
 	}
 
 	// Get their diagnostics and cache them
-	diagnosticsPerFile := p.program.GetSemanticDiagnosticsWithoutNoEmitFiltering(ctx, affectedFiles)
+	diagnosticsPerFile := p.program.GetSemanticDiagnosticsForIncremental(ctx, affectedFiles)
 	// commit changes if no err
 	if ctx.Err() != nil {
 		return

tsc/internal/execute/tsctests/tsc_test.go

@@ -26,6 +26,13 @@ func TestTscCommandline(t *testing.T) {
 		}
 	}
 	testCases := []*tscInput{
+		{
+			subScenario: "global diagnostics produced during ordinary semantic checking",
+			files: FileMap{
+				"/home/src/workspaces/project/index.ts": `export function* values() { yield 1; }`,
+			},
+			commandLineArgs: []string{"index.ts", "--noEmit"},
+		},
 		{
 			subScenario: "show help with ExitStatus.DiagnosticsPresent_OutputsSkipped",
 			env: map[string]string{
@@ -1349,6 +1356,33 @@ func TestTscIgnoreConfig(t *testing.T) {
 
 func TestTscIncremental(t *testing.T) {
 	t.Parallel()
+	libWithReadonlyArray := strings.Replace(tscDefaultLibContent, "interface ReadonlyArray<T> {}", "interface ReadonlyArray<T> { readonly length: number; readonly [n: number]: T; }", 1)
+	getRecursiveTypeTest := func(name string, source string) *tscInput {
+		return &tscInput{
+			subScenario: name + " after comment only edit",
+			files: FileMap{
+				"/home/src/workspaces/project/tsconfig.json": `{"compilerOptions": {"strict": true, "noEmit": true, "incremental": true}}`,
+				"/home/src/workspaces/project/repro.ts":      stringtestutil.Dedent(source),
+				tscLibPath + "/lib.es2025.full.d.ts":         libWithReadonlyArray,
+			},
+			edits: []*tscEdit{
+				{
+					caption: "add a comment",
+					edit: func(sys *TestSys) {
+						sys.appendFile("/home/src/workspaces/project/repro.ts", "\n// comment-only edit\n")
+					},
+				},
+				noChange,
+				{
+					caption: "add another comment",
+					edit: func(sys *TestSys) {
+						sys.appendFile("/home/src/workspaces/project/repro.ts", "\n// another comment\n")
+					},
+				},
+				noChange,
+			},
+		}
+	}
 	getConstEnumTest := func(bdsContents string, changeEnumFile string, testSuffix string) *tscInput {
 		return &tscInput{
 			subScenario: "const enums" + testSuffix,
@@ -2270,6 +2304,85 @@ func TestTscIncremental(t *testing.T) {
 			},
 			commandLineArgs: []string{"--noEmit"},
 		},
+		getRecursiveTypeTest("recursive mapped type", `
+			type Json = string | Json[];
+			type Parsed<T> = T extends object ? { [K in keyof T]: Parsed<T[K]> } : T;
+			declare function wrap<T>(value: T): Parsed<T>;
+			export const value = wrap({ items: [] as Json[] });
+		`),
+		getRecursiveTypeTest("recursive readonly mapped type", `
+			type Json = string | readonly Json[];
+			type Parsed<T> = T extends object ? { [K in keyof T]: Parsed<T[K]> } : T;
+			declare function wrap<T>(value: T): Parsed<T>;
+			export const value = wrap({ items: [] as readonly Json[] });
+		`),
+		{
+			subScenario: "global diagnostics produced during semantic checking",
+			files: FileMap{
+				"/home/src/workspaces/project/tsconfig.json": `{"compilerOptions": {"noEmit": true, "incremental": true}}`,
+				"/home/src/workspaces/project/repro.ts":      `export function* values() { yield 1; }`,
+			},
+			edits: []*tscEdit{
+				noChange,
+				{
+					caption: "add a comment",
+					edit: func(sys *TestSys) {
+						sys.appendFile("/home/src/workspaces/project/repro.ts", "\n// comment-only edit\n")
+					},
+					expectedDiff: "Like Strada, signature generation produces the missing-global diagnostic before semantic checking, so it is excluded from the file's semantic diagnostics.",
+				},
+				{
+					caption:      "no change",
+					edit:         noChange.edit,
+					expectedDiff: "Like Strada, the cached semantic diagnostics do not include the missing-global diagnostic produced during signature generation.",
+				},
+				{
+					caption: "delete build info to restore the semantic diagnostic",
+					edit: func(sys *TestSys) {
+						sys.removeNoError("/home/src/workspaces/project/tsconfig.tsbuildinfo")
+					},
+				},
+			},
+		},
+		{
+			subScenario: "global diagnostics from function bodies after incremental edits",
+			files: FileMap{
+				"/home/src/workspaces/project/tsconfig.json": `{"compilerOptions": {"noEmit": true, "incremental": true}}`,
+				"/home/src/workspaces/project/repro.ts": stringtestutil.Dedent(`
+					export function values() {
+						// @ts-ignore
+						function* generator() { yield 1; }
+					}
+				`),
+			},
+			edits: []*tscEdit{
+				noChange,
+				{
+					caption: "add a comment",
+					edit: func(sys *TestSys) {
+						sys.appendFile("/home/src/workspaces/project/repro.ts", "\n// comment-only edit\n")
+					},
+				},
+				noChange,
+			},
+		},
+		{
+			subScenario: "global diagnostics produced during unchecked javascript checking",
+			files: FileMap{
+				"/home/src/workspaces/project/tsconfig.json": `{"compilerOptions": {"allowJs": true, "noEmit": true, "incremental": true}}`,
+				"/home/src/workspaces/project/repro.js":      `export function* values() { yield 1; }`,
+			},
+			edits: []*tscEdit{
+				noChange,
+				{
+					caption: "enable javascript checking",
+					edit: func(sys *TestSys) {
+						sys.replaceFileText("/home/src/workspaces/project/tsconfig.json", `"allowJs": true`, `"allowJs": true, "checkJs": true`)
+					},
+				},
+				noChange,
+			},
+		},
 		{
 			subScenario: "json module diagnostics are cleared after fixing the json file",
 			files: FileMap{

tsc/internal/project/checkerpool.go

@@ -351,7 +351,10 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec
 			p.log(fmt.Sprintf("checkerpool: Checker %d for request %s was canceled, disposing", index, holdTag(requestID)))
 			p.disposeCheckerLocked(index, c)
 		} else {
-			p.mergeGlobalDiagnosticsFromCheckerLocked(index, c)
+			// Query checkers can produce incidental errors while serializing types.
+			if index == 0 {
+				p.mergeGlobalDiagnosticsFromCheckerLocked(index, c)
+			}
 			p.heldBy[index] = ""
 			p.lastReleased[index] = time.Now()
 			if !p.discarded {
@@ -491,8 +494,8 @@ func (p *checkerPool) mergeGlobalDiagnosticsFromCheckerLocked(index int, c *chec
 	}
 }
 
-// GetGlobalDiagnostics returns the accumulated global diagnostics collected from
-// all checkers that have been used so far in this pool's lifetime.
+// GetGlobalDiagnostics returns the global diagnostics accumulated from the dedicated
+// diagnostics checker across its instances during this pool's lifetime.
 func (p *checkerPool) GetGlobalDiagnostics() []*ast.Diagnostic {
 	p.mu.Lock()
 	defer p.mu.Unlock()

tsc/internal/project/checkerpool_test.go

@@ -1157,31 +1157,26 @@ func TestCheckerPoolTakeNewGlobalDiagnostics(t *testing.T) {
 
 	// Use a checker and trigger diagnostics, then release to run the merge.
 	ctx := core.WithRequestID(context.Background(), "global-diag-req")
-	ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary)
+	ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics)
 	sourceFile := pool.program.GetSourceFile("/src/index.ts")
 	c, release := pool.GetChecker(ctx, sourceFile)
 	assert.Assert(t, c != nil)
 	c.GetDiagnostics(ctx, sourceFile)
 	release()
 
-	// Whether globals were produced depends on the program, but the flag
-	// should reflect the merge result.
-	firstTake := pool.TakeNewGlobalDiagnostics()
+	assert.Assert(t, pool.TakeNewGlobalDiagnostics(), "diagnostics checker should publish missing-lib globals")
 
 	// After taking, a second call should always return false (flag is reset).
 	assert.Assert(t, !pool.TakeNewGlobalDiagnostics(), "TakeNewGlobalDiagnostics should reset after first call")
 
 	// Releasing the same checker again with the same state should not set the flag.
 	ctx2 := core.WithRequestID(context.Background(), "global-diag-req-2")
-	ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeTemporary)
+	ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeDiagnostics)
 	c2, release2 := pool.GetChecker(ctx2, sourceFile)
 	assert.Assert(t, c2 != nil)
 	c2.GetDiagnostics(ctx2, sourceFile)
 	release2()
 
-	// If first call produced globals, the count is now stable, so no new change.
-	// If first call produced no globals, still no change.
-	_ = firstTake
 	assert.Assert(t, !pool.TakeNewGlobalDiagnostics(), "should not report new globals when checker state is unchanged")
 }