Validate context expressions

languageserver/src/connection.ts

@@ -1,4 +1,4 @@
-import {documentLinks, documentSymbols, hover, validate, ValidationConfig} from "@actions/languageservice";
+import {documentLinks, hover, validate, ValidationConfig} from "@actions/languageservice";
 import {registerLogger, setLogLevel} from "@actions/languageservice/log";
 import {clearCache, clearCacheEntry} from "@actions/languageservice/utils/workflow-cache";
 import {Octokit} from "@octokit/rest";
@@ -7,8 +7,6 @@ import {
   Connection,
   DocumentLink,
   DocumentLinkParams,
-  DocumentSymbol,
-  DocumentSymbolParams,
   ExecuteCommandParams,
   Hover,
   HoverParams,
@@ -74,10 +72,6 @@ export function initConnection(connection: Connection) {
         hoverProvider: true,
         documentLinkProvider: {
           resolveProvider: false
-        },
-        documentSymbolProvider: {
-          label: "GitHub Actions",
-          workDoneProgress: false
         }
       }
     };
@@ -164,11 +158,6 @@ export function initConnection(connection: Connection) {
     return documentLinks(getDocument(documents, textDocument), repoContext?.workspaceUri);
   });
 
-  connection.onDocumentSymbol(async ({textDocument}: DocumentSymbolParams): Promise<DocumentSymbol[] | null> => {
-    const repoContext = repos.find(repo => textDocument.uri.startsWith(repo.workspaceUri));
-    return documentSymbols(getDocument(documents, textDocument), repoContext?.workspaceUri);
-  });
-
   // Make the text document manager listen on the connection
   // for open, change and close text document events
   documents.listen(connection);

languageservice/src/document-symbols.ts

@@ -1,70 +0,0 @@
-import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
-import {File} from "@actions/workflow-parser/workflows/file";
-import {TextDocument} from "vscode-languageserver-textdocument";
-import {DocumentSymbol, SymbolKind} from "vscode-languageserver-types";
-import {mapRange} from "./utils/range";
-import {fetchOrConvertWorkflowTemplate, fetchOrParseWorkflow} from "./utils/workflow-cache";
-
-export async function documentSymbols(
-  document: TextDocument,
-  workspace: string | undefined
-): Promise<DocumentSymbol[]> {
-  const file: File = {
-    name: document.uri,
-    content: document.getText()
-  };
-
-  const parsedWorkflow = fetchOrParseWorkflow(file, document.uri);
-  if (!parsedWorkflow?.value) {
-    return [];
-  }
-
-  const template = await fetchOrConvertWorkflowTemplate(
-    parsedWorkflow.context,
-    parsedWorkflow.value,
-    document.uri,
-    undefined,
-    {
-      errorPolicy: ErrorPolicy.TryConversion
-    }
-  );
-
-  if (!template) {
-    return [];
-  }
-
-  const symbols: DocumentSymbol[] = [];
-
-  const onSymbol = DocumentSymbol.create(
-    "`on` Triggers",
-    undefined,
-    SymbolKind.Key,
-    mapRange(template.events.range),
-    mapRange(template.events.range)
-  );
-  symbols.push(onSymbol);
-
-  const jobsSymbol = DocumentSymbol.create(
-    "Jobs",
-    undefined,
-    SymbolKind.Namespace,
-    mapRange(template.jobs?.[0].id.range),
-    mapRange(template.jobs?.[0].id.range),
-    []
-  );
-  symbols.push(jobsSymbol);
-
-  for (const job of template.jobs || []) {
-    const jobSymbol = DocumentSymbol.create(
-      `Job ${job.name?.toDisplayString() || job.id?.toString()}`,
-      "detail",
-      SymbolKind.Class,
-      mapRange(job.id.range),
-      mapRange(job.id.range)
-    );
-
-    jobsSymbol.children!.push(jobSymbol);
-  }
-
-  return symbols;
-}

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

@@ -1,44 +0,0 @@
-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/visitor.test.ts

@@ -0,0 +1,129 @@
+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

@@ -0,0 +1,148 @@
+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/index.ts

@@ -1,8 +1,7 @@
 export {complete} from "./complete";
 export {ContextProviderConfig} from "./context-providers/config";
 export {documentLinks} from "./document-links";
-export {documentSymbols} from "./document-symbols";
 export {hover} from "./hover";
-export {LogLevel, Logger, registerLogger, setLogLevel} from "./log";
-export {ActionsMetadataProvider, ValidationConfig, validate} from "./validate";
+export {Logger, LogLevel, registerLogger, setLogLevel} from "./log";
+export {validate, ValidationConfig, ActionsMetadataProvider} from "./validate";
 export {ValueProviderConfig, ValueProviderKind} from "./value-providers/config";

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";

languageservice/src/validate.ts

@@ -1,6 +1,6 @@
-import {Evaluator, ExpressionEvaluationError, Lexer, Parser} from "@actions/expressions";
+import {Lexer, Parser} from "@actions/expressions";
 import {Expr} from "@actions/expressions/ast";
-import {isBasicExpression, isString, ParseWorkflowResult, WorkflowTemplate} from "@actions/workflow-parser";
+import {ParseWorkflowResult, WorkflowTemplate, isBasicExpression, isString} from "@actions/workflow-parser";
 import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
 import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
 import {BasicExpressionToken} from "@actions/workflow-parser/templates/tokens/basic-expression-token";
@@ -13,10 +13,9 @@ import {TextDocument} from "vscode-languageserver-textdocument";
 import {Diagnostic, DiagnosticSeverity, URI} from "vscode-languageserver-types";
 import {ActionMetadata, ActionReference} from "./action";
 import {ContextProviderConfig} from "./context-providers/config";
-import {getContext, Mode} from "./context-providers/default";
-import {getWorkflowContext, WorkflowContext} from "./context/workflow-context";
-import {AccessError, wrapDictionary} from "./expression-validation/error-dictionary";
-import {validatorFunctions} from "./expression-validation/functions";
+import {Mode, getContext} from "./context-providers/default";
+import {WorkflowContext, getWorkflowContext} from "./context/workflow-context";
+import {ValidationVisitor} from "./expression-validation/visitor";
 import {error} from "./log";
 import {findToken} from "./utils/find-token";
 import {mapRange} from "./utils/range";
@@ -202,28 +201,17 @@ async function validateExpression(
       continue;
     }
 
-    try {
-      const context = await getContext(namedContexts, contextProviderConfig, workflowContext, Mode.Validation);
+    const context = await getContext(namedContexts, contextProviderConfig, workflowContext, Mode.Validation);
 
-      const e = new Evaluator(expr, wrapDictionary(context), validatorFunctions);
-      e.evaluate();
+    const e = new ValidationVisitor(expr, context);
+    e.validate();
 
-      // 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) {
-        diagnostics.push({
-          message: `Context access might be invalid: ${e.keyName}`,
-          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)
-        });
-      }
-    }
+    diagnostics.push(
+      ...e.errors.map(e => ({
+        message: e.message,
+        range: mapRange(expression.range),
+        severity: e.severity === "error" ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning
+      }))
+    );
   }
 }

workflow-parser/src/model/convert.ts

@@ -9,7 +9,7 @@ import {handleTemplateTokenErrors} from "./converter/handle-errors";
 import {convertJobs} from "./converter/jobs";
 import {convertReferencedWorkflow} from "./converter/referencedWorkflow";
 import {isReusableWorkflowJob} from "./type-guards";
-import {EventsConfig, WorkflowTemplate} from "./workflow-template";
+import {WorkflowTemplate} from "./workflow-template";
 
 export enum ErrorPolicy {
   ReturnErrorsOnly,
@@ -73,9 +73,7 @@ export async function convertWorkflowTemplate(
 
       switch (key.value) {
         case "on":
-          result.events = handleTemplateTokenErrors(root, context, {range: item.key.range} as EventsConfig, () =>
-            convertOn(context, item.value)
-          );
+          result.events = handleTemplateTokenErrors(root, context, {}, () => convertOn(context, item.value));
           break;
 
         case "jobs":

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

@@ -23,15 +23,12 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
     const event = token.assertString("on");
 
     return {
-      range: token.range,
       [event.value]: {}
     } as EventsConfig;
   }
 
   if (isSequence(token)) {
-    const result = {
-      range: token.range
-    } as EventsConfig;
+    const result = {} as EventsConfig;
 
     for (const item of token) {
       const event = item.assertString("on");
@@ -42,9 +39,7 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
   }
 
   if (isMapping(token)) {
-    const result = {
-      range: token.range
-    } as EventsConfig;
+    const result = {} as EventsConfig;
 
     for (const item of token) {
       const eventKey = item.key.assertString("event name");
@@ -82,9 +77,7 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
   }
 
   context.error(token, "Invalid format for 'on'");
-  return {
-    range: token.range
-  } as EventsConfig;
+  return {};
 }
 
 function convertPatternFilter<T extends BranchFilterConfig & TagFilterConfig & PathFilterConfig>(

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

@@ -6,7 +6,6 @@ import {
   StringToken,
   TemplateToken
 } from "../templates/tokens";
-import {TokenRange} from "../templates/tokens/token-range";
 
 export type WorkflowTemplate = {
   events: EventsConfig;
@@ -100,8 +99,6 @@ export type ActionStep = BaseStep & {
 };
 
 export type EventsConfig = {
-  range: TokenRange;
-
   schedule?: ScheduleConfig[];
   workflow_dispatch?: WorkflowDispatchConfig;
   workflow_call?: WorkflowCallConfig;