From 7aa9c3a635070b877e773c27fcf64e9d64081506 Mon Sep 17 00:00:00 2001 From: Isak Nilsson <143806924+anderson4j@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:37:56 +0100 Subject: [PATCH] Added hint when using function as procedure and vice versa (#322) --- .changeset/cuddly-vans-flash.md | 5 + .../src/syntaxValidation/syntaxValidation.ts | 73 +++++++++--- .../functionsValidation.test.ts | 108 ++++++++++++++++++ .../proceduresValidation.test.ts | 72 ++++++++++++ 4 files changed, 244 insertions(+), 14 deletions(-) create mode 100644 .changeset/cuddly-vans-flash.md diff --git a/.changeset/cuddly-vans-flash.md b/.changeset/cuddly-vans-flash.md new file mode 100644 index 000000000..fddae6a48 --- /dev/null +++ b/.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 diff --git a/packages/language-support/src/syntaxValidation/syntaxValidation.ts b/packages/language-support/src/syntaxValidation/syntaxValidation.ts index 9dda8ff55..178e20335 100644 --- a/packages/language-support/src/syntaxValidation/syntaxValidation.ts +++ b/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, + procedureSchema: Record, ): 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, +): 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)); + } } }); } diff --git a/packages/language-support/src/tests/syntaxValidation/functionsValidation.test.ts b/packages/language-support/src/tests/syntaxValidation/functionsValidation.test.ts index c96623f39..5a6a33bbb 100644 --- a/packages/language-support/src/tests/syntaxValidation/functionsValidation.test.ts +++ b/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")`; diff --git a/packages/language-support/src/tests/syntaxValidation/proceduresValidation.test.ts b/packages/language-support/src/tests/syntaxValidation/proceduresValidation.test.ts index acc079e4c..26517cb98 100644 --- a/packages/language-support/src/tests/syntaxValidation/proceduresValidation.test.ts +++ b/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")`;