triggering a param add pop-up if running a cypher with missing params (#452)

Author: nnecla

package-lock.json

@@ -7,7 +7,12 @@ export { _internalFeatureFlags } from './featureFlags';
 export { formatQuery } from './formatting/formatting';
 export { antlrUtils } from './helpers';
 export { CypherTokenType, lexerSymbols } from './lexerSymbols';
-export { parse, parserWrapper, parseStatementsStrs } from './parserWrapper';
+export {
+  parse,
+  parseParameters,
+  parserWrapper,
+  parseStatementsStrs,
+} from './parserWrapper';
 export { signatureHelp, toSignatureInformation } from './signatureHelp';
 export {
   applySyntaxColouring,

packages/language-support/src/index.ts

@@ -236,6 +236,24 @@ export function createParsingResult(
   return parsingResult;
 }
 
+const getClearParamName = (name: string): string => {
+  if (name.startsWith('`') && name.endsWith('`')) {
+    return name.slice(1, -1);
+  }
+  return name;
+};
+
+export function parseParameters(
+  query: string,
+  consoleCommandsEnabled: boolean,
+): string[] {
+  const parsingResult = createParsingResult(query, consoleCommandsEnabled);
+  const parameters = parsingResult.statementsParsing.flatMap((statement) =>
+    statement.collectedParameters.map((param) => getClearParamName(param.name)),
+  );
+  return [...new Set(parameters)];
+}
+
 // This listener collects all labels and relationship types
 class LabelAndRelTypesCollector extends ParseTreeListener {
   labelOrRelTypes: LabelOrRelType[] = [];

packages/language-support/src/parserWrapper.ts

@@ -392,8 +392,7 @@ function errorOnUndeclaredParameters(
       if (parameterName.startsWith('`') && parameterName.endsWith('`')) {
         parameterName = parameterName.substring(1, parameterName.length - 1);
       }
-      const paramExists = !!dbSchema.parameters?.[parameterName];
-      if (!paramExists) {
+      if (dbSchema.parameters?.[parameterName] === undefined) {
         errors.push(
           generateSyntaxDiagnostic(
             parameter.rawText,

packages/language-support/src/syntaxValidation/syntaxValidation.ts

@@ -44,7 +44,7 @@ export function validateParamInput(
   return undefined;
 }
 
-export async function addParameter(): Promise<void> {
+export async function addParameter(defaultParamName?: string): Promise<void> {
   const connected = await isConnected();
 
   if (!connected) {
@@ -54,12 +54,14 @@ export async function addParameter(): Promise<void> {
     return;
   }
 
-  const paramName = await window.showInputBox({
-    prompt: 'Parameter name',
-    placeHolder:
-      'The name you want to store your parameter with, for example: param, p, `my parameter`',
-    ignoreFocusOut: true,
-  });
+  const paramName =
+    defaultParamName ??
+    (await window.showInputBox({
+      prompt: 'Parameter name',
+      placeHolder:
+        'The name you want to store your parameter with, for example: param, p, `my parameter`',
+      ignoreFocusOut: true,
+    }));
   if (!paramName) {
     await window.showErrorMessage('Parameter name cannot be empty.');
     return;
@@ -68,7 +70,9 @@ export async function addParameter(): Promise<void> {
   const dbSchema = schemaPoller.metadata.dbSchema;
   let timeout: NodeJS.Timeout;
   const paramValue = await window.showInputBox({
-    prompt: 'Parameter value',
+    prompt: defaultParamName
+      ? `Parameter value for the parameter ${defaultParamName}`
+      : 'Parameter value',
     placeHolder:
       'The value for your parameter (anything you could evaluate in a RETURN), for example: 1234, "some string", datetime(), {a: 1, b: "some string"}',
     ignoreFocusOut: true,
@@ -84,7 +88,7 @@ export async function addParameter(): Promise<void> {
   });
 
   if (!paramValue) {
-    await window.showErrorMessage('Parameter value cannot be empty.');
+    void window.showErrorMessage('Parameter value cannot be empty.');
     return;
   }
 
@@ -150,7 +154,7 @@ export async function evaluateParam(
     const db = getCurrentDatabase();
 
     if (db.type === 'system') {
-      await window.showErrorMessage(
+      void window.showErrorMessage(
         'Parameters cannot be evaluated against a system database. Please connect to a user database.',
       );
       return;
@@ -186,7 +190,7 @@ export async function evaluateParam(
     if (e instanceof Neo4jError) {
       //If we can get past linting-check with invalid query but still have failing query
       //when executing, we catch here as a backup
-      await window.showErrorMessage(
+      void window.showErrorMessage(
         'Failed to evaluate parameter: ' + e.message,
       );
     } else {

packages/vscode-extension/src/commandHandlers/params.ts

@@ -1,6 +1,11 @@
-import { parseStatementsStrs } from '@neo4j-cypher/language-support';
+import {
+  parseParameters,
+  parseStatementsStrs,
+} from '@neo4j-cypher/language-support';
 import { Uri } from 'vscode';
+import { addParameter } from './commandHandlers/params';
 import { Connection } from './connectionService';
+import { getDeserializedParams } from './parameterService';
 import ResultWindow from './webviews/resultWindow';
 
 export default class CypherRunner {
@@ -10,7 +15,15 @@ export default class CypherRunner {
 
   async run(connection: Connection, uri: Uri, input: string) {
     const statements = parseStatementsStrs(input);
+    const statementParams = parseParameters(input, false);
     const filePath = uri.toString();
+    const parameters = getDeserializedParams();
+
+    for (const param of statementParams) {
+      if (parameters[param] === undefined) {
+        await addParameter(param);
+      }
+    }
 
     if (this.results.has(filePath)) {
       const resultWindow = this.results.get(filePath);

packages/vscode-extension/src/cypherRunner.ts

@@ -1 +1 @@
-RETURN $a, $b, $`some param`, $`some-param`
+RETURN $a, $b, $`some param`, $`some-param`, $a + $b;

packages/vscode-extension/tests/fixtures/params.cypher

@@ -1,6 +1,7 @@
 import { browser } from '@wdio/globals';
 import { before } from 'mocha';
 import { Workbench } from 'wdio-vscode-service';
+import { Key } from 'webdriverio';
 import {
   checkResultsContent,
   executeFile,
@@ -14,6 +15,14 @@ suite('Params panel testing', () => {
     workbench = await browser.getWorkbench();
   });
 
+  async function escapeModal(count: number) {
+    for (let i = 0; i < count; i++) {
+      await browser.pause(500);
+      await browser.keys([Key.Escape]);
+      await waitUntilNotification(browser, 'Parameter value cannot be empty.');
+    }
+  }
+
   async function addParamWithInputBox() {
     await browser.executeWorkbench(async (vscode) => {
       // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
@@ -100,10 +109,13 @@ suite('Params panel testing', () => {
     await clearParams();
 
     await executeFile(workbench, 'params.cypher');
+
+    await escapeModal(4);
+
     await checkResultsContent(workbench, async () => {
       const text = await (await $('#query-error')).getText();
       await expect(text).toContain(
-        'Error executing query RETURN $a, $b, $`some param`, $`some-param`:\nExpected parameter(s): a, b, some param, some-param',
+        'Error executing query RETURN $a, $b, $`some param`, $`some-param`, $a + $b;:\nExpected parameter(s): a, b, some param, some-param',
       );
     });
   });
@@ -155,10 +167,13 @@ suite('Params panel testing', () => {
     await forceDeleteParam('b');
 
     await executeFile(workbench, 'params.cypher');
+
+    await escapeModal(2);
+
     await checkResultsContent(workbench, async () => {
       const text = await (await $('#query-error')).getText();
       await expect(text).toContain(
-        'Error executing query RETURN $a, $b, $`some param`, $`some-param`:\nExpected parameter(s): a, b',
+        'Error executing query RETURN $a, $b, $`some param`, $`some-param`, $a + $b;:\nExpected parameter(s): a, b',
       );
     });
   });
@@ -187,11 +202,41 @@ suite('Params panel testing', () => {
     // to execute the file we need to be on the user database
     await forceSwitchDatabase('neo4j');
     await executeFile(workbench, 'params.cypher');
+
+    await escapeModal(4);
+
     await checkResultsContent(workbench, async () => {
       const text = await (await $('#query-error')).getText();
       await expect(text).toContain(
-        'Error executing query RETURN $a, $b, $`some param`, $`some-param`:\nExpected parameter(s): a, b, some param, some-param',
+        'Error executing query RETURN $a, $b, $`some param`, $`some-param`, $a + $b;:\nExpected parameter(s): a, b',
       );
     });
   });
+
+  test('Should trigger parameter add pop-up when running a query with an unknown parameter', async () => {
+    await clearParams();
+    await forceAddParam('a', '1998');
+    await executeFile(workbench, 'params.cypher');
+
+    // initial pop-up for param b
+    await browser.pause(1000);
+    await browser.keys(['1', '2', Key.Enter]);
+
+    // initial pop-up for param `some param`
+    await browser.pause(1000);
+    await browser.keys(['f', 'a', 'l', 's', 'e', Key.Enter]);
+
+    // initial pop-up for param `some-param`
+    await browser.pause(1000);
+    await browser.keys(['5', Key.Enter]);
+
+    await checkResultsContent(workbench, async () => {
+      const queryResult = await (await $('#query-result')).getText();
+      await expect(queryResult).toContain('1998');
+      await expect(queryResult).toContain('12');
+      await expect(queryResult).toContain('false');
+      await expect(queryResult).toContain('5');
+      await expect(queryResult).toContain('2010');
+    });
+  });
 });