Strict null on some query and connection (#7300)

* wip

* make connection work with strict-nulls

* change comments

* fix tests; remove unneeded type forcing

* address feedback

* adjust the logic of query editor

* clean up typing

src/sql/platform/connection/common/connectionConfig.ts

@@ -18,7 +18,7 @@ const CONNECTIONS_CONFIG_KEY = 'datasource.connections';
 
 export interface ISaveGroupResult {
 	groups: IConnectionProfileGroup[];
-	newGroupId: string;
+	newGroupId?: string;
 }
 
 /**
@@ -37,7 +37,9 @@ export class ConnectionConfig {
 	public getAllGroups(): IConnectionProfileGroup[] {
 
 		let allGroups: IConnectionProfileGroup[] = [];
-		let { user, workspace } = this.configurationService.inspect<IConnectionProfileGroup[]>(GROUPS_CONFIG_KEY);
+		const config = this.configurationService.inspect<IConnectionProfileGroup[]>(GROUPS_CONFIG_KEY);
+		let { user } = config;
+		const { workspace } = config;
 
 		if (user) {
 			if (workspace) {
@@ -109,7 +111,7 @@ export class ConnectionConfig {
 			let result = this.saveGroup(groups, profile.groupFullName, undefined, undefined);
 			groups = result.groups;
 
-			return this.configurationService.updateValue(GROUPS_CONFIG_KEY, groups, ConfigurationTarget.USER).then(() => result.newGroupId);
+			return this.configurationService.updateValue(GROUPS_CONFIG_KEY, groups, ConfigurationTarget.USER).then(() => result.newGroupId!);
 		}
 	}
 
@@ -129,7 +131,7 @@ export class ConnectionConfig {
 				let result = this.saveGroup(groups, profileGroup.name, profileGroup.color, profileGroup.description);
 				groups = result.groups;
 
-				return this.configurationService.updateValue(GROUPS_CONFIG_KEY, groups, ConfigurationTarget.USER).then(() => result.newGroupId);
+				return this.configurationService.updateValue(GROUPS_CONFIG_KEY, groups, ConfigurationTarget.USER).then(() => result.newGroupId!);
 			}
 		}
 	}
@@ -138,18 +140,22 @@ export class ConnectionConfig {
 		let configs = this.configurationService.inspect<IConnectionProfileStore[]>(CONNECTIONS_CONFIG_KEY);
 		let profiles: IConnectionProfileStore[];
 		if (configs) {
+			let fromConfig: IConnectionProfileStore[] | undefined;
 			if (configTarget === ConfigurationTarget.USER) {
-				profiles = configs.user;
+				fromConfig = configs.user;
 			} else if (configTarget === ConfigurationTarget.WORKSPACE) {
-				profiles = configs.workspace;
+				fromConfig = configs.workspace || [];
 			}
-			if (profiles) {
+			if (fromConfig) {
+				profiles = fromConfig;
 				if (this.fixConnectionIds(profiles)) {
 					this.configurationService.updateValue(CONNECTIONS_CONFIG_KEY, profiles, configTarget);
 				}
 			} else {
 				profiles = [];
 			}
+		} else {
+			profiles = [];
 		}
 
 		return profiles;
@@ -315,7 +321,7 @@ export class ConnectionConfig {
 		}
 	}
 
-	public saveGroup(groups: IConnectionProfileGroup[], groupFullName: string, color: string, description: string): ISaveGroupResult {
+	public saveGroup(groups: IConnectionProfileGroup[], groupFullName?: string, color?: string, description?: string): ISaveGroupResult {
 		let groupNames = ConnectionProfileGroup.getGroupFullNameParts(groupFullName);
 		return this.saveGroupInTree(groups, undefined, groupNames, color, description, 0);
 	}
@@ -348,21 +354,21 @@ export class ConnectionConfig {
 		return sameGroupName;
 	}
 
-	private saveGroupInTree(groupTree: IConnectionProfileGroup[], parentId: string, groupNames: string[], color: string, description: string, index: number): ISaveGroupResult {
+	private saveGroupInTree(groupTree: IConnectionProfileGroup[], parentId: string | undefined, groupNames: string[], color: string | undefined, description: string | undefined, index: number): ISaveGroupResult {
 		if (!groupTree) {
 			groupTree = [];
 		}
-		let newGroupId: string;
+		let newGroupId: string | undefined;
 
 		if (index < groupNames.length) {
 			let groupName: string = groupNames[index];
-			let newGroup: IConnectionProfileGroup = {
+			let newGroup = <unknown>{ // workaround to make this work properly
 				name: groupName,
 				id: undefined,
 				parentId: parentId,
 				color: color,
 				description: description
-			};
+			} as IConnectionProfileGroup;
 			let found = groupTree.find(group => this.isSameGroupName(group, newGroup));
 			if (found) {
 				if (index === groupNames.length - 1) {

src/sql/platform/connection/common/connectionProfile.ts

@@ -25,7 +25,7 @@ export class ConnectionProfile extends ProviderConnectionInfo implements interfa
 	public parent?: ConnectionProfileGroup;
 	private _id: string;
 	public savePassword: boolean;
-	private _groupName: string;
+	private _groupName?: string;
 	public groupId: string;
 	public saveProfile: boolean;
 
@@ -119,11 +119,11 @@ export class ConnectionProfile extends ProviderConnectionInfo implements interfa
 		this.options['registeredServerDescription'] = value;
 	}
 
-	public get groupFullName(): string {
+	public get groupFullName(): string | undefined {
 		return this._groupName;
 	}
 
-	public set groupFullName(value: string) {
+	public set groupFullName(value: string | undefined) {
 		this._groupName = value;
 	}
 
@@ -214,15 +214,12 @@ export class ConnectionProfile extends ProviderConnectionInfo implements interfa
 		return !profile.databaseName || profile.databaseName.trim() === '';
 	}
 
-	public static fromIConnectionProfile(capabilitiesService: ICapabilitiesService, profile: azdata.IConnectionProfile) {
-		if (profile) {
-			if (profile instanceof ConnectionProfile) {
-				return profile;
-			} else {
-				return new ConnectionProfile(capabilitiesService, profile);
-			}
+	public static fromIConnectionProfile(capabilitiesService: ICapabilitiesService, profile: azdata.IConnectionProfile): ConnectionProfile {
+		if (profile instanceof ConnectionProfile) {
+			return profile;
+		} else {
+			return new ConnectionProfile(capabilitiesService, profile);
 		}
-		return undefined;
 	}
 
 	public static createFromStoredProfile(profile: interfaces.IConnectionProfileStore, capabilitiesService: ICapabilitiesService): ConnectionProfile {
@@ -244,24 +241,18 @@ export class ConnectionProfile extends ProviderConnectionInfo implements interfa
 
 	public static convertToProfileStore(
 		capabilitiesService: ICapabilitiesService,
-		connectionProfile: interfaces.IConnectionProfile): interfaces.IConnectionProfileStore | undefined {
-		if (connectionProfile) {
-			let connectionInfo = ConnectionProfile.fromIConnectionProfile(capabilitiesService, connectionProfile);
-			if (connectionInfo) {
-				let profile: interfaces.IConnectionProfileStore = {
-					options: {},
-					groupId: connectionProfile.groupId,
-					providerName: connectionInfo.providerName,
-					savePassword: connectionInfo.savePassword,
-					id: connectionInfo.id
-				};
+		connectionProfile: interfaces.IConnectionProfile): interfaces.IConnectionProfileStore {
+		let connectionInfo = ConnectionProfile.fromIConnectionProfile(capabilitiesService, connectionProfile);
+		let profile: interfaces.IConnectionProfileStore = {
+			options: {},
+			groupId: connectionProfile.groupId,
+			providerName: connectionInfo.providerName,
+			savePassword: connectionInfo.savePassword,
+			id: connectionInfo.id
+		};
 
-				profile.options = connectionInfo.options;
+		profile.options = connectionInfo.options;
 
-				return profile;
-			}
-		}
-
-		return undefined;
+		return profile;
 	}
 }

src/sql/platform/connection/common/connectionProfileGroup.ts

@@ -5,13 +5,14 @@
 
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
 import { Disposable } from 'vs/base/common/lifecycle';
+import { isUndefinedOrNull } from 'vs/base/common/types';
 
 export interface IConnectionProfileGroup {
 	id: string;
 	parentId?: string;
 	name: string;
-	color: string;
-	description: string;
+	color?: string;
+	description?: string;
 }
 
 export class ConnectionProfileGroup extends Disposable implements IConnectionProfileGroup {
@@ -22,10 +23,10 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
 	private _isRenamed: boolean;
 	public constructor(
 		public name: string,
-		public parent: ConnectionProfileGroup,
+		public parent: ConnectionProfileGroup | undefined,
 		public id: string,
-		public color: string,
-		public description: string
+		public color?: string,
+		public description?: string
 	) {
 		super();
 		this.parentId = parent ? parent.id : undefined;
@@ -150,7 +151,7 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
 		});
 	}
 
-	public getParent(): ConnectionProfileGroup {
+	public getParent(): ConnectionProfileGroup | undefined {
 		return this.parent;
 	}
 
@@ -167,7 +168,7 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
 		return isAncestor;
 	}
 
-	public static getGroupFullNameParts(groupFullName: string): string[] {
+	public static getGroupFullNameParts(groupFullName?: string): string[] {
 		groupFullName = groupFullName ? groupFullName : '';
 		let groupNames: string[] = groupFullName.split(ConnectionProfileGroup.GroupNameSeparator);
 		groupNames = groupNames.filter(g => !!g);
@@ -185,13 +186,19 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
 			name === ConnectionProfileGroup.GroupNameSeparator);
 	}
 
-	public static sameGroupName(name1: string, name2: string): boolean {
-		let sameGroupName: boolean =
-			(!name1 && !name2) ||
-			name1.toUpperCase() === name2.toUpperCase() ||
-			(ConnectionProfileGroup.isRoot(name1) && ConnectionProfileGroup.isRoot(name2));
-
-		return sameGroupName;
+	public static sameGroupName(name1?: string, name2?: string): boolean {
+		const isName1Undefined = isUndefinedOrNull(name1);
+		const isName2Undefined = isUndefinedOrNull(name2);
+		if (isName1Undefined && isName2Undefined) {
+			return true;
+		}
+		if ((isName1Undefined && !isName2Undefined) || !isName1Undefined && isName2Undefined) {
+			return false;
+		}
+		if (name1!.toUpperCase() === name2!.toUpperCase()) {
+			return true;
+		}
+		return ConnectionProfileGroup.isRoot(name1!) && ConnectionProfileGroup.isRoot(name2!);
 	}
 
 	public static getConnectionsInGroup(group: ConnectionProfileGroup): ConnectionProfile[] {

src/sql/platform/connection/common/connectionStatusManager.ts

@@ -27,7 +27,7 @@ export class ConnectionStatusManager {
 		this._connections = {};
 	}
 
-	public findConnection(uri: string): ConnectionManagementInfo {
+	public findConnection(uri: string): ConnectionManagementInfo | undefined {
 		if (uri in this._connections) {
 			return this._connections[uri];
 		} else {
@@ -39,7 +39,7 @@ export class ConnectionStatusManager {
 		return Object.values(this._connections).find((connection: ConnectionManagementInfo) => connection.connectionProfile.id === profileId);
 	}
 
-	public findConnectionProfile(connectionProfile: IConnectionProfile): ConnectionManagementInfo {
+	public findConnectionProfile(connectionProfile: IConnectionProfile): ConnectionManagementInfo | undefined {
 		let id = Utils.generateUri(connectionProfile);
 		return this.findConnection(id);
 	}
@@ -65,7 +65,7 @@ export class ConnectionStatusManager {
 		}
 	}
 
-	public getConnectionProfile(id: string): ConnectionProfile {
+	public getConnectionProfile(id: string): ConnectionProfile | undefined {
 		let connectionInfoForId = this.findConnection(id);
 		return connectionInfoForId ? connectionInfoForId.connectionProfile : undefined;
 	}
@@ -178,7 +178,7 @@ export class ConnectionStatusManager {
 		return ownerUriToReturn;
 	}
 
-	public onConnectionChanged(changedConnInfo: azdata.ChangedConnectionInfo): IConnectionProfile {
+	public onConnectionChanged(changedConnInfo: azdata.ChangedConnectionInfo): IConnectionProfile | undefined {
 		let connection = this._connections[changedConnInfo.connectionUri];
 		if (connection && connection.connectionProfile) {
 			connection.connectionProfile.serverName = changedConnInfo.connection.serverName;
@@ -190,14 +190,14 @@ export class ConnectionStatusManager {
 	}
 
 	private isSharedSession(fileUri: string): boolean {
-		return fileUri && fileUri.startsWith('vsls:');
+		return !!(fileUri && fileUri.startsWith('vsls:'));
 	}
 
 	public isConnected(id: string): boolean {
 		if (this.isSharedSession(id)) {
 			return true;
 		}
-		return (id in this._connections && this._connections[id].connectionId && !!this._connections[id].connectionId);
+		return !!(id in this._connections && this._connections[id].connectionId && !!this._connections[id].connectionId);
 	}
 
 	public isConnecting(id: string): boolean {
@@ -205,7 +205,7 @@ export class ConnectionStatusManager {
 	}
 
 	public isDefaultTypeUri(uri: string): boolean {
-		return uri && uri.startsWith(Utils.uriPrefixes.default);
+		return !!(uri && uri.startsWith(Utils.uriPrefixes.default));
 	}
 
 	public getProviderIdFromUri(ownerUri: string): string {

src/sql/platform/connection/common/connectionStore.ts

@@ -29,7 +29,7 @@ const CRED_PROFILE_USER = 'Profile';
  * @export
  */
 export class ConnectionStore {
-	private groupIdMap = new ReverseLookUpMap<string, string>();
+	private groupIdMap = new ReverseLookUpMap<string, string | undefined>();
 	private connectionConfig = new ConnectionConfig(this.configurationService, this.capabilitiesService);
 	private mru: Array<IConnectionProfile>;
 
@@ -57,8 +57,7 @@ export class ConnectionStore {
 	 * @returns formatted string with server, DB and username
 	 */
 	private formatCredentialId(connectionProfile: IConnectionProfile, itemType?: string): string {
-		const connectionProfileInstance: ConnectionProfile = ConnectionProfile.fromIConnectionProfile(
-			this.capabilitiesService, connectionProfile);
+		const connectionProfileInstance = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, connectionProfile);
 		const cred: string[] = [CRED_PREFIX];
 		if (!itemType) {
 			itemType = CRED_PROFILE_USER;
@@ -74,12 +73,8 @@ export class ConnectionStore {
 	 * @param connection profile
 	 */
 	public isPasswordRequired(connection: IConnectionProfile): boolean {
-		if (connection) {
-			const connectionProfile = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, connection);
-			return connectionProfile.isPasswordRequired();
-		} else {
-			return false;
-		}
+		const connectionProfile = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, connection);
+		return connectionProfile.isPasswordRequired();
 	}
 
 	public addSavedPassword(credentialsItem: IConnectionProfile): Promise<{ profile: IConnectionProfile, savedCred: boolean }> {
@@ -162,34 +157,26 @@ export class ConnectionStore {
 
 	private convertConfigValuesToConnectionProfiles(configValues: IConnectionProfile[]): ConnectionProfile[] {
 		return configValues.map(c => {
-			if (c) {
-				const connectionProfile = new ConnectionProfile(this.capabilitiesService, c);
-				if (connectionProfile.saveProfile) {
-					if (!connectionProfile.groupFullName && connectionProfile.groupId) {
-						connectionProfile.groupFullName = this.getGroupFullName(connectionProfile.groupId);
-					}
-					if (!connectionProfile.groupId && connectionProfile.groupFullName) {
-						connectionProfile.groupId = this.getGroupId(connectionProfile.groupFullName);
-					} else if (!connectionProfile.groupId && !connectionProfile.groupFullName) {
-						connectionProfile.groupId = this.getGroupId('');
-					}
+			const connectionProfile = new ConnectionProfile(this.capabilitiesService, c);
+			if (connectionProfile.saveProfile) {
+				if (!connectionProfile.groupFullName && connectionProfile.groupId) {
+					connectionProfile.groupFullName = this.getGroupFullName(connectionProfile.groupId);
+				}
+				if (!connectionProfile.groupId && connectionProfile.groupFullName) {
+					connectionProfile.groupId = this.getGroupId(connectionProfile.groupFullName);
+				} else if (!connectionProfile.groupId && !connectionProfile.groupFullName) {
+					connectionProfile.groupId = this.getGroupId('');
 				}
-				return connectionProfile;
-			} else {
-				return undefined;
 			}
+			return connectionProfile;
 		});
 	}
 
 	public getProfileWithoutPassword(conn: IConnectionProfile): ConnectionProfile {
-		if (conn) {
-			let savedConn: ConnectionProfile = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, conn);
-			savedConn = savedConn.withoutPassword();
+		let savedConn = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, conn);
+		savedConn = savedConn.withoutPassword();
 
-			return savedConn;
-		} else {
-			return undefined;
-		}
+		return savedConn;
 	}
 
 	/**
@@ -221,7 +208,7 @@ export class ConnectionStore {
 	}
 
 	private addToConnectionList(conn: IConnectionProfile, list: ConnectionProfile[]): IConnectionProfile[] {
-		const savedProfile: ConnectionProfile = this.getProfileWithoutPassword(conn);
+		const savedProfile = this.getProfileWithoutPassword(conn);
 
 		// Remove the connection from the list if it already exists
 		list = list.filter(value => {
@@ -239,7 +226,7 @@ export class ConnectionStore {
 	}
 
 	private removeFromConnectionList(conn: IConnectionProfile, list: ConnectionProfile[]): IConnectionProfile[] {
-		const savedProfile: ConnectionProfile = this.getProfileWithoutPassword(conn);
+		const savedProfile = this.getProfileWithoutPassword(conn);
 
 		// Remove the connection from the list if it already exists
 		list = list.filter(value => {
@@ -286,7 +273,7 @@ export class ConnectionStore {
 	}
 
 	public getConnectionProfileGroups(withoutConnections?: boolean, providers?: string[]): ConnectionProfileGroup[] {
-		let profilesInConfiguration: ConnectionProfile[];
+		let profilesInConfiguration: ConnectionProfile[] | undefined;
 		if (!withoutConnections) {
 			profilesInConfiguration = this.connectionConfig.getConnections(true);
 			if (providers && providers.length > 0) {
@@ -295,10 +282,10 @@ export class ConnectionStore {
 		}
 		const groups = this.connectionConfig.getAllGroups();
 
-		return this.convertToConnectionGroup(groups, profilesInConfiguration, undefined);
+		return this.convertToConnectionGroup(groups, profilesInConfiguration);
 	}
 
-	private convertToConnectionGroup(groups: IConnectionProfileGroup[], connections: ConnectionProfile[], parent: ConnectionProfileGroup = undefined): ConnectionProfileGroup[] {
+	private convertToConnectionGroup(groups: IConnectionProfileGroup[], connections?: ConnectionProfile[], parent?: ConnectionProfileGroup): ConnectionProfileGroup[] {
 		const result: ConnectionProfileGroup[] = [];
 		const children = groups.filter(g => g.parentId === (parent ? parent.id : undefined));
 		if (children) {
@@ -307,7 +294,7 @@ export class ConnectionStore {
 				this.addGroupFullNameToMap(group.id, connectionGroup.fullName);
 				if (connections) {
 					let connectionsForGroup = connections.filter(conn => conn.groupId === connectionGroup.id);
-					let conns = [];
+					let conns: ConnectionProfile[] = [];
 					connectionsForGroup.forEach((conn) => {
 						conn.groupFullName = connectionGroup.fullName;
 						conns.push(conn);
@@ -359,7 +346,7 @@ export class ConnectionStore {
 		return this.connectionConfig.changeGroupIdForConnection(source, targetGroupId).then();
 	}
 
-	private addGroupFullNameToMap(groupId: string, groupFullName: string): void {
+	private addGroupFullNameToMap(groupId: string, groupFullName?: string): void {
 		if (groupId) {
 			this.groupIdMap.set(groupId, groupFullName);
 		}
@@ -373,7 +360,7 @@ export class ConnectionStore {
 			// Load the cache
 			this.getConnectionProfileGroups(true);
 		}
-		return this.groupIdMap.get(groupId);
+		return this.groupIdMap.get(groupId)!;
 	}
 
 	private getGroupId(groupFullName: string): string {
@@ -385,6 +372,6 @@ export class ConnectionStore {
 			// Load the cache
 			this.getConnectionProfileGroups(true);
 		}
-		return this.groupIdMap.reverseGet(key);
+		return this.groupIdMap.reverseGet(key)!;
 	}
 }

src/sql/platform/connection/common/utils.ts

@@ -115,7 +115,7 @@ export function generateUriWithPrefix(connection: IConnectionProfile, prefix: st
 	return uri;
 }
 
-export function findProfileInGroup(og: IConnectionProfile, groups: ConnectionProfileGroup[]): ConnectionProfile {
+export function findProfileInGroup(og: IConnectionProfile, groups: ConnectionProfileGroup[]): ConnectionProfile | undefined {
 	for (let group of groups) {
 		for (let conn of group.connections) {
 			if (conn.id === og.id) {