diff --git a/actions-expressions/src/errors.ts b/actions-expressions/src/errors.ts index 5edba0e..0a2db07 100644 --- a/actions-expressions/src/errors.ts +++ b/actions-expressions/src/errors.ts @@ -50,3 +50,5 @@ function errorDescription(typ: ErrorType): string { return "Unknown error"; } } + +export class ExpressionEvaluationError extends Error {} diff --git a/actions-expressions/src/evaluator.test.ts b/actions-expressions/src/evaluator.test.ts index cb282eb..e1fa8fd 100644 --- a/actions-expressions/src/evaluator.test.ts +++ b/actions-expressions/src/evaluator.test.ts @@ -1,27 +1,42 @@ import * as data from "./data"; +import {ExpressionEvaluationError} from "./errors"; import {Evaluator} from "./evaluator"; import {Lexer} from "./lexer"; import {Parser} from "./parser"; -test("evaluator", () => { - const input = "foo['']"; +describe("evaluator", () => { + const lexAndParse = (input: string) => { + const lexer = new Lexer(input); + const result = lexer.lex(); - const lexer = new Lexer(input); - const result = lexer.lex(); + // Parse + const parser = new Parser(result.tokens, ["foo"], []); + const expr = parser.parse(); + return expr; + }; - // Parse - const parser = new Parser(result.tokens, ["foo"], []); - const expr = parser.parse(); + it("basic evaluation", () => { + const expr = lexAndParse("foo['']"); - // Evaluate expression - const evaluator = new Evaluator( - expr, - new data.Dictionary({ - key: "foo", - value: new data.Dictionary({key: "bar", value: new data.NumberData(42)}) - }) - ); - const eresult = evaluator.evaluate(); + // Evaluate expression + const evaluator = new Evaluator( + expr, + new data.Dictionary({ + key: "foo", + value: new data.Dictionary({key: "bar", value: new data.NumberData(42)}) + }) + ); + const eresult = evaluator.evaluate(); - expect(eresult.kind).toBe(data.Kind.Null); + expect(eresult.kind).toBe(data.Kind.Null); + }); + + it("handle runtime errors", () => { + const expr = lexAndParse("fromJson('test') == 123"); + const evaluator = new Evaluator(expr, new data.Dictionary()); + + expect(() => evaluator.evaluate()).toThrowError( + new ExpressionEvaluationError("Error parsing JSON when evaluating fromJson") + ); + }); }); diff --git a/actions-expressions/src/evaluator.ts b/actions-expressions/src/evaluator.ts index 65556ff..54c27e6 100644 --- a/actions-expressions/src/evaluator.ts +++ b/actions-expressions/src/evaluator.ts @@ -14,14 +14,19 @@ import { import * as data from "./data"; import {FilteredArray} from "./filtered_array"; import {wellKnownFunctions} from "./funcs"; +import {FunctionDefinition} from "./funcs/info"; import {idxHelper} from "./idxHelper"; import {TokenType} from "./lexer"; import {equals, falsy, greaterThan, lessThan, truthy} from "./result"; -export class EvaluationError extends Error {} - export class Evaluator implements ExprVisitor { - constructor(private n: Expr, private context: data.Dictionary) {} + /** + * Creates a new evaluator + * @param n Parsed expression to evaluate + * @param context Context data to use + * @param functions Optional map of function implementations. If given, these will be preferred over the built-in functions. + */ + constructor(private n: Expr, private context: data.Dictionary, private functions?: Map) {} public evaluate(): data.ExpressionData { return this.eval(this.n); @@ -109,7 +114,7 @@ export class Evaluator implements ExprVisitor { try { idxResult = this.eval(ia.index); } catch (e) { - throw new Error(`could not evaluate index for index access: ${e}`); + throw new Error(`could not evaluate index for index access: ${e}`, {cause: e}); } idx = new idxHelper(false, idxResult); } @@ -150,16 +155,14 @@ export class Evaluator implements ExprVisitor { // Evaluate arguments const args = functionCall.args.map(arg => this.eval(arg)); - return fcall(functionCall, args); + // Get function definitions + const functionName = functionCall.functionName.lexeme.toLowerCase(); + const f = this.functions?.get(functionName) || wellKnownFunctions[functionName]; + + return f.call(...args); } } -function fcall(fc: FunctionCall, args: data.ExpressionData[]): data.ExpressionData { - const f = wellKnownFunctions[fc.functionName.lexeme.toLowerCase()]; - - return f.call(...args); -} - function filteredArrayAccess(fa: FilteredArray, idx: idxHelper): data.ExpressionData { const result = new FilteredArray(); diff --git a/actions-expressions/src/funcs/fromjson.ts b/actions-expressions/src/funcs/fromjson.ts index b014400..6546b2a 100644 --- a/actions-expressions/src/funcs/fromjson.ts +++ b/actions-expressions/src/funcs/fromjson.ts @@ -1,5 +1,6 @@ import {ExpressionData} from "../data"; import {reviver} from "../data/reviver"; +import {ExpressionEvaluationError} from "../errors"; import {FunctionDefinition} from "./info"; export const fromjson: FunctionDefinition = { @@ -16,7 +17,10 @@ export const fromjson: FunctionDefinition = { throw new Error("empty input"); } - const d = JSON.parse(is, reviver); - return d; + try { + return JSON.parse(is, reviver); + } catch (e) { + throw new ExpressionEvaluationError("Error parsing JSON when evaluating fromJson", {cause: e}); + } } }; diff --git a/actions-expressions/src/index.ts b/actions-expressions/src/index.ts index 06eda82..72e53c3 100644 --- a/actions-expressions/src/index.ts +++ b/actions-expressions/src/index.ts @@ -2,7 +2,8 @@ export {Expr} from "./ast"; export {complete, CompletionItem} from "./completion"; export {DescriptionDictionary, DescriptionPair, isDescriptionDictionary} from "./completion/descriptionDictionary"; export * as data from "./data"; -export {ExpressionError} from "./errors"; +export {ExpressionError, ExpressionEvaluationError} from "./errors"; export {Evaluator} from "./evaluator"; +export {wellKnownFunctions} from "./funcs"; export {Lexer, Result} from "./lexer"; export {Parser} from "./parser"; diff --git a/actions-expressions/tsconfig.json b/actions-expressions/tsconfig.json index bbc5196..7da45bc 100644 --- a/actions-expressions/tsconfig.json +++ b/actions-expressions/tsconfig.json @@ -3,6 +3,7 @@ "compilerOptions": { "module": "esnext", "target": "ES2020", + "lib": ["ES2022"], "esModuleInterop": true, "forceConsistentCasingInFileNames": true, "sourceMap": true, diff --git a/actions-languageservice/src/validate.expressions.test.ts b/actions-languageservice/src/validate.expressions.test.ts index 0233569..85e0a4f 100644 --- a/actions-languageservice/src/validate.expressions.test.ts +++ b/actions-languageservice/src/validate.expressions.test.ts @@ -359,7 +359,7 @@ jobs: runs-on: ubuntu-latest steps: - name: step a - env: + env: step_env: job_a_env run: echo "hello \${{ env.step_env }} `; @@ -376,7 +376,7 @@ env: jobs: a: runs-on: ubuntu-latest - env: + env: envjoba: job_a_env steps: - name: step a @@ -395,7 +395,7 @@ env: jobs: a: runs-on: ubuntu-latest - env: + env: envjoba: job_a_env steps: - name: step a @@ -1185,6 +1185,9 @@ jobs: - run: echo "hello \${{ github.event.inputs.name }}" - run: echo "hello \${{ github.event.inputs.third-name }}" - run: echo "hello \${{ github.event.inputs.random }}" + - run: echo \${{ fromJSON(inputs.random2) }} + - run: echo "hello \${{ inputs.random }}" + name: "\${{ fromJSON('test') == inputs.name }}" `; const result = await validate(createDocument("wf.yaml", input)); @@ -1202,6 +1205,34 @@ jobs: } }, severity: DiagnosticSeverity.Warning + }, + { + message: "Context access might be invalid: random2", + range: { + end: { + character: 47, + line: 20 + }, + start: { + character: 16, + line: 20 + } + }, + severity: 2 + }, + { + message: "Context access might be invalid: random", + range: { + end: { + character: 43, + line: 21 + }, + start: { + character: 23, + line: 21 + } + }, + severity: 2 } ]); }); diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index d5e9884..25e2d40 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -1,4 +1,11 @@ -import {Evaluator, Lexer, Parser} from "@github/actions-expressions"; +import { + data, + Evaluator, + ExpressionEvaluationError, + Lexer, + Parser, + wellKnownFunctions +} from "@github/actions-expressions"; import {Expr} from "@github/actions-expressions/ast"; import { convertWorkflowTemplate, @@ -202,7 +209,7 @@ async function validateExpression( try { const context = await getContext(namedContexts, contextProviderConfig, workflowContext, Mode.Validation); - const e = new Evaluator(expr, wrapDictionary(context)); + const e = new Evaluator(expr, wrapDictionary(context), validatorFunctions); e.evaluate(); // Any invalid context access would've thrown an error via the `ErrorDictionary`, for now we don't have to check the actual @@ -214,7 +221,25 @@ async function validateExpression( severity: DiagnosticSeverity.Warning, range: mapRange(expression.range) }); + } else if (e instanceof ExpressionEvaluationError) { + diagnostics.push({ + message: `Expression might be invalid: ${e.message}`, + severity: DiagnosticSeverity.Error, + range: mapRange(expression.range) + }); } } } } + +// Custom implementations for standard actions-expression functions used during validation. For example, +// for fromJson we'll most likely not have a valid input. In order to not throw, we'll always return an +// empty dictionary. +const validatorFunctions = new Map( + Object.entries({ + fromjson: { + ...wellKnownFunctions.fromjson, + call: () => new data.Dictionary() + } + }) +);