Merge pull request #17 from actions/release/0.3.2

Release version 0.3.2

.github/CODEOWNERS

@@ -1 +1 @@
-* @actions/actions-experience
+* @actions/actions-workflow-development-reviewers

expressions/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/expressions",
-  "version": "0.3.1",
+  "version": "0.3.2",
   "license": "MIT",
   "type": "module",
   "source": "./src/index.ts",

expressions/src/evaluator.ts

@@ -32,7 +32,7 @@ export class Evaluator implements ExprVisitor<data.ExpressionData> {
     return this.eval(this.n);
   }
 
-  private eval(n: Expr): data.ExpressionData {
+  protected eval(n: Expr): data.ExpressionData {
     return n.accept(this);
   }
 

languageserver/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/languageserver",
-  "version": "0.3.1",
+  "version": "0.3.2",
   "description": "Language server for GitHub Actions",
   "license": "MIT",
   "type": "module",
@@ -43,8 +43,8 @@
     "watch": "tsc --build tsconfig.build.json --watch"
   },
   "dependencies": {
-    "@actions/languageservice": "^0.3.1",
-    "@actions/workflow-parser": "^0.3.1",
+    "@actions/languageservice": "^0.3.2",
+    "@actions/workflow-parser": "^0.3.2",
     "@octokit/rest": "^19.0.7",
     "@octokit/types": "^9.0.0",
     "vscode-languageserver": "^8.0.2",

languageservice/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/languageservice",
-  "version": "0.3.1",
+  "version": "0.3.2",
   "description": "Language service for GitHub Actions",
   "license": "MIT",
   "type": "module",
@@ -44,8 +44,8 @@
     "watch": "tsc --build tsconfig.build.json --watch"
   },
   "dependencies": {
-    "@actions/expressions": "^0.3.1",
-    "@actions/workflow-parser": "^0.3.1",
+    "@actions/expressions": "^0.3.2",
+    "@actions/workflow-parser": "^0.3.2",
     "vscode-languageserver-textdocument": "^1.0.7",
     "vscode-languageserver-types": "^3.17.2",
     "vscode-uri": "^3.0.7",

languageservice/src/expression-validation/error-dictionary.ts

@@ -0,0 +1,44 @@
+import {data, isDescriptionDictionary} from "@actions/expressions";
+import {isDictionary} from "@actions/expressions/data/dictionary";
+import {ExpressionData, Pair} from "@actions/expressions/data/expressiondata";
+
+export class AccessError extends Error {
+  constructor(message: string, public readonly keyName: string) {
+    super(message);
+  }
+}
+
+export class ErrorDictionary extends data.Dictionary {
+  constructor(...pairs: Pair[]) {
+    super(...pairs);
+  }
+  public complete = true;
+
+  get(key: string): ExpressionData | undefined {
+    const value = super.get(key);
+    if (value) {
+      return value;
+    }
+
+    if (this.complete) {
+      throw new AccessError(`Invalid context access: ${key}`, key);
+    }
+  }
+}
+
+export function wrapDictionary(d: data.Dictionary): ErrorDictionary {
+  const e = new ErrorDictionary();
+  if (isDescriptionDictionary(d)) {
+    e.complete = d.complete;
+  }
+
+  for (const {key, value} of d.pairs()) {
+    if (isDictionary(value)) {
+      e.add(key, wrapDictionary(value));
+    } else {
+      e.add(key, value);
+    }
+  }
+
+  return e;
+}

languageservice/src/expression-validation/evaluator.ts

@@ -0,0 +1,61 @@
+import {Evaluator, ExpressionEvaluationError, data} from "@actions/expressions";
+import {Expr, Logical} from "@actions/expressions/ast";
+import {ExpressionData} from "@actions/expressions/data/expressiondata";
+import {TokenType} from "@actions/expressions/lexer";
+import {falsy, truthy} from "@actions/expressions/result";
+import {AccessError} from "./error-dictionary";
+
+export type ValidationError = {
+  message: string;
+  severity: "error" | "warning";
+};
+
+export class ValidationEvaluator extends Evaluator {
+  public readonly errors: ValidationError[] = [];
+
+  public validate() {
+    super.evaluate();
+  }
+
+  protected override eval(n: Expr): ExpressionData {
+    try {
+      return super.eval(n);
+    } catch (e) {
+      // Record error
+      if (e instanceof AccessError) {
+        this.errors.push({
+          message: `Context access might be invalid: ${e.keyName}`,
+          severity: "warning"
+        });
+      } else if (e instanceof ExpressionEvaluationError) {
+        this.errors.push({
+          message: `Expression might be invalid: ${e.message}`,
+          severity: "error"
+        });
+      }
+    }
+
+    // Return null but continue with the validation
+    return new data.Null();
+  }
+
+  override visitLogical(logical: Logical): ExpressionData {
+    let result: data.ExpressionData | undefined;
+
+    for (const arg of logical.args) {
+      const r = this.eval(arg);
+
+      // Simulate short-circuit behavior but continue to evalute all arguments for validation purposes
+      if (
+        !result &&
+        ((logical.operator.type === TokenType.AND && falsy(r)) || (logical.operator.type === TokenType.OR && truthy(r)))
+      ) {
+        result = r;
+      }
+    }
+
+    // result is always assigned before we return here
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    return result!;
+  }
+}

languageservice/src/expression-validation/visitor.test.ts

@@ -1,129 +0,0 @@
-import {Lexer, Parser} from "@actions/expressions";
-import {Dictionary} from "@actions/expressions/data/dictionary";
-import {StringData} from "@actions/expressions/data/string";
-import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
-import {ValidationVisitor} from "./visitor";
-
-const testContext = new Dictionary({
-  key: "github",
-  value: new Dictionary(
-    {
-      key: "event",
-      value: new StringData("push")
-    },
-    {
-      key: "repo",
-      value: new Dictionary({
-        key: "name",
-        value: new StringData("test")
-      })
-    }
-  )
-});
-
-function useVisitor(expression: string, allowedContext: string[]): any[] {
-  const {namedContexts, functions} = splitAllowedContext(allowedContext);
-
-  const l = new Lexer(expression);
-  const lr = l.lex();
-
-  const p = new Parser(lr.tokens, namedContexts, functions);
-  const expr = p.parse();
-
-  const e = new ValidationVisitor(expr, testContext);
-  e.validate();
-
-  return e.errors;
-}
-
-describe("validation visitor", () => {
-  it("invalid context access", () => {
-    expect(useVisitor("github.foo", ["github"])).toEqual([
-      {
-        message: "Context access might be invalid: foo",
-        range: {
-          end: {
-            column: 10,
-            line: 0
-          },
-          start: {
-            column: 0,
-            line: 0
-          }
-        },
-        severity: "warning"
-      }
-    ]);
-  });
-
-  it("invalid context access as index", () => {
-    expect(useVisitor("github[github.foo]", ["github"])).toEqual([
-      {
-        message: "Context access might be invalid: foo",
-        range: {
-          end: {
-            column: 17,
-            line: 0
-          },
-          start: {
-            column: 7,
-            line: 0
-          }
-        },
-        severity: "warning"
-      }
-    ]);
-  });
-
-  it("invalid nested context access", () => {
-    expect(useVisitor("github.repo.name", ["github"])).toEqual([
-      {
-        message: "Context access might be invalid: name",
-        range: {
-          end: {
-            column: 16,
-            line: 0
-          },
-          start: {
-            column: 0,
-            line: 0
-          }
-        },
-        severity: "warning"
-      }
-    ]);
-  });
-
-  it("invalid context accesses", () => {
-    expect(useVisitor("github.foo || github.foo.bar", ["github"])).toEqual([
-      {
-        message: "Context access might be invalid: foo",
-        range: {
-          end: {
-            column: 10,
-            line: 0
-          },
-          start: {
-            column: 0,
-            line: 0
-          }
-        },
-        severity: "warning"
-      },
-      {
-        message: "Context access might be invalid: bar",
-        range: {
-          end: {
-            column: 28,
-            line: 0
-          },
-          start: {
-            column: 14,
-            line: 0
-          }
-        },
-        severity: "warning"
-      }
-    ]);
-  });
-});

languageservice/src/expression-validation/visitor.ts

@@ -1,148 +0,0 @@
-import {DescriptionDictionary} from "@actions/expressions";
-import {
-  Binary,
-  ContextAccess,
-  Expr,
-  ExprVisitor,
-  FunctionCall,
-  Grouping,
-  IndexAccess,
-  Literal,
-  Logical,
-  Unary
-} from "@actions/expressions/ast";
-import {Dictionary} from "@actions/expressions/data/dictionary";
-import {ExpressionData} from "@actions/expressions/data/expressiondata";
-import {Range} from "@actions/expressions/lexer";
-
-export type ValidationError = {
-  range: Range;
-  message: string;
-  severity: "error" | "warning";
-};
-
-export class ValidationVisitor implements ExprVisitor<void> {
-  public readonly errors: ValidationError[] = [];
-
-  constructor(private expr: Expr, private context: Dictionary) {}
-
-  validate(): void {
-    this._validate(this.expr);
-  }
-
-  private _validate(expr: Expr) {
-    expr.accept(this);
-  }
-
-  visitLiteral() {
-    return undefined;
-  }
-
-  visitUnary(unary: Unary) {
-    this._validate(unary.expr);
-  }
-
-  visitBinary(binary: Binary) {
-    this._validate(binary.left);
-    this._validate(binary.right);
-  }
-
-  visitLogical(logical: Logical) {
-    for (const arg of logical.args) {
-      this._validate(arg);
-    }
-  }
-
-  visitGrouping(grouping: Grouping) {
-    this._validate(grouping.group);
-  }
-
-  visitContextAccess(contextAccess: ContextAccess) {
-    const contextName = contextAccess.name.lexeme;
-    if (this.context.get(contextName) === undefined) {
-      this.errors.push({
-        message: `Context access might be invalid: ${contextName}`,
-        range: contextAccess.name.range,
-        severity: "error"
-      });
-    }
-  }
-
-  visitIndexAccess(indexAccess: IndexAccess) {
-    let contextAccess: ContextAccess | undefined;
-
-    const s: ExpressionData[] = [];
-    let i: Expr = indexAccess;
-    while (i) {
-      if (i instanceof IndexAccess) {
-        if (!(i.index instanceof Literal)) {
-          // Not a literal, validate independently
-          this._validate(i.index);
-          return;
-        }
-        s.push(i.index.literal);
-        i = i.expr;
-      }
-
-      if (i instanceof ContextAccess) {
-        contextAccess = i;
-        break;
-      }
-    }
-
-    if (!contextAccess) {
-      // Context not found, should not happen, ignore in this case
-      return;
-    }
-
-    const contextName = contextAccess.name.lexeme;
-    let contextValue = this.context.get(contextName);
-    if (contextValue === undefined || !(contextValue instanceof Dictionary)) {
-      const contextName = contextAccess.name.lexeme;
-      if (this.context.get(contextName) === undefined) {
-        this.errors.push({
-          message: `Context access might be invalid: ${contextName}`,
-          range: contextAccess.name.range,
-          severity: "warning"
-        });
-      }
-      return;
-    }
-
-    while (s.length > 0) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      const idx = s.pop()!;
-
-      const key = idx.coerceString();
-      const v: ExpressionData | undefined = contextValue.get(key);
-      if (v === undefined) {
-        if (contextValue instanceof DescriptionDictionary && !contextValue.complete) {
-          // If the context dictionary is not complete, we cannot validate the expression
-          return;
-        }
-
-        this.errors.push({
-          range: {
-            start: contextAccess.name.range.start,
-            end: (indexAccess.index as Literal).token.range.end
-          },
-          message: `Context access might be invalid: ${key}`,
-          severity: "warning"
-        });
-        return;
-      }
-
-      if (!(v instanceof Dictionary)) {
-        return;
-      }
-
-      contextValue = v;
-    }
-  }
-
-  visitFunctionCall(functionCall: FunctionCall) {
-    for (const arg of functionCall.args) {
-      this._validate(arg);
-    }
-  }
-}

languageservice/src/validate.expressions.test.ts

@@ -1,4 +1,4 @@
-import {DescriptionDictionary} from "@actions/expressions";
+import {DescriptionDictionary} from "@actions/expressions/.";
 import {DiagnosticSeverity} from "vscode-languageserver-types";
 import {ContextProviderConfig} from "./context-providers/config";
 import {registerLogger} from "./log";
@@ -46,6 +46,52 @@ jobs:
     ]);
   });
 
+  it("access invalid context field in short-circuited expression", async () => {
+    const result = await validate(
+      createDocument(
+        "wf.yaml",
+        `on: push
+run-name: name-\${{ github.does-not-exist || github.does-not-exist2 }}
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo`
+      )
+    );
+
+    expect(result).toEqual([
+      {
+        message: "Context access might be invalid: does-not-exist",
+        range: {
+          end: {
+            character: 69,
+            line: 1
+          },
+          start: {
+            character: 15,
+            line: 1
+          }
+        },
+        severity: DiagnosticSeverity.Warning
+      },
+      {
+        message: "Context access might be invalid: does-not-exist2",
+        range: {
+          end: {
+            character: 69,
+            line: 1
+          },
+          start: {
+            character: 15,
+            line: 1
+          }
+        },
+        severity: DiagnosticSeverity.Warning
+      }
+    ]);
+  });
+
   it("partial skip access invalid context on incomplete", async () => {
     const contextProviderConfig: ContextProviderConfig = {
       getContext: (context: string) => {

languageservice/src/validate.ts

@@ -15,7 +15,9 @@ import {ActionMetadata, ActionReference} from "./action";
 import {ContextProviderConfig} from "./context-providers/config";
 import {Mode, getContext} from "./context-providers/default";
 import {WorkflowContext, getWorkflowContext} from "./context/workflow-context";
-import {ValidationVisitor} from "./expression-validation/visitor";
+import {wrapDictionary} from "./expression-validation/error-dictionary";
+import {ValidationEvaluator} from "./expression-validation/evaluator";
+import {validatorFunctions} from "./expression-validation/functions";
 import {error} from "./log";
 import {findToken} from "./utils/find-token";
 import {mapRange} from "./utils/range";
@@ -203,7 +205,7 @@ async function validateExpression(
 
     const context = await getContext(namedContexts, contextProviderConfig, workflowContext, Mode.Validation);
 
-    const e = new ValidationVisitor(expr, context);
+    const e = new ValidationEvaluator(expr, wrapDictionary(context), validatorFunctions);
     e.validate();
 
     diagnostics.push(

lerna.json

@@ -1,5 +1,5 @@
 {
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
   "useWorkspaces": true,
-  "version": "0.3.1"
+  "version": "0.3.2"
 }

package-lock.json

@@ -135,7 +135,7 @@
     },
     "expressions": {
       "name": "@actions/expressions",
-      "version": "0.3.1",
+      "version": "0.3.2",
       "license": "MIT",
       "devDependencies": {
         "@types/jest": "^29.0.3",
@@ -395,11 +395,11 @@
     },
     "languageserver": {
       "name": "@actions/languageserver",
-      "version": "0.3.1",
+      "version": "0.3.2",
       "license": "MIT",
       "dependencies": {
-        "@actions/languageservice": "^0.3.1",
-        "@actions/workflow-parser": "^0.3.1",
+        "@actions/languageservice": "^0.3.2",
+        "@actions/workflow-parser": "^0.3.2",
         "@octokit/rest": "^19.0.7",
         "@octokit/types": "^9.0.0",
         "vscode-languageserver": "^8.0.2",
@@ -678,11 +678,11 @@
     },
     "languageservice": {
       "name": "@actions/languageservice",
-      "version": "0.3.1",
+      "version": "0.3.2",
       "license": "MIT",
       "dependencies": {
-        "@actions/expressions": "^0.3.1",
-        "@actions/workflow-parser": "^0.3.1",
+        "@actions/expressions": "^0.3.2",
+        "@actions/workflow-parser": "^0.3.2",
         "vscode-languageserver-textdocument": "^1.0.7",
         "vscode-languageserver-types": "^3.17.2",
         "vscode-uri": "^3.0.7",
@@ -6652,9 +6652,10 @@
       "license": "MIT"
     },
     "node_modules/http-cache-semantics": {
-      "version": "4.1.0",
-      "dev": true,
-      "license": "BSD-2-Clause"
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/http-cache-semantics/-/http-cache-semantics-4.1.1.tgz",
+      "integrity": "sha512-er295DKPVsV82j5kw1Gjt+ADA/XYHsajl82cGNQG2eyoPkvgUhX+nDIyelzhIWbbsXP39EHcI6l5tYs2FYqYXQ==",
+      "dev": true
     },
     "node_modules/http-proxy-agent": {
       "version": "5.0.0",
@@ -11719,10 +11720,10 @@
     },
     "workflow-parser": {
       "name": "@actions/workflow-parser",
-      "version": "0.3.1",
+      "version": "0.3.2",
       "license": "MIT",
       "dependencies": {
-        "@actions/expressions": "^0.3.1",
+        "@actions/expressions": "^0.3.2",
         "cronstrue": "^2.21.0",
         "yaml": "^2.0.0-8"
       },

workflow-parser/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/workflow-parser",
-  "version": "0.3.1",
+  "version": "0.3.2",
   "license": "MIT",
   "type": "module",
   "source": "./src/index.ts",
@@ -43,7 +43,7 @@
     "watch": "tsc --build tsconfig.build.json --watch"
   },
   "dependencies": {
-    "@actions/expressions": "^0.3.1",
+    "@actions/expressions": "^0.3.2",
     "cronstrue": "^2.21.0",
     "yaml": "^2.0.0-8"
   },

workflow-parser/src/model/converter/events.ts

@@ -59,17 +59,24 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
 
       // All other events are defined as mappings. During schema validation we already ensure that events
       // receive only known keys, so here we can focus on the values and whether they are valid.
+
       const eventToken = item.value.assertMapping(`event ${eventName}`);
+      if (eventName === "workflow_call") {
+        result.workflow_call = convertEventWorkflowCall(context, eventToken);
+        continue;
+      }
+
+      if (eventName === "workflow_dispatch") {
+        result.workflow_dispatch = convertEventWorkflowDispatchInputs(context, eventToken);
+        continue;
+      }
 
       result[eventName] = {
         ...convertPatternFilter("branches", eventToken),
         ...convertPatternFilter("tags", eventToken),
         ...convertPatternFilter("paths", eventToken),
         ...convertFilter("types", eventToken),
-        ...convertFilter("workflows", eventToken),
-        // workflow_call and workflow_dispatch share input parsing
-        ...convertEventWorkflowDispatchInputs(context, eventToken),
-        ...convertEventWorkflowCall(context, eventToken)
+        ...convertFilter("workflows", eventToken)
       };
     }
 

workflow-parser/src/model/converter/workflow-call.ts

@@ -1,7 +1,9 @@
 import {TemplateContext} from "../../templates/template-context";
 import {MappingToken, TemplateToken} from "../../templates/tokens";
 import {isMapping} from "../../templates/tokens/type-guards";
-import {SecretConfig, WorkflowCallConfig} from "../workflow-template";
+import {SecretConfig, WorkflowCallConfig, InputConfig, InputType} from "../workflow-template";
+import {convertStringList} from "./string-list";
+import {ScalarToken} from "../../templates/tokens/scalar-token";
 
 export function convertEventWorkflowCall(context: TemplateContext, token: MappingToken): WorkflowCallConfig {
   const result: WorkflowCallConfig = {};
@@ -11,7 +13,7 @@ export function convertEventWorkflowCall(context: TemplateContext, token: Mappin
 
     switch (key.value) {
       case "inputs":
-        // Ignore, these are handled by convertEventWorkflowDispatchInputs
+        result.inputs = convertWorkflowInputs(context, item.value.assertMapping("workflow dispatch inputs"));
         break;
 
       case "secrets":
@@ -27,6 +29,94 @@ export function convertEventWorkflowCall(context: TemplateContext, token: Mappin
   return result;
 }
 
+export function convertWorkflowInputs(
+  context: TemplateContext,
+  token: MappingToken
+): {
+  [inputName: string]: InputConfig;
+} {
+  const result: {[inputName: string]: InputConfig} = {};
+
+  for (const item of token) {
+    const inputName = item.key.assertString("input name");
+    const inputMapping = item.value.assertMapping("input configuration");
+
+    result[inputName.value] = convertWorkflowInput(context, inputMapping);
+  }
+
+  return result;
+}
+
+export function convertWorkflowInput(context: TemplateContext, token: MappingToken): InputConfig {
+  const result: InputConfig = {
+    type: InputType.string // Default to string
+  };
+
+  let defaultValue: undefined | ScalarToken;
+
+  for (const item of token) {
+    const key = item.key.assertString("workflow dispatch input key");
+
+    switch (key.value) {
+      case "description":
+        result.description = item.value.assertString("input description").value;
+        break;
+
+      case "required":
+        result.required = item.value.assertBoolean("input required").value;
+        break;
+
+      case "default":
+        defaultValue = item.value.assertScalar("input default");
+        break;
+
+      case "type":
+        result.type = InputType[item.value.assertString("input type").value as keyof typeof InputType];
+        break;
+
+      case "options":
+        result.options = convertStringList("input options", item.value.assertSequence("input options"));
+        break;
+
+      default:
+        context.error(item.key, `Invalid key '${key.value}'`);
+    }
+  }
+
+  // Validate default value
+  if (defaultValue !== undefined && !defaultValue.isExpression) {
+    try {
+      switch (result.type) {
+        case InputType.boolean:
+          result.default = defaultValue.assertBoolean("input default").value;
+
+          break;
+
+        case InputType.string:
+        case InputType.choice:
+        case InputType.environment:
+          result.default = defaultValue.assertString("input default").value;
+          break;
+      }
+    } catch (e) {
+      context.error(defaultValue, e);
+    }
+  }
+
+  // Validate `options` for `choice` type
+  if (result.type === InputType.choice) {
+    if (result.options === undefined || result.options.length === 0) {
+      context.error(token, "Missing 'options' for choice input");
+    }
+  } else {
+    if (result.options !== undefined) {
+      context.error(token, "Input type is not 'choice', but 'options' is defined");
+    }
+  }
+
+  return result;
+}
+
 function convertWorkflowCallSecrets(
   context: TemplateContext,
   token: MappingToken

workflow-parser/src/model/workflow-template.ts

@@ -158,7 +158,7 @@ export type WorkflowDispatchConfig = {
 };
 
 export type WorkflowCallConfig = {
-  inputs?: {[inputName: string]: InputConfig};
+  inputs?: {[inputName: string]: InputConfig & {default?: string | boolean | number | ScalarToken}};
   secrets?: {[secretName: string]: SecretConfig};
   // TODO - these are supported in C# and Go but not in TS yet
   // outputs: { [outputName: string]: OutputConfig }

workflow-parser/src/workflow-v1.0.json

@@ -576,7 +576,8 @@
     "merge-group-mapping": {
       "mapping": {
         "properties": {
-          "types": "merge-group-activity"
+          "types": "merge-group-activity",
+          "branches": "event-branches"
         }
       }
     },

workflow-parser/testdata/reader/events-mapping-all.yml

@@ -72,7 +72,11 @@ on:
       - edited
       - deleted
   merge_group:
-    types: checks_requested
+    branches: 
+        - master 
+        - main
+    types: 
+      - checks_requested
   milestone:
     types:
       - created
@@ -313,6 +317,10 @@ jobs:
       ]
     },
     "merge_group": {
+      "branches": [
+        "master",
+        "main"
+      ],
       "types": [
         "checks_requested"
       ]