Make type signature of groupBy more accurate (#272395)

See #272263, it currently assumes that all keys of K will be defined. This still isn't perfect, because it says that the returned object might contain a key with value `undefined`, but in reality, if a key is defined, it will have a value. So now usages of this with Object.entries and so on are slighly wrong, but that's preferable IMO.

src/vs/base/common/collections.ts

@@ -19,8 +19,8 @@ export type INumberDictionary<V> = Record<number, V>;
  * Groups the collection into a dictionary based on the provided
  * group function.
  */
-export function groupBy<K extends string | number | symbol, V>(data: readonly V[], groupFn: (element: V) => K): Record<K, V[]> {
+export function groupBy<K extends string | number | symbol, V>(data: readonly V[], groupFn: (element: V) => K): Partial<Record<K, V[]>> {
-	const result: Record<K, V[]> = Object.create(null);
+	const result: Partial<Record<K, V[]>> = Object.create(null);
 	for (const element of data) {
 		const key = groupFn(element);
 		let target = result[key];

src/vs/base/common/lifecycle.ts

@@ -206,7 +206,9 @@ export class DisposableTracker implements IDisposableTracker {
 				const continuations = groupBy([...prevStarts].map(d => getStackTracePath(d)[i]), v => v);
 				delete continuations[stackTracePath[i]];
 				for (const [cont, set] of Object.entries(continuations)) {
-					stackTraceFormattedLines.unshift(`    - stacktraces of ${set.length} other leaks continue with ${cont}`);
+					if (set) {
+						stackTraceFormattedLines.unshift(`    - stacktraces of ${set.length} other leaks continue with ${cont}`);
+					}
 				}
 
 				stackTraceFormattedLines.unshift(line);

src/vs/base/test/common/collections.test.ts

@@ -24,12 +24,12 @@ suite('Collections', () => {
 		const grouped = collections.groupBy(source, x => x.key);
 
 		// Group 1
-		assert.strictEqual(grouped[group1].length, 2);
+		assert.strictEqual(grouped[group1]?.length, 2);
 		assert.strictEqual(grouped[group1][0].value, value1);
 		assert.strictEqual(grouped[group1][1].value, value2);
 
 		// Group 2
-		assert.strictEqual(grouped[group2].length, 1);
+		assert.strictEqual(grouped[group2]?.length, 1);
 		assert.strictEqual(grouped[group2][0].value, value3);
 	});
 

src/vs/workbench/contrib/chat/browser/modelPicker/modePickerActionItem.ts

@@ -79,11 +79,11 @@ export class ModePickerActionItem extends ActionWidgetDropdownActionViewItem {
 					mode => mode.source?.storage === PromptsStorage.extension && mode.source.extensionId.value === productService.defaultChatAgent?.chatExtensionId ?
 						'builtin' : 'custom');
 
-				const customBuiltinModeActions = customModes.builtin.map(mode => {
+				const customBuiltinModeActions = customModes.builtin?.map(mode => {
 					const action = makeActionFromCustomMode(mode, currentMode);
 					action.category = builtInCategory;
 					return action;
-				});
+				}) ?? [];
 
 				const orderedModes = coalesce([
 					agentMode && makeAction(agentMode, currentMode),

src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts

@@ -410,9 +410,9 @@ export class BaseIssueReporterService extends Disposable {
 			return ext.isTheme ? 'themes' : 'nonThemes';
 		});
 
-		const numberOfThemeExtesions = themes && themes.length;
+		const numberOfThemeExtesions = (themes && themes.length) ?? 0;
 		this.issueReporterModel.update({ numberOfThemeExtesions, enabledNonThemeExtesions: nonThemes, allExtensions: installedExtensions });
-		this.updateExtensionTable(nonThemes, numberOfThemeExtesions);
+		this.updateExtensionTable(nonThemes ?? [], numberOfThemeExtesions);
 		if (this.disableExtensions || installedExtensions.length === 0) {
 			(<HTMLButtonElement>this.getElementById('disableExtensions')).disabled = true;
 		}

src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts

@@ -19,9 +19,11 @@ class TestMarkersModel extends MarkersModel {
 
 		Object.keys(byResource).forEach(key => {
 			const markers = byResource[key];
-			const resource = markers[0].resource;
+			if (markers) {
+				const resource = markers[0].resource;
 
-			this.setResourceMarkers([[resource, markers]]);
+				this.setResourceMarkers([[resource, markers]]);
+			}
 		});
 	}
 }

src/vs/workbench/contrib/mcp/browser/mcpCommands.ts

@@ -117,9 +117,9 @@ export class ListMcpServerCommand extends Action2 {
 			const firstRun = pick.items.length === 0;
 			pick.items = [
 				{ id: '$add', label: localize('mcp.addServer', 'Add Server'), description: localize('mcp.addServer.description', 'Add a new server configuration'), alwaysShow: true, iconClass: ThemeIcon.asClassName(Codicon.add) },
-				...Object.values(servers).filter(s => s.length).flatMap((servers): (ItemType | IQuickPickSeparator)[] => [
+				...Object.values(servers).filter(s => s!.length).flatMap((servers): (ItemType | IQuickPickSeparator)[] => [
-					{ type: 'separator', label: servers[0].collection.label, id: servers[0].collection.id },
+					{ type: 'separator', label: servers![0].collection.label, id: servers![0].collection.id },
-					...servers.map(server => ({
+					...servers!.map(server => ({
 						id: server.definition.id,
 						label: server.definition.label,
 						description: McpConnectionState.toString(server.connectionState.read(reader)),

src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts

@@ -778,7 +778,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
 
 		for (const _handle in deletesByHandle) {
 			const handle = parseInt(_handle);
-			const ids = deletesByHandle[handle];
+			const ids = deletesByHandle[handle]!;
 			const cell = this.getCellByHandle(handle);
 			cell?.deltaCellStatusBarItems(ids, []);
 			ids.forEach(id => this._statusBarItemIdToCellMap.delete(id));

src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts

@@ -504,11 +504,15 @@ class HistoryItemRenderer implements ICompressibleTreeRenderer<SCMHistoryItemVie
 					continue;
 				}
 
+				if (!historyItemRefs) {
+					continue;
+				}
+
 				// Group history item references by icon
 				const historyItemRefByIconId = groupBy2(historyItemRefs, ref => ThemeIcon.isThemeIcon(ref.icon) ? ref.icon.id : '');
 				for (const [key, historyItemRefs] of Object.entries(historyItemRefByIconId)) {
 					// Skip badges without an icon
-					if (key === '') {
+					if (key === '' || !historyItemRefs) {
 						continue;
 					}