Merge pull request #69921 from mjbvz/strict-null-textFileService

Strict null check textFileService

src/tsconfig.strictNullChecks.json

@@ -489,6 +489,7 @@
 		"./vs/workbench/services/textMate/electron-browser/textMateService.ts",
 		"./vs/workbench/services/textfile/common/textFileEditorModel.ts",
 		"./vs/workbench/services/textfile/common/textFileEditorModelManager.ts",
+		"./vs/workbench/services/textfile/common/textFileService.ts",
 		"./vs/workbench/services/textfile/common/textfiles.ts",
 		"./vs/workbench/services/textfile/node/textResourcePropertiesService.ts",
 		"./vs/workbench/services/textmodelResolver/common/textModelResolverService.ts",

src/vs/editor/contrib/codeAction/codeAction.ts

@@ -16,9 +16,26 @@ import { IModelService } from 'vs/editor/common/services/modelService';
 import { CodeActionKind, CodeActionTrigger, filtersAction, mayIncludeActionsOfKind, CodeActionFilter } from './codeActionTrigger';
 
 export class CodeActionSet {
-	public constructor(
-		public readonly actions: ReadonlyArray<CodeAction>
-	) { }
+
+	private static codeActionsComparator(a: CodeAction, b: CodeAction): number {
+		if (isNonEmptyArray(a.diagnostics)) {
+			if (isNonEmptyArray(b.diagnostics)) {
+				return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message);
+			} else {
+				return -1;
+			}
+		} else if (isNonEmptyArray(b.diagnostics)) {
+			return 1;
+		} else {
+			return 0;	// both have no diagnostics
+		}
+	}
+
+	public readonly actions: ReadonlyArray<CodeAction>;
+
+	public constructor(actions: CodeAction[]) {
+		this.actions = mergeSort(actions, CodeActionSet.codeActionsComparator);
+	}
 
 	public get hasAutoFix() {
 		return this.actions.some(fix => !!fix.kind && CodeActionKind.QuickFix.contains(new CodeActionKind(fix.kind)) && !!fix.isPreferred);
@@ -56,7 +73,6 @@ export function getCodeActions(
 
 	return Promise.all(promises)
 		.then(flatten)
-		.then(allCodeActions => mergeSort(allCodeActions, codeActionsComparator))
 		.then(actions => new CodeActionSet(actions));
 }
 
@@ -75,20 +91,6 @@ function getCodeActionProviders(
 		});
 }
 
-function codeActionsComparator(a: CodeAction, b: CodeAction): number {
-	if (isNonEmptyArray(a.diagnostics)) {
-		if (isNonEmptyArray(b.diagnostics)) {
-			return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message);
-		} else {
-			return -1;
-		}
-	} else if (isNonEmptyArray(b.diagnostics)) {
-		return 1;
-	} else {
-		return 0;	// both have no diagnostics
-	}
-}
-
 registerLanguageCommand('_executeCodeActionProvider', function (accessor, args): Promise<ReadonlyArray<CodeAction>> {
 	const { resource, range, kind } = args;
 	if (!(resource instanceof URI) || !Range.isIRange(range)) {

src/vs/workbench/services/textfile/common/textFileService.ts

@@ -123,7 +123,7 @@ export class TextFileService extends Disposable implements ITextFileService {
 		});
 	}
 
-	promptForPath(resource: URI, defaultUri: URI): Promise<URI> {
+	promptForPath(resource: URI, defaultUri: URI): Promise<URI | undefined> {
 
 		// Help user to find a name for the file by opening it first
 		return this.editorService.openEditor({ resource, options: { revealIfOpened: true, preserveFocus: true, } }).then(() => {
@@ -145,7 +145,7 @@ export class TextFileService extends Disposable implements ITextFileService {
 		interface IFilter { name: string; extensions: string[]; }
 
 		// Build the file filter by using our known languages
-		const ext: string = defaultUri ? extname(defaultUri) : undefined;
+		const ext: string | undefined = defaultUri ? extname(defaultUri) : undefined;
 		let matchingFilter: IFilter | undefined;
 		const filters: IFilter[] = coalesce(this.modeService.getRegisteredLanguageNames().map(languageName => {
 			const extensions = this.modeService.getExtensions(languageName);
@@ -256,7 +256,7 @@ export class TextFileService extends Disposable implements ITextFileService {
 						return this.handleDirtyBeforeShutdown(remainingDirty, reason);
 					}
 
-					return undefined;
+					return false;
 				});
 			}
 
@@ -344,7 +344,10 @@ export class TextFileService extends Disposable implements ITextFileService {
 		const untitledToBackup: URI[] = [];
 		dirtyToBackup.forEach(s => {
 			if (this.fileService.canHandleResource(s)) {
-				filesToBackup.push(textFileEditorModelManager.get(s));
+				const model = textFileEditorModelManager.get(s);
+				if (model) {
+					filesToBackup.push(model);
+				}
 			} else if (s.scheme === Schemas.untitled) {
 				untitledToBackup.push(s);
 			}
@@ -354,9 +357,16 @@ export class TextFileService extends Disposable implements ITextFileService {
 	}
 
 	private doBackupAll(dirtyFileModels: ITextFileEditorModel[], untitledResources: URI[]): Promise<void> {
+		const promises = dirtyFileModels.map(model => {
+			const snapshot = model.createSnapshot();
+			if (snapshot) {
+				return this.backupFileService.backupResource(model.getResource(), snapshot, model.getVersionId());
+			}
+			return Promise.resolve();
+		});
 
 		// Handle file resources first
-		return Promise.all(dirtyFileModels.map(model => this.backupFileService.backupResource(model.getResource(), model.createSnapshot(), model.getVersionId()))).then(results => {
+		return Promise.all(promises).then(results => {
 
 			// Handle untitled resources
 			const untitledModelPromises = untitledResources
@@ -365,7 +375,11 @@ export class TextFileService extends Disposable implements ITextFileService {
 
 			return Promise.all(untitledModelPromises).then(untitledModels => {
 				const untitledBackupPromises = untitledModels.map(model => {
-					return this.backupFileService.backupResource(model.getResource(), model.createSnapshot(), model.getVersionId());
+					const snapshot = model.createSnapshot();
+					if (snapshot) {
+						return this.backupFileService.backupResource(model.getResource(), snapshot, model.getVersionId());
+					}
+					return Promise.resolve();
 				});
 
 				return Promise.all(untitledBackupPromises).then(() => undefined);
@@ -402,7 +416,7 @@ export class TextFileService extends Disposable implements ITextFileService {
 				return true; // veto
 			}
 
-			return undefined;
+			return false;
 		});
 	}
 
@@ -620,7 +634,10 @@ export class TextFileService extends Disposable implements ITextFileService {
 		return Promise.all(dirtyFileModels.map(model => {
 			return model.save(options).then(() => {
 				if (!model.isDirty()) {
-					mapResourceToResult.get(model.getResource()).success = true;
+					const result = mapResourceToResult.get(model.getResource());
+					if (result) {
+						result.success = true;
+					}
 				}
 			});
 		})).then(r => ({ results: mapResourceToResult.values() }));
@@ -647,10 +664,10 @@ export class TextFileService extends Disposable implements ITextFileService {
 		return this.getFileModels(arg1).filter(model => model.isDirty());
 	}
 
-	saveAs(resource: URI, target?: URI, options?: ISaveOptions): Promise<URI> {
+	saveAs(resource: URI, target?: URI, options?: ISaveOptions): Promise<URI | undefined> {
 
 		// Get to target resource
-		let targetPromise: Promise<URI>;
+		let targetPromise: Promise<URI | undefined>;
 		if (target) {
 			targetPromise = Promise.resolve(target);
 		} else {
@@ -662,9 +679,9 @@ export class TextFileService extends Disposable implements ITextFileService {
 			targetPromise = this.promptForPath(resource, dialogPath);
 		}
 
-		return targetPromise.then(target => {
+		return targetPromise.then<URI | undefined>(target => {
 			if (!target) {
-				return null; // user canceled
+				return undefined; // user canceled
 			}
 
 			// Just save if target is same as models own resource
@@ -677,10 +694,10 @@ export class TextFileService extends Disposable implements ITextFileService {
 		});
 	}
 
-	private doSaveAs(resource: URI, target?: URI, options?: ISaveOptions): Promise<URI> {
+	private doSaveAs(resource: URI, target: URI, options?: ISaveOptions): Promise<URI> {
 
 		// Retrieve text model from provided resource if any
-		let modelPromise: Promise<ITextFileEditorModel | UntitledEditorModel> = Promise.resolve(null);
+		let modelPromise: Promise<ITextFileEditorModel | UntitledEditorModel | undefined> = Promise.resolve(undefined);
 		if (this.fileService.canHandleResource(resource)) {
 			modelPromise = Promise.resolve(this._models.get(resource));
 		} else if (resource.scheme === Schemas.untitled && this.untitledEditorService.exists(resource)) {
@@ -725,7 +742,7 @@ export class TextFileService extends Disposable implements ITextFileService {
 
 		// Otherwise create the target file empty if it does not exist already and resolve it from there
 		else {
-			targetModelResolver = this.fileService.existsFile(target).then(exists => {
+			targetModelResolver = this.fileService.existsFile(target).then<any>(exists => {
 				targetExists = exists;
 
 				// create target model adhoc if file does not exist yet
@@ -757,7 +774,12 @@ export class TextFileService extends Disposable implements ITextFileService {
 
 				// take over encoding and model value from source model
 				targetModel.updatePreferredEncoding(sourceModel.getEncoding());
-				this.modelService.updateModel(targetModel.textEditorModel, createTextBufferFactoryFromSnapshot(sourceModel.createSnapshot()));
+				if (targetModel.textEditorModel) {
+					const snapshot = sourceModel.createSnapshot();
+					if (snapshot) {
+						this.modelService.updateModel(targetModel.textEditorModel, createTextBufferFactoryFromSnapshot(snapshot));
+					}
+				}
 
 				// save model
 				return targetModel.save(options).then(() => true);
@@ -775,7 +797,6 @@ export class TextFileService extends Disposable implements ITextFileService {
 
 	private suggestFileName(untitledResource: URI): URI {
 		const untitledFileName = this.untitledEditorService.suggestFileName(untitledResource);
-
 		const remoteAuthority = this.windowService.getConfiguration().remoteAuthority;
 		const schemeFilter = remoteAuthority ? REMOTE_HOST_SCHEME : Schemas.file;
 
@@ -823,13 +844,19 @@ export class TextFileService extends Disposable implements ITextFileService {
 		return Promise.all(fileModels.map(model => {
 			return model.revert(options && options.soft).then(() => {
 				if (!model.isDirty()) {
-					mapResourceToResult.get(model.getResource()).success = true;
+					const result = mapResourceToResult.get(model.getResource());
+					if (result) {
+						result.success = true;
+					}
 				}
 			}, error => {
 
 				// FileNotFound means the file got deleted meanwhile, so still record as successful revert
 				if ((<FileOperationError>error).fileOperationResult === FileOperationResult.FILE_NOT_FOUND) {
-					mapResourceToResult.get(model.getResource()).success = true;
+					const result = mapResourceToResult.get(model.getResource());
+					if (result) {
+						result.success = true;
+					}
 				}
 
 				// Otherwise bubble up the error
@@ -915,7 +942,11 @@ export class TextFileService extends Disposable implements ITextFileService {
 						dirtyTargetModels.push(targetModelResource);
 
 						// Backup dirty source model to the target resource it will become later
-						return this.backupFileService.backupResource(targetModelResource, sourceModel.createSnapshot(), sourceModel.getVersionId());
+						const snapshot = sourceModel.createSnapshot();
+						if (snapshot) {
+							return this.backupFileService.backupResource(targetModelResource, snapshot, sourceModel.getVersionId());
+						}
+						return Promise.resolve();
 					}));
 				} else {
 					handleDirtySourceModels = Promise.resolve();

src/vs/workbench/services/textfile/common/textfiles.ts

@@ -101,7 +101,7 @@ export interface IResult {
 }
 
 export interface IAutoSaveConfiguration {
-	autoSaveDelay: number;
+	autoSaveDelay?: number;
 	autoSaveFocusChange: boolean;
 	autoSaveApplicationChange: boolean;
 }
@@ -257,6 +257,8 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport
 
 export interface IResolvedTextFileEditorModel extends ITextFileEditorModel {
 	readonly textEditorModel: ITextModel;
+
+	createSnapshot(): ITextSnapshot;
 }
 
 
@@ -317,9 +319,9 @@ export interface ITextFileService extends IDisposable {
 	 * @param resource the resource to save as.
 	 * @param targetResource the optional target to save to.
 	 * @param options optional save options
-	 * @return true if the file was saved.
+	 * @return Path of the saved resource.
 	 */
-	saveAs(resource: URI, targetResource?: URI, options?: ISaveOptions): Promise<URI>;
+	saveAs(resource: URI, targetResource?: URI, options?: ISaveOptions): Promise<URI | undefined>;
 
 	/**
 	 * Saves the set of resources and returns a promise with the operation result.

src/vs/workbench/services/untitled/common/untitledEditorService.ts

@@ -15,6 +15,7 @@ import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorMo
 import { Schemas } from 'vs/base/common/network';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
+import { basename } from 'vs/base/common/resources';
 
 export const IUntitledEditorService = createDecorator<IUntitledEditorService>('untitledEditorService');
 
@@ -96,7 +97,7 @@ export interface IUntitledEditorService {
 	/**
 	 * Suggests a filename for the given untitled resource if it is known.
 	 */
-	suggestFileName(resource: URI): string | undefined;
+	suggestFileName(resource: URI): string;
 
 	/**
 	 * Get the configured encoding for the given untitled resource if any.
@@ -266,10 +267,10 @@ export class UntitledEditorService extends Disposable implements IUntitledEditor
 		return this.mapResourceToAssociatedFilePath.has(resource);
 	}
 
-	suggestFileName(resource: URI): string | undefined {
+	suggestFileName(resource: URI): string {
 		const input = this.get(resource);
 
-		return input ? input.suggestFileName() : undefined;
+		return input ? input.suggestFileName() : basename(resource);
 	}
 
 	getEncoding(resource: URI): string | undefined {