From 7bac376599571f66ba5f6ca265de5404a3a40640 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 17 Feb 2025 09:34:34 +0100 Subject: [PATCH 1/5] Initial js validation --- .../common/CodeEditor/CodeEditor.svelte | 4 + .../common/CodeEditor/validator/js.ts | 80 +++++++++++++++++++ .../common/bindings/BindingPanel.svelte | 1 + 3 files changed, 85 insertions(+) create mode 100644 packages/builder/src/components/common/CodeEditor/validator/js.ts diff --git a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte index fbf74d1e9b..0f4bc64e2a 100644 --- a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte +++ b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte @@ -49,6 +49,7 @@ import type { EditorMode } from "@budibase/types" import type { BindingCompletion, CodeValidator } from "@/types" import { validateHbsTemplate } from "./validator/hbs" + import { validateJsTemplate } from "./validator/js" export let label: string | undefined = undefined export let completions: BindingCompletion[] = [] @@ -356,6 +357,9 @@ if (mode === EditorModes.Handlebars) { const diagnostics = validateHbsTemplate(value, validations) editor.dispatch(setDiagnostics(editor.state, diagnostics)) + } else if (mode === EditorModes.JS) { + const diagnostics = validateJsTemplate(value, validations) + editor.dispatch(setDiagnostics(editor.state, diagnostics)) } } diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts new file mode 100644 index 0000000000..6e85dc41ec --- /dev/null +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -0,0 +1,80 @@ +import { Parser } from "acorn" +import { simple as walk } from "acorn-walk" + +import { iifeWrapper } from "@budibase/string-templates" +import type { Diagnostic } from "@codemirror/lint" +import { CodeValidator } from "@/types" + +export function validateJsTemplate( + code: string, + validations: CodeValidator +): Diagnostic[] { + const diagnostics: Diagnostic[] = [] + + try { + // const helperUsages = new RegExp(/\bhelpers\.(\w)+\b/).exec(code) + const ast = Parser.parse(iifeWrapper(code), { + ecmaVersion: "latest", + locations: true, + }) + + const lineOffsets: number[] = [0] + let offset = 0 + for (const line of code.split("\n")) { + lineOffsets.push(offset) + offset += line.length + 1 // +1 for newline character + } + + walk(ast, { + CallExpression(node) { + const callee: any = node.callee + if ( + node.type === "CallExpression" && + callee.object?.name === "helpers" && + node.loc + ) { + const functionName = callee.property.name + const from = + lineOffsets[node.loc.start.line - 1] + node.loc.start.column + const to = lineOffsets[node.loc.end.line - 1] + node.loc.end.column + + if (!(functionName in validations)) { + diagnostics.push({ + from, + to, + severity: "warning", + message: `"${functionName}" function does not exist.`, + }) + return + } + + const { arguments: expectedArguments } = validations[functionName] + if ( + expectedArguments && + node.arguments.length !== expectedArguments.length + ) { + diagnostics.push({ + from, + to, + severity: "error", + message: `Function "${functionName}" expects ${ + expectedArguments.length + } parameters (${expectedArguments.join(", ")}), but got ${ + node.arguments.length + }.`, + }) + } + } + }, + }) + } catch (e: any) { + diagnostics.push({ + from: 0, + to: code.length, + severity: "error", + message: `Syntax error: ${e.message}`, + }) + } + + return diagnostics +} diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte index 2c35acdf2d..9ebbc91309 100644 --- a/packages/builder/src/components/common/bindings/BindingPanel.svelte +++ b/packages/builder/src/components/common/bindings/BindingPanel.svelte @@ -377,6 +377,7 @@ value={jsValue ? decodeJSBinding(jsValue) : jsValue} on:change={onChangeJSValue} {completions} + {validations} mode={EditorModes.JS} bind:getCaretPosition bind:insertAtPos From 608fcb42be494dbf418fff23fc4eb75bfb2ca9de Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:24:19 +0100 Subject: [PATCH 2/5] Validate return value --- .../components/common/CodeEditor/validator/js.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts index 6e85dc41ec..6d7c3ea8bf 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/js.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -25,7 +25,11 @@ export function validateJsTemplate( offset += line.length + 1 // +1 for newline character } + let hasReturnStatement = false walk(ast, { + ReturnStatement(node) { + hasReturnStatement = !!node.argument + }, CallExpression(node) { const callee: any = node.callee if ( @@ -67,6 +71,15 @@ export function validateJsTemplate( } }, }) + + if (!hasReturnStatement) { + diagnostics.push({ + from: 0, + to: code.length, + severity: "error", + message: "Your code must return a value.", + }) + } } catch (e: any) { diagnostics.push({ from: 0, From c5f2ef9354917c930cab6aef42470b2bfdcad7c8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:35:52 +0100 Subject: [PATCH 3/5] Add js validation tests --- .../CodeEditor/validator/tests/js.spec.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts new file mode 100644 index 0000000000..be6977f35a --- /dev/null +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -0,0 +1,71 @@ +import { validateJsTemplate } from "../js" +import { CodeValidator } from "@/types" + +describe("js validator", () => { + it("validates valid code", () => { + const text = "return 7" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("does not validate runtime errors", () => { + const text = "return a" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("validates multiline code", () => { + const text = "const foo='bar'\nreturn 123" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + describe("helpers", () => { + const validators: CodeValidator = { + helperFunction: { + arguments: ["a", "b", "c"], + }, + } + + it("validates helpers with valid params", () => { + const text = "return helpers.helperFunction(1, 99, 'a')" + + const result = validateJsTemplate(text, validators) + expect(result).toHaveLength(0) + }) + + it("throws on too few params", () => { + const text = "return helpers.helperFunction(100)" + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 7, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 1.`, + severity: "error", + to: 34, + }, + ]) + }) + + it("throws on too many params", () => { + const text = "return helpers.helperFunction( 1, 99, 'a', 100)" + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 7, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 4.`, + severity: "error", + to: 47, + }, + ]) + }) + }) +}) From fba5663e66aeb34605529896e7bb085e34a4d5b9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:44:47 +0100 Subject: [PATCH 4/5] Improve tests --- .../CodeEditor/validator/tests/js.spec.ts | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts index be6977f35a..d0c25182f7 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -26,6 +26,29 @@ describe("js validator", () => { expect(result).toEqual([]) }) + it("allows return not being on the last line", () => { + const text = "const foo='bar'\nreturn 123\nconsole.log(foo)" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("throws on missing return", () => { + const text = "const foo='bar'\nbar='foo'" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 0, + message: "Your code must return a value.", + severity: "error", + to: 25, + }, + ]) + }) + describe("helpers", () => { const validators: CodeValidator = { helperFunction: { @@ -37,7 +60,7 @@ describe("js validator", () => { const text = "return helpers.helperFunction(1, 99, 'a')" const result = validateJsTemplate(text, validators) - expect(result).toHaveLength(0) + expect(result).toEqual([]) }) it("throws on too few params", () => { @@ -67,5 +90,49 @@ describe("js validator", () => { }, ]) }) + + it("validates helpers on inner functions", () => { + const text = `function call(){ + return helpers.helperFunction(1, 99) + } + return call()` + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 46, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 2.`, + severity: "error", + to: 75, + }, + ]) + }) + + it("validates multiple helpers", () => { + const text = + "return helpers.helperFunction(1, 99, 'a') + helpers.helperFunction(1) + helpers.another(1) + helpers.another()" + const validators: CodeValidator = { + helperFunction: { + arguments: ["a", "b", "c"], + }, + another: { arguments: [] }, + } + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 44, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 1.`, + severity: "error", + to: 69, + }, + { + from: 72, + message: `Function "another" expects 0 parameters (), but got 1.`, + severity: "error", + to: 90, + }, + ]) + }) }) }) From 1a4abb7630b5c72ac18e9c5267e2f583caa28a99 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 13:14:40 +0100 Subject: [PATCH 5/5] Validate return --- .../common/CodeEditor/validator/js.ts | 24 ++++++++++++------- .../CodeEditor/validator/tests/js.spec.ts | 18 ++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts index 6d7c3ea8bf..20fb5abd07 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/js.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -1,7 +1,6 @@ import { Parser } from "acorn" -import { simple as walk } from "acorn-walk" +import * as walk from "acorn-walk" -import { iifeWrapper } from "@budibase/string-templates" import type { Diagnostic } from "@codemirror/lint" import { CodeValidator } from "@/types" @@ -12,13 +11,13 @@ export function validateJsTemplate( const diagnostics: Diagnostic[] = [] try { - // const helperUsages = new RegExp(/\bhelpers\.(\w)+\b/).exec(code) - const ast = Parser.parse(iifeWrapper(code), { + const ast = Parser.parse(code, { ecmaVersion: "latest", locations: true, + allowReturnOutsideFunction: true, }) - const lineOffsets: number[] = [0] + const lineOffsets: number[] = [] let offset = 0 for (const line of code.split("\n")) { lineOffsets.push(offset) @@ -26,9 +25,18 @@ export function validateJsTemplate( } let hasReturnStatement = false - walk(ast, { - ReturnStatement(node) { - hasReturnStatement = !!node.argument + walk.ancestor(ast, { + ReturnStatement(node, _state, ancestors) { + if ( + // it returns a value + node.argument && + // and it is top level + ancestors.length === 2 && + ancestors[0].type === "Program" && + ancestors[1].type === "ReturnStatement" + ) { + hasReturnStatement = true + } }, CallExpression(node) { const callee: any = node.callee diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts index d0c25182f7..cd9fe4684c 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -49,6 +49,24 @@ describe("js validator", () => { ]) }) + it("checks that returns are at top level", () => { + const text = ` + function call(){ + return 1 + }` + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 0, + message: "Your code must return a value.", + severity: "error", + to: text.length, + }, + ]) + }) + describe("helpers", () => { const validators: CodeValidator = { helperFunction: {