From 0d647c1765c996cf3c477d2fc5aa4baf41d12135 Mon Sep 17 00:00:00 2001
From: Josh Gross <joshmgross@github.com>
Date: Tue, 14 Feb 2023 16:41:34 -0500
Subject: [PATCH] Move `ErrorPolicy` into template converter options

---
 actions-languageservice/src/complete.ts       | 13 ++++------
 actions-languageservice/src/document-links.ts |  4 +++-
 .../src/expression-hover/visitor.test.ts      |  4 +++-
 actions-languageservice/src/hover.ts          |  8 +++++--
 .../src/test-utils/test-workflow-context.ts   |  2 +-
 actions-languageservice/src/validate.ts       | 13 ++++------
 .../src/model/convert.test.ts                 | 24 ++++++++++++++-----
 actions-workflow-parser/src/model/convert.ts  | 18 +++++++++-----
 actions-workflow-parser/src/xlang.test.ts     |  1 -
 9 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts
index c12bacd..8050570 100644
--- a/actions-languageservice/src/complete.ts
+++ b/actions-languageservice/src/complete.ts
@@ -77,15 +77,10 @@ export async function complete(
   }
 
   const {token, keyToken, parent, path} = findToken(newPos, result.value);
-  const template = await convertWorkflowTemplate(
-    result.context,
-    result.value,
-    ErrorPolicy.TryConversion,
-    config?.fileProvider,
-    {
-      fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0
-    }
-  );
+  const template = await convertWorkflowTemplate(result.context, result.value, config?.fileProvider, {
+    fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
+    errorPolicy: ErrorPolicy.TryConversion
+  });
   const workflowContext = getWorkflowContext(textDocument.uri, template, path);
 
   // If we are inside an expression, take a different code-path. The workflow parser does not correctly create
diff --git a/actions-languageservice/src/document-links.ts b/actions-languageservice/src/document-links.ts
index a22d358..42ec015 100644
--- a/actions-languageservice/src/document-links.ts
+++ b/actions-languageservice/src/document-links.ts
@@ -19,7 +19,9 @@ export async function documentLinks(document: TextDocument): Promise<DocumentLin
     return [];
   }
 
-  const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+  const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+    errorPolicy: ErrorPolicy.TryConversion
+  });
 
   // Add links to referenced actions
   const actionLinks: DocumentLink[] = [];
diff --git a/actions-languageservice/src/expression-hover/visitor.test.ts b/actions-languageservice/src/expression-hover/visitor.test.ts
index cffefff..bc6b798 100644
--- a/actions-languageservice/src/expression-hover/visitor.test.ts
+++ b/actions-languageservice/src/expression-hover/visitor.test.ts
@@ -110,7 +110,9 @@ async function hoverExpression(input: string) {
     return undefined;
   }
 
-  const template = await convertWorkflowTemplate(result.context, result.value, ErrorPolicy.TryConversion);
+  const template = await convertWorkflowTemplate(result.context, result.value, undefined, {
+    errorPolicy: ErrorPolicy.TryConversion
+  });
   const workflowContext = getWorkflowContext(td.uri, template, []);
   const context = await getContext(allowedContext, contextProviderConfig, workflowContext, Mode.Completion);
 
diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts
index b36cf16..01affd3 100644
--- a/actions-languageservice/src/hover.ts
+++ b/actions-languageservice/src/hover.ts
@@ -53,7 +53,9 @@ export async function hover(document: TextDocument, position: Position, config?:
       const allowedContext = tokenDefinitionInfo.allowedContext || [];
       const {namedContexts, functions} = splitAllowedContext(allowedContext);
 
-      const template = await convertWorkflowTemplate(result.context, result.value, ErrorPolicy.TryConversion);
+      const template = await convertWorkflowTemplate(result.context, result.value, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion
+      });
       const workflowContext = getWorkflowContext(document.uri, template, tokenResult.path);
       const context = await getContext(namedContexts, config?.contextProviderConfig, workflowContext, Mode.Completion);
 
@@ -107,7 +109,9 @@ async function getDescription(
     return defaultDescription;
   }
 
-  const template = await convertWorkflowTemplate(result.context, result.value, ErrorPolicy.TryConversion);
+  const template = await convertWorkflowTemplate(result.context, result.value, undefined, {
+    errorPolicy: ErrorPolicy.TryConversion
+  });
   const workflowContext = getWorkflowContext(document.uri, template, path);
   const description = await config.descriptionProvider.getDescription(workflowContext, token, path);
   return description || defaultDescription;
diff --git a/actions-languageservice/src/test-utils/test-workflow-context.ts b/actions-languageservice/src/test-utils/test-workflow-context.ts
index e671ba2..86624d1 100644
--- a/actions-languageservice/src/test-utils/test-workflow-context.ts
+++ b/actions-languageservice/src/test-utils/test-workflow-context.ts
@@ -18,7 +18,7 @@ export async function testGetWorkflowContext(input: string): Promise<WorkflowCon
   let template: WorkflowTemplate | undefined;
 
   if (result.value) {
-    template = await convertWorkflowTemplate(result.context, result.value, undefined, testFileProvider, {
+    template = await convertWorkflowTemplate(result.context, result.value, testFileProvider, {
       fetchReusableWorkflowDepth: 1
     });
   }
diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts
index 544a163..b466642 100644
--- a/actions-languageservice/src/validate.ts
+++ b/actions-languageservice/src/validate.ts
@@ -57,15 +57,10 @@ export async function validate(textDocument: TextDocument, config?: ValidationCo
     const result: ParseWorkflowResult = parseWorkflow(file, nullTrace);
     if (result.value) {
       // Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information
-      const template = await convertWorkflowTemplate(
-        result.context,
-        result.value,
-        ErrorPolicy.TryConversion,
-        config?.fileProvider,
-        {
-          fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0
-        }
-      );
+      const template = await convertWorkflowTemplate(result.context, result.value, config?.fileProvider, {
+        fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
+        errorPolicy: ErrorPolicy.TryConversion
+      });
 
       // Validate expressions and value providers
       await additionalValidations(diagnostics, textDocument.uri, template, result.value, config);
diff --git a/actions-workflow-parser/src/model/convert.test.ts b/actions-workflow-parser/src/model/convert.test.ts
index 78b1f68..5fb1ec8 100644
--- a/actions-workflow-parser/src/model/convert.test.ts
+++ b/actions-workflow-parser/src/model/convert.test.ts
@@ -19,7 +19,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(serializeTemplate(template)).toEqual({
       events: {
@@ -59,7 +61,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(serializeTemplate(template)).toEqual({
       events: {
@@ -105,7 +109,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(serializeTemplate(template)).toEqual({
       errors: [
@@ -152,7 +158,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(serializeTemplate(template)).toEqual({
       errors: [
@@ -229,7 +237,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(template.jobs).not.toBeUndefined();
     expect(template.jobs).toHaveLength(1);
@@ -282,7 +292,9 @@ jobs:
       nullTrace
     );
 
-    const template = await convertWorkflowTemplate(result.context, result.value!, ErrorPolicy.TryConversion);
+    const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+      errorPolicy: ErrorPolicy.TryConversion
+    });
 
     expect(template.jobs).not.toBeUndefined();
     expect(template.jobs).toHaveLength(0);
diff --git a/actions-workflow-parser/src/model/convert.ts b/actions-workflow-parser/src/model/convert.ts
index 944ba75..47e9373 100644
--- a/actions-workflow-parser/src/model/convert.ts
+++ b/actions-workflow-parser/src/model/convert.ts
@@ -31,31 +31,36 @@ export type WorkflowTemplateConverterOptions = {
    * Default: 0
    */
   fetchReusableWorkflowDepth?: number;
+
+  /**
+   * The error policy to use when converting the workflow.
+   * By default, conversion will be skipped if there are errors in the {@link TemplateContext}.
+   */
+  errorPolicy?: ErrorPolicy;
 };
 
 const defaultOptions: Required<WorkflowTemplateConverterOptions> = {
   maxReusableWorkflowDepth: 4,
-  fetchReusableWorkflowDepth: 0
+  fetchReusableWorkflowDepth: 0,
+  errorPolicy: ErrorPolicy.ReturnErrorsOnly
 };
 
 export async function convertWorkflowTemplate(
   context: TemplateContext,
   root: TemplateToken,
-  errorPolicy: ErrorPolicy = ErrorPolicy.ReturnErrorsOnly,
   fileProvider?: FileProvider,
   options: WorkflowTemplateConverterOptions = defaultOptions
 ): Promise<WorkflowTemplate> {
   const result = {} as WorkflowTemplate;
+  const opts = getOptionsWithDefaults(options);
 
-  if (context.errors.getErrors().length > 0 && errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
+  if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
     result.errors = context.errors.getErrors().map(x => ({
       Message: x.message
     }));
     return result;
   }
 
-  const opts = getOptionsWithDefaults(options);
-
   if (fileProvider === undefined && opts.fetchReusableWorkflowDepth > 0) {
     context.error(root, new Error("A file provider is required to fetch reusable workflows"));
   }
@@ -136,6 +141,7 @@ function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Requ
     fetchReusableWorkflowDepth:
       options.fetchReusableWorkflowDepth !== undefined
         ? options.fetchReusableWorkflowDepth
-        : defaultOptions.fetchReusableWorkflowDepth
+        : defaultOptions.fetchReusableWorkflowDepth,
+    errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy
   };
 }
diff --git a/actions-workflow-parser/src/xlang.test.ts b/actions-workflow-parser/src/xlang.test.ts
index e9e2ab1..42a2b77 100644
--- a/actions-workflow-parser/src/xlang.test.ts
+++ b/actions-workflow-parser/src/xlang.test.ts
@@ -85,7 +85,6 @@ describe("x-lang tests", () => {
       const workflowTemplate = await convertWorkflowTemplate(
         parseResult.context,
         parseResult.value!,
-        undefined,
         testFileProvider,
         {
           fetchReusableWorkflowDepth: 1