From 40905b95ac88cea03619a4fe4a39edcea00a2594 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 6 Mar 2019 15:23:00 +0100 Subject: [PATCH] eng - strict null checks for remoteFileService --- src/tsconfig.strictNullChecks.json | 3 +- src/vs/platform/files/common/files.ts | 2 +- .../externalTerminal.contribution.ts | 2 +- .../contrib/stats/node/workspaceStats.ts | 4 +- .../services/files/node/remoteFileService.ts | 87 +++++++++---------- 5 files changed, 47 insertions(+), 51 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 0815dc052f9..9ececdd3e50 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -433,6 +433,7 @@ "./vs/workbench/services/extensions/node/rpcProtocol.ts", "./vs/workbench/services/extensions/test/node/rpcProtocol.test.ts", "./vs/workbench/services/files/node/encoding.ts", + "./vs/workbench/services/files/node/remoteFileService.ts", "./vs/workbench/services/files/node/fileService.ts", "./vs/workbench/services/files/node/streams.ts", "./vs/workbench/services/files/test/electron-browser/utils.ts", @@ -521,4 +522,4 @@ "./typings/require-monaco.d.ts", "./vs/workbench/contrib/comments/electron-browser/commentThreadWidget.ts" ] -} \ No newline at end of file +} diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index fa87bbd829f..399a6001e09 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -455,7 +455,7 @@ export interface IFileStat extends IBaseStat { } export interface IResolveFileResult { - stat: IFileStat; + stat?: IFileStat; success: boolean; } diff --git a/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts b/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts index d72561ce0a0..34c5c845166 100644 --- a/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts +++ b/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts @@ -91,7 +91,7 @@ CommandsRegistry.registerCommand({ const resources = getMultiSelectedResources(resource, accessor.get(IListService), editorService); return fileService.resolveFiles(resources.map(r => ({ resource: r }))).then(stats => { - const directoriesToOpen = distinct(stats.map(({ stat }) => stat.isDirectory ? stat.resource.fsPath : paths.dirname(stat.resource.fsPath))); + const directoriesToOpen = distinct(stats.filter(data => data.success).map(({ stat }) => stat!.isDirectory ? stat!.resource.fsPath : paths.dirname(stat!.resource.fsPath))); return directoriesToOpen.map(dir => { if (configurationService.getValue().terminal.explorerKind === 'integrated') { const instance = integratedTerminalService.createTerminal({ cwd: dir }, true); diff --git a/src/vs/workbench/contrib/stats/node/workspaceStats.ts b/src/vs/workbench/contrib/stats/node/workspaceStats.ts index 10ea1691f99..ea4cc981bee 100644 --- a/src/vs/workbench/contrib/stats/node/workspaceStats.ts +++ b/src/vs/workbench/contrib/stats/node/workspaceStats.ts @@ -368,7 +368,7 @@ export class WorkspaceStats implements IWorkbenchContribution { } return this.fileService.resolveFiles(folders.map(resource => ({ resource }))).then((files: IResolveFileResult[]) => { - const names = ([]).concat(...files.map(result => result.success ? (result.stat.children || []) : [])).map(c => c.name); + const names = ([]).concat(...files.map(result => result.success ? (result.stat!.children || []) : [])).map(c => c.name); const nameSet = names.reduce((s, n) => s.add(n.toLowerCase()), new Set()); if (participant) { @@ -664,7 +664,7 @@ export class WorkspaceStats implements IWorkbenchContribution { }); return this.fileService.resolveFiles(uris.map(resource => ({ resource }))).then( results => { - const names = ([]).concat(...results.map(result => result.success ? (result.stat.children || []) : [])).map(c => c.name); + const names = ([]).concat(...results.map(result => result.success ? (result.stat!.children || []) : [])).map(c => c.name); const referencesAzure = WorkspaceStats.searchArray(names, /azure/i); if (referencesAzure) { tags['node'] = true; diff --git a/src/vs/workbench/services/files/node/remoteFileService.ts b/src/vs/workbench/services/files/node/remoteFileService.ts index e9beb32812c..36687a83c68 100644 --- a/src/vs/workbench/services/files/node/remoteFileService.ts +++ b/src/vs/workbench/services/files/node/remoteFileService.ts @@ -70,7 +70,7 @@ function toIFileStat(provider: IFileSystemProvider, tuple: [URI, IStat], recurse return Promise.resolve(fileStat); } -export function toDeepIFileStat(provider: IFileSystemProvider, tuple: [URI, IStat], to: URI[]): Promise { +export function toDeepIFileStat(provider: IFileSystemProvider, tuple: [URI, IStat], to?: URI[]): Promise { const trie = TernarySearchTree.forPaths(); trie.set(tuple[0].toString(), true); @@ -281,7 +281,7 @@ export class RemoteFileService extends FileService { FileOperationResult.FILE_NOT_FOUND ); } else { - return data[0].stat; + return data[0].stat!; } }); } @@ -319,7 +319,7 @@ export class RemoteFileService extends FileService { return toDeepIFileStat(provider, [item.resource, stat], item.options && item.options.resolveTo).then(fileStat => { result[idx] = { stat: fileStat, success: true }; }); - }, err => { + }, _err => { result[idx] = { stat: undefined, success: false }; }); }); @@ -440,7 +440,7 @@ export class RemoteFileService extends FileService { return RemoteFileService._mkdirp(provider, resources.dirname(resource)).then(() => { const encoding = this.encoding.getWriteEncoding(resource); - return this._writeFile(provider, resource, new StringSnapshot(content), encoding, { create: true, overwrite: Boolean(options && options.overwrite) }); + return this._writeFile(provider, resource, new StringSnapshot(content || ''), encoding, { create: true, overwrite: Boolean(options && options.overwrite) }); }); }).then(fileStat => { @@ -449,7 +449,7 @@ export class RemoteFileService extends FileService { }, err => { const message = localize('err.create', "Failed to create file {0}", resource.toString(false)); const result = this._tryParseFileOperationResult(err); - throw new FileOperationError(message, result, options); + throw new FileOperationError(message, result || -1, options); }); } } @@ -467,7 +467,7 @@ export class RemoteFileService extends FileService { } } - private _writeFile(provider: IFileSystemProvider, resource: URI, snapshot: ITextSnapshot, preferredEncoding: string, options: FileWriteOptions): Promise { + private _writeFile(provider: IFileSystemProvider, resource: URI, snapshot: ITextSnapshot, preferredEncoding: string | undefined = undefined, options: FileWriteOptions): Promise { const readable = createReadableOfSnapshot(snapshot); const encoding = this.encoding.getWriteEncoding(resource, preferredEncoding); const encoder = encodeStream(encoding); @@ -549,13 +549,13 @@ export class RemoteFileService extends FileService { } } - private _doMoveWithInScheme(source: URI, target: URI, overwrite?: boolean): Promise { + private async _doMoveWithInScheme(source: URI, target: URI, overwrite: boolean = false): Promise { - const prepare = overwrite - ? Promise.resolve(this.del(target, { recursive: true }).then(undefined, err => { /*ignore*/ })) - : Promise.resolve(null); + if (overwrite) { + await this.del(target, { recursive: true }).catch(_err => { /*ignore*/ }); + } - return prepare.then(() => this._withProvider(source)).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { + return this._withProvider(source).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { return RemoteFileService._mkdirp(provider, resources.dirname(target)).then(() => { return provider.rename(source, target, { overwrite }).then(() => { return this.resolveFile(target); @@ -589,11 +589,11 @@ export class RemoteFileService extends FileService { return super.copyFile(source, target, overwrite); } - return this._withProvider(target).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { + return this._withProvider(target).then(RemoteFileService._throwIfFileSystemIsReadonly).then(async provider => { if (source.scheme === target.scheme && (provider.capabilities & FileSystemProviderCapabilities.FileFolderCopy)) { // good: provider supports copy withing scheme - return provider.copy(source, target, { overwrite: !!overwrite }).then(() => { + return provider.copy!(source, target, { overwrite: !!overwrite }).then(() => { return this.resolveFile(target); }).then(fileStat => { this._onAfterOperation.fire(new FileOperationEvent(source, FileOperation.COPY, fileStat)); @@ -607,51 +607,46 @@ export class RemoteFileService extends FileService { }); } - const prepare = overwrite - ? Promise.resolve(this.del(target, { recursive: true }).then(undefined, err => { /*ignore*/ })) - : Promise.resolve(null); + if (overwrite) { + await this.del(target, { recursive: true }).catch(_err => { /*ignore*/ }); + } - return prepare.then(() => { - // todo@ben, can only copy text files - // https://github.com/Microsoft/vscode/issues/41543 - return this.resolveContent(source, { acceptTextOnly: true }).then(content => { - return this._withProvider(target).then(provider => { - return this._writeFile( - provider, target, - new StringSnapshot(content.value), - content.encoding, - { create: true, overwrite: !!overwrite } - ).then(fileStat => { - this._onAfterOperation.fire(new FileOperationEvent(source, FileOperation.COPY, fileStat)); - return fileStat; - }); - }, err => { - const result = this._tryParseFileOperationResult(err); - if (result === FileOperationResult.FILE_MOVE_CONFLICT) { - throw new FileOperationError(localize('fileMoveConflict', "Unable to move/copy. File already exists at destination."), result); - } else if (err instanceof Error && err.name === 'ENOPRO') { - // file scheme - return super.updateContent(target, content.value, { encoding: content.encoding }); - } else { - return Promise.reject(err); - } + // todo@ben, can only copy text files + // https://github.com/Microsoft/vscode/issues/41543 + return this.resolveContent(source, { acceptTextOnly: true }).then(content => { + return this._withProvider(target).then(provider => { + return this._writeFile( + provider, target, + new StringSnapshot(content.value), + content.encoding, + { create: true, overwrite: !!overwrite } + ).then(fileStat => { + this._onAfterOperation.fire(new FileOperationEvent(source, FileOperation.COPY, fileStat)); + return fileStat; }); + }, err => { + const result = this._tryParseFileOperationResult(err); + if (result === FileOperationResult.FILE_MOVE_CONFLICT) { + throw new FileOperationError(localize('fileMoveConflict', "Unable to move/copy. File already exists at destination."), result); + } else if (err instanceof Error && err.name === 'ENOPRO') { + // file scheme + return super.updateContent(target, content.value, { encoding: content.encoding }); + } else { + return Promise.reject(err); + } }); }); + }); } private _activeWatches = new Map, count: number }>(); - watchFileChanges(resource: URI, opts?: IWatchOptions): void { + watchFileChanges(resource: URI, opts: IWatchOptions = { recursive: false, excludes: [] }): void { if (resource.scheme === Schemas.file) { return super.watchFileChanges(resource); } - if (!opts) { - opts = { recursive: false, excludes: [] }; - } - const key = resource.toString(); const entry = this._activeWatches.get(key); if (entry) { @@ -663,7 +658,7 @@ export class RemoteFileService extends FileService { count: 1, unwatch: this._withProvider(resource).then(provider => { return provider.watch(resource, opts); - }, err => { + }, _err => { return { dispose() { } }; }) });