From b9480e9d7d4575e3074b30fbb21e0c92beb1d187 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 20 Oct 2025 21:13:56 -0700 Subject: [PATCH] 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 | 4 ++-- src/vs/base/common/lifecycle.ts | 4 +++- src/vs/base/test/common/collections.test.ts | 4 ++-- .../chat/browser/modelPicker/modePickerActionItem.ts | 4 ++-- .../contrib/issue/browser/baseIssueReporterService.ts | 4 ++-- .../contrib/markers/test/browser/markersModel.test.ts | 6 ++++-- src/vs/workbench/contrib/mcp/browser/mcpCommands.ts | 6 +++--- .../notebook/browser/viewModel/notebookViewModelImpl.ts | 2 +- src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts | 6 +++++- 9 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/vs/base/common/collections.ts b/src/vs/base/common/collections.ts index 24c0b9767d7..845c5d9a2a9 100644 --- a/src/vs/base/common/collections.ts +++ b/src/vs/base/common/collections.ts @@ -19,8 +19,8 @@ export type INumberDictionary = Record; * Groups the collection into a dictionary based on the provided * group function. */ -export function groupBy(data: readonly V[], groupFn: (element: V) => K): Record { - const result: Record = Object.create(null); +export function groupBy(data: readonly V[], groupFn: (element: V) => K): Partial> { + const result: Partial> = Object.create(null); for (const element of data) { const key = groupFn(element); let target = result[key]; diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index ff7684f382b..0de9f6e37c8 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/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); diff --git a/src/vs/base/test/common/collections.test.ts b/src/vs/base/test/common/collections.test.ts index fbf8f6e77b3..5bd2206d538 100644 --- a/src/vs/base/test/common/collections.test.ts +++ b/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); }); diff --git a/src/vs/workbench/contrib/chat/browser/modelPicker/modePickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/modelPicker/modePickerActionItem.ts index 07d9543ec15..36fb1e79077 100644 --- a/src/vs/workbench/contrib/chat/browser/modelPicker/modePickerActionItem.ts +++ b/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), diff --git a/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts b/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts index 9f1f8ade96f..ca6f65cbadb 100644 --- a/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts +++ b/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) { (this.getElementById('disableExtensions')).disabled = true; } diff --git a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts b/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts index 34990da558e..48596e960b4 100644 --- a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts +++ b/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]]); + } }); } } diff --git a/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts b/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts index 3c60794dc2d..eae0316f8c0 100644 --- a/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts +++ b/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)[] => [ - { type: 'separator', label: servers[0].collection.label, id: servers[0].collection.id }, - ...servers.map(server => ({ + ...Object.values(servers).filter(s => s!.length).flatMap((servers): (ItemType | IQuickPickSeparator)[] => [ + { type: 'separator', label: servers![0].collection.label, id: servers![0].collection.id }, + ...servers!.map(server => ({ id: server.definition.id, label: server.definition.label, description: McpConnectionState.toString(server.connectionState.read(reader)), diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts index 44d5b94af14..ca7a0dabfe6 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts +++ b/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)); diff --git a/src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts b/src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts index 8ca4b2be266..21a3f1e9ae0 100644 --- a/src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts +++ b/src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts @@ -504,11 +504,15 @@ class HistoryItemRenderer implements ICompressibleTreeRenderer 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; }