From aee00a209b0b3855e88f44a0fce7750fc8650d35 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 20 Mar 2018 10:35:46 -0700 Subject: [PATCH] Ts suggest diagnostics (#45433) * Support basic suggest diagnostics from TS * Removing duplicate diagnostic paths * Avoid duplicate code around handling multiple diagnostic kinds --- .../typescript/src/features/diagnostics.ts | 51 +++++++++++------- .../src/features/quickFixProvider.ts | 2 +- extensions/typescript/src/languageProvider.ts | 10 ++-- extensions/typescript/src/protocol.const.ts | 2 +- .../src/typeScriptServiceClientHost.ts | 28 +++++----- .../typescript/src/typescriptServiceClient.ts | 52 +++++++++---------- 6 files changed, 78 insertions(+), 67 deletions(-) diff --git a/extensions/typescript/src/features/diagnostics.ts b/extensions/typescript/src/features/diagnostics.ts index 2972fbf5857..57401ef0eef 100644 --- a/extensions/typescript/src/features/diagnostics.ts +++ b/extensions/typescript/src/features/diagnostics.ts @@ -30,18 +30,27 @@ class DiagnosticSet { } } -export default class DiagnosticsManager { +export enum DiagnosticKind { + Syntax, + Semantic, + Suggestion +} - private readonly syntaxDiagnostics: DiagnosticSet; - private readonly semanticDiagnostics: DiagnosticSet; +const allDiagnosticKinds = [DiagnosticKind.Syntax, DiagnosticKind.Semantic, DiagnosticKind.Suggestion]; + +export class DiagnosticsManager { + + private readonly diagnostics = new Map(); private readonly currentDiagnostics: DiagnosticCollection; private _validate: boolean = true; constructor( language: string ) { - this.syntaxDiagnostics = new DiagnosticSet(); - this.semanticDiagnostics = new DiagnosticSet(); + for (const kind of allDiagnosticKinds) { + this.diagnostics.set(kind, new DiagnosticSet()); + } + this.currentDiagnostics = languages.createDiagnosticCollection(language); } @@ -51,8 +60,10 @@ export default class DiagnosticsManager { public reInitialize(): void { this.currentDiagnostics.clear(); - this.syntaxDiagnostics.clear(); - this.semanticDiagnostics.clear(); + + for (const diagnosticSet of this.diagnostics.values()) { + diagnosticSet.clear(); + } } public set validate(value: boolean) { @@ -65,14 +76,16 @@ export default class DiagnosticsManager { } } - public syntaxDiagnosticsReceived(file: Uri, syntaxDiagnostics: Diagnostic[]): void { - this.syntaxDiagnostics.set(file, syntaxDiagnostics); - this.updateCurrentDiagnostics(file); - } - - public semanticDiagnosticsReceived(file: Uri, semanticDiagnostics: Diagnostic[]): void { - this.semanticDiagnostics.set(file, semanticDiagnostics); - this.updateCurrentDiagnostics(file); + public diagnosticsReceived( + kind: DiagnosticKind, + file: Uri, + syntaxDiagnostics: Diagnostic[] + ): void { + const diagnostics = this.diagnostics.get(kind); + if (diagnostics) { + diagnostics.set(file, syntaxDiagnostics); + this.updateCurrentDiagnostics(file); + } } public configFileDiagnosticsReceived(file: Uri, diagnostics: Diagnostic[]): void { @@ -88,9 +101,11 @@ export default class DiagnosticsManager { return; } - const semanticDiagnostics = this.semanticDiagnostics.get(file); - const syntaxDiagnostics = this.syntaxDiagnostics.get(file); - this.currentDiagnostics.set(file, semanticDiagnostics.concat(syntaxDiagnostics)); + const allDiagnostics = allDiagnosticKinds.reduce((sum, kind) => { + sum.push(...this.diagnostics.get(kind)!.get(file)); + return sum; + }, [] as Diagnostic[]); + this.currentDiagnostics.set(file, allDiagnostics); } public getDiagnostics(file: Uri): Diagnostic[] { diff --git a/extensions/typescript/src/features/quickFixProvider.ts b/extensions/typescript/src/features/quickFixProvider.ts index e17b8ab2350..8714241d850 100644 --- a/extensions/typescript/src/features/quickFixProvider.ts +++ b/extensions/typescript/src/features/quickFixProvider.ts @@ -11,7 +11,7 @@ import * as typeConverters from '../utils/typeConverters'; import FormattingConfigurationManager from './formattingConfigurationManager'; import { getEditForCodeAction, applyCodeActionCommands } from '../utils/codeAction'; import { Command, CommandManager } from '../utils/commandManager'; -import DiagnosticsManager from './diagnostics'; +import { DiagnosticsManager } from './diagnostics'; import * as nls from 'vscode-nls'; const localize = nls.loadMessageBundle(); diff --git a/extensions/typescript/src/languageProvider.ts b/extensions/typescript/src/languageProvider.ts index f2007577e9f..cddd5e3fd30 100644 --- a/extensions/typescript/src/languageProvider.ts +++ b/extensions/typescript/src/languageProvider.ts @@ -14,7 +14,7 @@ import TypingsStatus from './utils/typingsStatus'; import FormattingConfigurationManager from './features/formattingConfigurationManager'; import * as languageConfigurations from './utils/languageConfigurations'; import { CommandManager } from './utils/commandManager'; -import DiagnosticsManager from './features/diagnostics'; +import { DiagnosticsManager, DiagnosticKind } from './features/diagnostics'; import { LanguageDescription } from './utils/languageDescription'; import * as fileSchemes from './utils/fileSchemes'; import { CachedNavTreeResponse } from './features/baseCodeLensProvider'; @@ -233,12 +233,8 @@ export default class LanguageProvider { this.bufferSyncSupport.requestAllDiagnostics(); } - public syntaxDiagnosticsReceived(file: Uri, syntaxDiagnostics: Diagnostic[]): void { - this.diagnosticsManager.syntaxDiagnosticsReceived(file, syntaxDiagnostics); - } - - public semanticDiagnosticsReceived(file: Uri, semanticDiagnostics: Diagnostic[]): void { - this.diagnosticsManager.semanticDiagnosticsReceived(file, semanticDiagnostics); + public diagnosticsReceived(diagnosticsKind: DiagnosticKind, file: Uri, syntaxDiagnostics: Diagnostic[]): void { + this.diagnosticsManager.diagnosticsReceived(diagnosticsKind, file, syntaxDiagnostics); } public configFileDiagnosticsReceived(file: Uri, diagnostics: Diagnostic[]): void { diff --git a/extensions/typescript/src/protocol.const.ts b/extensions/typescript/src/protocol.const.ts index 822e05d2b21..25cb015316b 100644 --- a/extensions/typescript/src/protocol.const.ts +++ b/extensions/typescript/src/protocol.const.ts @@ -36,6 +36,6 @@ export class Kind { export class DiagnosticCategory { public static readonly error = 'error'; - public static readonly warning = 'warning'; + public static readonly suggestion = 'suggestion'; } \ No newline at end of file diff --git a/extensions/typescript/src/typeScriptServiceClientHost.ts b/extensions/typescript/src/typeScriptServiceClientHost.ts index 38c3826f22e..7e3b170d968 100644 --- a/extensions/typescript/src/typeScriptServiceClientHost.ts +++ b/extensions/typescript/src/typeScriptServiceClientHost.ts @@ -24,6 +24,7 @@ import { CommandManager } from './utils/commandManager'; import { LanguageDescription } from './utils/languageDescription'; import LogDirectoryProvider from './utils/logDirectoryProvider'; import { disposeAll } from './utils/dipose'; +import { DiagnosticKind } from './features/diagnostics'; // Style check diagnostics that can be reported as warnings const styleCheckDiagnostics = [ @@ -70,8 +71,10 @@ export default class TypeScriptServiceClientHost { this.client = new TypeScriptServiceClient(workspaceState, version => this.versionStatus.onDidChangeTypeScriptVersion(version), plugins, logDirectoryProvider); this.disposables.push(this.client); - this.client.onSyntaxDiagnosticsReceived(({ resource, diagnostics }) => this.syntaxDiagnosticsReceived(resource, diagnostics), null, this.disposables); - this.client.onSemanticDiagnosticsReceived(({ resource, diagnostics }) => this.semanticDiagnosticsReceived(resource, diagnostics), null, this.disposables); + this.client.onDiagnosticsReceived(({ kind, resource, diagnostics }) => { + this.diagnosticsReceived(kind, resource, diagnostics); + }, null, this.disposables); + this.client.onConfigDiagnosticsReceived(diag => this.configFileDiagnosticsReceived(diag), null, this.disposables); this.client.onResendModelsRequested(() => this.populateService(), null, this.disposables); @@ -170,19 +173,15 @@ export default class TypeScriptServiceClientHost { }); } - private async syntaxDiagnosticsReceived(resource: Uri, diagnostics: Proto.Diagnostic[]): Promise { + private async diagnosticsReceived( + kind: DiagnosticKind, + resource: Uri, + diagnostics: Proto.Diagnostic[] + ): Promise { const language = await this.findLanguage(resource); if (language) { - language.syntaxDiagnosticsReceived( - resource, - this.createMarkerDatas(diagnostics, language.diagnosticSource)); - } - } - - private async semanticDiagnosticsReceived(resource: Uri, diagnostics: Proto.Diagnostic[]): Promise { - const language = await this.findLanguage(resource); - if (language) { - language.semanticDiagnosticsReceived( + language.diagnosticsReceived( + kind, resource, this.createMarkerDatas(diagnostics, language.diagnosticSource)); } @@ -267,6 +266,9 @@ export default class TypeScriptServiceClientHost { case PConst.DiagnosticCategory.warning: return DiagnosticSeverity.Warning; + case PConst.DiagnosticCategory.suggestion: + return DiagnosticSeverity.Hint; + default: return DiagnosticSeverity.Error; } diff --git a/extensions/typescript/src/typescriptServiceClient.ts b/extensions/typescript/src/typescriptServiceClient.ts index 61cfe79967d..b1798d93dd4 100644 --- a/extensions/typescript/src/typescriptServiceClient.ts +++ b/extensions/typescript/src/typescriptServiceClient.ts @@ -29,6 +29,7 @@ import * as fileSchemes from './utils/fileSchemes'; import { inferredProjectConfig } from './utils/tsconfig'; import LogDirectoryProvider from './utils/logDirectoryProvider'; import { disposeAll } from './utils/dipose'; +import { DiagnosticKind } from './features/diagnostics'; const localize = nls.loadMessageBundle(); @@ -140,6 +141,7 @@ class ForkedTsServerProcess { } export interface TsDiagnostics { + readonly kind: DiagnosticKind; readonly resource: Uri; readonly diagnostics: Proto.Diagnostic[]; } @@ -237,11 +239,8 @@ export default class TypeScriptServiceClient implements ITypeScriptServiceClient this.disposables.push(this.telemetryReporter); } - private _onSyntaxDiagnosticsReceived = new EventEmitter(); - public get onSyntaxDiagnosticsReceived(): Event { return this._onSyntaxDiagnosticsReceived.event; } - - private _onSemanticDiagnosticsReceived = new EventEmitter(); - public get onSemanticDiagnosticsReceived(): Event { return this._onSemanticDiagnosticsReceived.event; } + private _onDiagnosticsReceived = new EventEmitter(); + public get onDiagnosticsReceived(): Event { return this._onDiagnosticsReceived.event; } private _onConfigDiagnosticsReceived = new EventEmitter(); public get onConfigDiagnosticsReceived(): Event { return this._onConfigDiagnosticsReceived.event; } @@ -266,8 +265,7 @@ export default class TypeScriptServiceClient implements ITypeScriptServiceClient } disposeAll(this.disposables); - this._onSyntaxDiagnosticsReceived.dispose(); - this._onSemanticDiagnosticsReceived.dispose(); + this._onDiagnosticsReceived.dispose(); this._onConfigDiagnosticsReceived.dispose(); this._onResendModelsRequested.dispose(); } @@ -803,27 +801,18 @@ export default class TypeScriptServiceClient implements ITypeScriptServiceClient private dispatchEvent(event: Proto.Event) { switch (event.event) { case 'syntaxDiag': - { - const diagnosticEvent: Proto.DiagnosticEvent = event; - if (diagnosticEvent.body && diagnosticEvent.body.diagnostics) { - this._onSyntaxDiagnosticsReceived.fire({ - resource: this.asUrl(diagnosticEvent.body.file), - diagnostics: diagnosticEvent.body.diagnostics - }); - } - break; - } case 'semanticDiag': - { - const diagnosticEvent: Proto.DiagnosticEvent = event; - if (diagnosticEvent.body && diagnosticEvent.body.diagnostics) { - this._onSemanticDiagnosticsReceived.fire({ - resource: this.asUrl(diagnosticEvent.body.file), - diagnostics: diagnosticEvent.body.diagnostics - }); - } - break; + case 'suggestionDiag': + const diagnosticEvent: Proto.DiagnosticEvent = event; + if (diagnosticEvent.body && diagnosticEvent.body.diagnostics) { + this._onDiagnosticsReceived.fire({ + kind: getDignosticsKind(event), + resource: this.asUrl(diagnosticEvent.body.file), + diagnostics: diagnosticEvent.body.diagnostics + }); } + break; + case 'configFileDiag': this._onConfigDiagnosticsReceived.fire(event as Proto.ConfigFileDiagnosticEvent); break; @@ -994,4 +983,13 @@ export default class TypeScriptServiceClient implements ITypeScriptServiceClient const getTsLocale = (configuration: TypeScriptServiceConfiguration): string | undefined => (configuration.locale ? configuration.locale - : env.language); \ No newline at end of file + : env.language); + +function getDignosticsKind(event: Proto.Event) { + switch (event.event) { + case 'syntaxDiag': return DiagnosticKind.Syntax; + case 'semanticDiag': return DiagnosticKind.Semantic; + case 'suggestionDiag': return DiagnosticKind.Suggestion; + } + throw new Error('Unknown dignostics kind'); +}