Improvement: nonNull calls with better debugging information

.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

.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

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': [

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';

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;

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 }),
     );

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 }));
 }

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<unknown> | 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) {

src/commands/newLocalConnection/ExecuteStep.ts

@@ -146,7 +146,9 @@ export class ExecuteStep extends AzureWizardExecuteStep<NewLocalConnectionWizard
                     emulatorConfiguration: { isEmulator, disableEmulatorSecurity: !!disableEmulatorSecurity },
                     availableAuthMethods: [],
                 },
-                secrets: { connectionString: nonNullValue(connectionString) },
+                secrets: {
+                    connectionString: nonNullValue(connectionString, 'secrets.connectionString', 'ExecuteStep.ts'),
+                },
             };
 
             await ConnectionStorageService.save(ConnectionType.Emulators, storageItem, true);

src/commands/renameConnection/ExecuteStep.ts

@@ -17,7 +17,7 @@ export class ExecuteStep extends AzureWizardExecuteStep<RenameConnectionWizardCo
         const connection = await ConnectionStorageService.get(context.storageId, resourceType);
 
         if (connection) {
-            connection.name = nonNullValue(context.newConnectionName);
+            connection.name = nonNullValue(context.newConnectionName, 'connection.name', 'ExecuteStep.ts');
 
             try {
                 await ConnectionStorageService.save(resourceType, connection, true);

src/commands/updateConnectionString/ExecuteStep.ts

@@ -28,7 +28,11 @@ export class ExecuteStep extends AzureWizardExecuteStep<UpdateCSWizardContext> {
         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);

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,
         });

src/documentdb/scrapbook/ScrapbookHelpers.ts

@@ -107,7 +107,7 @@ class FindMongoCommandsVisitor extends MongoVisitor<MongoCommand[]> {
 
     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<MongoCommand[]> {
                     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<MongoCommand[]> {
                         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<MongoCommand[]> {
             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<MongoCommand[]> {
                 mongoParser.PropertyAssignmentContext,
             );
             for (const propertyAssignment of propertyAssignments) {
-                const propertyAssignmentChildren = nonNullProp(propertyAssignment, 'children');
+                const propertyAssignmentChildren = nonNullProp(
+                    propertyAssignment,
+                    'children',
+                    'propertyAssignment.children',
+                    'ScrapbookHelpers.ts',
+                );
                 const propertyName = <mongoParser.PropertyNameContext>propertyAssignmentChildren[0];
                 const propertyValue = <mongoParser.PropertyValueContext>propertyAssignmentChildren[2];
                 parsedObject[stripQuotes(propertyName.text)] = this.contextToObject(propertyValue);
@@ -261,10 +271,15 @@ class FindMongoCommandsVisitor extends MongoVisitor<MongoCommand[]> {
         // 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<MongoCommand[]> {
     ): 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(