From b35bc5e02109682f50ec2ff4ef6537c50ed75cb9 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 22 Jun 2021 18:42:50 +0200 Subject: [PATCH] improve extension kind computation for web - include web kind if extension supports web - support multiple extension kinds for contribution point --- .../common/jsonValidationExtensionPoint.ts | 2 +- .../contrib/debug/common/debugSchemas.ts | 2 +- .../contrib/terminal/common/terminal.ts | 2 +- .../extensionEnablementService.test.ts | 3 +- .../extensionManifestPropertiesService.ts | 51 ++++++++++++------- .../extensions/common/extensionsRegistry.ts | 13 +++-- ...extensionManifestPropertiesService.test.ts | 38 +++++++++----- 7 files changed, 67 insertions(+), 44 deletions(-) diff --git a/src/vs/workbench/api/common/jsonValidationExtensionPoint.ts b/src/vs/workbench/api/common/jsonValidationExtensionPoint.ts index e768c889fb9..895d36a310f 100644 --- a/src/vs/workbench/api/common/jsonValidationExtensionPoint.ts +++ b/src/vs/workbench/api/common/jsonValidationExtensionPoint.ts @@ -15,7 +15,7 @@ interface IJSONValidationExtensionPoint { const configurationExtPoint = ExtensionsRegistry.registerExtensionPoint({ extensionPoint: 'jsonValidation', - defaultExtensionKind: 'workspace', + defaultExtensionKind: ['workspace', 'web'], jsonSchema: { description: nls.localize('contributes.jsonValidation', 'Contributes json schema configuration.'), type: 'array', diff --git a/src/vs/workbench/contrib/debug/common/debugSchemas.ts b/src/vs/workbench/contrib/debug/common/debugSchemas.ts index d2e735701be..a32cb7dd2e2 100644 --- a/src/vs/workbench/contrib/debug/common/debugSchemas.ts +++ b/src/vs/workbench/contrib/debug/common/debugSchemas.ts @@ -13,7 +13,7 @@ import { inputsSchema } from 'vs/workbench/services/configurationResolver/common // debuggers extension point export const debuggersExtPoint = extensionsRegistry.ExtensionsRegistry.registerExtensionPoint({ extensionPoint: 'debuggers', - defaultExtensionKind: 'workspace', + defaultExtensionKind: ['workspace'], jsonSchema: { description: nls.localize('vscode.extension.contributes.debuggers', 'Contributes debug adapters.'), type: 'array', diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index cf8107862b1..29d7f53303c 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -595,7 +595,7 @@ export interface ITerminalProfileContribution { export const terminalContributionsDescriptor: IExtensionPointDescriptor = { extensionPoint: 'terminal', - defaultExtensionKind: 'workspace', + defaultExtensionKind: ['workspace'], jsonSchema: { description: nls.localize('vscode.extension.contributes.terminal', 'Contributes terminal functionality.'), type: 'object', diff --git a/src/vs/workbench/services/extensionManagement/test/browser/extensionEnablementService.test.ts b/src/vs/workbench/services/extensionManagement/test/browser/extensionEnablementService.test.ts index 1f138c595e5..f3ab9b6b9dd 100644 --- a/src/vs/workbench/services/extensionManagement/test/browser/extensionEnablementService.test.ts +++ b/src/vs/workbench/services/extensionManagement/test/browser/extensionEnablementService.test.ts @@ -35,6 +35,7 @@ import { ExtensionManifestPropertiesService, IExtensionManifestPropertiesService import { TestContextService, TestProductService } from 'vs/workbench/test/common/workbenchTestServices'; import { TestWorkspace } from 'vs/platform/workspace/test/common/testWorkspace'; import { ExtensionManagementService } from 'vs/workbench/services/extensionManagement/common/extensionManagementService'; +import { NullLogService } from 'vs/platform/log/common/log'; function createStorageService(instantiationService: TestInstantiationService): IStorageService { let service = instantiationService.get(IStorageService); @@ -83,7 +84,7 @@ export class TestExtensionEnablementService extends ExtensionEnablementService { new class extends mock() { override isDisabledByBisect() { return false; } }, workspaceTrustManagementService, new class extends mock() { override requestWorkspaceTrust(options?: WorkspaceTrustRequestOptions): Promise { return Promise.resolve(true); } }, - instantiationService.get(IExtensionManifestPropertiesService) || instantiationService.stub(IExtensionManifestPropertiesService, new ExtensionManifestPropertiesService(TestProductService, new TestConfigurationService(), workspaceTrustManagementService)), + instantiationService.get(IExtensionManifestPropertiesService) || instantiationService.stub(IExtensionManifestPropertiesService, new ExtensionManifestPropertiesService(TestProductService, new TestConfigurationService(), workspaceTrustManagementService, new NullLogService())), instantiationService ); } diff --git a/src/vs/workbench/services/extensions/common/extensionManifestPropertiesService.ts b/src/vs/workbench/services/extensions/common/extensionManifestPropertiesService.ts index 026e8f5da1f..f7c2660da3d 100644 --- a/src/vs/workbench/services/extensions/common/extensionManifestPropertiesService.ts +++ b/src/vs/workbench/services/extensions/common/extensionManifestPropertiesService.ts @@ -16,6 +16,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { WORKSPACE_TRUST_EXTENSION_SUPPORT } from 'vs/workbench/services/workspaces/common/workspaceTrust'; import { isBoolean } from 'vs/base/common/types'; import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust'; +import { ILogService } from 'vs/platform/log/common/log'; export const IExtensionManifestPropertiesService = createDecorator('extensionManifestPropertiesService'); @@ -39,7 +40,7 @@ export class ExtensionManifestPropertiesService extends Disposable implements IE readonly _serviceBrand: undefined; - private _uiExtensionPoints: Set | null = null; + private _extensionPointExtensionKindsMap: Map | null = null; private _productExtensionKindsMap: Map | null = null; private _configuredExtensionKindsMap: Map | null = null; @@ -52,7 +53,8 @@ export class ExtensionManifestPropertiesService extends Disposable implements IE constructor( @IProductService private readonly productService: IProductService, @IConfigurationService private readonly configurationService: IConfigurationService, - @IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService + @IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService, + @ILogService private readonly logService: ILogService, ) { super(); @@ -115,13 +117,20 @@ export class ExtensionManifestPropertiesService extends Disposable implements IE return result; } + const deducedExtensionKind = this.deduceExtensionKind(manifest); + // check the manifest itself result = manifest.extensionKind; if (typeof result !== 'undefined') { - return this.toArray(result); + result = this.toArray(result); + // Add web kind if the extension can run as web extension + if (deducedExtensionKind.includes('web') && !result.includes('web')) { + result.push('web'); + } + return result; } - return this.deduceExtensionKind(manifest); + return deducedExtensionKind; } getExtensionUntrustedWorkspaceSupportType(manifest: IExtensionManifest): ExtensionUntrustedWorkpaceSupportType { @@ -193,7 +202,7 @@ export class ExtensionManifestPropertiesService extends Disposable implements IE return true; } - deduceExtensionKind(manifest: IExtensionManifest): ExtensionKind[] { + private deduceExtensionKind(manifest: IExtensionManifest): ExtensionKind[] { // Not an UI extension if it has main if (manifest.main) { if (manifest.browser) { @@ -206,32 +215,36 @@ export class ExtensionManifestPropertiesService extends Disposable implements IE return ['web']; } - // Not an UI nor web extension if it has dependencies or an extension pack + let result: ExtensionKind[] = ['ui', 'workspace', 'web']; + + // Not an UI extension if extension has dependencies or an extension pack if (isNonEmptyArray(manifest.extensionDependencies) || isNonEmptyArray(manifest.extensionPack)) { - return ['workspace']; + result = ['workspace', 'web']; } if (manifest.contributes) { - // Not an UI nor web extension if it has no ui contributions for (const contribution of Object.keys(manifest.contributes)) { - if (!this.isUIExtensionPoint(contribution)) { - return ['workspace']; + const supportedExtensionKinds = this.getSupportedExtensionKindsForExtensionPoint(contribution); + if (supportedExtensionKinds.length) { + result = result.filter(extensionKind => supportedExtensionKinds.includes(extensionKind)); } } } - return ['ui', 'workspace', 'web']; + if (!result.length) { + this.logService.warn('Cannot deduce extensionKind for extension', getGalleryExtensionId(manifest.publisher, manifest.name)); + } + + return result; } - private isUIExtensionPoint(extensionPoint: string): boolean { - if (this._uiExtensionPoints === null) { - const uiExtensionPoints = new Set(); - ExtensionsRegistry.getExtensionPoints().filter(e => e.defaultExtensionKind !== 'workspace').forEach(e => { - uiExtensionPoints.add(e.name); - }); - this._uiExtensionPoints = uiExtensionPoints; + private getSupportedExtensionKindsForExtensionPoint(extensionPoint: string): ExtensionKind[] { + if (this._extensionPointExtensionKindsMap === null) { + const extensionPointExtensionKindsMap = new Map(); + ExtensionsRegistry.getExtensionPoints().forEach(e => extensionPointExtensionKindsMap.set(e.name, e.defaultExtensionKind || [] /* supports all */)); + this._extensionPointExtensionKindsMap = extensionPointExtensionKindsMap; } - return this._uiExtensionPoints.has(extensionPoint); + return this._extensionPointExtensionKindsMap.get(extensionPoint) || ['workspace'] /* Unknown extension point => workspace */; } private getProductExtensionKind(manifest: IExtensionManifest): ExtensionKind[] | undefined { diff --git a/src/vs/workbench/services/extensions/common/extensionsRegistry.ts b/src/vs/workbench/services/extensions/common/extensionsRegistry.ts index 7efa9576b0d..f9989af0505 100644 --- a/src/vs/workbench/services/extensions/common/extensionsRegistry.ts +++ b/src/vs/workbench/services/extensions/common/extensionsRegistry.ts @@ -11,10 +11,9 @@ import { EXTENSION_IDENTIFIER_PATTERN } from 'vs/platform/extensionManagement/co import { Extensions, IJSONContributionRegistry } from 'vs/platform/jsonschemas/common/jsonContributionRegistry'; import { Registry } from 'vs/platform/registry/common/platform'; import { IMessage } from 'vs/workbench/services/extensions/common/extensions'; -import { ExtensionIdentifier, IExtensionDescription, EXTENSION_CATEGORIES } from 'vs/platform/extensions/common/extensions'; +import { ExtensionIdentifier, IExtensionDescription, EXTENSION_CATEGORIES, ExtensionKind } from 'vs/platform/extensions/common/extensions'; const schemaRegistry = Registry.as(Extensions.JSONContribution); -export type ExtensionKind = 'workspace' | 'ui' | undefined; export class ExtensionMessageCollector { @@ -63,9 +62,9 @@ export interface IExtensionPointUser { export type IExtensionPointHandler = (extensions: readonly IExtensionPointUser[], delta: ExtensionPointUserDelta) => void; export interface IExtensionPoint { - name: string; + readonly name: string; setHandler(handler: IExtensionPointHandler): void; - defaultExtensionKind: ExtensionKind; + readonly defaultExtensionKind: ExtensionKind[] | undefined; } export class ExtensionPointUserDelta { @@ -104,13 +103,13 @@ export class ExtensionPointUserDelta { export class ExtensionPoint implements IExtensionPoint { public readonly name: string; - public readonly defaultExtensionKind: ExtensionKind; + public readonly defaultExtensionKind: ExtensionKind[] | undefined; private _handler: IExtensionPointHandler | null; private _users: IExtensionPointUser[] | null; private _delta: ExtensionPointUserDelta | null; - constructor(name: string, defaultExtensionKind: ExtensionKind) { + constructor(name: string, defaultExtensionKind: ExtensionKind[] | undefined) { this.name = name; this.defaultExtensionKind = defaultExtensionKind; this._handler = null; @@ -511,7 +510,7 @@ export interface IExtensionPointDescriptor { extensionPoint: string; deps?: IExtensionPoint[]; jsonSchema: IJSONSchema; - defaultExtensionKind?: ExtensionKind; + defaultExtensionKind?: ExtensionKind[]; } export class ExtensionsRegistryImpl { diff --git a/src/vs/workbench/services/extensions/test/common/extensionManifestPropertiesService.test.ts b/src/vs/workbench/services/extensions/test/common/extensionManifestPropertiesService.test.ts index 82b82e59c67..300a8a216f4 100644 --- a/src/vs/workbench/services/extensions/test/common/extensionManifestPropertiesService.test.ts +++ b/src/vs/workbench/services/extensions/test/common/extensionManifestPropertiesService.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { IExtensionManifest, ExtensionKind, ExtensionUntrustedWorkpaceSupportType } from 'vs/platform/extensions/common/extensions'; +import { IExtensionManifest, ExtensionUntrustedWorkpaceSupportType } from 'vs/platform/extensions/common/extensions'; import { ExtensionManifestPropertiesService } from 'vs/workbench/services/extensions/common/extensionManifestPropertiesService'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { TestProductService } from 'vs/workbench/test/common/workbenchTestServices'; @@ -14,40 +14,50 @@ import { IProductService } from 'vs/platform/product/common/productService'; import { isWeb } from 'vs/base/common/platform'; import { TestWorkspaceTrustManagementService } from 'vs/workbench/services/workspaces/test/common/testWorkspaceTrustService'; import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust'; +import { NullLogService } from 'vs/platform/log/common/log'; suite('ExtensionManifestPropertiesService - ExtensionKind', () => { - function check(manifest: Partial, expected: ExtensionKind[]): void { - const extensionManifestPropertiesService = new ExtensionManifestPropertiesService(TestProductService, new TestConfigurationService(), new TestWorkspaceTrustManagementService()); - assert.deepStrictEqual(extensionManifestPropertiesService.deduceExtensionKind(manifest), expected); - } + const testObject = new ExtensionManifestPropertiesService(TestProductService, new TestConfigurationService(), new TestWorkspaceTrustManagementService(), new NullLogService()); - test('declarative with extension dependencies => workspace', () => { - check({ extensionDependencies: ['ext1'] }, ['workspace']); + test('declarative with extension dependencies => workspace, web', () => { + assert.deepStrictEqual(testObject.getExtensionKind({ extensionDependencies: ['ext1'] }), ['workspace', 'web']); }); - test('declarative extension pack => workspace', () => { - check({ extensionPack: ['ext1', 'ext2'] }, ['workspace']); + test('declarative extension pack => workspace, web', () => { + assert.deepStrictEqual(testObject.getExtensionKind({ extensionPack: ['ext1', 'ext2'] }), ['workspace', 'web']); }); test('declarative with unknown contribution point => workspace', () => { - check({ contributes: { 'unknownPoint': { something: true } } }, ['workspace']); + assert.deepStrictEqual(testObject.getExtensionKind({ contributes: { 'unknownPoint': { something: true } } }), ['workspace']); + }); + + test('declarative extension pack with unknown contribution point => workspace', () => { + assert.deepStrictEqual(testObject.getExtensionKind({ extensionPack: ['ext1', 'ext2'], contributes: { 'unknownPoint': { something: true } } }), ['workspace']); }); test('simple declarative => ui, workspace, web', () => { - check({}, ['ui', 'workspace', 'web']); + assert.deepStrictEqual(testObject.getExtensionKind({}), ['ui', 'workspace', 'web']); }); test('only browser => web', () => { - check({ browser: 'main.browser.js' }, ['web']); + assert.deepStrictEqual(testObject.getExtensionKind({ browser: 'main.browser.js' }), ['web']); }); test('only main => workspace', () => { - check({ main: 'main.js' }, ['workspace']); + assert.deepStrictEqual(testObject.getExtensionKind({ main: 'main.js' }), ['workspace']); }); test('main and browser => workspace, web', () => { - check({ main: 'main.js', browser: 'main.browser.js' }, ['workspace', 'web']); + assert.deepStrictEqual(testObject.getExtensionKind({ main: 'main.js', browser: 'main.browser.js' }), ['workspace', 'web']); + }); + + test('browser entry point with workspace extensionKind => workspace, web', () => { + assert.deepStrictEqual(testObject.getExtensionKind({ main: 'main.js', browser: 'main.browser.js', extensionKind: ['workspace'] }), ['workspace', 'web']); + }); + + test('simple descriptive with workspace, ui extensionKind => workspace, ui, web', () => { + assert.deepStrictEqual(testObject.getExtensionKind({ extensionKind: ['workspace', 'ui'] }), ['workspace', 'ui', 'web']); }); });