From 7bac376599571f66ba5f6ca265de5404a3a40640 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 17 Feb 2025 09:34:34 +0100 Subject: [PATCH 1/9] 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/9] 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/9] 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/9] 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/9] 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: { From 406c60c9737248b626a4d6863746e98d55c8e2e8 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 20 Feb 2025 09:24:49 +0000 Subject: [PATCH 6/9] Bump version to 3.4.14 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index b6eb31f2b0..79a0eac346 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "3.4.13", + "version": "3.4.14", "npmClient": "yarn", "concurrency": 20, "command": { From 4092f4c3e158d67401883d94c5c6cbd85fd1805c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 20 Feb 2025 10:11:04 +0000 Subject: [PATCH 7/9] Fix loops being given empty strings. --- .../src/automations/tests/steps/loop.spec.ts | 22 +++++++++++++++++++ packages/server/src/threads/automation.ts | 8 +++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/server/src/automations/tests/steps/loop.spec.ts b/packages/server/src/automations/tests/steps/loop.spec.ts index 883732330f..19f7e5506f 100644 --- a/packages/server/src/automations/tests/steps/loop.spec.ts +++ b/packages/server/src/automations/tests/steps/loop.spec.ts @@ -7,6 +7,8 @@ import { CreateRowStepOutputs, FieldType, FilterCondition, + AutomationStatus, + AutomationStepStatus, } from "@budibase/types" import { createAutomationBuilder } from "../utilities/AutomationTestBuilder" import TestConfiguration from "../../../tests/utilities/TestConfiguration" @@ -560,5 +562,25 @@ describe("Attempt to run a basic loop automation", () => { status: "stopped", }) }) + + it("should not fail if queryRows returns nothing", async () => { + const table = await config.api.table.save(basicTable()) + const results = await createAutomationBuilder(config) + .onAppAction() + .queryRows({ + tableId: table._id!, + }) + .loop({ + option: LoopStepType.ARRAY, + binding: "{{ steps.1.rows }}", + }) + .serverLog({ text: "Message {{loop.currentItem}}" }) + .test({ fields: {} }) + + expect(results.steps[1].outputs.success).toBe(true) + expect(results.steps[1].outputs.status).toBe( + AutomationStepStatus.NO_ITERATIONS + ) + }) }) }) diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 8b2aac662c..762da1cbc1 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -68,7 +68,11 @@ function getLoopIterable(step: LoopStep): any[] { let input = step.inputs.binding if (option === LoopStepType.ARRAY && typeof input === "string") { - input = JSON.parse(input) + if (input === "") { + input = [] + } else { + input = JSON.parse(input) + } } if (option === LoopStepType.STRING && Array.isArray(input)) { @@ -492,7 +496,7 @@ class Orchestrator { } const status = - iterations === 0 ? AutomationStatus.NO_CONDITION_MET : undefined + iterations === 0 ? AutomationStepStatus.NO_ITERATIONS : undefined return stepSuccess(stepToLoop, { status, iterations, items }) }) } From ef9cbfce5936cb26fe981b6e2a4903ae3bd9bc1d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 20 Feb 2025 10:16:13 +0000 Subject: [PATCH 8/9] Fix lint. --- packages/server/src/automations/tests/steps/loop.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/automations/tests/steps/loop.spec.ts b/packages/server/src/automations/tests/steps/loop.spec.ts index 19f7e5506f..2bdf33b253 100644 --- a/packages/server/src/automations/tests/steps/loop.spec.ts +++ b/packages/server/src/automations/tests/steps/loop.spec.ts @@ -7,7 +7,6 @@ import { CreateRowStepOutputs, FieldType, FilterCondition, - AutomationStatus, AutomationStepStatus, } from "@budibase/types" import { createAutomationBuilder } from "../utilities/AutomationTestBuilder" From 1328076d03f88d656edb1a188c24d121904297fa Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 20 Feb 2025 10:26:46 +0000 Subject: [PATCH 9/9] Bump version to 3.4.15 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 79a0eac346..91980e0a15 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "3.4.14", + "version": "3.4.15", "npmClient": "yarn", "concurrency": 20, "command": {