Expression validation

actions-languageservice/src/complete.expressions.test.ts

@@ -4,7 +4,7 @@ import {ContextProviderConfig} from "./context-providers/config";
 import {getPositionFromCursor} from "./test-utils/cursor-position";
 
 const contextProviderConfig: ContextProviderConfig = {
-  getContext: async (context: string) => {
+  getContext: (context: string) => {
     switch (context) {
       case "github":
         return new data.Dictionary({

actions-languageservice/src/context-providers/config.ts

@@ -3,7 +3,7 @@ import {Dictionary} from "@github/actions-expressions/data/dictionary";
 import {ExpressionData, Pair} from "@github/actions-expressions/data/expressiondata";
 
 export type ContextProviderConfig = {
-  getContext: (name: string) => Promise<data.Dictionary | undefined>;
+  getContext: (name: string) => data.Dictionary | undefined;
 };
 
 /**

actions-languageservice/src/context-providers/default.ts

@@ -1,16 +1,16 @@
 import {data} from "@github/actions-expressions";
 import {ContextProviderConfig} from "./config";
 
-export async function getContext(names: string[], config: ContextProviderConfig | undefined): Promise<data.Dictionary> {
+export function getContext(names: string[], config: ContextProviderConfig | undefined): data.Dictionary {
   const context = new data.Dictionary();
 
   for (const contextName of names) {
     let value: data.Dictionary | undefined;
 
-    value = await getDefaultContext(contextName);
+    value = getDefaultContext(contextName);
 
     if (!value) {
-      value = await config?.getContext(contextName);
+      value = config?.getContext(contextName);
     }
 
     if (!value) {
@@ -23,7 +23,7 @@ export async function getContext(names: string[], config: ContextProviderConfig
   return context;
 }
 
-async function getDefaultContext(name: string): Promise<data.Dictionary | undefined> {
+function getDefaultContext(name: string): data.Dictionary | undefined {
   switch (name) {
     case "runner":
       return objectToDictionary({

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

@@ -0,0 +1,38 @@
+import {data} from "@github/actions-expressions";
+import {isDictionary} from "@github/actions-expressions/data/dictionary";
+import {ExpressionData, Pair} from "@github/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);
+  }
+
+  get(key: string): ExpressionData | undefined {
+    const value = super.get(key);
+    if (value) {
+      return value;
+    }
+
+    throw new AccessError(`Invalid context access: ${key}`, key);
+  }
+}
+
+export function wrapDictionary(d: data.Dictionary): ErrorDictionary {
+  const e = new ErrorDictionary();
+
+  for (const {key, value} of d.pairs()) {
+    if (isDictionary(value)) {
+      e.add(key, wrapDictionary(value));
+    } else {
+      e.add(key, value);
+    }
+  }
+
+  return e;
+}

actions-languageservice/src/test-utils/cursor-position.ts

@@ -1,9 +1,10 @@
-import {TextDocument, Position} from "vscode-languageserver-textdocument";
+import {Position, TextDocument} from "vscode-languageserver-textdocument";
+import {createDocument} from "./document";
 
 // Calculates the position of the cursor and the document without that cursor
 // Cursor is represented by a `|` character
 export function getPositionFromCursor(input: string): [TextDocument, Position] {
-  const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input);
+  const doc = createDocument("test.yaml", input);
 
   const cursorIndex = doc.getText().indexOf("|");
   if (cursorIndex === -1) {

actions-languageservice/src/test-utils/document.ts

@@ -0,0 +1,5 @@
+import {TextDocument} from "vscode-languageserver-textdocument";
+
+export function createDocument(fileName: string, content: string): TextDocument {
+  return TextDocument.create("test://test/" + fileName, "yaml", 0, content);
+}

actions-languageservice/src/validate.expressions.test.ts

@@ -0,0 +1,57 @@
+import {DiagnosticSeverity} from "vscode-languageserver-types";
+import {createDocument} from "./test-utils/document";
+import {validate} from "./validate";
+
+describe("expression validation", () => {
+  it("access invalid context field", async () => {
+    const result = await validate(
+      createDocument(
+        "wf.yaml",
+        "on: push\nrun-name: name-${{ github.does-not-exist }}\njobs:\n  build:\n    runs-on: ubuntu-latest\n    steps:\n    - run: echo"
+      )
+    );
+
+    expect(result).toEqual([
+      {
+        message: "Context might be invalid: does-not-exist",
+        range: {
+          end: {
+            character: 43,
+            line: 1
+          },
+          start: {
+            character: 10,
+            line: 1
+          }
+        },
+        severity: DiagnosticSeverity.Warning
+      }
+    ]);
+  });
+
+  it("access invalid nested context field", async () => {
+    const result = await validate(
+      createDocument(
+        "wf.yaml",
+        "on: push\nrun-name: name-${{ github.does-not-exist.again }}\njobs:\n  build:\n    runs-on: ubuntu-latest\n    steps:\n    - run: echo"
+      )
+    );
+
+    expect(result).toEqual([
+      {
+        message: "Context might be invalid: does-not-exist",
+        range: {
+          end: {
+            character: 49,
+            line: 1
+          },
+          start: {
+            character: 10,
+            line: 1
+          }
+        },
+        severity: DiagnosticSeverity.Warning
+      }
+    ]);
+  });
+});

actions-languageservice/src/validate.test.ts

@@ -1,68 +1,60 @@
-import {parseWorkflow} from "@github/actions-workflow-parser";
-import {TemplateValidationError} from "@github/actions-workflow-parser/templates/template-validation-error";
-import {nullTrace} from "./nulltrace";
+import {Diagnostic} from "vscode-languageserver-types";
+import {createDocument} from "./test-utils/document";
+import {validate} from "./validate";
 
 describe("validation", () => {
-  it("valid workflow", () => {
-    const result = parseWorkflow(
-      "wf.yaml",
-      [
-        {
-          name: "wf.yaml",
-          content: "on: push\njobs:\n  build:\n    runs-on: ubuntu-latest"
-        }
-      ],
-      nullTrace
-    );
+  it("valid workflow", async () => {
+    const result = await validate(createDocument("wf.yaml", "on: push\njobs:\n  build:\n    runs-on: ubuntu-latest"));
 
-    expect(result.context.errors.getErrors().length).toBe(0);
+    expect(result.length).toBe(0);
   });
 
-  it("missing jobs key", () => {
-    const result = parseWorkflow(
-      "wf.yaml",
-      [
-        {
-          name: "wf.yaml",
-          content: "on: push"
-        }
-      ],
-      nullTrace
-    );
+  it("missing jobs key", async () => {
+    const result = await validate(createDocument("wf.yaml", "on: push"));
 
-    expect(result.context.errors.getErrors().length).toBe(1);
-    expect(result.context.errors.getErrors()[0]).toEqual(
-      new TemplateValidationError("Required property is missing: jobs", "wf.yaml (Line: 1, Col: 1)", undefined, {
-        start: [1, 1],
-        end: [1, 9]
-      })
-    );
+    expect(result.length).toBe(1);
+    expect(result[0]).toEqual({
+      message: "Required property is missing: jobs",
+      range: {
+        start: {
+          line: 0,
+          character: 0
+        },
+        end: {
+          line: 0,
+          character: 8
+        }
+      }
+    } as Diagnostic);
   });
 
-  it("extraneous key", () => {
-    const result = parseWorkflow(
-      "wf.yaml",
-      [
-        {
-          name: "wf.yaml",
-          content: `on: push
+  it("extraneous key", async () => {
+    const result = await validate(
+      createDocument(
+        "wf.yaml",
+        `on: push
 unknown-key: foo
 jobs:
   build:
     runs-on: ubuntu-latest
     steps:
     - run: echo`
-        }
-      ],
-      nullTrace
+      )
     );
 
-    expect(result.context.errors.getErrors().length).toBe(1);
-    expect(result.context.errors.getErrors()[0]).toEqual(
-      new TemplateValidationError("Unexpected value 'unknown-key'", "wf.yaml (Line: 2, Col: 1)", undefined, {
-        start: [2, 1],
-        end: [2, 12]
-      })
-    );
+    expect(result.length).toBe(1);
+    expect(result[0]).toEqual({
+      message: "Unexpected value 'unknown-key'",
+      range: {
+        end: {
+          character: 11,
+          line: 1
+        },
+        start: {
+          character: 0,
+          line: 1
+        }
+      }
+    } as Diagnostic);
   });
 });

actions-languageservice/src/validate.ts

@@ -1,9 +1,23 @@
-import {convertWorkflowTemplate, parseWorkflow, ParseWorkflowResult} from "@github/actions-workflow-parser";
+import {Evaluator, Lexer, Parser} from "@github/actions-expressions";
+import {Expr} from "@github/actions-expressions/ast";
+import {
+  convertWorkflowTemplate,
+  isBasicExpression,
+  parseWorkflow,
+  ParseWorkflowResult
+} from "@github/actions-workflow-parser";
+import {splitAllowedContext} from "@github/actions-workflow-parser/templates/tokens/expression-token";
+import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token";
 import {TokenRange} from "@github/actions-workflow-parser/templates/tokens/token-range";
 import {File} from "@github/actions-workflow-parser/workflows/file";
 import {TextDocument} from "vscode-languageserver-textdocument";
-import {Diagnostic, Range} from "vscode-languageserver-types";
+import {Diagnostic, DiagnosticSeverity, Range} from "vscode-languageserver-types";
+
+import {ContextProviderConfig} from "./context-providers/config";
+import {getContext} from "./context-providers/default";
+import {AccessError, wrapDictionary} from "./expression-validation/error-dictionary";
 import {nullTrace} from "./nulltrace";
+import {ValueProviderConfig} from "./value-providers/config";
 
 /**
  * Validates a workflow file
@@ -12,53 +26,56 @@ import {nullTrace} from "./nulltrace";
  * @returns Array of diagnostics
  */
 export async function validate(
-  textDocument: TextDocument
+  textDocument: TextDocument,
   // TODO: Support multiple files, context for API calls
+  valueProviderConfig?: ValueProviderConfig,
+  contextProviderConfig?: ContextProviderConfig
 ): Promise<Diagnostic[]> {
   const file: File = {
     name: textDocument.uri,
     content: textDocument.getText()
   };
 
+  const diagnostics: Diagnostic[] = [];
+
   try {
     const result: ParseWorkflowResult = parseWorkflow(file.name, [file], nullTrace);
-
     if (result.value) {
       // Errors will be updated in the context
       convertWorkflowTemplate(result.context, result.value);
     }
 
-    // For now map parser errors directly to diagnostics
-    return result.context.errors.getErrors().map(error => {
-      let range = mapRange(error.range);
-      if (!range) {
-        // Use default range
-        range = {
-          start: {
-            line: 1,
-            character: 1
-          },
-          end: {
-            line: 1,
-            character: 1
-          }
-        };
-      }
+    // Validate expressions
+    validateExpressions(diagnostics, result, contextProviderConfig);
 
-      return {
+    // For now map parser errors directly to diagnostics
+    for (const error of result.context.errors.getErrors()) {
+      let range = mapRange(error.range);
+
+      diagnostics.push({
         message: error.rawMessage,
         range
-      };
-    });
+      });
+    }
   } catch (e) {
     // TODO: Handle error here
-    return [];
   }
+
+  return diagnostics;
 }
 
-function mapRange(range: TokenRange | undefined): Range | undefined {
+function mapRange(range: TokenRange | undefined): Range {
   if (!range) {
-    return undefined;
+    return {
+      start: {
+        line: 1,
+        character: 1
+      },
+      end: {
+        line: 1,
+        character: 1
+      }
+    };
   }
 
   return {
@@ -72,3 +89,57 @@ function mapRange(range: TokenRange | undefined): Range | undefined {
     }
   };
 }
+
+function validateExpressions(
+  diagnotics: Diagnostic[],
+  result: ParseWorkflowResult,
+  contextProviderConfig: ContextProviderConfig | undefined
+) {
+  if (!result.value) {
+    return;
+  }
+
+  // Iterate over the parsed workflow
+  for (const token of TemplateToken.traverse(result.value)) {
+    if (isBasicExpression(token)) {
+      // Validate the expression
+      for (const expression of token.originalExpressions || [token]) {
+        const allowedContexts = token.definition?.readerContext || [];
+        const {namedContexts, functions} = splitAllowedContext(allowedContexts);
+
+        let expr: Expr | undefined;
+
+        try {
+          const l = new Lexer(expression.expression);
+          const lr = l.lex();
+
+          const p = new Parser(lr.tokens, namedContexts, functions);
+          expr = p.parse();
+        } catch {
+          // Ignore any error here, we should've caught this earlier in the parsing process
+          continue;
+        }
+
+        try {
+          const context = getContext(namedContexts, contextProviderConfig);
+
+          const e = new Evaluator(expr, wrapDictionary(context));
+          e.evaluate();
+
+          // Any invalid context access would've thrown an error via the `ErrorDictionary`, for now we don't have to check the actual
+          // result of the evaluation.
+        } catch (e) {
+          if (e instanceof AccessError) {
+            diagnotics.push({
+              message: `Context access might be invalid: ${e.keyName}`,
+              severity: DiagnosticSeverity.Warning,
+              range: mapRange(expression.range)
+            });
+          } else {
+            // Ignore error
+          }
+        }
+      }
+    }
+  }
+}