From 878af0771b2084516c8a0911f830d4cd0362a693 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Wed, 19 Jun 2024 15:12:57 -0700 Subject: [PATCH] [typescript-language-features] Region-based semantic diagnostics for TypeScript (#208713) * WIP * invalidate diagnostics in range * check whether should use region based diagnostics * add ts-expect-errors * make region opt off by default * bump to expected 5.6 * update comments to refer to 5.6 * make region diagnostics on by default for insiders --- .../typescript-language-features/package.json | 6 +++ .../package.nls.json | 1 + .../src/configuration/configuration.ts | 6 +++ .../src/languageFeatures/diagnostics.ts | 31 +++++++++++-- .../src/languageProvider.ts | 8 +++- .../src/tsServer/bufferSyncSupport.ts | 46 +++++++++++++++---- .../src/tsServer/protocol/protocol.const.ts | 1 + .../src/typeConverters.ts | 14 +++++- .../src/typeScriptServiceClientHost.ts | 10 ++-- .../src/typescriptServiceClient.ts | 9 +++- 10 files changed, 108 insertions(+), 24 deletions(-) diff --git a/extensions/typescript-language-features/package.json b/extensions/typescript-language-features/package.json index 01981b1db95..3f7fdcf252f 100644 --- a/extensions/typescript-language-features/package.json +++ b/extensions/typescript-language-features/package.json @@ -1315,6 +1315,12 @@ "markdownDescription": "%typescript.workspaceSymbols.excludeLibrarySymbols%", "scope": "window" }, + "typescript.tsserver.enableRegionDiagnostics": { + "type": "boolean", + "default": true, + "description": "%typescript.tsserver.enableRegionDiagnostics%", + "scope": "window" + }, "javascript.experimental.updateImportsOnPaste": { "scope": "window", "type": "boolean", diff --git a/extensions/typescript-language-features/package.nls.json b/extensions/typescript-language-features/package.nls.json index cdc6264424c..cba24007314 100644 --- a/extensions/typescript-language-features/package.nls.json +++ b/extensions/typescript-language-features/package.nls.json @@ -16,6 +16,7 @@ "typescript.tsserver.pluginPaths": "Additional paths to discover TypeScript Language Service plugins.", "typescript.tsserver.pluginPaths.item": "Either an absolute or relative path. Relative path will be resolved against workspace folder(s).", "typescript.tsserver.trace": "Enables tracing of messages sent to the TS server. This trace can be used to diagnose TS Server issues. The trace may contain file paths, source code, and other potentially sensitive information from your project.", + "typescript.tsserver.enableRegionDiagnostics": "Enables region-based diagnostics in TypeScript. Requires using TypeScript 5.6+ in the workspace.", "typescript.validate.enable": "Enable/disable TypeScript validation.", "typescript.format.enable": "Enable/disable default TypeScript formatter.", "javascript.format.enable": "Enable/disable default JavaScript formatter.", diff --git a/extensions/typescript-language-features/src/configuration/configuration.ts b/extensions/typescript-language-features/src/configuration/configuration.ts index a08ca921e0c..639f3d346e0 100644 --- a/extensions/typescript-language-features/src/configuration/configuration.ts +++ b/extensions/typescript-language-features/src/configuration/configuration.ts @@ -124,6 +124,7 @@ export interface TypeScriptServiceConfiguration { readonly localNodePath: string | null; readonly globalNodePath: string | null; readonly workspaceSymbolsExcludeLibrarySymbols: boolean; + readonly enableRegionDiagnostics: boolean; } export function areServiceConfigurationsEqual(a: TypeScriptServiceConfiguration, b: TypeScriptServiceConfiguration): boolean { @@ -162,6 +163,7 @@ export abstract class BaseServiceConfigurationProvider implements ServiceConfigu localNodePath: this.readLocalNodePath(configuration), globalNodePath: this.readGlobalNodePath(configuration), workspaceSymbolsExcludeLibrarySymbols: this.readWorkspaceSymbolsExcludeLibrarySymbols(configuration), + enableRegionDiagnostics: this.readEnableRegionDiagnostics(configuration), }; } @@ -267,4 +269,8 @@ export abstract class BaseServiceConfigurationProvider implements ServiceConfigu private readWebTypeAcquisition(configuration: vscode.WorkspaceConfiguration): boolean { return configuration.get('typescript.tsserver.web.typeAcquisition.enabled', false); } + + private readEnableRegionDiagnostics(configuration: vscode.WorkspaceConfiguration): boolean { + return configuration.get('typescript.tsserver.enableRegionDiagnostics', true); + } } diff --git a/extensions/typescript-language-features/src/languageFeatures/diagnostics.ts b/extensions/typescript-language-features/src/languageFeatures/diagnostics.ts index 190e6a99bf7..990aefdfa56 100644 --- a/extensions/typescript-language-features/src/languageFeatures/diagnostics.ts +++ b/extensions/typescript-language-features/src/languageFeatures/diagnostics.ts @@ -34,6 +34,7 @@ export const enum DiagnosticKind { Syntax, Semantic, Suggestion, + RegionSemantic, } class FileDiagnostics { @@ -48,7 +49,8 @@ class FileDiagnostics { public updateDiagnostics( language: DiagnosticLanguage, kind: DiagnosticKind, - diagnostics: ReadonlyArray + diagnostics: ReadonlyArray, + ranges: ReadonlyArray | undefined ): boolean { if (language !== this.language) { this._diagnostics.clear(); @@ -61,6 +63,9 @@ class FileDiagnostics { return false; } + if (kind === DiagnosticKind.RegionSemantic) { + return this.updateRegionDiagnostics(diagnostics, ranges!); + } this._diagnostics.set(kind, diagnostics); return true; } @@ -83,6 +88,23 @@ class FileDiagnostics { } } + /** + * @param ranges The ranges whose diagnostics were updated. + */ + private updateRegionDiagnostics( + diagnostics: ReadonlyArray, + ranges: ReadonlyArray): boolean { + if (!this._diagnostics.get(DiagnosticKind.Semantic)) { + this._diagnostics.set(DiagnosticKind.Semantic, diagnostics); + return true; + } + const oldDiagnostics = this._diagnostics.get(DiagnosticKind.Semantic)!; + const newDiagnostics = oldDiagnostics.filter(diag => !ranges.some(range => diag.range.intersection(range))); + newDiagnostics.push(...diagnostics); + this._diagnostics.set(DiagnosticKind.Semantic, newDiagnostics); + return true; + } + private getSuggestionDiagnostics(settings: DiagnosticSettings) { const enableSuggestions = settings.getEnableSuggestions(this.language); return this.get(DiagnosticKind.Suggestion).filter(x => { @@ -284,15 +306,16 @@ export class DiagnosticsManager extends Disposable { file: vscode.Uri, language: DiagnosticLanguage, kind: DiagnosticKind, - diagnostics: ReadonlyArray + diagnostics: ReadonlyArray, + ranges: ReadonlyArray | undefined, ): void { let didUpdate = false; const entry = this._diagnostics.get(file); if (entry) { - didUpdate = entry.updateDiagnostics(language, kind, diagnostics); + didUpdate = entry.updateDiagnostics(language, kind, diagnostics, ranges); } else if (diagnostics.length) { const fileDiagnostics = new FileDiagnostics(file, language); - fileDiagnostics.updateDiagnostics(language, kind, diagnostics); + fileDiagnostics.updateDiagnostics(language, kind, diagnostics, ranges); this._diagnostics.set(file, fileDiagnostics); didUpdate = true; } diff --git a/extensions/typescript-language-features/src/languageProvider.ts b/extensions/typescript-language-features/src/languageProvider.ts index a1927409869..7b95591604b 100644 --- a/extensions/typescript-language-features/src/languageProvider.ts +++ b/extensions/typescript-language-features/src/languageProvider.ts @@ -138,7 +138,11 @@ export default class LanguageProvider extends Disposable { this.client.bufferSyncSupport.requestAllDiagnostics(); } - public diagnosticsReceived(diagnosticsKind: DiagnosticKind, file: vscode.Uri, diagnostics: (vscode.Diagnostic & { reportUnnecessary: any; reportDeprecated: any })[]): void { + public diagnosticsReceived( + diagnosticsKind: DiagnosticKind, + file: vscode.Uri, + diagnostics: (vscode.Diagnostic & { reportUnnecessary: any; reportDeprecated: any })[], + ranges: vscode.Range[] | undefined): void { if (diagnosticsKind !== DiagnosticKind.Syntax && !this.client.hasCapabilityForResource(file, ClientCapability.Semantic)) { return; } @@ -175,7 +179,7 @@ export default class LanguageProvider extends Disposable { } } return true; - })); + }), ranges); } public configFileDiagnosticsReceived(file: vscode.Uri, diagnostics: vscode.Diagnostic[]): void { diff --git a/extensions/typescript-language-features/src/tsServer/bufferSyncSupport.ts b/extensions/typescript-language-features/src/tsServer/bufferSyncSupport.ts index 87c715982c2..32707f1c049 100644 --- a/extensions/typescript-language-features/src/tsServer/bufferSyncSupport.ts +++ b/extensions/typescript-language-features/src/tsServer/bufferSyncSupport.ts @@ -275,12 +275,12 @@ class SyncedBufferMap extends ResourceMap { } class PendingDiagnostics extends ResourceMap { - public getOrderedFileSet(): ResourceMap { + public getOrderedFileSet(): ResourceMap { const orderedResources = Array.from(this.entries()) .sort((a, b) => a.value - b.value) .map(entry => entry.resource); - const map = new ResourceMap(this._normalizePath, this.config); + const map = new ResourceMap(this._normalizePath, this.config); for (const resource of orderedResources) { map.set(resource, undefined); } @@ -292,7 +292,7 @@ class GetErrRequest { public static executeGetErrRequest( client: ITypeScriptServiceClient, - files: ResourceMap, + files: ResourceMap, onDone: () => void ) { return new GetErrRequest(client, files, onDone); @@ -303,7 +303,7 @@ class GetErrRequest { private constructor( private readonly client: ITypeScriptServiceClient, - public readonly files: ResourceMap, + public readonly files: ResourceMap, onDone: () => void ) { if (!this.isErrorReportingEnabled()) { @@ -313,19 +313,39 @@ class GetErrRequest { } const supportsSyntaxGetErr = this.client.apiVersion.gte(API.v440); - const allFiles = coalesce(Array.from(files.entries()) - .filter(entry => supportsSyntaxGetErr || client.hasCapabilityForResource(entry.resource, ClientCapability.Semantic)) + const fileEntries = Array.from(files.entries()).filter(entry => supportsSyntaxGetErr || client.hasCapabilityForResource(entry.resource, ClientCapability.Semantic)); + const allFiles = coalesce(fileEntries .map(entry => client.toTsFilePath(entry.resource))); if (!allFiles.length) { this._done = true; setImmediate(onDone); } else { - const request = this.areProjectDiagnosticsEnabled() + let request; + if (this.areProjectDiagnosticsEnabled()) { // Note that geterrForProject is almost certainly not the api we want here as it ends up computing far // too many diagnostics - ? client.executeAsync('geterrForProject', { delay: 0, file: allFiles[0] }, this._token.token) - : client.executeAsync('geterr', { delay: 0, files: allFiles }, this._token.token); + request = client.executeAsync('geterrForProject', { delay: 0, file: allFiles[0] }, this._token.token); + } + else { + let requestFiles; + if (this.areRegionDiagnosticsEnabled()) { + requestFiles = coalesce(fileEntries + .map(entry => { + const file = client.toTsFilePath(entry.resource); + const ranges = entry.value; + if (file && ranges) { + return typeConverters.Range.toFileRangesRequestArgs(file, ranges); + } + + return file; + })); + } + else { + requestFiles = allFiles; + } + request = client.executeAsync('geterr', { delay: 0, files: requestFiles }, this._token.token); + } request.finally(() => { if (this._done) { @@ -350,6 +370,10 @@ class GetErrRequest { return this.client.configuration.enableProjectDiagnostics && this.client.capabilities.has(ClientCapability.Semantic); } + private areRegionDiagnosticsEnabled() { + return this.client.configuration.enableRegionDiagnostics && this.client.apiVersion.gte(API.v560); + } + public cancel(): any { if (!this._done) { this._token.cancel(); @@ -722,7 +746,9 @@ export default class BufferSyncSupport extends Disposable { // Add all open TS buffers to the geterr request. They might be visible for (const buffer of this.syncedBuffers.values()) { - orderedFileSet.set(buffer.resource, undefined); + const editors = vscode.window.visibleTextEditors.filter(editor => editor.document.uri.toString() === buffer.resource.toString()); + const visibleRanges = editors.flatMap(editor => editor.visibleRanges); + orderedFileSet.set(buffer.resource, visibleRanges.length ? visibleRanges : undefined); } for (const { resource } of orderedFileSet.entries()) { diff --git a/extensions/typescript-language-features/src/tsServer/protocol/protocol.const.ts b/extensions/typescript-language-features/src/tsServer/protocol/protocol.const.ts index 4f02ed29427..f1b0cca26a4 100644 --- a/extensions/typescript-language-features/src/tsServer/protocol/protocol.const.ts +++ b/extensions/typescript-language-features/src/tsServer/protocol/protocol.const.ts @@ -78,6 +78,7 @@ export enum EventName { syntaxDiag = 'syntaxDiag', semanticDiag = 'semanticDiag', suggestionDiag = 'suggestionDiag', + regionSemanticDiag = 'regionSemanticDiag', configFileDiag = 'configFileDiag', telemetry = 'telemetry', projectLanguageServiceState = 'projectLanguageServiceState', diff --git a/extensions/typescript-language-features/src/typeConverters.ts b/extensions/typescript-language-features/src/typeConverters.ts index 58babe2bda3..067a1ff3c0a 100644 --- a/extensions/typescript-language-features/src/typeConverters.ts +++ b/extensions/typescript-language-features/src/typeConverters.ts @@ -26,14 +26,24 @@ export namespace Range { Math.max(0, start.line - 1), Math.max(start.offset - 1, 0), Math.max(0, end.line - 1), Math.max(0, end.offset - 1)); - export const toFileRangeRequestArgs = (file: string, range: vscode.Range): Proto.FileRangeRequestArgs => ({ - file, + // @ts-expect-error until ts 5.6 + export const toFileRange = (range: vscode.Range): Proto.FileRange => ({ startLine: range.start.line + 1, startOffset: range.start.character + 1, endLine: range.end.line + 1, endOffset: range.end.character + 1 }); + export const toFileRangeRequestArgs = (file: string, range: vscode.Range): Proto.FileRangeRequestArgs => ({ + file, + ...toFileRange(range) + }); + // @ts-expect-error until ts 5.6 + export const toFileRangesRequestArgs = (file: string, ranges: vscode.Range[]): Proto.FileRangesRequestArgs => ({ + file, + ranges: ranges.map(toFileRange) + }); + export const toFormattingRequestArgs = (file: string, range: vscode.Range): Proto.FormatRequestArgs => ({ file, line: range.start.line + 1, diff --git a/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts b/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts index da651e71044..c44bfc3ac3f 100644 --- a/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts +++ b/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts @@ -90,8 +90,8 @@ export default class TypeScriptServiceClientHost extends Disposable { services, allModeIds)); - this.client.onDiagnosticsReceived(({ kind, resource, diagnostics }) => { - this.diagnosticsReceived(kind, resource, diagnostics); + this.client.onDiagnosticsReceived(({ kind, resource, diagnostics, spans }) => { + this.diagnosticsReceived(kind, resource, diagnostics, spans); }, null, this._disposables); this.client.onConfigDiagnosticsReceived(diag => this.configFileDiagnosticsReceived(diag), null, this._disposables); @@ -236,14 +236,16 @@ export default class TypeScriptServiceClientHost extends Disposable { private async diagnosticsReceived( kind: DiagnosticKind, resource: vscode.Uri, - diagnostics: Proto.Diagnostic[] + diagnostics: Proto.Diagnostic[], + spans: Proto.TextSpan[] | undefined, ): Promise { const language = await this.findLanguage(resource); if (language) { language.diagnosticsReceived( kind, resource, - this.createMarkerDatas(diagnostics, language.diagnosticSource)); + this.createMarkerDatas(diagnostics, language.diagnosticSource), + spans?.map(span => typeConverters.Range.fromTextSpan(span))); } } diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index 24742f99219..da6408b827b 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/extensions/typescript-language-features/src/typescriptServiceClient.ts @@ -37,6 +37,7 @@ export interface TsDiagnostics { readonly kind: DiagnosticKind; readonly resource: vscode.Uri; readonly diagnostics: Proto.Diagnostic[]; + readonly spans?: Proto.TextSpan[]; } interface ToCancelOnResourceChanged { @@ -947,7 +948,8 @@ export default class TypeScriptServiceClient extends Disposable implements IType switch (event.event) { case EventName.syntaxDiag: case EventName.semanticDiag: - case EventName.suggestionDiag: { + case EventName.suggestionDiag: + case EventName.regionSemanticDiag: { // This event also roughly signals that projects have been loaded successfully (since the TS server is synchronous) this.loadingIndicator.reset(); @@ -956,7 +958,9 @@ export default class TypeScriptServiceClient extends Disposable implements IType this._onDiagnosticsReceived.fire({ kind: getDiagnosticsKind(event), resource: this.toResource(diagnosticEvent.body.file), - diagnostics: diagnosticEvent.body.diagnostics + diagnostics: diagnosticEvent.body.diagnostics, + // @ts-expect-error until ts 5.6 + spans: diagnosticEvent.body.spans, }); } break; @@ -1261,6 +1265,7 @@ function getDiagnosticsKind(event: Proto.Event) { case 'syntaxDiag': return DiagnosticKind.Syntax; case 'semanticDiag': return DiagnosticKind.Semantic; case 'suggestionDiag': return DiagnosticKind.Suggestion; + case 'regionSemanticDiag': return DiagnosticKind.RegionSemantic; } throw new Error('Unknown dignostics kind'); }