Fixes the empty parameter cell and multi parameters on one line (#15234)

* fix two bugs

* work on empty parameter cell and format properly

* add empty cell tests

src/sql/workbench/contrib/notebook/browser/notebookActions.ts

@@ -45,6 +45,8 @@ const msgLocalHost = localize('localhost', "localhost");
 export const noKernel: string = localize('noKernel', "No Kernel");
 const baseIconClass = 'codicon';
 const maskedIconClass = 'masked-icon';
+export const noParameterCell: string = localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization).");
+export const noParametersInCell: string = localize('noParametersInCell', "This notebook cannot run with parameters until there are parameters added to the parameter cell. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization).");
 
 // Action to add a cell to notebook based on cell type(code/markdown).
 export class AddCellAction extends Action {
@@ -308,14 +310,29 @@ export class RunParametersAction extends TooltipFromLabelAction {
 		const editor = this._notebookService.findNotebookEditor(context);
 		// Set defaultParameters to the parameter values in parameter cell
 		let defaultParameters = new Map<string, string>();
-		editor.cells.forEach(cell => {
+		for (let cell of editor?.cells) {
 			if (cell.isParameter) {
+				// Check if parameter cell is empty
+				const cellSource = typeof cell.source === 'string' ? [cell.source] : cell.source;
+				// Check to see if every line in the cell is empty or contains whitespace
+				const emptyParameterCell = cellSource.every(s => /^\s*$/.test(s));
+				if (emptyParameterCell) {
+					// If there is no parameters in the cell indicate to user to add them
+					this.notificationService.notify({
+						severity: Severity.Info,
+						message: noParametersInCell,
+					});
+					return;
+				}
 				for (let parameter of cell.source) {
-					let param = parameter.split('=', 2);
+					// Only add parameters that contain the proper parameters format (ex. x = 1) shown in the Parameterization Doc.
-					defaultParameters.set(param[0].trim(), param[1].trim());
+					if (parameter.includes('=')) {
+						let param = parameter.split('=', 2);
+						defaultParameters.set(param[0].trim(), param[1].trim());
+					}
 				}
 			}
-		});
+		}
 
 		// Store new parameters values the user inputs
 		let inputParameters = new Map<string, string>();
@@ -325,7 +342,7 @@ export class RunParametersAction extends TooltipFromLabelAction {
 			// If there is no parameter cell indicate to user to create one
 			this.notificationService.notify({
 				severity: Severity.Info,
-				message: localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization)."),
+				message: noParameterCell,
 			});
 			return;
 		} else {

src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts

@@ -7,7 +7,7 @@ import * as assert from 'assert';
 import * as azdata from 'azdata';
 import * as sinon from 'sinon';
 import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService';
-import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, KernelsDropdown, msgChanging, NewNotebookAction, noKernelName, RunAllCellsAction, RunParametersAction, TrustedAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
+import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, KernelsDropdown, msgChanging, NewNotebookAction, noKernelName, noParameterCell, noParametersInCell, RunAllCellsAction, RunParametersAction, TrustedAction } from 'sql/workbench/contrib/notebook/browser/notebookActions';
 import { ClientSessionStub, ContextViewProviderStub, NotebookComponentStub, NotebookModelStub, NotebookServiceStub } from 'sql/workbench/contrib/notebook/test/stubs';
 import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon';
 import { ICellModel, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
@@ -20,13 +20,12 @@ import { TestCommandService } from 'vs/editor/test/browser/editorTestServices';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { IConfigurationChangeEvent, IConfigurationOverrides, IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
-import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
+import { INotificationService } from 'vs/platform/notification/common/notification';
 import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
 import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
 import { URI } from 'vs/base/common/uri';
 import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
 import { MockQuickInputService } from 'sql/workbench/contrib/notebook/test/common/quickInputServiceMock';
-import { localize } from 'vs/nls';
 
 class TestClientSession extends ClientSessionStub {
 	private _errorState: boolean = false;
@@ -302,7 +301,7 @@ suite('Notebook Actions', function (): void {
 		assert.call(mockNotebookService.object.openNotebook(testUri, testShowOptions), 'Should Open Parameterized Notebook');
 	});
 
-	test('Run with Parameters Action with no parameters in notebook', async function (): Promise<void> {
+	test('Run with Parameters Action with no parameter cell in notebook', async function (): Promise<void> {
 		const testContents: azdata.nb.INotebookContents = {
 			cells: [{
 				cell_type: CellTypes.Code,
@@ -320,11 +319,7 @@ suite('Notebook Actions', function (): void {
 			nbformat: 4,
 			nbformat_minor: 5
 		};
-		let expectedMsg: string = localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization).");
+		let expectedMsg: string = noParameterCell;
-		let notification = {
-			severity: Severity.Info,
-			message: expectedMsg,
-		};
 
 		let actualMsg: string;
 		let mockNotification = TypeMoq.Mock.ofType<INotificationService>(TestNotificationService);
@@ -342,8 +337,137 @@ suite('Notebook Actions', function (): void {
 		// Run Parameters Action
 		await action.run(testUri);
 
-		assert.ok(actualMsg !== undefined, 'Should have received a notification');
+		assert.strictEqual(actualMsg, expectedMsg);
-		assert.call(mockNotification.object.notify(notification), 'Should notify user there is no parameter cell');
+	});
+
+	test('Run with Parameters Action with empty string parameter cell in notebook', async function (): Promise<void> {
+		const testContents: azdata.nb.INotebookContents = {
+			cells: [{
+				cell_type: CellTypes.Code,
+				source: [' '],
+				metadata: { language: 'python' },
+				execution_count: 1
+			}],
+			metadata: {
+				kernelspec: {
+					name: 'python',
+					language: 'python',
+					display_name: 'Python 3'
+				}
+			},
+			nbformat: 4,
+			nbformat_minor: 5
+		};
+		let expectedMsg: string = noParametersInCell;
+
+		let actualMsg: string;
+		let mockNotification = TypeMoq.Mock.ofType<INotificationService>(TestNotificationService);
+		mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => {
+			actualMsg = notification.message;
+			return undefined;
+		});
+		let quickInputService = new MockQuickInputService;
+		let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents);
+
+		let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object);
+		const testCells = [<ICellModel>{
+			isParameter: true,
+			source: [' ']
+		}];
+		mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel);
+		mockNotebookEditor.setup(x => x.cells).returns(() => testCells);
+
+		// Run Parameters Action
+		await action.run(testUri);
+
+		assert.strictEqual(actualMsg, expectedMsg);
+	});
+
+	test('Run with Parameters Action with empty array string parameter cell in notebook', async function (): Promise<void> {
+		const testContents: azdata.nb.INotebookContents = {
+			cells: [{
+				cell_type: CellTypes.Code,
+				source: [' ', ' '],
+				metadata: { language: 'python' },
+				execution_count: 1
+			}],
+			metadata: {
+				kernelspec: {
+					name: 'python',
+					language: 'python',
+					display_name: 'Python 3'
+				}
+			},
+			nbformat: 4,
+			nbformat_minor: 5
+		};
+		let expectedMsg: string = noParametersInCell;
+
+		let actualMsg: string;
+		let mockNotification = TypeMoq.Mock.ofType<INotificationService>(TestNotificationService);
+		mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => {
+			actualMsg = notification.message;
+			return undefined;
+		});
+		let quickInputService = new MockQuickInputService;
+		let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents);
+
+		let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object);
+
+		const testCells = [<ICellModel>{
+			isParameter: true,
+			source: [' ', ' ']
+		}];
+		mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel);
+		mockNotebookEditor.setup(x => x.cells).returns(() => testCells);
+
+		// Run Parameters Action
+		await action.run(testUri);
+
+		assert.strictEqual(actualMsg, expectedMsg);
+	});
+
+	test('Run with Parameters Action with empty parameter cell in notebook', async function (): Promise<void> {
+		const testContents: azdata.nb.INotebookContents = {
+			cells: [{
+				cell_type: CellTypes.Code,
+				source: [],
+				metadata: { language: 'python' },
+				execution_count: 1
+			}],
+			metadata: {
+				kernelspec: {
+					name: 'python',
+					language: 'python',
+					display_name: 'Python 3'
+				}
+			},
+			nbformat: 4,
+			nbformat_minor: 5
+		};
+		let expectedMsg: string = noParametersInCell;
+
+		let actualMsg: string;
+		let mockNotification = TypeMoq.Mock.ofType<INotificationService>(TestNotificationService);
+		mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => {
+			actualMsg = notification.message;
+			return undefined;
+		});
+		let quickInputService = new MockQuickInputService;
+		let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents);
+
+		let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object);
+
+		const testCells = [<ICellModel>{
+			isParameter: true,
+			source: []
+		}];
+		mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel);
+		mockNotebookEditor.setup(x => x.cells).returns(() => testCells);
+
+		// Run Parameters Action
+		await action.run(testUri);
+
 		assert.strictEqual(actualMsg, expectedMsg);
 	});