From 182dfd85183ad42d626422fdb7907dca7fa8c2b0 Mon Sep 17 00:00:00 2001 From: Dirk Baeumer Date: Mon, 15 Jun 2020 22:15:28 +0200 Subject: [PATCH] First cut for full by position support --- .vscode/launch.json | 12 +- jsonrpc/src/common/connection.ts | 165 ++++++++++++++++------- jsonrpc/src/common/messages.ts | 40 +++++- jsonrpc/src/node/test/connection.test.ts | 5 +- tsconfig-watch.json | 2 +- 5 files changed, 159 insertions(+), 65 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 9be18f9ad..92b36cde3 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -14,7 +14,7 @@ "env": { }, "sourceMaps": true, "outFiles": ["${workspaceRoot}/types/lib/umd/**/*.js"], - "preLaunchTask": "watch:types" + "preLaunchTask": "npm: watch" }, { "request": "launch", @@ -29,7 +29,7 @@ "env": { }, "sourceMaps": true, "outFiles": ["${workspaceRoot}/textDocument/lib/umd/**/*.js"], - "preLaunchTask": "watch:textDocument" + "preLaunchTask": "npm: watch" }, { "request": "launch", @@ -44,7 +44,7 @@ "env": { }, "sourceMaps": true, "outFiles": ["${workspaceRoot}/jsonrpc/lib/**/*.js"], - "preLaunchTask": "watch:jsonrpc" + "preLaunchTask": "npm: watch" }, { "request": "launch", @@ -59,7 +59,7 @@ "env": { }, "sourceMaps": true, "outFiles": ["${workspaceRoot}/protocol/lib/**/*.js"], - "preLaunchTask": "watch:protocol" + "preLaunchTask": "npm: watch" }, { "request": "launch", @@ -74,7 +74,7 @@ "env": { }, "sourceMaps": true, "outFiles": ["${workspaceRoot}/server/lib/**/*.js"], - "preLaunchTask": "watch:server" + "preLaunchTask": "npm: watch" }, { "type": "extensionHost", @@ -85,7 +85,7 @@ "stopOnEntry": false, "sourceMaps": true, "outFiles": ["${workspaceRoot}/client-tests/lib/test/**/*.js"], - "preLaunchTask": "watch:client" + "preLaunchTask": "npm: watch" }, { "type": "extensionHost", diff --git a/jsonrpc/src/common/connection.ts b/jsonrpc/src/common/connection.ts index 2713f6042..1097315d1 100644 --- a/jsonrpc/src/common/connection.ts +++ b/jsonrpc/src/common/connection.ts @@ -11,7 +11,7 @@ import { RequestType4, RequestType5, RequestType6, RequestType7, RequestType8, RequestType9, ResponseMessage, isResponseMessage, ResponseError, ErrorCodes, NotificationMessage, isNotificationMessage, NotificationType, NotificationType0, NotificationType1, NotificationType2, NotificationType3, NotificationType4, NotificationType5, NotificationType6, NotificationType7, NotificationType8, - NotificationType9, LSPMessageType, _EM + NotificationType9, LSPMessageType, _EM, ParameterStructures } from './messages'; import { LinkedMap } from './linkedMap'; @@ -62,7 +62,13 @@ export class ProgressType

{ export type HandlerResult = R | ResponseError | Thenable | Thenable> | Thenable>; export interface StarRequestHandler { - (method: string, ...params: any[]): HandlerResult; + (method: string, params: any[] | object | undefined, token: CancellationToken): HandlerResult; +} + +namespace StarRequestHandler { + export function is(value: any): value is StarRequestHandler { + return Is.func(value); + } } export interface GenericRequestHandler { @@ -114,7 +120,7 @@ export interface RequestHandler9 { } export interface StarNotificationHandler { - (method: string, ...params: any[]): void; + (method: string, params: any[] | object | undefined): void; } export interface GenericNotificationHandler { @@ -373,7 +379,7 @@ export interface MessageConnection { sendRequest(type: RequestType7, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7, token?: CancellationToken): Promise; sendRequest(type: RequestType8, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7, p8: P8, token?: CancellationToken): Promise; sendRequest(type: RequestType9, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7, p8: P8, p9: P9, token?: CancellationToken): Promise; - sendRequest(method: string, ...params: any[]): Promise; + sendRequest(method: string, r0?: ParameterStructures | any, ...rest: any[]): Promise; onRequest(type: RequestType0, handler: RequestHandler0): void; onRequest(type: RequestType, handler: RequestHandler): void; @@ -400,7 +406,7 @@ export interface MessageConnection { sendNotification(type: NotificationType7, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7): void; sendNotification(type: NotificationType8, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7, p8: P8): void; sendNotification(type: NotificationType9, p1: P1, p2: P2, p3: P3, p4: P4, p5: P5, p6: P6, p7: P7, p8: P8, p9: P9): void; - sendNotification(method: string, ...params: any[]): void; + sendNotification(method: string, r0?: ParameterStructures | any, ...rest: any[]): void; onNotification(type: NotificationType0, handler: NotificationHandler0): void; onNotification(type: NotificationType, handler: NotificationHandler

): void; @@ -672,18 +678,28 @@ export function createMessageConnection(messageReader: MessageReader, messageWri requestTokens[tokenKey] = cancellationSource; try { let handlerResult: any; - if (requestMessage.params === undefined || (type !== undefined && type.numberOfParams === 0)) { - handlerResult = requestHandler - ? requestHandler(cancellationSource.token) - : starRequestHandler!(requestMessage.method, cancellationSource.token); - } else if (Is.array(requestMessage.params) && (type === undefined || type.numberOfParams > 1)) { - handlerResult = requestHandler - ? requestHandler(...requestMessage.params, cancellationSource.token) - : starRequestHandler!(requestMessage.method, ...requestMessage.params, cancellationSource.token); - } else { - handlerResult = requestHandler - ? requestHandler(requestMessage.params, cancellationSource.token) - : starRequestHandler!(requestMessage.method, requestMessage.params, cancellationSource.token); + if (requestHandler) { + if (requestMessage.params === undefined) { + if (type !== undefined && type.numberOfParams !== 0) { + replyError(new ResponseError(ErrorCodes.InvalidParams, `Request ${requestMessage.method} defines ${type.numberOfParams} params but recevied none.`), requestMessage.method, startTime); + return; + } + handlerResult = requestHandler(cancellationSource.token); + } else if (Array.isArray(requestMessage.params)) { + if (type !== undefined && type.parameterStructures === ParameterStructures.byName) { + replyError(new ResponseError(ErrorCodes.InvalidParams, `Request ${requestMessage.method} defines parameters by name but received parameters by position`), requestMessage.method, startTime); + return; + } + handlerResult = requestHandler(...requestMessage.params, cancellationSource.token); + } else { + if (type !== undefined && type.parameterStructures === ParameterStructures.byPosition) { + replyError(new ResponseError(ErrorCodes.InvalidParams, `Request ${requestMessage.method} defines parameters by position but received parameters by name`), requestMessage.method, startTime); + return; + } + handlerResult = requestHandler(requestMessage.params, cancellationSource.token); + } + } else if (starRequestHandler) { + handlerResult = starRequestHandler(requestMessage.method, requestMessage.params, cancellationSource.token); } const promise = handlerResult as Thenable>; @@ -787,12 +803,25 @@ export function createMessageConnection(messageReader: MessageReader, messageWri if (notificationHandler || starNotificationHandler) { try { traceReceivedNotification(message); - if (message.params === undefined || (type !== undefined && type.numberOfParams === 0)) { - notificationHandler ? notificationHandler() : starNotificationHandler!(message.method); - } else if (Is.array(message.params) && (type === undefined || type.numberOfParams > 1)) { - notificationHandler ? notificationHandler(...message.params) : starNotificationHandler!(message.method, ...message.params); - } else { - notificationHandler ? notificationHandler(message.params) : starNotificationHandler!(message.method, message.params); + if (notificationHandler) { + if (message.params === undefined) { + if (type !== undefined && type.numberOfParams !== 0) { + logger.error(`Notification ${message.method} defines ${type.numberOfParams} params but recevied none.`); + } + notificationHandler(); + } else if (Array.isArray(message.params)) { + if (type !== undefined && type.parameterStructures === ParameterStructures.byName) { + logger.error(`Notification ${message.method} defines parameters by name but received parameters by position`); + } + notificationHandler(...message.params); + } else { + if (type !== undefined && type.parameterStructures === ParameterStructures.byPosition) { + logger.error(`Notification ${message.method} defines parameters by position but received parameters by name`); + } + notificationHandler(message.params); + } + } else if (starNotificationHandler) { + starNotificationHandler(message.method, message.params); } } catch (error) { if (error.message) { @@ -1000,7 +1029,11 @@ export function createMessageConnection(messageReader: MessageReader, messageWri result = null; break; case 1: - result = undefinedToNull(params[0]); + if (type.parameterStructures === ParameterStructures.byPosition) { + result = [undefinedToNull(params[0])]; + } else { + result = undefinedToNull(params[0]); + } break; default: result = []; @@ -1018,25 +1051,43 @@ export function createMessageConnection(messageReader: MessageReader, messageWri } const connection: MessageConnection = { - sendNotification: (type: string | MessageSignature, ...params: any[]): void => { + sendNotification: (type: string | MessageSignature, ...args: any[]): void => { throwIfClosedOrDisposed(); let method: string; let messageParams: object | [] | undefined; if (Is.string(type)) { method = type; - switch (params.length) { + const first: unknown = args[0]; + let paramStart: number = 0; + let parameterStructures: ParameterStructures | undefined = undefined; + if (first === ParameterStructures.byName || first === ParameterStructures.byPosition) { + paramStart = 1; + parameterStructures = first as ParameterStructures; + } + let paramEnd: number = args.length; + const numberOfParams = paramEnd - paramStart; + switch (numberOfParams) { case 0: messageParams = undefined; break; case 1: - messageParams = params[0]; + const param = args[paramStart]; + if (parameterStructures === ParameterStructures.byPosition) { + messageParams = [undefinedToNull(param)]; + } else { + messageParams = undefinedToNull(param); + } break; default: - messageParams = params; + if (parameterStructures === ParameterStructures.byName) { + throw new Error(`Recevied ${numberOfParams} parameters for 'by Name' notification parameter structure.`); + } + messageParams = args.slice(paramStart, paramEnd).map(value => undefinedToNull(value)); break; } } else { + const params = args; method = type.method; messageParams = computeMessageParams(type, params); } @@ -1075,7 +1126,7 @@ export function createMessageConnection(messageReader: MessageReader, messageWri connection.sendNotification(ProgressNotification.type, { token, value }); }, onUnhandledProgress: unhandledProgressEmitter.event, - sendRequest: (type: string | MessageSignature, ...params: any[]) => { + sendRequest: (type: string | MessageSignature, ...args: any[]) => { throwIfClosedOrDisposed(); throwIfNotListening(); @@ -1084,34 +1135,44 @@ export function createMessageConnection(messageReader: MessageReader, messageWri let token: CancellationToken | undefined = undefined; if (Is.string(type)) { method = type; - switch (params.length) { + const first: unknown = args[0]; + const last: unknown = args[args.length - 1]; + let paramStart: number = 0; + let parameterStructures: ParameterStructures | undefined = undefined; + if (first === ParameterStructures.byName || first === ParameterStructures.byPosition) { + paramStart = 1; + parameterStructures = first as ParameterStructures; + } + let paramEnd: number = args.length; + if (CancellationToken.is(last)) { + paramEnd = paramEnd - 1; + token = last; + } + const numberOfParams = paramEnd - paramStart; + switch (numberOfParams) { case 0: messageParams = undefined; break; case 1: - // The cancellation token is optional so it can also be undefined. - if (CancellationToken.is(params[0])) { - messageParams = undefined; - token = params[0]; + const param = args[paramStart]; + if (parameterStructures === ParameterStructures.byPosition) { + messageParams = [undefinedToNull(param)]; } else { - messageParams = undefinedToNull(params[0]); + messageParams = undefinedToNull(param); + if (messageParams !== null && typeof messageParams !== 'object') { + throw new Error(`Recevied parameters by name but param is not an object literal.`); + } } break; default: - const last = params.length - 1; - if (CancellationToken.is(params[last])) { - token = params[last]; - if (params.length === 2) { - messageParams = undefinedToNull(params[0]); - } else { - messageParams = params.slice(0, last).map(value => undefinedToNull(value)); - } - } else { - messageParams = params.map(value => undefinedToNull(value)); + if (parameterStructures === ParameterStructures.byName) { + throw new Error(`Recevied ${numberOfParams} parameters for 'by Name' request parameter structure.`); } + messageParams = args.slice(paramStart, paramEnd).map(value => undefinedToNull(value)); break; } } else { + const params = args; method = type.method; messageParams = computeMessageParams(type, params); const numberOfParams = type.numberOfParams; @@ -1163,12 +1224,14 @@ export function createMessageConnection(messageReader: MessageReader, messageWri onRequest: (type: string | MessageSignature | StarRequestHandler, handler?: GenericRequestHandler): void => { throwIfClosedOrDisposed(); - if (Is.func(type)) { - starRequestHandler = type as StarRequestHandler; - } else if (handler) { - if (Is.string(type)) { - requestHandlers[type] = { type: undefined, handler }; - } else { + if (StarRequestHandler.is(type)) { + starRequestHandler = type; + } else if (Is.string(type)) { + if (handler !== undefined) { + requestHandlers[type] = { handler: handler, type: undefined }; + } + } else { + if (handler !== undefined) { requestHandlers[type.method] = { type, handler }; } } diff --git a/jsonrpc/src/common/messages.ts b/jsonrpc/src/common/messages.ts index ee70bff73..93fa6564a 100644 --- a/jsonrpc/src/common/messages.ts +++ b/jsonrpc/src/common/messages.ts @@ -30,7 +30,7 @@ export interface RequestMessage extends Message { /** * The method's params. */ - params?: [] | object + params?: any[] | object } /** @@ -138,12 +138,22 @@ export interface LSPLogMessage { timestamp: number; } +export class ParameterStructures { + public static readonly byPosition = new ParameterStructures('byPosition'); + public static readonly byName = new ParameterStructures('byName'); + + private constructor(private readonly kind: string) { + this.kind; + } +} + /** * An interface to type messages. */ export interface MessageSignature { readonly method: string; readonly numberOfParams: number; + readonly parameterStructures: ParameterStructures; } /** @@ -160,6 +170,10 @@ export abstract class AbstractMessageSignature implements MessageSignature { get numberOfParams(): number { return this._numberOfParams; } + + get parameterStructures(): ParameterStructures { + return ParameterStructures.byPosition; + } } /** @@ -191,9 +205,13 @@ export class RequestType extends AbstractMessageSignature { * Clients must not use this property. It is here to ensure correct typing. */ public readonly _?: [P, R, E, RO, _EM]; - constructor(method: string) { + constructor(method: string, private _parameterStructures: ParameterStructures = ParameterStructures.byName) { super(method, 1); } + + get parameterStructures(): ParameterStructures { + return this._parameterStructures; + } } export class RequestType1 extends AbstractMessageSignature { @@ -201,9 +219,13 @@ export class RequestType1 extends AbstractMessageSignature * Clients must not use this property. It is here to ensure correct typing. */ public readonly _?: [P1, R, E, RO, _EM]; - constructor(method: string) { + constructor(method: string, private _parameterStructures: ParameterStructures = ParameterStructures.byName) { super(method, 1); } + + get parameterStructures(): ParameterStructures { + return this._parameterStructures; + } } export class RequestType2 extends AbstractMessageSignature { @@ -311,10 +333,14 @@ export class NotificationType extends AbstractMessageSignature { * Clients must not use this property. It is here to ensure correct typing. */ public readonly _?: [P, RO, _EM]; - constructor(method: string) { + constructor(method: string, private _parameterStructures: ParameterStructures = ParameterStructures.byName) { super(method, 1); this._ = undefined; } + + get parameterStructures(): ParameterStructures { + return this._parameterStructures; + } } export class NotificationType0 extends AbstractMessageSignature { @@ -332,9 +358,13 @@ export class NotificationType1 extends AbstractMessageSignature * Clients must not use this property. It is here to ensure correct typing. */ public readonly _?: [P1, RO, _EM]; - constructor(method: string) { + constructor(method: string, private _parameterStructures: ParameterStructures = ParameterStructures.byName) { super(method, 1); } + + get parameterStructures(): ParameterStructures { + return this._parameterStructures; + } } export class NotificationType2 extends AbstractMessageSignature { diff --git a/jsonrpc/src/node/test/connection.test.ts b/jsonrpc/src/node/test/connection.test.ts index 90c84c6b1..696419599 100644 --- a/jsonrpc/src/node/test/connection.test.ts +++ b/jsonrpc/src/node/test/connection.test.ts @@ -12,6 +12,7 @@ import { CancellationTokenSource, RequestType, RequestType3, ResponseError, Noti import * as hostConnection from '../main'; import { getCustomCancellationStrategy } from './customCancellationStrategy'; +import { ParameterStructures } from '../../common/messages'; interface TestDuplex extends Duplex { } @@ -358,7 +359,7 @@ suite('Connection', () => { }); test('One Param as array in request', (done) => { - let type = new RequestType('add'); + let type = new RequestType('add', ParameterStructures.byPosition); let duplexStream1 = new TestDuplex('ds1'); let duplexStream2 = new TestDuplex('ds2'); @@ -385,7 +386,7 @@ suite('Connection', () => { }); test('One Param as array in notification', (done) => { - let type = new NotificationType('add'); + let type = new NotificationType('add', ParameterStructures.byPosition); let duplexStream1 = new TestDuplex('ds1'); let duplexStream2 = new TestDuplex('ds2'); diff --git a/tsconfig-watch.json b/tsconfig-watch.json index f50428080..bdceb9a0f 100644 --- a/tsconfig-watch.json +++ b/tsconfig-watch.json @@ -9,6 +9,6 @@ { "path": "./textDocument/tsconfig-watch.json" }, { "path": "./server/tsconfig-watch.json" }, { "path": "./client/tsconfig-watch.json" }, - { "path": "./client-tests/tsconfig-watch.json" } + { "path": "./client-node-tests/tsconfig-watch.json" } ] } \ No newline at end of file