Skip to content

Commit

Permalink
Add mode flags and context to diagnostic pull
Browse files Browse the repository at this point in the history
  • Loading branch information
@dbaeumer
dbaeumer committed Feb 17, 2021
1 parent 92688d6 commit 645eb3d
Show file tree
Hide file tree
Showing 11 changed files with 252 additions and 80 deletions.
1 change: 1 addition & 0 deletions .vscode/settings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"ASAR",
"LSIF",
"Struct",
"Undispatched",
"Unregistration",
"XCOPY",
"asColorInformations",
Expand Down
24 changes: 18 additions & 6 deletions client/src/common/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2676,6 +2676,14 @@ class OnReady {
}
}

export class LSPCancellationError extends CancellationError {
public readonly data: object | Object;
constructor(data: object | Object) {
super();
this.data = data;
}
}

export abstract class BaseLanguageClient {

private _id: string;
Expand Down Expand Up @@ -2973,7 +2981,7 @@ export abstract class BaseLanguageClient {
});
}

private data2String(data: any): string {
private data2String(data: Object): string {
if (data instanceof ResponseError) {
const responseError = data as ResponseError<any>;
return ` Message: ${responseError.message}\n Code: ${responseError.code} ${responseError.data ? '\n' + responseError.data.toString() : ''}`;
Expand All @@ -2992,7 +3000,7 @@ export abstract class BaseLanguageClient {

public info(message: string, data?: any, showNotification: boolean = true): void {
this.outputChannel.appendLine(`[Info - ${(new Date().toLocaleTimeString())}] ${message}`);
if (data) {
if (data !== null && data !== undefined) {
this.outputChannel.appendLine(this.data2String(data));
}
if (showNotification && this._clientOptions.revealOutputChannelOn <= RevealOutputChannelOn.Info) {
Expand All @@ -3002,7 +3010,7 @@ export abstract class BaseLanguageClient {

public warn(message: string, data?: any, showNotification: boolean = true): void {
this.outputChannel.appendLine(`[Warn - ${(new Date().toLocaleTimeString())}] ${message}`);
if (data) {
if (data !== null && data !== undefined) {
this.outputChannel.appendLine(this.data2String(data));
}
if (showNotification && this._clientOptions.revealOutputChannelOn <= RevealOutputChannelOn.Warn) {
Expand All @@ -3012,7 +3020,7 @@ export abstract class BaseLanguageClient {

public error(message: string, data?: any, showNotification: boolean = true): void {
this.outputChannel.appendLine(`[Error - ${(new Date().toLocaleTimeString())}] ${message}`);
if (data) {
if (data !== null && data !== undefined) {
this.outputChannel.appendLine(this.data2String(data));
}
if (showNotification && this._clientOptions.revealOutputChannelOn <= RevealOutputChannelOn.Error) {
Expand Down Expand Up @@ -3733,11 +3741,15 @@ export abstract class BaseLanguageClient {
public handleFailedRequest<T>(type: MessageSignature, token: CancellationToken | undefined, error: any, defaultValue: T): T {
// If we get a request cancel or a content modified don't log anything.
if (error instanceof ResponseError) {
if (error.code === LSPErrorCodes.RequestCancelled) {
if (error.code === LSPErrorCodes.RequestCancelled || error.code === LSPErrorCodes.ServerCancelled) {
if (token !== undefined && token.isCancellationRequested) {
return defaultValue;
} else {
throw new CancellationError();
if (error.data !== undefined) {
throw new LSPCancellationError(error.data);
} else {
throw new CancellationError();
}
}
} else if (error.code === LSPErrorCodes.ContentModified) {
if (BaseLanguageClient.RequestsToCancelOnContentModified.has(type.method)) {
Expand Down
110 changes: 70 additions & 40 deletions client/src/common/proposed.diagnostic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import {
Disposable, languages as Languages, window as Window, CancellationToken, ProviderResult,
Diagnostic as VDiagnostic, CancellationTokenSource, TextDocument
Diagnostic as VDiagnostic, CancellationTokenSource, TextDocument, CancellationError
} from 'vscode';

import {
Expand All @@ -15,7 +15,7 @@ import {

import {
TextDocumentFeature, BaseLanguageClient, Middleware, DidOpenTextDocumentFeatureShape, DidChangeTextDocumentFeatureShape, DidSaveTextDocumentFeatureShape,
DidCloseTextDocumentFeatureShape
DidCloseTextDocumentFeatureShape, LSPCancellationError
} from './client';

function ensure<T, K extends keyof T>(target: T, key: K): T[K] {
Expand All @@ -25,16 +25,20 @@ function ensure<T, K extends keyof T>(target: T, key: K): T[K] {
return target[key];
}

interface VDiagnosticList {
items: VDiagnostic[];
}

interface DiagnosticProvider {
provideDiagnostics (textDocument: TextDocument, token: CancellationToken): ProviderResult<VDiagnostic[]>;
provideDiagnostics (textDocument: TextDocument, context: Proposed.DiagnosticContext, token: CancellationToken): ProviderResult<VDiagnosticList>;
}

export interface ProvideDiagnosticSignature {
(this: void, textDocument: TextDocument, token: CancellationToken): ProviderResult<VDiagnostic[]>;
(this: void, textDocument: TextDocument, context: Proposed.DiagnosticContext, token: CancellationToken): ProviderResult<VDiagnosticList>;
}

export interface DiagnosticProviderMiddleware {
provideDiagnostics?: (this: void, document: TextDocument, token: CancellationToken, next: ProvideDiagnosticSignature) => ProviderResult<VDiagnostic[]>;
provideDiagnostics?: (this: void, document: TextDocument, context: Proposed.DiagnosticContext, token: CancellationToken, next: ProvideDiagnosticSignature) => ProviderResult<VDiagnosticList>;
}


Expand All @@ -47,16 +51,18 @@ enum RequestStateKind {
type RequestState = {
state: RequestStateKind.active;
textDocument: TextDocument;
trigger: Proposed.DiagnosticTriggerKind;
tokenSource: CancellationTokenSource;
} | {
state: RequestStateKind.reschedule;
textDocument: TextDocument;
trigger: Proposed.DiagnosticTriggerKind;
} | {
state: RequestStateKind.outDated;
textDocument: TextDocument;
};

export class DiagnosticFeature extends TextDocumentFeature<Proposed.DiagnosticOptions, Proposed.DiagnosticRegistrationOptions, DiagnosticProvider> {
export class DiagnosticFeature extends TextDocumentFeature<boolean | Proposed.DiagnosticOptions, Proposed.DiagnosticRegistrationOptions, DiagnosticProvider> {

private readonly openFeature: DidOpenTextDocumentFeatureShape;
private readonly changeFeature: DidChangeTextDocumentFeatureShape;
Expand Down Expand Up @@ -86,7 +92,7 @@ export class DiagnosticFeature extends TextDocumentFeature<Proposed.DiagnosticOp

protected registerLanguageProvider(options: Proposed.DiagnosticRegistrationOptions): [Disposable, DiagnosticProvider] {
const documentSelector = options.documentSelector!;
const mode = Proposed.DiagnosticPullMode.is(options.mode) ? options.mode : Proposed.DiagnosticPullMode.onType;
const mode = Proposed.DiagnosticPullModeFlags.is(options.mode) ? options.mode : (Proposed.DiagnosticPullModeFlags.onOpen | Proposed.DiagnosticPullModeFlags.onType);
const disposables: Disposable[] = [];
const collection = Languages.createDiagnosticCollection(options.identifier);
disposables.push(collection);
Expand All @@ -102,57 +108,74 @@ export class DiagnosticFeature extends TextDocumentFeature<Proposed.DiagnosticOp
};

const provider: DiagnosticProvider = {
provideDiagnostics: (textDocument, token) => {
provideDiagnostics: (textDocument, context, token) => {
const client = this._client;
const provideDiagnostics: ProvideDiagnosticSignature = (textDocument, token) => {
const provideDiagnostics: ProvideDiagnosticSignature = (textDocument, context, token) => {
const params: Proposed.DiagnosticParams = {
textDocument: { uri: client.code2ProtocolConverter.asUri(textDocument.uri) }
textDocument: { uri: client.code2ProtocolConverter.asUri(textDocument.uri) },
context: context
};
return client.sendRequest(Proposed.DiagnosticRequest.type, params, token).then((result) => {
if (result === null) {
return [];
if (result === null || !result.items) {
return { items: [] };
}
return client.protocol2CodeConverter.asDiagnostics(result);
return { items: client.protocol2CodeConverter.asDiagnostics(result.items) };
}, (error) => {
return client.handleFailedRequest(Proposed.DiagnosticRequest.type, token, error, []);
return client.handleFailedRequest(Proposed.DiagnosticRequest.type, token, error, { items: [] });
});
};
const middleware: Middleware & DiagnosticProviderMiddleware = client.clientOptions.middleware!;
return middleware.provideDiagnostics
? middleware.provideDiagnostics(textDocument, token, provideDiagnostics)
: provideDiagnostics(textDocument, token);
? middleware.provideDiagnostics(textDocument, context, token, provideDiagnostics)
: provideDiagnostics(textDocument, context, token);
}
};

const requestStates: Map<string, RequestState> = new Map();
const pullDiagnostics = async (textDocument: TextDocument): Promise<void> => {
const pullDiagnostics = async (textDocument: TextDocument, trigger: Proposed.DiagnosticTriggerKind): Promise<void> => {
const key = textDocument.uri.toString();
const currentState = requestStates.get(key);
if (currentState !== undefined) {
if (currentState.state === RequestStateKind.active) {
currentState.tokenSource.cancel();
requestStates.set(key, { state: RequestStateKind.reschedule, textDocument });
}
requestStates.set(key, { state: RequestStateKind.reschedule, textDocument, trigger: trigger });
// We have a state. Wait until the request returns.
return;
}
const tokenSource = new CancellationTokenSource();
requestStates.set(key, { state: RequestStateKind.active, textDocument, tokenSource });
const diagnostics = await provider.provideDiagnostics(textDocument, tokenSource.token) ?? [];
const afterState = requestStates.get(key);
requestStates.set(key, { state: RequestStateKind.active, textDocument, trigger, tokenSource });
let diagnostics: VDiagnosticList | undefined;
let afterState: RequestState | undefined;
try {
diagnostics = await provider.provideDiagnostics(textDocument, { triggerKind: trigger }, tokenSource.token) ?? { items: [] };
} catch (error: unknown) {
if (error instanceof LSPCancellationError && Proposed.DiagnosticServerCancellationData.is(error.data) && error.data.retriggerRequest === false) {
afterState = { state: RequestStateKind.outDated, textDocument };
}
if (afterState === undefined && error instanceof CancellationError) {
afterState = { state: RequestStateKind.reschedule, textDocument, trigger };
} else {
throw error;
}
}
afterState = afterState ?? requestStates.get(key);
if (afterState === undefined) {
// This shouldn't happen. Log it
this._client.error(`Lost request state in diagnostic pull model. Clearing diagnostics for ${key}`);
collection.delete(textDocument.uri);
return;
}
requestStates.delete(key);
if (afterState.state === RequestStateKind.outDated) {
if (afterState.state === RequestStateKind.outDated || !manages(textDocument)) {
return;
}
collection.set(textDocument.uri, diagnostics);
// diagnostics is only undefined if the request has thrown.
if (diagnostics !== undefined) {
collection.set(textDocument.uri, diagnostics.items);
}
if (afterState.state === RequestStateKind.reschedule) {
pullDiagnostics(textDocument);
pullDiagnostics(textDocument, afterState.trigger);
}
};

Expand All @@ -165,37 +188,44 @@ export class DiagnosticFeature extends TextDocumentFeature<Proposed.DiagnosticOp
openEditorsHandler();
disposables.push(Window.onDidChangeOpenEditors(openEditorsHandler));

disposables.push(this.openFeature.onNotificationSent((event) => {
const textDocument = event.original;
if (matches(textDocument)) {
managedDocuments.add(textDocument.uri.toString());
pullDiagnostics(event.original);
}
}));
// Pull all diagnostics for documents that are already open
for (const textDocument of this.openFeature.openDocuments) {
if (matches(textDocument)) {
managedDocuments.add(textDocument.uri.toString());
pullDiagnostics(textDocument);
if (Proposed.DiagnosticPullModeFlags.isOpen(mode)) {
disposables.push(this.openFeature.onNotificationSent((event) => {
const textDocument = event.original;
if (matches(textDocument)) {
managedDocuments.add(textDocument.uri.toString());
pullDiagnostics(event.original, Proposed.DiagnosticTriggerKind.Opened);
}
}));
// Pull all diagnostics for documents that are already open
for (const textDocument of this.openFeature.openDocuments) {
if (matches(textDocument)) {
managedDocuments.add(textDocument.uri.toString());
pullDiagnostics(textDocument, Proposed.DiagnosticTriggerKind.Opened);
}
}
}
if (mode === Proposed.DiagnosticPullMode.onType) {
if (Proposed.DiagnosticPullModeFlags.isType(mode)) {
disposables.push(this.changeFeature.onNotificationSent((event) => {
const textDocument = event.original.document;
if (manages(textDocument) && event.original.contentChanges.length > 0) {
pullDiagnostics(textDocument);
pullDiagnostics(textDocument, Proposed.DiagnosticTriggerKind.Typed);
}
}));
} else if (mode === Proposed.DiagnosticPullMode.onSave) {
}
if (Proposed.DiagnosticPullModeFlags.isSave(mode)) {
disposables.push(this.saveFeature.onNotificationSent((event) => {
const textDocument = event.original;
if (manages(textDocument)) {
pullDiagnostics(event.original);
pullDiagnostics(event.original, Proposed.DiagnosticTriggerKind.Saved);
}
}));
}
disposables.push(this.closeFeature.onNotificationSent((event) => {
const textDocument = event.original;
const requestState = requestStates.get(textDocument.uri.toString());
if (requestState !== undefined) {
requestStates.set(textDocument.uri.toString(),{ state: RequestStateKind.outDated, textDocument });
}
if (manages(textDocument)) {
collection.delete(textDocument.uri);
managedDocuments.delete(textDocument.uri.toString());
Expand Down
12 changes: 6 additions & 6 deletions jsonrpc/src/common/connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class ProgressType<PR> {
}
}

export type HandlerResult<R, E> = R | ResponseError<E> | Thenable<R> | Thenable<ResponseError<E>> | Thenable<R | ResponseError<E>>;
export type HandlerResult<R, E> = R | Thenable<R> | Thenable<ResponseError<E>> | Thenable<R | ResponseError<E>>;

export interface StarRequestHandler {
(method: string, params: any[] | object | undefined, token: CancellationToken): HandlerResult<any, any>;
Expand Down Expand Up @@ -477,8 +477,8 @@ export function createMessageConnection(messageReader: MessageReader, messageWri
const logger: Logger = _logger !== undefined ? _logger : NullLogger;

let sequenceNumber = 0;
let notificationSquenceNumber = 0;
let unknownResponseSquenceNumber = 0;
let notificationSequenceNumber = 0;
let unknownResponseSequenceNumber = 0;
const version: string = '2.0';

let starRequestHandler: StarRequestHandler | undefined = undefined;
Expand Down Expand Up @@ -515,14 +515,14 @@ export function createMessageConnection(messageReader: MessageReader, messageWri

function createResponseQueueKey(id: string | number | null): string {
if (id === null) {
return 'res-unknown-' + (++unknownResponseSquenceNumber).toString();
return 'res-unknown-' + (++unknownResponseSequenceNumber).toString();
} else {
return 'res-' + id.toString();
}
}

function createNotificationQueueKey(): string {
return 'not-' + (++notificationSquenceNumber).toString();
return 'not-' + (++notificationSequenceNumber).toString();
}

function addMessageToQueue(queue: MessageQueue, message: Message): void {
Expand Down Expand Up @@ -839,7 +839,7 @@ export function createMessageConnection(messageReader: MessageReader, messageWri
logger.error(`Notification ${message.method} defines parameters by name but received parameters by position`);
}
if (type.numberOfParams !== message.params.length) {
logger.error(`Notification ${message.method} defines ${type.numberOfParams} params but received ${message.params.length} argumennts`);
logger.error(`Notification ${message.method} defines ${type.numberOfParams} params but received ${message.params.length} arguments`);
}
}
notificationHandler(...message.params);
Expand Down
17 changes: 10 additions & 7 deletions jsonrpc/src/common/messages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ export namespace ErrorCodes {
export const serverErrorEnd: number = jsonrpcReservedErrorRangeEnd;
}

export interface ResponseErrorLiteral<D> {
export interface ResponseErrorLiteral<D = void> {
/**
* A number indicating the error type that occured.
* A number indicating the error type that occurred.
*/
code: number;

/**
* A string providing a short decription of the error.
* A string providing a short description of the error.
*/
message: string;

Expand All @@ -96,7 +96,7 @@ export interface ResponseErrorLiteral<D> {
* An error object return in a response in case a request
* has failed.
*/
export class ResponseError<D> extends Error {
export class ResponseError<D = void> extends Error {

public readonly code: number;
public readonly data: D | undefined;
Expand All @@ -109,11 +109,14 @@ export class ResponseError<D> extends Error {
}

public toJson(): ResponseErrorLiteral<D> {
return {
const result: ResponseErrorLiteral<D> = {
code: this.code,
message: this.message,
data: this.data,
message: this.message
};
if (this.data !== undefined) {
result.data = this.data;
}
return result;
}
}

Expand Down
Loading

0 comments on commit 645eb3d

Please sign in to comment.