From 3a0be849744cab03679d1f25b6f3823bf6611425 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Thu, 13 Dec 2018 23:11:11 +0100 Subject: [PATCH] #64755 Check strict nulls in ExtensionWorkbenchService --- src/tsconfig.strictNullChecks.json | 3 +- .../extensions/browser/extensionsWidgets.ts | 10 +- .../parts/extensions/common/extensions.ts | 22 ++-- .../electron-browser/extensionEditor.ts | 37 +++--- .../node/extensionsWorkbenchService.ts | 112 +++++++++--------- 5 files changed, 96 insertions(+), 88 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index ffacc7493de..4e9f4b9ecd7 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -766,7 +766,8 @@ "./vs/workbench/test/electron-browser/api/mock.ts", "./vs/platform/extensionManagement/node/multiExtensionManagement.ts", "./vs/platform/extensionManagement/node/extensionManagementService.ts", - "./vs/platform/extensionManagement/node/extensionGalleryService.ts" + "./vs/platform/extensionManagement/node/extensionGalleryService.ts", + "./vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts" ], "exclude": [ "./typings/require-monaco.d.ts" diff --git a/src/vs/workbench/parts/extensions/browser/extensionsWidgets.ts b/src/vs/workbench/parts/extensions/browser/extensionsWidgets.ts index 611327322ca..361ef310087 100644 --- a/src/vs/workbench/parts/extensions/browser/extensionsWidgets.ts +++ b/src/vs/workbench/parts/extensions/browser/extensionsWidgets.ts @@ -67,7 +67,7 @@ export class InstallCountWidget implements IDisposable { const installCount = this.extension.installCount; - if (installCount === null) { + if (installCount === undefined) { return; } @@ -126,16 +126,16 @@ export class RatingsWidget implements IDisposable { return; } - const rating = Math.round(this.extension.rating * 2) / 2; - - if (this.extension.rating === null) { + if (this.extension.rating === undefined) { return; } - if (this.options.small && this.extension.ratingCount === 0) { + if (this.options.small && !this.extension.ratingCount) { return; } + const rating = Math.round(this.extension.rating * 2) / 2; + if (this.options.small) { append(this.container, $('span.full.star')); diff --git a/src/vs/workbench/parts/extensions/common/extensions.ts b/src/vs/workbench/parts/extensions/common/extensions.ts index 1831da27394..e1e4bf832d9 100644 --- a/src/vs/workbench/parts/extensions/common/extensions.ts +++ b/src/vs/workbench/parts/extensions/common/extensions.ts @@ -27,32 +27,32 @@ export const enum ExtensionState { } export interface IExtension { - type: LocalExtensionType; + type?: LocalExtensionType; state: ExtensionState; name: string; displayName: string; id: string; - uuid: string; + uuid?: string; publisher: string; publisherDisplayName: string; version: string; latestVersion: string; description: string; - url: string; - repository: string; + url?: string; + repository?: string; iconUrl: string; iconUrlFallback: string; - licenseUrl: string; - installCount: number; - rating: number; - ratingCount: number; + licenseUrl?: string; + installCount?: number; + rating?: number; + ratingCount?: number; outdated: boolean; enablementState: EnablementState; dependencies: string[]; extensionPack: string[]; telemetryData: any; preview: boolean; - getManifest(token: CancellationToken): Promise; + getManifest(token: CancellationToken): Promise; getReadme(token: CancellationToken): Promise; hasReadme(): boolean; getChangelog(token: CancellationToken): Promise; @@ -68,7 +68,7 @@ export interface IExtensionDependencies { hasDependencies: boolean; identifier: string; extension: IExtension; - dependent: IExtensionDependencies; + dependent: IExtensionDependencies | null; } export const SERVICE_ID = 'extensionsWorkbenchService'; @@ -88,7 +88,7 @@ export interface IExtensionsWorkbenchService { installVersion(extension: IExtension, version: string): Promise; reinstall(extension: IExtension): Promise; setEnablement(extensions: IExtension | IExtension[], enablementState: EnablementState): Promise; - loadDependencies(extension: IExtension, token: CancellationToken): Promise; + loadDependencies(extension: IExtension, token: CancellationToken): Promise; open(extension: IExtension, sideByside?: boolean): Promise; checkForUpdates(): Promise; allowedBadgeProviders: string[]; diff --git a/src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts b/src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts index 21bbe3a63e5..16ee2a8d670 100644 --- a/src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts +++ b/src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts @@ -602,24 +602,29 @@ export class ExtensionEditor extends BaseEditor { } return this.loadContents(() => this.extensionDependencies.get()) - .then(extensionDependencies => { - const content = $('div', { class: 'subcontent' }); - const scrollableContent = new DomScrollableElement(content, {}); - append(this.content, scrollableContent.getDomNode()); - this.contentDisposables.push(scrollableContent); + .then(extensionDependencies => { + if (extensionDependencies) { + const content = $('div', { class: 'subcontent' }); + const scrollableContent = new DomScrollableElement(content, {}); + append(this.content, scrollableContent.getDomNode()); + this.contentDisposables.push(scrollableContent); - const dependenciesTree = this.renderDependencies(content, extensionDependencies); - const layout = () => { - scrollableContent.scanDomNode(); - const scrollDimensions = scrollableContent.getScrollDimensions(); - dependenciesTree.layout(scrollDimensions.height); - }; - const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout }); - this.contentDisposables.push(toDisposable(removeLayoutParticipant)); + const dependenciesTree = this.renderDependencies(content, extensionDependencies); + const layout = () => { + scrollableContent.scanDomNode(); + const scrollDimensions = scrollableContent.getScrollDimensions(); + dependenciesTree.layout(scrollDimensions.height); + }; + const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout }); + this.contentDisposables.push(toDisposable(removeLayoutParticipant)); - this.contentDisposables.push(dependenciesTree); - scrollableContent.scanDomNode(); - return { focus() { dependenciesTree.domFocus(); } }; + this.contentDisposables.push(dependenciesTree); + scrollableContent.scanDomNode(); + return { focus() { dependenciesTree.domFocus(); } }; + } else { + append(this.content, $('p.nocontent')).textContent = localize('noDependencies', "No Dependencies"); + return Promise.resolve(this.content); + } }, error => { append(this.content, $('p.nocontent')).textContent = error; this.notificationService.error(error); diff --git a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts index 529027e10f7..fb834e57a31 100644 --- a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts +++ b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts @@ -51,13 +51,13 @@ class Extension implements IExtension { private galleryService: IExtensionGalleryService, private stateProvider: IExtensionStateProvider, public locals: ILocalExtension[], - public gallery: IGalleryExtension, + public gallery: IGalleryExtension | undefined, private telemetryService: ITelemetryService, private logService: ILogService ) { } - get type(): LocalExtensionType { - return this.local ? this.local.type : null; + get type(): LocalExtensionType | undefined { + return this.local ? this.local.type : undefined; } get name(): string { @@ -79,7 +79,7 @@ class Extension implements IExtension { return getGalleryExtensionIdFromLocal(this.local); } - get uuid(): string { + get uuid(): string | undefined { return this.gallery ? this.gallery.identifier.uuid : this.local.identifier.uuid; } @@ -100,7 +100,7 @@ class Extension implements IExtension { } get version(): string { - return this.local ? this.local.manifest.version : this.gallery.version; + return this.local ? this.local.manifest.version : this.latestVersion; } get latestVersion(): string { @@ -108,12 +108,12 @@ class Extension implements IExtension { } get description(): string { - return this.gallery ? this.gallery.description : this.local.manifest.description; + return this.gallery ? this.gallery.description : this.local.manifest.description || ''; } - get url(): string { + get url(): string | undefined { if (!product.extensionsGallery || !this.gallery) { - return null; + return undefined; } return `${product.extensionsGallery.itemUrl}?itemName=${this.publisher}.${this.name}`; @@ -127,19 +127,19 @@ class Extension implements IExtension { return this.galleryIconUrlFallback || this.localIconUrl || this.defaultIconUrl; } - private get localIconUrl(): string { + private get localIconUrl(): string | null { if (this.local && this.local.manifest.icon) { return resources.joinPath(this.local.location, this.local.manifest.icon).toString(); } return null; } - private get galleryIconUrl(): string { - return this.gallery && this.gallery.assets.icon.uri; + private get galleryIconUrl(): string | null { + return this.gallery ? this.gallery.assets.icon.uri : null; } - private get galleryIconUrlFallback(): string { - return this.gallery && this.gallery.assets.icon.fallbackUri; + private get galleryIconUrlFallback(): string | null { + return this.gallery ? this.gallery.assets.icon.fallbackUri : null; } private get defaultIconUrl(): string { @@ -156,12 +156,12 @@ class Extension implements IExtension { return require.toUrl('../electron-browser/media/defaultIcon.png'); } - get repository(): string { - return this.gallery && this.gallery.assets.repository && this.gallery.assets.repository.uri; + get repository(): string | undefined { + return this.gallery && this.gallery.assets.repository ? this.gallery.assets.repository.uri : undefined; } - get licenseUrl(): string { - return this.gallery && this.gallery.assets.license && this.gallery.assets.license.uri; + get licenseUrl(): string | undefined { + return this.gallery && this.gallery.assets.license ? this.gallery.assets.license.uri : undefined; } get state(): ExtensionState { @@ -170,16 +170,16 @@ class Extension implements IExtension { public isMalicious: boolean = false; - get installCount(): number { - return this.gallery ? this.gallery.installCount : null; + get installCount(): number | undefined { + return this.gallery ? this.gallery.installCount : undefined; } - get rating(): number { - return this.gallery ? this.gallery.rating : null; + get rating(): number | undefined { + return this.gallery ? this.gallery.rating : undefined; } - get ratingCount(): number { - return this.gallery ? this.gallery.ratingCount : null; + get ratingCount(): number | undefined { + return this.gallery ? this.gallery.ratingCount : undefined; } get outdated(): boolean { @@ -201,16 +201,16 @@ class Extension implements IExtension { } private isGalleryOutdated(): boolean { - return this.local && this.gallery && semver.gt(this.local.manifest.version, this.gallery.version); + return this.local && this.gallery ? semver.gt(this.local.manifest.version, this.gallery.version) : false; } - getManifest(token: CancellationToken): Promise { + getManifest(token: CancellationToken): Promise { if (this.gallery && !this.isGalleryOutdated()) { if (this.gallery.assets.manifest) { return this.galleryService.getManifest(this.gallery, token); } this.logService.error(nls.localize('Manifest is not found', "Manifest is not found"), this.id); - return Promise.resolve(undefined); + return Promise.resolve(null); } return Promise.resolve(this.local.manifest); @@ -333,7 +333,7 @@ class ExtensionDependencies implements IExtensionDependencies { return this._identifier; } - get dependent(): IExtensionDependencies { + get dependent(): IExtensionDependencies | null { return this._dependent; } @@ -443,7 +443,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, const locals = groupById[getGalleryExtensionIdFromLocal(local)]; locals.splice(locals.indexOf(local), 1); locals.splice(0, 0, local); - const extension = installedById[local.identifier.id] || new Extension(this.galleryService, this.stateProvider, locals, null, this.telemetryService, this.logService); + const extension = installedById[local.identifier.id] || new Extension(this.galleryService, this.stateProvider, locals, void 0, this.telemetryService, this.logService); extension.locals = locals; extension.enablementState = this.extensionEnablementService.getEnablementState(local); return extension; @@ -471,9 +471,9 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, }); } - loadDependencies(extension: IExtension, token: CancellationToken): Promise { + loadDependencies(extension: IExtension, token: CancellationToken): Promise { if (!extension.dependencies.length) { - return Promise.resolve(null); + return Promise.resolve(null); } return this.extensionService.getExtensionsReport() @@ -481,9 +481,8 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, const maliciousSet = getMaliciousExtensionsSet(report); return this.galleryService.loadAllDependencies((extension).dependencies.map(id => ({ id })), token) - .then(galleryExtensions => galleryExtensions.map(galleryExtension => this.fromGallery(galleryExtension, maliciousSet))) - .then(extensions => [...this.local, ...extensions]) - .then(extensions => { + .then(galleryExtensions => { + const extensions: IExtension[] = [...this.local, ...galleryExtensions.map(galleryExtension => this.fromGallery(galleryExtension, maliciousSet))]; const map = new Map(); for (const extension of extensions) { map.set(extension.id, extension); @@ -494,7 +493,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } open(extension: IExtension, sideByside: boolean = false): Promise { - return Promise.resolve(this.editorService.openEditor(this.instantiationService.createInstance(ExtensionsInput, extension), null, sideByside ? SIDE_GROUP : ACTIVE_GROUP)); + return Promise.resolve(this.editorService.openEditor(this.instantiationService.createInstance(ExtensionsInput, extension), void 0, sideByside ? SIDE_GROUP : ACTIVE_GROUP)); } private getDistinctInstalledExtensions(allInstalled: ILocalExtension[]): Promise { @@ -557,7 +556,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, // Otherwise falling back to old way so that we will not make many roundtrips if (gallery.properties.engine) { this.galleryService.loadCompatibleVersion(gallery) - .then(compatible => compatible ? this.syncLocalWithGalleryExtension(result, compatible) : null); + .then(compatible => compatible ? this.syncLocalWithGalleryExtension(result!, compatible) : null); } else { this.syncLocalWithGalleryExtension(result, gallery); } @@ -572,7 +571,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, return result; } - private getInstalledExtensionMatchingGallery(gallery: IGalleryExtension): Extension { + private getInstalledExtensionMatchingGallery(gallery: IGalleryExtension): Extension | null { for (const installed of this.installed) { if (installed.uuid) { // Installed from Gallery if (installed.uuid === gallery.identifier.uuid) { @@ -612,7 +611,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, private eventuallySyncWithGallery(immediate = false): void { const shouldSync = this.isAutoUpdateEnabled() || this.isAutoCheckUpdatesEnabled(); - const loop = () => (shouldSync ? this.syncWithGallery() : Promise.resolve(null)).then(() => this.eventuallySyncWithGallery()); + const loop = () => (shouldSync ? this.syncWithGallery() : Promise.resolve(void 0)).then(() => this.eventuallySyncWithGallery()); const delay = immediate ? 0 : ExtensionsWorkbenchService.SyncPeriod; this.syncDelayer.trigger(loop, delay) @@ -639,7 +638,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, promises.push(this.queryGallery({ names, pageSize: names.length })); } - return Promise.all(promises).then(() => null); + return Promise.all(promises).then(() => void 0); } private eventuallyAutoUpdateExtensions(): void { @@ -649,7 +648,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, private autoUpdateExtensions(): Promise { if (!this.isAutoUpdateEnabled()) { - return Promise.resolve(null); + return Promise.resolve(); } const toUpdate = this.local.filter(e => @@ -703,7 +702,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, uninstall(extension: IExtension): Promise { if (!(extension instanceof Extension)) { - return undefined; + return Promise.resolve(); } const ext = extension as Extension; @@ -718,7 +717,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, location: ProgressLocation.Extensions, title: nls.localize('uninstallingExtension', 'Uninstalling extension....'), source: `${toUninstall[0].identifier.id}` - }, () => Promise.all(toUninstall.map(local => this.extensionService.uninstall(local))).then(() => null)); + }, () => Promise.all(toUninstall.map(local => this.extensionService.uninstall(local))).then(() => void 0)); } installVersion(extension: IExtension, version: string): Promise { @@ -733,7 +732,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, return this.galleryService.getExtension(extension.gallery.identifier, version) .then(gallery => { if (!gallery) { - return null; + return void 0; } return this.installWithProgress( () => this.extensionService.installFromGallery(gallery) @@ -744,7 +743,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, reinstall(extension: IExtension): Promise { if (!(extension instanceof Extension)) { - return undefined; + return Promise.resolve(); } const ext = extension as Extension; @@ -757,7 +756,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, return this.progressService.withProgress({ location: ProgressLocation.Extensions, source: `${toReinstall[0].identifier.id}` - }, () => Promise.all(toReinstall.map(local => this.extensionService.reinstallFromGallery(local))).then(() => null)); + }, () => Promise.all(toReinstall.map(local => this.extensionService.reinstallFromGallery(local))).then(() => void 0)); } private installWithProgress(installTask: () => Promise, extensionName?: string): Promise { @@ -769,14 +768,14 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } private checkAndEnableDisabledDependencies(extensionIdentifier: IExtensionIdentifier): Promise { - const extension = this.local.filter(e => (e.local || e.gallery) && areSameExtensions(extensionIdentifier, e.local ? e.local.identifier : e.gallery.identifier))[0]; + const extension = this.local.filter(e => (e.local || e.gallery) && areSameExtensions(extensionIdentifier, e.local ? e.local.identifier : e.gallery!.identifier))[0]; if (extension) { const disabledDepencies = this.getExtensionsRecursively([extension], this.local, EnablementState.Enabled, { dependencies: true, pack: false }); if (disabledDepencies.length) { return this.setEnablement(disabledDepencies, EnablementState.Enabled); } } - return Promise.resolve(null); + return Promise.resolve(); } private promptAndSetEnablement(extensions: IExtension[], enablementState: EnablementState): Promise { @@ -877,7 +876,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } private doSetEnablement(extension: IExtension, enablementState: EnablementState): Promise { - return this.extensionEnablementService.setEnablement(extension.local, enablementState) + return this.extensionEnablementService.setEnablement(extension.local!, enablementState) .then(changed => { if (changed) { /* __GDPR__ @@ -930,15 +929,18 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, private onDidInstallExtension(event: DidInstallExtensionEvent): void { const { local, zipPath, error, gallery } = event; const installingExtension = gallery ? this.installing.filter(e => areSameExtensions(e, gallery.identifier))[0] : null; - let extension: Extension = installingExtension ? installingExtension : zipPath ? new Extension(this.galleryService, this.stateProvider, [local], null, this.telemetryService, this.logService) : null; + let extension: Extension | undefined = installingExtension ? installingExtension : zipPath ? new Extension(this.galleryService, this.stateProvider, local ? [local] : [], void 0, this.telemetryService, this.logService) : undefined; if (extension) { this.installing = installingExtension ? this.installing.filter(e => e !== installingExtension) : this.installing; - if (!error) { - const installed = this.installed.filter(e => e.id === extension.id)[0]; + if (local) { + const installed = this.installed.filter(e => e.id === extension!.id)[0]; if (installed) { extension = installed; - const server = this.extensionManagementServerService.getExtensionManagementServer(local.location); - const existingLocal = installed.locals.filter(l => this.extensionManagementServerService.getExtensionManagementServer(l.location).authority === server.authority)[0]; + const newServer = this.extensionManagementServerService.getExtensionManagementServer(local.location); + const existingLocal = newServer && installed.locals.filter(l => { + const server = this.extensionManagementServerService.getExtensionManagementServer(l.location); + return server && server.authority === newServer.authority; + })[0]; if (existingLocal) { const locals = [...installed.locals]; locals.splice(installed.locals.indexOf(existingLocal), 1, local); @@ -952,7 +954,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } } } - this._onChange.fire(error ? null : extension); + this._onChange.fire(error ? undefined : extension); } private onUninstallExtension({ id }: IExtensionIdentifier): void { @@ -997,7 +999,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } private getExtensionState(extension: Extension): ExtensionState { - if (extension.gallery && this.installing.some(e => e.gallery && areSameExtensions(e.gallery.identifier, extension.gallery.identifier))) { + if (extension.gallery && this.installing.some(e => !!e.gallery && areSameExtensions(e.gallery.identifier, extension.gallery!.identifier))) { return ExtensionState.Installing; } @@ -1099,7 +1101,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService, } private resetIgnoreAutoUpdateExtensions(): void { - this.ignoredAutoUpdateExtensions = this.ignoredAutoUpdateExtensions.filter(extensionId => this.local.some(local => local.local && local.local.identifier.id.toLowerCase() === extensionId)); + this.ignoredAutoUpdateExtensions = this.ignoredAutoUpdateExtensions.filter(extensionId => this.local.some(local => !!local.local && local.local.identifier.id.toLowerCase() === extensionId)); } dispose(): void { -- GitLab