simplify rpc logic + improve typings

packages/core/src/common/cancellation.ts

@@ -46,6 +46,15 @@ export namespace CancellationToken {
         isCancellationRequested: true,
         onCancellationRequested: shortcutEvent
     });
+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    export function Is(what: any): what is CancellationToken {
+        return typeof what === 'object'
+            // eslint-disable-next-line no-null/no-null
+            && what !== null
+            && typeof what.onCancellationRequested === 'function'
+            && typeof what.isCancellationRequested === 'boolean';
+    }
 }
 
 export class CancellationError extends Error {

packages/core/src/common/messaging/json-rpc-service.ts

@@ -0,0 +1,36 @@
+/********************************************************************************
+ * Copyright (C) 2021 Ericsson and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { MaybePromise, Unpromise } from '../types';
+
+export type UndefinedOrNull<T> = T extends object
+    ? { [K in keyof T]: UndefinedOrNull<T[K]> }
+    : T extends undefined ? undefined | null : T;
+
+export interface JsonRpcMethods {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    [method: string]: (...args: any[]) => MaybePromise<any>
+}
+
+/**
+ * Use this type to wrap your JSON-RPC service APIs to ensure you only use types that can be serialized through JSON-RPC.
+ *
+ * It replaces any occurence of `undefined` in the signature with `undefined | null` because when passing `undefined` it
+ * becomes `null` when serialized in JSON. The implementations must be able to handle both cases.
+ */
+export type JsonRpcService<T extends JsonRpcMethods> = {
+    [K in keyof T]: (...args: UndefinedOrNull<Parameters<T[K]>>) => Promise<UndefinedOrNull<Unpromise<ReturnType<T[K]>>>>
+};

packages/core/src/common/messaging/proxy-factory.ts

@@ -32,14 +32,6 @@ export type JsonRpcServer<Client> = Disposable & {
     getClient?(): Client | undefined;
 };
 
-/**
- * Since neither JSON nor JSON-RPC handle undefined values in method argument arrays, we'll transform those
- * into a structure of [argument_index, argument][] so that we can omit certain values and properly reconstruct
- * the original array with undefined values. See `createJsonRpcParameters` and `parseJsonRpcParameters`.
- * Note that any nested array will see its `undefined` values replaced by `null` because of the JSON serialization.
- */
-export type JsonRpcParameters = [number, any][];
-
 export interface JsonRpcConnectionEventEmitter {
     readonly onDidOpenConnection: Event<void>;
     readonly onDidCloseConnection: Event<void>;
@@ -136,23 +128,23 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
     listen(connection: MessageConnection): void {
         // When a method is called without arguments then `params` is `undefined`.
         // `token` seems to always be set by `vscode-jsonrpc` on the receiving side.
-        connection.onRequest((method: string, params: JsonRpcParameters | undefined, token: CancellationToken) => {
+        connection.onRequest((method: string, params: object | unknown[] | undefined, token: CancellationToken) => {
             if (params === undefined) {
                 return this.onRequest(method, token);
             }
             if (!Array.isArray(params)) {
                 throw new Error('Unexpected JSON-RPC params format');
             }
-            return this.onRequest(method, ...this.parseJsonRpcParameters(params), token);
+            return this.onRequest(method, ...params, token);
         });
-        connection.onNotification((method: string, params: JsonRpcParameters | undefined) => {
+        connection.onNotification((method: string, params: object | unknown[] | undefined) => {
             if (params === undefined) {
                 return this.onNotification(method);
             }
             if (!Array.isArray(params)) {
                 throw new Error('Unexpected JSON-RPC params format');
             }
-            return this.onNotification(method, ...this.parseJsonRpcParameters(params));
+            return this.onNotification(method, ...params);
         });
         connection.onDispose(() => this.waitForConnection());
         connection.onClose(() => {
@@ -261,23 +253,11 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
             const method = property.toString();
             const capturedError = new Error(`Request '${method}' failed`);
             const connection = await this.connectionPromise;
-            // Extract the cancellation token if it is the last argument passed to the method to call over JSON-RPC.
-            let token: CancellationToken | undefined;
-            if (CancellationToken.is(args[args.length - 1])) {
-                token = args.pop();
-            }
-            // Convert the list of arguments into a structure that allows us to conserve undefined values.
-            const params: any[] = this.createJsonRpcParameters(args);
-            // Push back the token if there was one.
-            // `vscode-jsonrpc` will remove the `CancellationToken` out of `params` when is is the last element.
-            if (token) {
-                params.push(token);
-            }
             if (isNotify) {
-                connection.sendNotification(method, ParameterStructures.byPosition, ...params);
+                connection.sendNotification(method, ParameterStructures.byPosition, ...args);
             } else {
                 try {
-                    return await connection.sendRequest(method, ParameterStructures.byPosition, ...params);
+                    return await connection.sendRequest(method, ParameterStructures.byPosition, ...args);
                 } catch (error) {
                     throw this.deserializeError(capturedError, error);
                 }
@@ -322,22 +302,4 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
         }
         return e;
     }
-
-    protected createJsonRpcParameters(args: any[]): JsonRpcParameters {
-        const params: JsonRpcParameters = [];
-        for (const [i, arg] of args.entries()) {
-            if (arg !== undefined) {
-                params.push([i, arg]);
-            }
-        }
-        return params;
-    }
-
-    protected parseJsonRpcParameters(params: JsonRpcParameters): any[] {
-        const args = [];
-        for (const [i, arg] of params) {
-            args[i] = arg;
-        }
-        return args;
-    }
 }

packages/core/src/common/types.ts

@@ -26,6 +26,8 @@ export type RecursivePartial<T> = {
 };
 export type MaybeArray<T> = T | T[];
 export type MaybePromise<T> = T | PromiseLike<T>;
+/** Extract `T` from `PromiseLike<T>`. */
+export type Unpromise<T> = T extends PromiseLike<infer U> ? U : T;
 
 export interface Prioritizeable<T> {
     readonly priority: number;

packages/plugin-ext/src/hosted/browser/hosted-plugin.ts

@@ -533,7 +533,13 @@ export class HostedPluginSupport {
 
     protected async getStoragePath(): Promise<string | undefined> {
         const roots = await this.workspaceService.roots;
-        return this.pluginPathsService.getHostStoragePath(this.workspaceService.workspace?.resource.toString(), roots.map(root => root.resource.toString()));
+        const storagePath = await this.pluginPathsService.getHostStoragePath(
+            this.workspaceService.workspace?.resource.toString(),
+            roots.map(root => root.resource.toString()),
+        );
+        if (storagePath) {
+            return storagePath;
+        }
     }
 
     protected async getHostGlobalStoragePath(): Promise<string> {

packages/plugin-ext/src/main/browser/custom-editors/glob.ts

@@ -589,6 +589,7 @@ function parsedExpression(expression: IExpression, options: IGlobOptions): Parse
 
     if (!parsedPatterns.some(parsedPattern => (<ParsedExpressionPattern>parsedPattern).requiresSiblings!)) {
         if (n === 1) {
+            // @ts-expect-error TS2322
             return <ParsedStringPattern>parsedPatterns[0];
         }
 
@@ -619,6 +620,7 @@ function parsedExpression(expression: IExpression, options: IGlobOptions): Parse
             resultExpression.allPaths = allPaths;
         }
 
+        // @ts-expect-error TS2322
         return resultExpression;
     }
 
@@ -656,6 +658,7 @@ function parsedExpression(expression: IExpression, options: IGlobOptions): Parse
         resultExpression.allPaths = allPaths;
     }
 
+    // @ts-expect-error TS2322
     return resultExpression;
 }
 

packages/plugin-ext/src/main/browser/quick-open-main.ts

@@ -209,8 +209,8 @@ export class QuickOpenMainImpl implements QuickOpenMain, Disposable {
                 quickPick.onDidChangeSelection((items: Array<monaco.quickInput.IQuickPickItem>) => {
                     this.proxy.$onDidChangeSelection(sessionId, items.map(item => (item as TransferQuickPickItems).handle));
                 });
-                quickPick.onDidTriggerButton((button: QuickInputButtonHandle) => {
-                    this.proxy.$acceptOnDidTriggerButton(sessionId, button);
+                quickPick.onDidTriggerButton(button => {
+                    this.proxy.$acceptOnDidTriggerButton(sessionId, button as QuickInputButtonHandle);
                 });
                 quickPick.onDidChangeValue((value: string) => {
                     this.proxy.$acceptDidChangeValue(sessionId, value);
@@ -227,8 +227,8 @@ export class QuickOpenMainImpl implements QuickOpenMain, Disposable {
                 inputBox.onDidAccept(() => {
                     this.proxy.$acceptOnDidAccept(sessionId);
                 });
-                inputBox.onDidTriggerButton((button: QuickInputButtonHandle) => {
-                    this.proxy.$acceptOnDidTriggerButton(sessionId, button);
+                inputBox.onDidTriggerButton(button => {
+                    this.proxy.$acceptOnDidTriggerButton(sessionId, button as QuickInputButtonHandle);
                 });
                 inputBox.onDidChangeValue((value: string) => {
                     this.proxy.$acceptDidChangeValue(sessionId, value);

packages/plugin-ext/src/main/common/plugin-paths-protocol.ts

@@ -14,13 +14,15 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
+import type { JsonRpcService } from '@theia/core/lib/common/messaging/json-rpc-service';
+
 export const pluginPathsServicePath = '/services/plugin-paths';
 
 // Service to create plugin configuration folders for different purpose.
 export const PluginPathsService = Symbol('PluginPathsService');
-export interface PluginPathsService {
+export type PluginPathsService = JsonRpcService<{
     /** Returns hosted log path. Create directory by this path if it is not exist on the file system. */
     getHostLogPath(): Promise<string>;
     /** Returns storage path for given workspace */
     getHostStoragePath(workspaceUri: string | undefined, rootUris: string[]): Promise<string | undefined>;
-}
+}>;

packages/plugin-ext/src/main/node/paths/plugin-paths-service.ts

@@ -57,7 +57,7 @@ export class PluginPathsServiceImpl implements PluginPathsService {
         return pluginDirPath;
     }
 
-    async getHostStoragePath(workspaceUri: string | undefined, rootUris: string[]): Promise<string | undefined> {
+    async getHostStoragePath(workspaceUri: string | null | undefined, rootUris: string[]): Promise<string | undefined> {
         const parentStorageDir = await this.getWorkspaceStorageDirPath();
 
         if (!parentStorageDir) {

packages/plugin-ext/src/main/node/plugin-server-handler.ts

@@ -42,16 +42,19 @@ export class PluginServerHandler implements PluginServer {
         return this.pluginDeployer.undeploy(pluginId);
     }
 
-    setStorageValue(key: string, value: KeysToAnyValues, kind: PluginStorageKind): Promise<boolean> {
-        return this.pluginsKeyValueStorage.set(key, value, kind);
+    setStorageValue(key: string, value: KeysToAnyValues, kind: PluginStorageKind | CancellationToken): Promise<boolean> {
+        return this.pluginsKeyValueStorage.set(key, value, this.ignoreCancellationToken(kind));
     }
 
-    getStorageValue(key: string, kind: PluginStorageKind): Promise<KeysToAnyValues> {
-        return this.pluginsKeyValueStorage.get(key, kind);
+    getStorageValue(key: string, kind: PluginStorageKind | CancellationToken): Promise<KeysToAnyValues> {
+        return this.pluginsKeyValueStorage.get(key, this.ignoreCancellationToken(kind));
     }
 
-    getAllStorageValues(kind: PluginStorageKind): Promise<KeysToKeysToAnyValue> {
-        return this.pluginsKeyValueStorage.getAll(kind);
+    getAllStorageValues(kind: PluginStorageKind | CancellationToken): Promise<KeysToKeysToAnyValue> {
+        return this.pluginsKeyValueStorage.getAll(this.ignoreCancellationToken(kind));
     }
 
+    protected ignoreCancellationToken<T>(what: T | CancellationToken): T | undefined {
+        return CancellationToken.Is(what) ? undefined : what;
+    }
 }

packages/plugin-ext/src/main/node/plugins-key-value-storage.ts

@@ -94,7 +94,9 @@ export class PluginsKeyValueStorage {
             return this.deferredGlobalDataPath.promise;
         }
         const storagePath = await this.pluginPathsService.getHostStoragePath(kind.workspace, kind.roots);
-        return storagePath ? path.join(storagePath, 'workspace-state.json') : undefined;
+        if (storagePath) {
+            return path.join(storagePath, 'workspace-state.json');
+        }
     }
 
     private async readFromFile(pathToFile: string): Promise<KeysToKeysToAnyValue> {

packages/plugin-ext/src/plugin/authentication-ext.ts

@@ -61,10 +61,26 @@ export class AuthenticationExtImpl implements AuthenticationExt {
         return Object.freeze(this._providers.slice());
     }
 
-    async getSession(requestingExtension: InternalPlugin, providerId: string, scopes: string[],
-                     options: theia.AuthenticationGetSessionOptions & { createIfNone: true }): Promise<theia.AuthenticationSession>;
-    async getSession(requestingExtension: InternalPlugin, providerId: string, scopes: string[],
-                     options: theia.AuthenticationGetSessionOptions = {}): Promise<theia.AuthenticationSession | undefined> {
+    async getSession(
+        requestingExtension: InternalPlugin,
+        providerId: string,
+        scopes: string[],
+        options?: theia.AuthenticationGetSessionOptions & { createIfNone: true }
+    ): Promise<theia.AuthenticationSession>;
+
+    async getSession(
+        requestingExtension: InternalPlugin,
+        providerId: string,
+        scopes: string[],
+        options?: theia.AuthenticationGetSessionOptions
+    ): Promise<theia.AuthenticationSession | undefined>;
+
+    async getSession(
+        requestingExtension: InternalPlugin,
+        providerId: string,
+        scopes: string[],
+        options: theia.AuthenticationGetSessionOptions & { createIfNone?: boolean } = {}
+    ): Promise<theia.AuthenticationSession | undefined> {
         const extensionName = requestingExtension.model.displayName || requestingExtension.model.name;
         const extensionId = requestingExtension.model.id.toLowerCase();
 
@@ -163,7 +179,7 @@ export class AuthenticationExtImpl implements AuthenticationExt {
         return Promise.resolve();
     }
 
-   async $onDidChangeAuthenticationProviders(added: theia.AuthenticationProviderInformation[], removed: theia.AuthenticationProviderInformation[]): Promise<void> {
+    async $onDidChangeAuthenticationProviders(added: theia.AuthenticationProviderInformation[], removed: theia.AuthenticationProviderInformation[]): Promise<void> {
         added.forEach(id => {
             if (this._providers.indexOf(id) === -1) {
                 this._providers.push(id);

packages/plugin-ext/src/plugin/command-registry.ts

@@ -197,8 +197,7 @@ export class CommandsConverter {
         // we're deprecating Command.id, so it has to be optional.
         // Existing code will have compiled against a non - optional version of the field, so asserting it to exist is ok
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        return KnownCommands.map((external.command || external.id)!, external.arguments, (mappedId: string, mappedArgs: any[]) =>
-        ({
+        return KnownCommands.map((external.command || external.id)!, external.arguments, (mappedId: string, mappedArgs?: any[]) => ({
             id: mappedId,
             title: external.title || external.label || ' ',
             tooltip: external.tooltip,

packages/plugin-ext/src/plugin/known-commands.spec.ts

@@ -34,7 +34,7 @@ describe('Known Command Conversions', () => {
 
         // when
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        KnownCommands.map(commandID, [uri, position], (mappedID: string, mappedArgs: any[]) => {
+        KnownCommands.map(commandID, [uri, position], (mappedID: string, mappedArgs: any[] = []) => {
 
             // then
             assert.strictEqual(commandID, mappedID);

packages/plugin-ext/src/plugin/plugin-context.ts

@@ -20,7 +20,7 @@
 import type * as theia from '@theia/plugin';
 import { CommandRegistryImpl } from './command-registry';
 import { Emitter } from '@theia/core/lib/common/event';
-import { CancellationError, CancellationToken, CancellationTokenSource } from '@theia/core/lib/common/cancellation';
+import { CancellationError, CancellationTokenSource } from '@theia/core/lib/common/cancellation';
 import { QuickOpenExtImpl } from './quick-open';
 import {
     MAIN_RPC_CONTEXT,
@@ -229,8 +229,8 @@ export function createAPIFactory(
             get providers(): ReadonlyArray<theia.AuthenticationProviderInformation> {
                 return authenticationExt.providers;
             },
-            getSession(providerId: string, scopes: string[], options: theia.AuthenticationGetSessionOptions) {
-                return authenticationExt.getSession(plugin, providerId, scopes, options as any);
+            getSession(providerId: string, scopes: string[], options: theia.AuthenticationGetSessionOptions = {}): Promise<theia.AuthenticationSession> {
+                return authenticationExt.getSession(plugin, providerId, scopes, { ...options, createIfNone: true });
             },
             logout(providerId: string, sessionId: string): Thenable<void> {
                 return authenticationExt.logout(providerId, sessionId);
@@ -530,13 +530,18 @@ export function createAPIFactory(
                 const data = await documents.openDocument(uri);
                 return data && data.document;
             },
-            createFileSystemWatcher: (pattern, ignoreCreate, ignoreChange, ignoreDelete): theia.FileSystemWatcher =>
-                extHostFileSystemEvent.createFileSystemWatcher(fromGlobPattern(pattern), ignoreCreate, ignoreChange, ignoreDelete),
-            findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | null, maxResults?: number, token?: CancellationToken): PromiseLike<URI[]> {
+            createFileSystemWatcher(pattern, ignoreCreate, ignoreChange, ignoreDelete): theia.FileSystemWatcher {
+                return extHostFileSystemEvent.createFileSystemWatcher(fromGlobPattern(pattern), ignoreCreate, ignoreChange, ignoreDelete);
+            },
+            findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | null, maxResults?: number, token?: theia.CancellationToken): PromiseLike<URI[]> {
                 return workspaceExt.findFiles(include, exclude, maxResults, token);
             },
-            findTextInFiles(query: theia.TextSearchQuery, optionsOrCallback: theia.FindTextInFilesOptions | ((result: theia.TextSearchResult) => void),
-                callbackOrToken?: CancellationToken | ((result: theia.TextSearchResult) => void), token?: CancellationToken): Promise<theia.TextSearchComplete> {
+            findTextInFiles(
+                query: theia.TextSearchQuery,
+                optionsOrCallback: theia.FindTextInFilesOptions | ((result: theia.TextSearchResult) => void),
+                callbackOrToken?: theia.CancellationToken | ((result: theia.TextSearchResult) => void),
+                token?: theia.CancellationToken
+            ): Promise<theia.TextSearchComplete> {
                 return workspaceExt.findTextInFiles(query, optionsOrCallback, callbackOrToken, token);
             },
             saveAll(includeUntitled?: boolean): PromiseLike<boolean> {