From 231dd7d377a1caa73539fe4415b4dfdaece97d05 Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 13:51:25 +0200 Subject: [PATCH 1/7] feat: improved nonNull* code, added optional `details` field --- src/utils/nonNull.ts | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/utils/nonNull.ts b/src/utils/nonNull.ts index fc40ecab1..931379c27 100644 --- a/src/utils/nonNull.ts +++ b/src/utils/nonNull.ts @@ -9,26 +9,38 @@ import * as l10n from '@vscode/l10n'; * Retrieves a property by name from an object and checks that it's not null and not undefined. It is strongly typed * for the property and will give a compile error if the given name is not a property of the source. */ +// NOTE: when calling these helpers from source files in this open-source repo, prefer passing a +// short file identifier (for example a repo-relative path) via the optional +// `details` parameter — it makes debugging and issue triage much easier for external contributors. export function nonNullProp( - source: TSource, + sourceObj: TSource, name: TKey, message?: string, + details?: string, ): NonNullable { - const value: NonNullable = >source[name]; + const value: NonNullable = >sourceObj[name]; if (message) { - return nonNullValue(value, `${name}, ${message}`); + return nonNullValue(value, `${name}, ${message}`, details); } - return nonNullValue(value, name); + return nonNullValue(value, name, details); } /** * Validates that a given value is not null and not undefined. */ -export function nonNullValue(value: T | undefined | null, propertyNameOrMessage?: string): T { +/** + * Validates that a given value is not null and not undefined. + * + * @param value The value to check. + * @param propertyNameOrMessage Optional property name or human message. + * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. + */ +export function nonNullValue(value: T | undefined | null, propertyNameOrMessage?: string, details?: string): T { if (value === undefined || value === null) { throw new Error( l10n.t('Internal error: Expected value to be neither null nor undefined') + - (propertyNameOrMessage ? `: ${propertyNameOrMessage}` : ''), + (propertyNameOrMessage ? `: ${propertyNameOrMessage}` : '') + + (details ? ` (${details})` : ''), ); } @@ -38,11 +50,23 @@ export function nonNullValue(value: T | undefined | null, propertyNameOrMessa /** * Validates that a given string is not null, undefined, nor empty */ -export function nonNullOrEmptyValue(value: string | undefined, propertyNameOrMessage?: string): string { +/** + * Validates that a given string is not null, undefined, nor empty + * + * @param value The string to check. + * @param propertyNameOrMessage Optional property name or human message. + * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. + */ +export function nonNullOrEmptyValue( + value: string | undefined, + propertyNameOrMessage?: string, + details?: string, +): string { if (!value) { throw new Error( l10n.t('Internal error: Expected value to be neither null, undefined, nor empty') + - (propertyNameOrMessage ? `: ${propertyNameOrMessage}` : ''), + (propertyNameOrMessage ? `: ${propertyNameOrMessage}` : '') + + (details ? ` (${details})` : ''), ); } From 441105723da8ad70f8e9693df56921bca24c725b Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 14:34:28 +0200 Subject: [PATCH 2/7] chore: improved nonNull* calls with more debug information --- .../chooseDataMigrationExtension.ts | 7 +++++-- src/commands/createCollection/createCollection.ts | 9 +++++++-- src/commands/createDatabase/createDatabase.ts | 5 +++-- src/commands/newLocalConnection/ExecuteStep.ts | 4 +++- src/commands/renameConnection/ExecuteStep.ts | 2 +- src/commands/updateConnectionString/ExecuteStep.ts | 6 +++++- src/documentdb/auth/NativeAuthHandler.ts | 3 ++- src/documentdb/scrapbook/ShellScriptRunner.ts | 12 +++++++++--- src/documentdb/scrapbook/mongoConnectionStrings.ts | 3 ++- .../discovery-tree/vm/AzureVMResourceItem.ts | 9 ++++----- .../documentdb/DocumentDBResourceItem.ts | 10 +++++++--- src/tree/api/createGenericElementWithContext.ts | 7 ++++--- src/tree/workspace-view/documentdb/ClusterItem.ts | 13 +++++++------ src/utils/workspacUtils.ts | 5 ++++- src/vscodeUriHandler.ts | 7 ++++--- 15 files changed, 67 insertions(+), 35 deletions(-) diff --git a/src/commands/chooseDataMigrationExtension/chooseDataMigrationExtension.ts b/src/commands/chooseDataMigrationExtension/chooseDataMigrationExtension.ts index 55be2ca9c..350102d26 100644 --- a/src/commands/chooseDataMigrationExtension/chooseDataMigrationExtension.ts +++ b/src/commands/chooseDataMigrationExtension/chooseDataMigrationExtension.ts @@ -3,13 +3,14 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { nonNullValue, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import { QuickPickItemKind, type QuickPickItem } from 'vscode'; import { CredentialCache } from '../../documentdb/CredentialCache'; import { DocumentDBConnectionString } from '../../documentdb/utils/DocumentDBConnectionString'; import { MigrationService } from '../../services/migrationServices'; import { type ClusterItemBase } from '../../tree/documentdb/ClusterItemBase'; +import { nonNullValue } from '../../utils/nonNull'; import { openUrl } from '../../utils/openUrl'; export async function chooseDataMigrationExtension(context: IActionContext, node: ClusterItemBase) { @@ -72,7 +73,9 @@ export async function chooseDataMigrationExtension(context: IActionContext, node // } if (migrationProviders.some((provider) => provider.id === selectedItem.id)) { - const selectedProvider = MigrationService.getProvider(nonNullValue(selectedItem.id, 'selectedItem.id')); + const selectedProvider = MigrationService.getProvider( + nonNullValue(selectedItem.id, 'selectedItem.id', 'chooseDataMigrationExtension.ts'), + ); if (selectedProvider) { context.telemetry.properties.migrationProvider = selectedProvider.id; diff --git a/src/commands/createCollection/createCollection.ts b/src/commands/createCollection/createCollection.ts index 030db4a8c..d7e3b9c00 100644 --- a/src/commands/createCollection/createCollection.ts +++ b/src/commands/createCollection/createCollection.ts @@ -3,10 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { AzureWizard, nonNullValue, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { AzureWizard, type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import { type DatabaseItem } from '../../tree/documentdb/DatabaseItem'; import { showConfirmationAsInSettings } from '../../utils/dialogs/showConfirmation'; +import { nonNullValue } from '../../utils/nonNull'; import { CollectionNameStep } from './CollectionNameStep'; import { type CreateCollectionWizardContext } from './CreateCollectionWizardContext'; import { ExecuteStep } from './ExecuteStep'; @@ -35,7 +36,11 @@ export async function createCollection(context: IActionContext, node: DatabaseIt await wizard.prompt(); await wizard.execute(); - const newCollectionName = nonNullValue(wizardContext.newCollectionName); + const newCollectionName = nonNullValue( + wizardContext.newCollectionName, + 'wizardContext.newCollectionName', + 'createCollection.ts', + ); showConfirmationAsInSettings( l10n.t('The "{newCollectionName}" collection has been created.', { newCollectionName }), ); diff --git a/src/commands/createDatabase/createDatabase.ts b/src/commands/createDatabase/createDatabase.ts index 2f4376053..b369423c5 100644 --- a/src/commands/createDatabase/createDatabase.ts +++ b/src/commands/createDatabase/createDatabase.ts @@ -3,11 +3,12 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { AzureWizard, type IActionContext, nonNullValue } from '@microsoft/vscode-azext-utils'; +import { AzureWizard, type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import { CredentialCache } from '../../documentdb/CredentialCache'; import { type ClusterItemBase } from '../../tree/documentdb/ClusterItemBase'; import { showConfirmationAsInSettings } from '../../utils/dialogs/showConfirmation'; +import { nonNullValue } from '../../utils/nonNull'; import { type CreateDatabaseWizardContext } from './CreateDatabaseWizardContext'; import { DatabaseNameStep } from './DatabaseNameStep'; import { ExecuteStep } from './ExecuteStep'; @@ -53,6 +54,6 @@ async function createMongoDatabase(context: IActionContext, node: ClusterItemBas await wizard.prompt(); await wizard.execute(); - const newDatabaseName = nonNullValue(wizardContext.databaseName); + const newDatabaseName = nonNullValue(wizardContext.databaseName, 'wizardContext.databaseName', 'createDatabase.ts'); showConfirmationAsInSettings(l10n.t('The "{name}" database has been created.', { name: newDatabaseName })); } diff --git a/src/commands/newLocalConnection/ExecuteStep.ts b/src/commands/newLocalConnection/ExecuteStep.ts index 8d42dbe6b..dfa343f79 100644 --- a/src/commands/newLocalConnection/ExecuteStep.ts +++ b/src/commands/newLocalConnection/ExecuteStep.ts @@ -146,7 +146,9 @@ export class ExecuteStep extends AzureWizardExecuteStep { try { connection.secrets = { ...connection.secrets, - connectionString: nonNullValue(context.newConnectionString?.toString(), 'newConnectionString'), + connectionString: nonNullValue( + context.newConnectionString?.toString(), + 'context.newConnectionString', + 'ExecuteStep.ts', + ), }; await ConnectionStorageService.save(resourceType, connection, true); diff --git a/src/documentdb/auth/NativeAuthHandler.ts b/src/documentdb/auth/NativeAuthHandler.ts index f5427ad67..c9ae7d457 100644 --- a/src/documentdb/auth/NativeAuthHandler.ts +++ b/src/documentdb/auth/NativeAuthHandler.ts @@ -30,7 +30,8 @@ export class NativeAuthHandler implements AuthHandler { return Promise.resolve({ connectionString: nonNullValue( this.clusterCredentials.connectionStringWithPassword, - 'connectionStringWithPassword', + 'clusterCredentials.connectionStringWithPassword', + 'NativeAuthHandler.ts', ), options, }); diff --git a/src/documentdb/scrapbook/ShellScriptRunner.ts b/src/documentdb/scrapbook/ShellScriptRunner.ts index 6cef1b48a..61972a432 100644 --- a/src/documentdb/scrapbook/ShellScriptRunner.ts +++ b/src/documentdb/scrapbook/ShellScriptRunner.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { nonNullValue, parseError, UserCancelledError, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { parseError, UserCancelledError, type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import * as fs from 'node:fs/promises'; import * as os from 'os'; @@ -14,6 +14,7 @@ import * as cpUtils from '../../utils/cp'; import { type EmulatorConfiguration } from '../../utils/emulatorConfiguration'; import { pathExists } from '../../utils/fs/pathExists'; import { InteractiveChildProcess } from '../../utils/InteractiveChildProcess'; +import { nonNullValue } from '../../utils/nonNull'; import { randomUtils } from '../../utils/randomUtils'; import { getBatchSizeSetting } from '../../utils/workspacUtils'; import { wrapError } from '../../utils/wrapError'; @@ -147,7 +148,13 @@ export class ShellScriptRunner extends vscode.Disposable { } ShellScriptRunner._cachedShellPathOrCmd = shellPath; - const timeout = 1000 * nonNullValue(config.get(ext.settingsKeys.shellTimeout), 'mongoShellTimeout'); + const timeout = + 1000 * + nonNullValue( + config.get(ext.settingsKeys.shellTimeout), + 'config.get(ext.settingsKeys.shellTimeout)', + 'ShellScriptRunner.ts', + ); return ShellScriptRunner.createShellProcessHelper( shellPath, shellArgs, @@ -347,7 +354,6 @@ export class ShellScriptRunner extends vscode.Disposable { openFile, ); if (response === openFile) { - // eslint-disable-next-line no-constant-condition while (true) { const newPath: vscode.Uri[] = await context.ui.showOpenDialog({ filters: { 'Executable Files': [process.platform === 'win32' ? 'exe' : ''] }, diff --git a/src/documentdb/scrapbook/mongoConnectionStrings.ts b/src/documentdb/scrapbook/mongoConnectionStrings.ts index d24d52276..26166f8e0 100644 --- a/src/documentdb/scrapbook/mongoConnectionStrings.ts +++ b/src/documentdb/scrapbook/mongoConnectionStrings.ts @@ -28,7 +28,8 @@ export function getDatabaseNameFromConnectionString(connectionString: string): s try { const [, , databaseName] = nonNullValue( connectionString.match(mongoConnectionStringRegExp), - 'databaseNameMatch', + 'connectionString.match(mongoConnectionStringRegExp)', + 'mongoConnectionStrings.ts', ); return databaseName; } catch { diff --git a/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts b/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts index cd2016807..d0bae560a 100644 --- a/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts +++ b/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts @@ -6,8 +6,6 @@ import { AzureWizard, callWithTelemetryAndErrorHandling, - nonNullProp, - nonNullValue, UserCancelledError, type IActionContext, } from '@microsoft/vscode-azext-utils'; @@ -26,6 +24,7 @@ import { ProvideUserNameStep } from '../../../../documentdb/wizards/authenticate import { ext } from '../../../../extensionVariables'; import { ClusterItemBase, type ClusterCredentials } from '../../../../tree/documentdb/ClusterItemBase'; import { type ClusterModel } from '../../../../tree/documentdb/ClusterModel'; +import { nonNullProp, nonNullValue } from '../../../../utils/nonNull'; // Define a model for VM, similar to ClusterModel but for VM properties export interface VirtualMachineModel extends ClusterModel { @@ -171,10 +170,10 @@ export class AzureVMResourceItem extends ClusterItemBase { // Construct the final connection string with user-provided credentials const connectionString = (await this.getCredentials())?.connectionString; - context.valuesToMask.push(nonNullValue(connectionString, 'connectionString')); + context.valuesToMask.push(nonNullValue(connectionString, 'connectionString', 'AzureVMResourceItem.ts')); const finalConnectionString = new DocumentDBConnectionString( - nonNullValue(connectionString, 'connectionString'), + nonNullValue(connectionString, 'connectionString', 'AzureVMResourceItem.ts'), ); maskSensitiveValuesInTelemetry(context, finalConnectionString); @@ -191,7 +190,7 @@ export class AzureVMResourceItem extends ClusterItemBase { return null; } - context.valuesToMask.push(nonNullProp(wizardContext, 'password')); + context.valuesToMask.push(nonNullProp(wizardContext, 'password', undefined, 'AzureVMResourceItem.ts')); finalConnectionString.username = wizardContext.selectedUserName; diff --git a/src/plugins/service-azure/discovery-tree/documentdb/DocumentDBResourceItem.ts b/src/plugins/service-azure/discovery-tree/documentdb/DocumentDBResourceItem.ts index 1e2509cd8..3346fafd0 100644 --- a/src/plugins/service-azure/discovery-tree/documentdb/DocumentDBResourceItem.ts +++ b/src/plugins/service-azure/discovery-tree/documentdb/DocumentDBResourceItem.ts @@ -7,7 +7,6 @@ import { type MongoCluster } from '@azure/arm-mongocluster'; import { AzureWizard, callWithTelemetryAndErrorHandling, - nonNullValue, UserCancelledError, type IActionContext, } from '@microsoft/vscode-azext-utils'; @@ -25,6 +24,7 @@ import { ProvideUserNameStep } from '../../../../documentdb/wizards/authenticate import { ext } from '../../../../extensionVariables'; import { ClusterItemBase, type ClusterCredentials } from '../../../../tree/documentdb/ClusterItemBase'; import { type ClusterModel } from '../../../../tree/documentdb/ClusterModel'; +import { nonNullValue } from '../../../../utils/nonNull'; import { extractCredentialsFromCluster, getClusterInformationFromAzure } from '../../utils/clusterHelpers'; export class DocumentDBResourceItem extends ClusterItemBase { @@ -116,8 +116,12 @@ export class DocumentDBResourceItem extends ClusterItemBase { // Cache credentials and attempt connection CredentialCache.setAuthCredentials( this.id, - nonNullValue(wizardContext.selectedAuthMethod, 'authMethod'), - nonNullValue(credentials.connectionString), + nonNullValue( + wizardContext.selectedAuthMethod, + 'wizardContext.selectedAuthMethod', + 'DocumentDBResourceItem.ts', + ), + nonNullValue(credentials.connectionString, 'credentials.connectionString', 'DocumentDBResourceItem.ts'), wizardContext.selectedUserName, wizardContext.password, ); diff --git a/src/tree/api/createGenericElementWithContext.ts b/src/tree/api/createGenericElementWithContext.ts index 382446cc4..ce92ba8f1 100644 --- a/src/tree/api/createGenericElementWithContext.ts +++ b/src/tree/api/createGenericElementWithContext.ts @@ -3,8 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { nonNullValue, type GenericElementOptions } from '@microsoft/vscode-azext-utils'; +import { type GenericElementOptions } from '@microsoft/vscode-azext-utils'; import { type TreeItem } from 'vscode'; +import { nonNullValue } from '../../utils/nonNull'; import { type TreeElement } from '../TreeElement'; import { type TreeElementWithContextValue } from '../TreeElementWithContextValue'; @@ -13,8 +14,8 @@ export function createGenericElementWithContext( ): TreeElement & TreeElementWithContextValue { let commandArgs = options.commandArgs; const item = { - id: nonNullValue(options.id, 'options.id'), - contextValue: nonNullValue(options.contextValue, 'options.contextValue'), + id: nonNullValue(options.id, 'options.id', 'createGenericElementWithContext.ts'), + contextValue: nonNullValue(options.contextValue, 'options.contextValue', 'createGenericElementWithContext.ts'), getTreeItem(): TreeItem { return { diff --git a/src/tree/workspace-view/documentdb/ClusterItem.ts b/src/tree/workspace-view/documentdb/ClusterItem.ts index 95bc11680..046b2e1db 100644 --- a/src/tree/workspace-view/documentdb/ClusterItem.ts +++ b/src/tree/workspace-view/documentdb/ClusterItem.ts @@ -6,8 +6,6 @@ import { AzureWizard, callWithTelemetryAndErrorHandling, - nonNullProp, - nonNullValue, UserCancelledError, type IActionContext, } from '@microsoft/vscode-azext-utils'; @@ -19,6 +17,7 @@ import { type AuthenticateWizardContext } from '../../../documentdb/wizards/auth import { ProvidePasswordStep } from '../../../documentdb/wizards/authenticate/ProvidePasswordStep'; import { ProvideUserNameStep } from '../../../documentdb/wizards/authenticate/ProvideUsernameStep'; import { ext } from '../../../extensionVariables'; +import { nonNullProp, nonNullValue } from '../../../utils/nonNull'; import { ClusterItemBase, type ClusterCredentials } from '../../documentdb/ClusterItemBase'; import { type AttachedClusterModel } from '../../documentdb/ClusterModel'; import { type TreeElementWithStorageId } from '../../TreeElementWithStorageId'; @@ -63,7 +62,9 @@ export class ClusterItem extends ClusterItemBase implements TreeElementWithStora let clustersClient: ClustersClient; - const connectionString = new DocumentDBConnectionString(nonNullValue(this.cluster.connectionString)); + const connectionString = new DocumentDBConnectionString( + nonNullValue(this.cluster.connectionString, 'cluster.connectionString', 'ClusterItem.ts'), + ); let username: string | undefined = connectionString.username; let password: string | undefined = connectionString.password; @@ -87,10 +88,10 @@ export class ClusterItem extends ClusterItemBase implements TreeElementWithStora return null; } - context.valuesToMask.push(nonNullProp(wizardContext, 'password')); + context.valuesToMask.push(nonNullProp(wizardContext, 'password', undefined, 'ClusterItem.ts')); - username = nonNullProp(wizardContext, 'selectedUserName'); - password = nonNullProp(wizardContext, 'password'); + username = nonNullProp(wizardContext, 'selectedUserName', undefined, 'ClusterItem.ts'); + password = nonNullProp(wizardContext, 'password', undefined, 'ClusterItem.ts'); } ext.outputChannel.append(l10n.t('Connecting to the cluster as "{username}"…', { username })); diff --git a/src/utils/workspacUtils.ts b/src/utils/workspacUtils.ts index ab0ceb885..6d45dc066 100644 --- a/src/utils/workspacUtils.ts +++ b/src/utils/workspacUtils.ts @@ -16,5 +16,8 @@ export function getRootPath(): string | undefined { export function getBatchSizeSetting(): number { const config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(); - return nonNullValue(config.get(ext.settingsKeys.batchSize), 'batchSize'); + return nonNullValue( + config.get(ext.settingsKeys.batchSize), + 'config.get(ext.settingsKeys.batchSize)', + ); } diff --git a/src/vscodeUriHandler.ts b/src/vscodeUriHandler.ts index c5ed18126..406e9b096 100644 --- a/src/vscodeUriHandler.ts +++ b/src/vscodeUriHandler.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { callWithTelemetryAndErrorHandling, nonNullValue, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import * as vscode from 'vscode'; import { openCollectionViewInternal } from './commands/openCollectionView/openCollectionView'; @@ -16,6 +16,7 @@ import { revealInConnectionsView, waitForConnectionsViewReady, } from './tree/connections-view/connectionsViewHelpers'; +import { nonNullValue } from './utils/nonNull'; import { generateDocumentDBStorageId } from './utils/storageUtils'; // #region Type Definitions @@ -450,7 +451,7 @@ async function openDedicatedView( return openCollectionViewInternal(context, { clusterId: clusterId, - databaseName: nonNullValue(database, 'database'), - collectionName: nonNullValue(collection, 'collection'), + databaseName: nonNullValue(database, 'database', 'vscodeUriHandler.ts'), + collectionName: nonNullValue(collection, 'collection', 'vscodeUriHandler.ts'), }); } From 0ef5a2cbe7afdf5d27faf56fb4f8668dd0f3e276 Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 15:00:19 +0200 Subject: [PATCH 3/7] Introduced ESLint rules for import monitorin of 'nonNull'-functions --- eslint.config.mjs | 15 ++++++- extension.bundle.ts | 1 + .../importDocuments/importDocuments.ts | 12 ++++- .../discovery-tree/AzureSubscriptionItem.ts | 5 ++- .../mongo-ru/MongoRUResourceItem.ts | 15 ++++++- .../MongoVCoreBranchDataProvider.ts | 45 +++++++++++-------- .../mongo-vcore/MongoVCoreResourceItem.ts | 17 ++++--- .../connections-view/DocumentDBClusterItem.ts | 9 +++- 8 files changed, 86 insertions(+), 33 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 59dd815b3..bf8126660 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -52,7 +52,20 @@ export default ts.config( 'no-case-declarations': 'error', 'no-constant-condition': 'error', 'no-inner-declarations': 'error', - 'no-restricted-imports': ['error', { patterns: ['**/*/extension.bundle'] }], + 'no-restricted-imports': [ + 'error', + { + paths: [ + { + name: '@microsoft/vscode-azext-utils', + importNames: ['nonNullValue', 'nonNullProp', 'nonNullOrEmptyValue'], + message: + "Do not import nonNull helpers from '@microsoft/vscode-azext-utils'. Use the local 'src/utils/nonNull' instead.", + }, + ], + patterns: ['**/*/extension.bundle'], + }, + ], 'no-unused-vars': ['error', { argsIgnorePattern: '^_' }], 'no-useless-escape': 'error', 'license-header/header': [ diff --git a/extension.bundle.ts b/extension.bundle.ts index 8a13ba230..547308d19 100644 --- a/extension.bundle.ts +++ b/extension.bundle.ts @@ -19,6 +19,7 @@ export { ObjectId } from 'bson'; // The tests should import '../extension.bundle.ts'. At design-time they live in tests/ and so will pick up this file (extension.bundle.ts). // At runtime the tests live in dist/tests and will therefore pick up the main webpack bundle at dist/extension.bundle.js. export { AzureAccountTreeItemBase, createAzureClient } from '@microsoft/vscode-azext-azureutils'; +// eslint-disable-next-line no-restricted-imports -- bundle intentionally re-exports many helpers for tests; nonNull helpers are provided locally in this repo export * from '@microsoft/vscode-azext-utils'; export { isWindows, wellKnownEmulatorPassword } from './src/constants'; export { connectToClient, isCosmosEmulatorConnectionString } from './src/documentdb/scrapbook/connectToClient'; diff --git a/src/commands/importDocuments/importDocuments.ts b/src/commands/importDocuments/importDocuments.ts index 79acd0468..2a1e0866c 100644 --- a/src/commands/importDocuments/importDocuments.ts +++ b/src/commands/importDocuments/importDocuments.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { nonNullProp, parseError, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { parseError, type IActionContext } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import { EJSON, type Document } from 'bson'; import * as fs from 'node:fs/promises'; @@ -17,6 +17,7 @@ import { import { ext } from '../../extensionVariables'; import { CollectionItem } from '../../tree/documentdb/CollectionItem'; import { BufferErrorCode, createMongoDbBuffer, type DocumentBuffer } from '../../utils/documentBuffer'; +import { nonNullProp } from '../../utils/nonNull'; import { getRootPath } from '../../utils/workspacUtils'; export async function importDocuments( @@ -114,7 +115,14 @@ export async function importDocumentsWithProgress(selectedItem: CollectionItem, let count = 0; let buffer: DocumentBuffer | undefined; if (selectedItem instanceof CollectionItem) { - const hosts = getHostsFromConnectionString(nonNullProp(selectedItem.cluster, 'connectionString')); + const hosts = getHostsFromConnectionString( + nonNullProp( + selectedItem.cluster, + 'connectionString', + 'selectedItem.cluster.connectionString', + 'importDocuments.ts', + ), + ); const isRuResource = hasDomainSuffix(AzureDomains.RU, ...hosts); if (isRuResource) { diff --git a/src/plugins/service-azure/discovery-tree/AzureSubscriptionItem.ts b/src/plugins/service-azure/discovery-tree/AzureSubscriptionItem.ts index 0ca266d5e..f4a4d4c6d 100644 --- a/src/plugins/service-azure/discovery-tree/AzureSubscriptionItem.ts +++ b/src/plugins/service-azure/discovery-tree/AzureSubscriptionItem.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { getResourceGroupFromId, uiUtils } from '@microsoft/vscode-azext-azureutils'; -import { callWithTelemetryAndErrorHandling, nonNullProp, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microsoft/vscode-azext-utils'; import { type AzureSubscription } from '@microsoft/vscode-azureresources-api'; import * as vscode from 'vscode'; import { MongoClustersExperience } from '../../../DocumentDBExperiences'; @@ -13,6 +13,7 @@ import { type TreeElement } from '../../../tree/TreeElement'; import { type TreeElementWithContextValue } from '../../../tree/TreeElementWithContextValue'; import { type ClusterModel } from '../../../tree/documentdb/ClusterModel'; import { createResourceManagementClient } from '../../../utils/azureClients'; +import { nonNullProp } from '../../../utils/nonNull'; import { DocumentDBResourceItem } from './documentdb/DocumentDBResourceItem'; export interface AzureSubscriptionModel { @@ -45,7 +46,7 @@ export class AzureSubscriptionItem implements TreeElement, TreeElementWithContex return accounts .sort((a, b) => (a.name || '').localeCompare(b.name || '')) .map((account) => { - const resourceId = nonNullProp(account, 'id'); + const resourceId = nonNullProp(account, 'id', 'account.id', 'AzureSubscriptionItem.ts'); const clusterInfo: ClusterModel = { ...account, diff --git a/src/tree/azure-resources-view/documentdb/mongo-ru/MongoRUResourceItem.ts b/src/tree/azure-resources-view/documentdb/mongo-ru/MongoRUResourceItem.ts index cf2abb420..9ac608193 100644 --- a/src/tree/azure-resources-view/documentdb/mongo-ru/MongoRUResourceItem.ts +++ b/src/tree/azure-resources-view/documentdb/mongo-ru/MongoRUResourceItem.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { callWithTelemetryAndErrorHandling, nonNullProp, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microsoft/vscode-azext-utils'; import { type AzureSubscription } from '@microsoft/vscode-azureresources-api'; import * as l10n from '@vscode/l10n'; import * as vscode from 'vscode'; @@ -12,6 +12,7 @@ import { CredentialCache } from '../../../../documentdb/CredentialCache'; import { DocumentDBConnectionString } from '../../../../documentdb/utils/DocumentDBConnectionString'; import { ext } from '../../../../extensionVariables'; import { createCosmosDBManagementClient } from '../../../../utils/azureClients'; +import { nonNullProp } from '../../../../utils/nonNull'; import { ClusterItemBase, type ClusterCredentials } from '../../../documentdb/ClusterItemBase'; import { type ClusterModel } from '../../../documentdb/ClusterModel'; @@ -41,7 +42,17 @@ export class MongoRUResourceItem extends ClusterItemBase { ); const connectionString: URL = new URL( - nonNullProp(nonNullProp(connectionStringsInfo, 'connectionStrings')[0], 'connectionString'), + nonNullProp( + nonNullProp( + connectionStringsInfo, + 'connectionStrings', + 'connectionStringsInfo.connectionStrings', + 'MongoRUResourceItem.ts', + )[0], + 'connectionString', + 'connectionStringsInfo.connectionStrings[0].connectionString', + 'MongoRUResourceItem.ts', + ), ); // for any Mongo connectionString, append this query param because the Cosmos Mongo API v3.6 doesn't support retrywrites diff --git a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreBranchDataProvider.ts b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreBranchDataProvider.ts index a10e91b2e..4be7965c7 100644 --- a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreBranchDataProvider.ts +++ b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreBranchDataProvider.ts @@ -5,10 +5,11 @@ import { type GenericResource } from '@azure/arm-resources'; import { getResourceGroupFromId, uiUtils } from '@microsoft/vscode-azext-azureutils'; -import { callWithTelemetryAndErrorHandling, nonNullProp, type IActionContext } from '@microsoft/vscode-azext-utils'; +import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microsoft/vscode-azext-utils'; import { type AzureResource, type AzureSubscription } from '@microsoft/vscode-azureresources-api'; import { API, MongoClustersExperience } from '../../../../DocumentDBExperiences'; import { createMongoClustersManagementClient } from '../../../../utils/azureClients'; +import { nonNullProp } from '../../../../utils/nonNull'; import { BaseCachedBranchDataProvider } from '../../../BaseCachedBranchDataProvider'; import { type ClusterModel } from '../../../documentdb/ClusterModel'; import { type TreeElement } from '../../../TreeElement'; @@ -98,24 +99,32 @@ export class MongoVCoreBranchDataProvider extends BaseCachedBranchDataProvider { - this.detailsCache.set(nonNullProp(mongoClusterAccount, 'id'), { - dbExperience: MongoClustersExperience, - id: mongoClusterAccount.id!, - name: mongoClusterAccount.name!, - resourceGroup: getResourceGroupFromId(mongoClusterAccount.id!), - - location: mongoClusterAccount.location, - serverVersion: mongoClusterAccount.properties?.serverVersion, - - systemData: { - createdAt: mongoClusterAccount.systemData?.createdAt, + this.detailsCache.set( + nonNullProp( + mongoClusterAccount, + 'id', + 'mongoClusterAccount.id', + 'MongoVCoreBranchDataProvider.ts', + ), + { + dbExperience: MongoClustersExperience, + id: mongoClusterAccount.id!, + name: mongoClusterAccount.name!, + resourceGroup: getResourceGroupFromId(mongoClusterAccount.id!), + + location: mongoClusterAccount.location, + serverVersion: mongoClusterAccount.properties?.serverVersion, + + systemData: { + createdAt: mongoClusterAccount.systemData?.createdAt, + }, + + sku: mongoClusterAccount.properties?.compute?.tier, + diskSize: mongoClusterAccount.properties?.storage?.sizeGb, + nodeCount: mongoClusterAccount.properties?.sharding?.shardCount, + enableHa: mongoClusterAccount.properties?.highAvailability?.targetMode !== 'Disabled', }, - - sku: mongoClusterAccount.properties?.compute?.tier, - diskSize: mongoClusterAccount.properties?.storage?.sizeGb, - nodeCount: mongoClusterAccount.properties?.sharding?.shardCount, - enableHa: mongoClusterAccount.properties?.highAvailability?.targetMode !== 'Disabled', - }); + ); }); } catch (e) { console.debug({ ...context, ...subscription }); diff --git a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts index fece4cac5..d193bd8c5 100644 --- a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts +++ b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts @@ -6,8 +6,6 @@ import { AzureWizard, callWithTelemetryAndErrorHandling, - nonNullProp, - nonNullValue, UserCancelledError, type IActionContext, } from '@microsoft/vscode-azext-utils'; @@ -23,6 +21,7 @@ import { ProvidePasswordStep } from '../../../../documentdb/wizards/authenticate import { ProvideUserNameStep } from '../../../../documentdb/wizards/authenticate/ProvideUsernameStep'; import { ext } from '../../../../extensionVariables'; import { createMongoClustersManagementClient } from '../../../../utils/azureClients'; +import { nonNullProp, nonNullValue } from '../../../../utils/nonNull'; import { ClusterItemBase, type ClusterCredentials } from '../../../documentdb/ClusterItemBase'; import { type ClusterModel } from '../../../documentdb/ClusterModel'; @@ -118,14 +117,20 @@ export class MongoVCoreResourceItem extends ClusterItemBase { return null; } - context.valuesToMask.push(nonNullProp(wizardContext, 'password')); + context.valuesToMask.push( + nonNullProp(wizardContext, 'password', 'wizardContext.password', 'MongoVCoreResourceItem.ts'), + ); // Cache the credentials CredentialCache.setCredentials( this.id, - nonNullValue(clusterInformation.properties.connectionString), - nonNullProp(wizardContext, 'selectedUserName'), - nonNullProp(wizardContext, 'password'), + nonNullValue( + clusterInformation.properties.connectionString, + 'clusterInformation.properties.connectionString', + 'MongoVCoreResourceItem.ts', + ), + nonNullProp(wizardContext, 'selectedUserName', undefined, 'MongoVCoreResourceItem.ts'), + nonNullProp(wizardContext, 'password', undefined, 'MongoVCoreResourceItem.ts'), // here, emulatorConfiguration is not set, as it's a resource item from Azure resources, not a workspace item, therefore, no emulator support needed ); diff --git a/src/tree/connections-view/DocumentDBClusterItem.ts b/src/tree/connections-view/DocumentDBClusterItem.ts index 4365c3aaf..cda50be24 100644 --- a/src/tree/connections-view/DocumentDBClusterItem.ts +++ b/src/tree/connections-view/DocumentDBClusterItem.ts @@ -6,12 +6,12 @@ import { AzureWizard, callWithTelemetryAndErrorHandling, - nonNullProp, UserCancelledError, type IActionContext, } from '@microsoft/vscode-azext-utils'; import * as l10n from '@vscode/l10n'; import * as vscode from 'vscode'; +import { nonNullProp } from '../../utils/nonNull'; import { authMethodFromString, AuthMethodId, authMethodsFromString } from '../../documentdb/auth/AuthMethod'; import { ClustersClient } from '../../documentdb/ClustersClient'; @@ -130,7 +130,12 @@ export class DocumentDBClusterItem extends ClusterItemBase implements TreeElemen username = wizardContext.selectedUserName; password = wizardContext.password; - authMethod = nonNullProp(wizardContext, 'selectedAuthMethod'); + authMethod = nonNullProp( + wizardContext, + 'selectedAuthMethod', + 'wizardContext.selectedAuthMethod', + 'DocumentDBClusterItem.ts', + ); if (wizardContext.saveCredentials) { ext.outputChannel.append( From b405794a6b8271befa46a73217f9cf8f0b87ec25 Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 15:10:49 +0200 Subject: [PATCH 4/7] nonNull.ts now requires message and details --- src/documentdb/scrapbook/ScrapbookHelpers.ts | 29 ++++++++++++++----- .../discovery-tree/vm/AzureVMResourceItem.ts | 4 ++- .../mongo-vcore/MongoVCoreResourceItem.ts | 9 ++++-- .../workspace-view/documentdb/ClusterItem.ts | 15 +++++++--- src/utils/nonNull.ts | 17 ++++------- src/utils/workspacUtils.ts | 1 + 6 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/documentdb/scrapbook/ScrapbookHelpers.ts b/src/documentdb/scrapbook/ScrapbookHelpers.ts index 5846771f0..b2bca937e 100644 --- a/src/documentdb/scrapbook/ScrapbookHelpers.ts +++ b/src/documentdb/scrapbook/ScrapbookHelpers.ts @@ -107,7 +107,7 @@ class FindMongoCommandsVisitor extends MongoVisitor { public visitCommand(ctx: mongoParser.CommandContext): MongoCommand[] { const funcCallCount: number = filterType(ctx.children, mongoParser.FunctionCallContext).length; - const stop = nonNullProp(ctx, 'stop'); + const stop = nonNullProp(ctx, 'stop', 'ctx.stop', 'ScrapbookHelpers.ts'); this.commands.push({ range: new vscode.Range( ctx.start.line - 1, @@ -146,7 +146,7 @@ class FindMongoCommandsVisitor extends MongoVisitor { const argAsObject = this.contextToObject(ctx); const argText = EJSON.stringify(argAsObject); - nonNullProp(lastCommand, 'arguments').push(argText); + nonNullProp(lastCommand, 'arguments', 'lastCommand.arguments', 'ScrapbookHelpers.ts').push(argText); const escapeHandled = this.deduplicateEscapesForRegex(argText); let ejsonParsed = {}; try { @@ -157,7 +157,12 @@ class FindMongoCommandsVisitor extends MongoVisitor { const parsedError: IParsedError = parseError(error); this.addErrorToCommand(parsedError.message, ctx); } - nonNullProp(lastCommand, 'argumentObjects').push(ejsonParsed); + nonNullProp( + lastCommand, + 'argumentObjects', + 'lastCommand.argumentObjects', + 'ScrapbookHelpers.ts', + ).push(ejsonParsed); } } } catch (error) { @@ -178,7 +183,7 @@ class FindMongoCommandsVisitor extends MongoVisitor { return {}; } // In a well formed expression, Argument and propertyValue tokens should have exactly one child, from their definitions in mongo.g4 - const child: ParseTree = nonNullProp(ctx, 'children')[0]; + const child: ParseTree = nonNullProp(ctx, 'children', 'ctx.children', 'ScrapbookHelpers.ts')[0]; if (child instanceof mongoParser.LiteralContext) { return this.literalContextToObject(child, ctx); } else if (child instanceof mongoParser.ObjectLiteralContext) { @@ -235,7 +240,12 @@ class FindMongoCommandsVisitor extends MongoVisitor { mongoParser.PropertyAssignmentContext, ); for (const propertyAssignment of propertyAssignments) { - const propertyAssignmentChildren = nonNullProp(propertyAssignment, 'children'); + const propertyAssignmentChildren = nonNullProp( + propertyAssignment, + 'children', + 'propertyAssignment.children', + 'ScrapbookHelpers.ts', + ); const propertyName = propertyAssignmentChildren[0]; const propertyValue = propertyAssignmentChildren[2]; parsedObject[stripQuotes(propertyName.text)] = this.contextToObject(propertyValue); @@ -261,10 +271,15 @@ class FindMongoCommandsVisitor extends MongoVisitor { // eslint-disable-next-line @typescript-eslint/no-wrapper-object-types ): Object { const functionTokens = child.children; - const constructorCall: TerminalNode = nonNullValue(findType(functionTokens, TerminalNode), 'constructorCall'); + const constructorCall: TerminalNode = nonNullValue( + findType(functionTokens, TerminalNode), + 'constructorCall', + 'ScrapbookHelpers.ts', + ); const argumentsToken: mongoParser.ArgumentsContext = nonNullValue( findType(functionTokens, mongoParser.ArgumentsContext), 'argumentsToken', + 'ScrapbookHelpers.ts', ); if (!(argumentsToken._CLOSED_PARENTHESIS && argumentsToken._OPEN_PARENTHESIS)) { //argumentsToken does not have '(' or ')' @@ -418,7 +433,7 @@ class FindMongoCommandsVisitor extends MongoVisitor { ): void { const command = this.commands[this.commands.length - 1]; command.errors = command.errors || []; - const stop = nonNullProp(ctx, 'stop'); + const stop = nonNullProp(ctx, 'stop', 'ctx.stop', 'ScrapbookHelpers.ts'); const currentErrorDesc: ErrorDescription = { message: errorMessage, range: new vscode.Range( diff --git a/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts b/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts index d0bae560a..30ffd04d9 100644 --- a/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts +++ b/src/plugins/service-azure-vm/discovery-tree/vm/AzureVMResourceItem.ts @@ -190,7 +190,9 @@ export class AzureVMResourceItem extends ClusterItemBase { return null; } - context.valuesToMask.push(nonNullProp(wizardContext, 'password', undefined, 'AzureVMResourceItem.ts')); + context.valuesToMask.push( + nonNullProp(wizardContext, 'password', 'wizardContext.password', 'AzureVMResourceItem.ts'), + ); finalConnectionString.username = wizardContext.selectedUserName; diff --git a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts index d193bd8c5..0a47319ba 100644 --- a/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts +++ b/src/tree/azure-resources-view/documentdb/mongo-vcore/MongoVCoreResourceItem.ts @@ -129,8 +129,13 @@ export class MongoVCoreResourceItem extends ClusterItemBase { 'clusterInformation.properties.connectionString', 'MongoVCoreResourceItem.ts', ), - nonNullProp(wizardContext, 'selectedUserName', undefined, 'MongoVCoreResourceItem.ts'), - nonNullProp(wizardContext, 'password', undefined, 'MongoVCoreResourceItem.ts'), + nonNullProp( + wizardContext, + 'selectedUserName', + 'wizardContext.selectedUserName', + 'MongoVCoreResourceItem.ts', + ), + nonNullProp(wizardContext, 'password', 'wizardContext.password', 'MongoVCoreResourceItem.ts'), // here, emulatorConfiguration is not set, as it's a resource item from Azure resources, not a workspace item, therefore, no emulator support needed ); diff --git a/src/tree/workspace-view/documentdb/ClusterItem.ts b/src/tree/workspace-view/documentdb/ClusterItem.ts index 046b2e1db..79147c195 100644 --- a/src/tree/workspace-view/documentdb/ClusterItem.ts +++ b/src/tree/workspace-view/documentdb/ClusterItem.ts @@ -88,10 +88,17 @@ export class ClusterItem extends ClusterItemBase implements TreeElementWithStora return null; } - context.valuesToMask.push(nonNullProp(wizardContext, 'password', undefined, 'ClusterItem.ts')); - - username = nonNullProp(wizardContext, 'selectedUserName', undefined, 'ClusterItem.ts'); - password = nonNullProp(wizardContext, 'password', undefined, 'ClusterItem.ts'); + context.valuesToMask.push( + nonNullProp(wizardContext, 'password', 'wizardContext.password', 'ClusterItem.ts'), + ); + + username = nonNullProp( + wizardContext, + 'selectedUserName', + 'wizardContext.selectedUserName', + 'ClusterItem.ts', + ); + password = nonNullProp(wizardContext, 'password', 'wizardContext.password', 'ClusterItem.ts'); } ext.outputChannel.append(l10n.t('Connecting to the cluster as "{username}"…', { username })); diff --git a/src/utils/nonNull.ts b/src/utils/nonNull.ts index 931379c27..3e660c886 100644 --- a/src/utils/nonNull.ts +++ b/src/utils/nonNull.ts @@ -15,14 +15,11 @@ import * as l10n from '@vscode/l10n'; export function nonNullProp( sourceObj: TSource, name: TKey, - message?: string, - details?: string, + message: string, + details: string, ): NonNullable { const value: NonNullable = >sourceObj[name]; - if (message) { - return nonNullValue(value, `${name}, ${message}`, details); - } - return nonNullValue(value, name, details); + return nonNullValue(value, `${name}, ${message}`, details); } /** @@ -35,7 +32,7 @@ export function nonNullProp( * @param propertyNameOrMessage Optional property name or human message. * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. */ -export function nonNullValue(value: T | undefined | null, propertyNameOrMessage?: string, details?: string): T { +export function nonNullValue(value: T | undefined | null, propertyNameOrMessage: string, details: string): T { if (value === undefined || value === null) { throw new Error( l10n.t('Internal error: Expected value to be neither null nor undefined') + @@ -57,11 +54,7 @@ export function nonNullValue(value: T | undefined | null, propertyNameOrMessa * @param propertyNameOrMessage Optional property name or human message. * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. */ -export function nonNullOrEmptyValue( - value: string | undefined, - propertyNameOrMessage?: string, - details?: string, -): string { +export function nonNullOrEmptyValue(value: string | undefined, propertyNameOrMessage: string, details: string): string { if (!value) { throw new Error( l10n.t('Internal error: Expected value to be neither null, undefined, nor empty') + diff --git a/src/utils/workspacUtils.ts b/src/utils/workspacUtils.ts index 6d45dc066..5141e904f 100644 --- a/src/utils/workspacUtils.ts +++ b/src/utils/workspacUtils.ts @@ -19,5 +19,6 @@ export function getBatchSizeSetting(): number { return nonNullValue( config.get(ext.settingsKeys.batchSize), 'config.get(ext.settingsKeys.batchSize)', + 'workspacUtils.ts', ); } From c32eff0a6c3aad2da6d62c0e60934c41ad4d1c1d Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 15:16:18 +0200 Subject: [PATCH 5/7] Added comments for code maintainers. --- src/utils/nonNull.ts | 64 +++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/utils/nonNull.ts b/src/utils/nonNull.ts index 3e660c886..0a1f0cf94 100644 --- a/src/utils/nonNull.ts +++ b/src/utils/nonNull.ts @@ -6,12 +6,34 @@ import * as l10n from '@vscode/l10n'; /** - * Retrieves a property by name from an object and checks that it's not null and not undefined. It is strongly typed - * for the property and will give a compile error if the given name is not a property of the source. + * NOTE: These helpers append a short context string to thrown errors to help with triage. + * + * Parameter guidelines: + * - `message` (required): A short, human-friendly identifier for the value being checked. + * Use the actual member access or assignment LHS from your code: + * - Member access: 'selectedItem.cluster.connectionString' + * - Wizard context property: 'wizardContext.password' + * - Local variable or expression: 'connectionString.match(...)' + * + * - `details` (required): A short file identifier using the actual file base name from your code. + * Since this is an open source project, use real file names like 'ExecuteStep.ts', 'ConnectionItem.ts', etc. + * Keep it short and inline (do not create a constant). + * + * Example usage with actual code references: + * nonNullProp(selectedItem.cluster, 'connectionString', 'selectedItem.cluster.connectionString', 'ExecuteStep.ts') + */ + +/** + * Retrieves a property by name from an object and asserts it's not null or undefined. + * Provides compile-time type checking for the property name. + * + * @param sourceObj - The object to read the property from + * @param name - The property name (compile-time checked) + * @param message - Short identifier describing the checked value (prefer member-access or LHS) + * @param details - Short file identifier (file base name) used to help triage runtime errors + * @returns The non-null property value + * @throws Error with message format: ", (details)" when value is missing */ -// NOTE: when calling these helpers from source files in this open-source repo, prefer passing a -// short file identifier (for example a repo-relative path) via the optional -// `details` parameter — it makes debugging and issue triage much easier for external contributors. export function nonNullProp( sourceObj: TSource, name: TKey, @@ -23,14 +45,19 @@ export function nonNullProp( } /** - * Validates that a given value is not null and not undefined. - */ -/** - * Validates that a given value is not null and not undefined. + * Validates that a given value is not null or undefined. * - * @param value The value to check. - * @param propertyNameOrMessage Optional property name or human message. - * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. + * @param value - The value to check + * @param propertyNameOrMessage - Property name or short human message describing the value + * (prefer member-access or the LHS of the assignment) + * @param details - Short file identifier (file base name) used to assist with triage + * @returns The validated non-null value + * @throws Error when value is null or undefined + * + * @example + * ```typescript + * nonNullValue(someVar, 'connectionString', 'ExecuteStep.ts') + * ``` */ export function nonNullValue(value: T | undefined | null, propertyNameOrMessage: string, details: string): T { if (value === undefined || value === null) { @@ -45,14 +72,13 @@ export function nonNullValue(value: T | undefined | null, propertyNameOrMessa } /** - * Validates that a given string is not null, undefined, nor empty - */ -/** - * Validates that a given string is not null, undefined, nor empty + * Validates that a given string is not null, undefined, or empty. * - * @param value The string to check. - * @param propertyNameOrMessage Optional property name or human message. - * @param details Optional short context (file name or identifier). Recommended for open-source issue triage. + * @param value - The string to check + * @param propertyNameOrMessage - Property name or message describing the value (e.g. 'database') + * @param details - Short file identifier (file base name) to help triage issues + * @returns The validated non-empty string + * @throws Error when value is null, undefined, or empty string */ export function nonNullOrEmptyValue(value: string | undefined, propertyNameOrMessage: string, details: string): string { if (!value) { From 2616a0177f9c4d6e22a5317241d351b5387cb96f Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 15:21:35 +0200 Subject: [PATCH 6/7] updated copilot instructions with nonNull info --- .github/copilot-instructions.md | 67 +++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6f8332db3..a8fd8e834 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -68,6 +68,7 @@ This document provides comprehensive guidelines and context for GitHub Copilot t - Use l10n for any user-facing strings with `vscode.l10n.t()`. - Use `npm run prettier-fix` to format your code before committing. - Use `npm run lint` to check for linting errors. +- Use `npm run build` to ensure the project builds successfully (do not use `npm run compile`). - Use `npm run l10n` to update localization files in case you change any user-facing strings. - Ensure TypeScript compilation passes without errors. @@ -422,3 +423,69 @@ const message = vscode.l10n.t( - Ensure compatibility with Node.js version specified in `.nvmrc`. - Follow the project's ESLint configuration for consistent code style. - Use webpack for bundling and ensure proper tree-shaking. + +--- + +## Null Safety with nonNull Helpers + +**Always use the nonNull utility functions** from `src/utils/nonNull.ts` instead of manual null checks for better error reporting and debugging. + +#### Available Functions + +- **`nonNullProp()`**: Extract and validate object properties +- **`nonNullValue()`**: Validate any value is not null/undefined +- **`nonNullOrEmptyValue()`**: Validate strings are not null/undefined/empty + +#### Parameter Guidelines + +Both `message` and `details` parameters are **required** for all nonNull functions: + +- **`message`**: Use the actual member access or assignment LHS from your code. Since this is open source, use real variable names: + - Member access: `'selectedItem.cluster.connectionString'` + - Wizard context: `'wizardContext.password'` + - Local variables: `'connectionString.match(...)'` + +- **`details`**: Use the actual file base name where the code is located: + - Examples: `'ExecuteStep.ts'`, `'ConnectionItem.ts'`, `'DatabaseTreeItem.ts'` + - Keep it short, use the actual file name, don't create constants + +#### Usage Examples + +```typescript +// ✅ Good - Property extraction with validation +const connectionString = nonNullProp( + selectedItem.cluster, + 'connectionString', + 'selectedItem.cluster.connectionString', + 'ExecuteStep.ts', +); + +// ✅ Good - Value validation +const validatedConnection = nonNullValue(await getConnection(id), 'getConnection(id)', 'ConnectionManager.ts'); + +// ✅ Good - String validation (not empty) +const databaseName = nonNullOrEmptyValue( + wizardContext.databaseName, + 'wizardContext.databaseName', + 'CreateDatabaseStep.ts', +); + +// ✅ Good - Manual null check for user-facing validation +if (!userInput.connectionString) { + void vscode.window.showErrorMessage(vscode.l10n.t('Connection string is required')); + return; +} + +// ❌ Bad - Manual null checks for internal validation (use nonNull helpers instead) +if (!selectedItem.cluster.connectionString) { + throw new Error('Connection string is required'); // This should use nonNullProp +} + +// ❌ Bad - Generic parameter values +const value = nonNullValue(data, 'some value', 'file.ts'); +``` + +**When to use each approach:** + +- **Use nonNull helpers**: For internal validation where you expect the value to exist (programming errors) +- **Use manual checks**: For user-facing validation with localized error messages shown to users From 8afcc137d687d8296b4930f288d549b0e5971b3d Mon Sep 17 00:00:00 2001 From: Tomasz Naumowicz Date: Mon, 25 Aug 2025 15:37:22 +0200 Subject: [PATCH 7/7] wip: github actions resilience --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2aedc58f8..879da1ee7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -60,7 +60,7 @@ jobs: ${{ runner.os }}-node- - name: 📦 Install Dependencies (npm ci) - run: npm ci + run: npm ci --verbose - name: 🌐 Check Localization Files run: npm run l10n:check