From cd1a6b4fbef676d2872e283ae298034c5f79e878 Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Mon, 5 Nov 2018 17:19:27 -0800 Subject: [PATCH] Add strict null checks in issue and launch services, #60565 --- src/tsconfig.strictNullChecks.json | 2 ++ .../issue/electron-main/issueService.ts | 35 ++++++++++--------- .../launch/electron-main/launchService.ts | 26 ++++++++------ 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 0e62ee09ce3..dd7b39ec7cd 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -429,6 +429,7 @@ "./vs/platform/integrity/common/integrity.ts", "./vs/platform/integrity/node/integrityServiceImpl.ts", "./vs/platform/issue/common/issue.ts", + "./vs/platform/issue/electron-main/issueService.ts", "./vs/platform/issue/node/issueIpc.ts", "./vs/platform/jsonschemas/common/jsonContributionRegistry.ts", "./vs/platform/keybinding/common/abstractKeybindingService.ts", @@ -440,6 +441,7 @@ "./vs/platform/keybinding/test/common/mockKeybindingService.ts", "./vs/platform/label/common/label.ts", "./vs/platform/label/electron-browser/label.contribution.ts", + "./vs/platform/launch/electron-main/launchService.ts", "./vs/platform/lifecycle/common/lifecycle.ts", "./vs/platform/lifecycle/electron-browser/lifecycleService.ts", "./vs/platform/lifecycle/electron-main/lifecycleMain.ts", diff --git a/src/vs/platform/issue/electron-main/issueService.ts b/src/vs/platform/issue/electron-main/issueService.ts index 1cb9f623661..fc0988603d0 100644 --- a/src/vs/platform/issue/electron-main/issueService.ts +++ b/src/vs/platform/issue/electron-main/issueService.ts @@ -15,14 +15,15 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment' import { isMacintosh, IProcessEnvironment } from 'vs/base/common/platform'; import { ILogService } from 'vs/platform/log/common/log'; import { IWindowsService } from 'vs/platform/windows/common/windows'; +import { IWindowState } from 'vs/platform/windows/electron-main/windows'; const DEFAULT_BACKGROUND_COLOR = '#1E1E1E'; export class IssueService implements IIssueService { _serviceBrand: any; - _issueWindow: BrowserWindow; + _issueWindow: BrowserWindow | null; _issueParentWindow: BrowserWindow; - _processExplorerWindow: BrowserWindow; + _processExplorerWindow: BrowserWindow | null; constructor( private machineId: string, @@ -108,7 +109,7 @@ export class IssueService implements IIssueService { this._issueWindow.focus(); - return TPromise.as(null); + return TPromise.as(undefined); } openProcessExplorer(data: ProcessExplorerData): TPromise { @@ -150,7 +151,7 @@ export class IssueService implements IIssueService { this._processExplorerWindow.loadURL(`${require.toUrl('vs/code/electron-browser/processExplorer/processExplorer.html')}?config=${encodeURIComponent(JSON.stringify(config))}`); - this._processExplorerWindow.on('close', () => this._processExplorerWindow = void 0); + this._processExplorerWindow.on('close', () => this._processExplorerWindow = null); parentWindow.on('close', () => { if (this._processExplorerWindow) { @@ -163,12 +164,12 @@ export class IssueService implements IIssueService { // Focus this._processExplorerWindow.focus(); - return TPromise.as(null); + return TPromise.as(undefined); } - private getWindowPosition(parentWindow: BrowserWindow, defaultWidth: number, defaultHeight: number) { + private getWindowPosition(parentWindow: BrowserWindow, defaultWidth: number, defaultHeight: number): IWindowState { // We want the new window to open on the same display that the parent is in - let displayToUse: Electron.Display; + let displayToUse: Electron.Display | undefined; const displays = screen.getAllDisplays(); // Single Display @@ -196,16 +197,14 @@ export class IssueService implements IIssueService { } } - let state = { + const state: IWindowState = { width: defaultWidth, - height: defaultHeight, - x: undefined, - y: undefined + height: defaultHeight }; const displayBounds = displayToUse.bounds; - state.x = displayBounds.x + (displayBounds.width / 2) - (state.width / 2); - state.y = displayBounds.y + (displayBounds.height / 2) - (state.height / 2); + state.x = displayBounds.x + (displayBounds.width / 2) - (state.width! / 2); + state.y = displayBounds.y + (displayBounds.height / 2) - (state.height! / 2); if (displayBounds.width > 0 && displayBounds.height > 0 /* Linux X11 sessions sometimes report wrong display bounds */) { if (state.x < displayBounds.x) { @@ -224,11 +223,11 @@ export class IssueService implements IIssueService { state.y = displayBounds.y; // prevent window from falling out of the screen to the bottom } - if (state.width > displayBounds.width) { + if (state.width! > displayBounds.width) { state.width = displayBounds.width; // prevent window from exceeding display bounds width } - if (state.height > displayBounds.height) { + if (state.height! > displayBounds.height) { state.height = displayBounds.height; // prevent window from exceeding display bounds height } } @@ -259,7 +258,11 @@ export class IssueService implements IIssueService { }); } - private getIssueReporterPath(data: IssueReporterData, features: IssueReporterFeatures) { + private getIssueReporterPath(data: IssueReporterData, features: IssueReporterFeatures): string { + if (!this._issueWindow) { + throw new Error('Issue window has been disposed'); + } + const windowConfiguration = { appRoot: this.environmentService.appRoot, nodeCachedDataDir: this.environmentService.nodeCachedDataDir, diff --git a/src/vs/platform/launch/electron-main/launchService.ts b/src/vs/platform/launch/electron-main/launchService.ts index e7981250608..4fb07311aa5 100644 --- a/src/vs/platform/launch/electron-main/launchService.ts +++ b/src/vs/platform/launch/electron-main/launchService.ts @@ -41,6 +41,10 @@ export interface IMainProcessInfo { windows: IWindowInfo[]; } +function isUri(uri: URI | null): uri is URI { + return !!uri; +} + function parseOpenUrl(args: ParsedArgs): URI[] { if (args['open-url'] && args._urls && args._urls.length > 0) { // --open-url must contain -- followed by the url(s) @@ -53,7 +57,7 @@ function parseOpenUrl(args: ParsedArgs): URI[] { return null; } }) - .filter(uri => !!uri); + .filter(isUri); } return []; @@ -99,7 +103,7 @@ export class LaunchChannel implements ILaunchChannel { return this.service.getLogsPath(); } - return undefined; + throw new Error(`Command '${command}' not found`); } } @@ -161,7 +165,7 @@ export class LaunchService implements ILaunchService { } }); - return TPromise.as(null); + return TPromise.as(undefined); } // Otherwise handle in windows service @@ -170,7 +174,7 @@ export class LaunchService implements ILaunchService { private startOpenWindow(args: ParsedArgs, userEnv: IProcessEnvironment): TPromise { const context = !!userEnv['VSCODE_CLI'] ? OpenContext.CLI : OpenContext.DESKTOP; - let usedWindows: ICodeWindow[]; + let usedWindows: ICodeWindow[] | undefined; // Special case extension development if (!!args.extensionDevelopmentPath) { @@ -231,14 +235,14 @@ export class LaunchService implements ILaunchService { // If the other instance is waiting to be killed, we hook up a window listener if one window // is being used and only then resolve the startup promise which will kill this second instance. // In addition, we poll for the wait marker file to be deleted to return. - if (args.wait && args.waitMarkerFilePath && usedWindows.length === 1 && usedWindows[0]) { + if (args.wait && args.waitMarkerFilePath && usedWindows && usedWindows.length === 1 && usedWindows[0]) { return Promise.race([ this.windowsMainService.waitForWindowCloseOrLoad(usedWindows[0].id), whenDeleted(args.waitMarkerFilePath) ]).then(() => void 0, () => void 0); } - return TPromise.as(null); + return TPromise.as(undefined); } getMainProcessId(): TPromise { @@ -279,10 +283,12 @@ export class LaunchService implements ILaunchService { if (window.openedFolderUri) { folderURIs.push(window.openedFolderUri); } else if (window.openedWorkspace) { - const rootFolders = this.workspacesMainService.resolveWorkspaceSync(window.openedWorkspace.configPath).folders; - rootFolders.forEach(root => { - folderURIs.push(root.uri); - }); + const workspace = this.workspacesMainService.resolveWorkspaceSync(window.openedWorkspace.configPath); + if (workspace) { + workspace.folders.forEach(root => { + folderURIs.push(root.uri); + }); + } } return this.browserWindowToInfo(window.win, folderURIs);