Added hint when using function as procedure and vice versa (#322)

.changeset/cuddly-vans-flash.md

@@ -0,0 +1,5 @@
+---
+'@neo4j-cypher/language-support': patch
+---
+
+Adds hints to the error when using a procedure/function where the other would be appropriate

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

@@ -14,7 +14,7 @@ import {
   ParsedStatement,
   parserWrapper,
 } from '../parserWrapper';
-import { Neo4jFunction } from '../types';
+import { Neo4jFunction, Neo4jProcedure } from '../types';
 import { wrappedSemanticAnalysis } from './semanticAnalysisWrapper';
 
 export type SyntaxDiagnostic = Diagnostic & {
@@ -85,25 +85,50 @@ function generateSyntaxDiagnostic(
 function detectNonDeclaredFunction(
   parsedFunction: ParsedFunction,
   functionsSchema: Record<string, Neo4jFunction>,
+  procedureSchema: Record<string, Neo4jProcedure>,
 ): SyntaxDiagnostic | undefined {
-  const lowercaseFunctionName = parsedFunction.name.toLowerCase();
+  const exists = functionExists(parsedFunction, functionsSchema);
+  if (!exists) {
+    const existsAsProcedure =
+      procedureSchema && Boolean(procedureSchema[parsedFunction.name]);
+    if (existsAsProcedure) {
+      return generateProcedureUsedAsFunctionError(parsedFunction);
+    }
+    return generateFunctionNotFoundError(parsedFunction);
+  }
+}
+
+function functionExists(
+  functionCandidate: ParsedFunction,
+  functionsSchema: Record<string, Neo4jFunction>,
+): boolean {
+  if (!functionCandidate || !functionsSchema) {
+    return false;
+  }
+  const functionExistsWithExactName = Boolean(
+    functionsSchema[functionCandidate.name],
+  );
+  const lowercaseFunctionName = functionCandidate.name.toLowerCase();
   const caseInsensitiveFunctionInDatabase =
     functionsSchema[lowercaseFunctionName];
 
   // Built-in functions are case-insensitive in the database
-  if (
-    caseInsensitiveFunctionInDatabase &&
-    caseInsensitiveFunctionInDatabase.isBuiltIn
-  ) {
-    return undefined;
-  }
+  return (
+    functionExistsWithExactName ||
+    (caseInsensitiveFunctionInDatabase &&
+      caseInsensitiveFunctionInDatabase.isBuiltIn)
+  );
+}
 
-  const functionExistsWithExactName = Boolean(
-    functionsSchema[parsedFunction.name],
+function generateFunctionUsedAsProcedureError(
+  parsedProcedure: ParsedProcedure,
+): SyntaxDiagnostic {
+  return generateSyntaxDiagnostic(
+    parsedProcedure.rawText,
+    parsedProcedure,
+    DiagnosticSeverity.Error,
+    `${parsedProcedure.name} is a function, not a procedure. Did you mean to use the function ${parsedProcedure.name} with a RETURN instead of a CALL clause?`,
   );
-  if (!functionExistsWithExactName) {
-    return generateFunctionNotFoundError(parsedFunction);
-  }
 }
 
 function generateFunctionNotFoundError(
@@ -117,6 +142,17 @@ function generateFunctionNotFoundError(
   );
 }
 
+function generateProcedureUsedAsFunctionError(
+  parsedFunction: ParsedFunction,
+): SyntaxDiagnostic {
+  return generateSyntaxDiagnostic(
+    parsedFunction.rawText,
+    parsedFunction,
+    DiagnosticSeverity.Error,
+    `${parsedFunction.name} is a procedure, not a function. Did you mean to call the procedure ${parsedFunction.name} inside a CALL clause?`,
+  );
+}
+
 function generateProcedureNotFoundError(
   parsedProcedure: ParsedProcedure,
 ): SyntaxDiagnostic {
@@ -342,6 +378,7 @@ function errorOnUndeclaredFunctions(
       const warning = detectNonDeclaredFunction(
         parsedFunction,
         dbSchema.functions,
+        dbSchema.procedures,
       );
 
       if (warning) warnings.push(warning);
@@ -365,7 +402,15 @@ function errorOnUndeclaredProcedures(
         dbSchema.procedures[parsedProcedure.name],
       );
       if (!procedureExists) {
-        errors.push(generateProcedureNotFoundError(parsedProcedure));
+        const existsAsFunction = functionExists(
+          parsedProcedure,
+          dbSchema.functions,
+        );
+        if (existsAsFunction) {
+          errors.push(generateFunctionUsedAsProcedureError(parsedProcedure));
+        } else {
+          errors.push(generateProcedureNotFoundError(parsedProcedure));
+        }
       }
     });
   }

packages/language-support/src/tests/syntaxValidation/functionsValidation.test.ts

@@ -74,6 +74,114 @@ describe('Functions semantic validation spec', () => {
     ]);
   });
 
+  test('Syntax validation error on function used as procedure returns helpful message', () => {
+    const query = `CALL abs(-50) YIELD *`;
+
+    expect(
+      getDiagnosticsForQuery({
+        query,
+        dbSchema: {
+          labels: ['Dog', 'Cat'],
+          relationshipTypes: ['Person'],
+          functions: testData.mockSchema.functions,
+          procedures: testData.mockSchema.procedures,
+        },
+      }),
+    ).toEqual([
+      {
+        message:
+          'abs is a function, not a procedure. Did you mean to use the function abs with a RETURN instead of a CALL clause?',
+        offsets: {
+          end: 8,
+          start: 5,
+        },
+        range: {
+          end: {
+            character: 8,
+            line: 0,
+          },
+          start: {
+            character: 5,
+            line: 0,
+          },
+        },
+        severity: 1,
+      },
+    ]);
+  });
+
+  test('Using improved message for function used as procedure is not case sensitive for built-in functions', () => {
+    const query = `CALL aBs(-50) YIELD *`;
+
+    expect(
+      getDiagnosticsForQuery({
+        query,
+        dbSchema: {
+          labels: ['Dog', 'Cat'],
+          relationshipTypes: ['Person'],
+          functions: testData.mockSchema.functions,
+          procedures: testData.mockSchema.procedures,
+        },
+      }),
+    ).toEqual([
+      {
+        message:
+          'aBs is a function, not a procedure. Did you mean to use the function aBs with a RETURN instead of a CALL clause?',
+        offsets: {
+          end: 8,
+          start: 5,
+        },
+        range: {
+          end: {
+            character: 8,
+            line: 0,
+          },
+          start: {
+            character: 5,
+            line: 0,
+          },
+        },
+        severity: 1,
+      },
+    ]);
+  });
+
+  test('Using improved message for function used as procedure is case sensitive for external functions', () => {
+    const query = `CALL apoc.text.TOUpperCase("message") YIELD *`;
+
+    expect(
+      getDiagnosticsForQuery({
+        query,
+        dbSchema: {
+          labels: ['Dog', 'Cat'],
+          relationshipTypes: ['Person'],
+          functions: testData.mockSchema.functions,
+          procedures: testData.mockSchema.procedures,
+        },
+      }),
+    ).toEqual([
+      {
+        message:
+          "Procedure apoc.text.TOUpperCase is not present in the database. Make sure you didn't misspell it or that it is available when you run this statement in your application",
+        offsets: {
+          end: 26,
+          start: 5,
+        },
+        range: {
+          end: {
+            character: 26,
+            line: 0,
+          },
+          start: {
+            character: 5,
+            line: 0,
+          },
+        },
+        severity: 1,
+      },
+    ]);
+  });
+
   test('Syntax validation errors on missing function when database can be contacted', () => {
     const query = `RETURN dontpanic("marvin")`;
 

packages/language-support/src/tests/syntaxValidation/proceduresValidation.test.ts

@@ -59,6 +59,78 @@ meaning that it expects at least 3 arguments of types NODE, STRING, ANY
     ]);
   });
 
+  test('Syntax validation error on procedure used as function returns helpful message', () => {
+    const query = `RETURN apoc.create.uuids(50)`;
+
+    expect(
+      getDiagnosticsForQuery({
+        query,
+        dbSchema: {
+          labels: ['Dog', 'Cat'],
+          relationshipTypes: ['Person'],
+          functions: testData.mockSchema.functions,
+          procedures: testData.mockSchema.procedures,
+        },
+      }),
+    ).toEqual([
+      {
+        message:
+          'apoc.create.uuids is a procedure, not a function. Did you mean to call the procedure apoc.create.uuids inside a CALL clause?',
+        offsets: {
+          end: 24,
+          start: 7,
+        },
+        range: {
+          end: {
+            character: 24,
+            line: 0,
+          },
+          start: {
+            character: 7,
+            line: 0,
+          },
+        },
+        severity: 1,
+      },
+    ]);
+  });
+
+  test('Using improved message for procedure used as function is case sensitive', () => {
+    const query = `RETURN apoc.cReAtE.uuids(50)`;
+
+    expect(
+      getDiagnosticsForQuery({
+        query,
+        dbSchema: {
+          labels: ['Dog', 'Cat'],
+          relationshipTypes: ['Person'],
+          functions: testData.mockSchema.functions,
+          procedures: testData.mockSchema.procedures,
+        },
+      }),
+    ).toEqual([
+      {
+        message:
+          "Function apoc.cReAtE.uuids is not present in the database. Make sure you didn't misspell it or that it is available when you run this statement in your application",
+        offsets: {
+          end: 24,
+          start: 7,
+        },
+        range: {
+          end: {
+            character: 24,
+            line: 0,
+          },
+          start: {
+            character: 7,
+            line: 0,
+          },
+        },
+        severity: 1,
+      },
+    ]);
+  });
+
   test('Syntax validation warns on missing procedures when database can be contacted', () => {
     const query = `CALL dontpanic("marvin")`;