From f4dde16b61170f314bc538c79d1042bd89db5337 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Mon, 6 Feb 2023 11:15:15 -0500 Subject: [PATCH 1/2] Add a `FileProvider` to `parseWorkflow` --- .../src/test-utils/workflow-context.ts | 2 +- actions-languageservice/src/complete.ts | 2 +- .../src/context/workflow-context.test.ts | 11 ++-- actions-languageservice/src/document-links.ts | 2 +- .../expression-hover/expression-pos.test.ts | 2 +- .../src/expression-hover/visitor.test.ts | 2 +- actions-languageservice/src/hover.ts | 2 +- .../src/utils/find-token.test.ts | 11 ++-- actions-languageservice/src/validate.ts | 2 +- .../src/workflow-provider.ts | 5 -- .../src/expressions.test.ts | 55 ++++++---------- actions-workflow-parser/src/index.test.ts | 46 +++++-------- .../src/model/convert.test.ts | 66 +++++++------------ .../templates/tokens/template-token.test.ts | 11 ++-- .../src/workflows/file-provider.ts | 3 + .../src/workflows/workflow-parser.test.ts | 2 +- .../src/workflows/workflow-parser.ts | 11 ++-- .../src/workflows/yaml-object-reader.test.ts | 11 ++-- actions-workflow-parser/src/xlang.test.ts | 11 ++-- 19 files changed, 96 insertions(+), 161 deletions(-) delete mode 100644 actions-languageservice/src/workflow-provider.ts create mode 100644 actions-workflow-parser/src/workflows/file-provider.ts diff --git a/actions-languageserver/src/test-utils/workflow-context.ts b/actions-languageserver/src/test-utils/workflow-context.ts index 88eb740..bd0ee97 100644 --- a/actions-languageserver/src/test-utils/workflow-context.ts +++ b/actions-languageserver/src/test-utils/workflow-context.ts @@ -9,7 +9,7 @@ const nullTrace: TraceWriter = { }; export function createWorkflowContext(workflow: string, job?: string, stepIndex?: number): WorkflowContext { - const parsed = parseWorkflow("test.yaml", [{name: "test.yaml", content: workflow}], nullTrace); + const parsed = parseWorkflow({name: "test.yaml", content: workflow}, nullTrace); if (!parsed.value) { throw new Error("Failed to parse workflow"); } diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index a028bd1..3047d63 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -65,7 +65,7 @@ export async function complete( name: textDocument.uri, content: newDoc.getText() }; - const result = parseWorkflow(file.name, [file], nullTrace); + const result = parseWorkflow(file, nullTrace); if (!result.value) { return []; } diff --git a/actions-languageservice/src/context/workflow-context.test.ts b/actions-languageservice/src/context/workflow-context.test.ts index 4a4d162..c336f3c 100644 --- a/actions-languageservice/src/context/workflow-context.test.ts +++ b/actions-languageservice/src/context/workflow-context.test.ts @@ -8,13 +8,10 @@ import {getWorkflowContext, WorkflowContext} from "./workflow-context"; function testGetWorkflowContext(input: string): WorkflowContext { const [textDocument, pos] = getPositionFromCursor(input); const result = parseWorkflow( - "wf.yaml", - [ - { - content: textDocument.getText(), - name: "wf.yaml" - } - ], + { + content: textDocument.getText(), + name: "wf.yaml" + }, nullTrace ); diff --git a/actions-languageservice/src/document-links.ts b/actions-languageservice/src/document-links.ts index cd739c0..625498d 100644 --- a/actions-languageservice/src/document-links.ts +++ b/actions-languageservice/src/document-links.ts @@ -14,7 +14,7 @@ export async function documentLinks(document: TextDocument): Promise Promise; -}; diff --git a/actions-workflow-parser/src/expressions.test.ts b/actions-workflow-parser/src/expressions.test.ts index c8f1664..1e46282 100644 --- a/actions-workflow-parser/src/expressions.test.ts +++ b/actions-workflow-parser/src/expressions.test.ts @@ -6,19 +6,16 @@ import {parseWorkflow} from "./workflows/workflow-parser"; describe("Workflow Expression Parsing", () => { it("preserves original expressions when building format", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push run-name: Test \${{ github.event_name }} \${{ github.ref }} jobs: build: runs-on: ubuntu-latest steps: - run: echo 'hello'` - } - ], + }, nullTrace ); @@ -44,11 +41,9 @@ jobs: it("preserves original expressions when building format for multi-line strings", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest @@ -56,8 +51,7 @@ jobs: - run: | echo \${{ github.event_name }} echo 'hello' \${{github.ref }}` - } - ], + }, nullTrace ); @@ -109,11 +103,9 @@ jobs: it("return errors and string token with preserved expressions for (multiple) expression errors", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest @@ -121,8 +113,7 @@ jobs: - run: | echo \${{ abc }} echo 'hello' \${{ gith }}` - } - ], + }, nullTrace ); @@ -147,11 +138,9 @@ jobs: it("reports all errors for multi-line expressions at the correct locations", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest @@ -159,8 +148,7 @@ jobs: - run: | echo \${{ fromJSON2('test') }} echo 'hello' \${{ toJSON2(inputs.test) }}` - } - ], + }, nullTrace ); @@ -186,19 +174,16 @@ jobs: it("parses isExpression strings into expression tokens", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest if: github.event_name == 'push' steps: - run: echo 'hello'` - } - ], + }, nullTrace ); diff --git a/actions-workflow-parser/src/index.test.ts b/actions-workflow-parser/src/index.test.ts index b316089..c1e2372 100644 --- a/actions-workflow-parser/src/index.test.ts +++ b/actions-workflow-parser/src/index.test.ts @@ -5,13 +5,10 @@ import {parseWorkflow} from "./workflows/workflow-parser"; describe("parseWorkflow", () => { it("parses valid workflow", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: "on: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo 'hello'" - } - ], + { + name: "test.yaml", + content: "on: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo 'hello'" + }, nullTrace ); @@ -20,13 +17,10 @@ describe("parseWorkflow", () => { it("contains range for error", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: "on: push\njobs:\n build:\n steps:\n - run: echo 'hello'" - } - ], + { + name: "test.yaml", + content: "on: push\njobs:\n build:\n steps:\n - run: echo 'hello'" + }, nullTrace ); @@ -40,19 +34,16 @@ describe("parseWorkflow", () => { it("error range for expression is constrained to scalar node", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: `on: push + { + name: "test.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest steps: - name: \${{ github.event = 12 }} run: echo 'hello'` - } - ], + }, nullTrace ); @@ -71,14 +62,11 @@ jobs: it("tokens contain descriptions", () => { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: - "on: push\nname: hello\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo 'hello'" - } - ], + { + name: "test.yaml", + content: + "on: push\nname: hello\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo 'hello'" + }, nullTrace ); diff --git a/actions-workflow-parser/src/model/convert.test.ts b/actions-workflow-parser/src/model/convert.test.ts index fa75ae8..93db6be 100644 --- a/actions-workflow-parser/src/model/convert.test.ts +++ b/actions-workflow-parser/src/model/convert.test.ts @@ -9,16 +9,13 @@ function serializeTemplate(template: unknown): unknown { describe("convertWorkflowTemplate", () => { it("converts workflow with one job", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push + { + name: "wf.yaml", + content: `on: push jobs: build: runs-on: ubuntu-latest` - } - ], + }, nullTrace ); @@ -48,11 +45,9 @@ jobs: it("converts workflow if expressions", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push + { + name: "wf.yaml", + content: `on: push jobs: build: if: \${{ true }} @@ -60,8 +55,7 @@ jobs: deploy: if: true runs-on: ubuntu-latest` - } - ], + }, nullTrace ); @@ -100,17 +94,14 @@ jobs: it("converts workflow with empty needs", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push + { + name: "wf.yaml", + content: `on: push jobs: build: needs: # comment to preserve whitespace in test runs-on: ubuntu-latest` - } - ], + }, nullTrace ); @@ -145,11 +136,9 @@ jobs: it("converts workflow with needs errors", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push + { + name: "wf.yaml", + content: `on: push jobs: job1: needs: [unknown-job, job3] @@ -159,8 +148,7 @@ jobs: job3: needs: job1 runs-on: ubuntu-latest` - } - ], + }, nullTrace ); @@ -225,11 +213,9 @@ jobs: it("converts workflow with invalid on", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: + { + name: "wf.yaml", + content: `on: workflow_dispatch: inputs: test: @@ -239,8 +225,7 @@ jobs: runs-on: ubuntu-latest steps: - run: echo hello` - } - ], + }, nullTrace ); @@ -288,15 +273,12 @@ jobs: it("converts workflow with invalid jobs", () => { const result = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push + { + name: "wf.yaml", + content: `on: push jobs: build:` - } - ], + }, nullTrace ); diff --git a/actions-workflow-parser/src/templates/tokens/template-token.test.ts b/actions-workflow-parser/src/templates/tokens/template-token.test.ts index 520eb28..34afdab 100644 --- a/actions-workflow-parser/src/templates/tokens/template-token.test.ts +++ b/actions-workflow-parser/src/templates/tokens/template-token.test.ts @@ -6,13 +6,10 @@ import {TemplateToken} from "./template-token"; describe("traverse", () => { it("returns parent token and key", () => { const workflow = parseWorkflow( - "wf.yaml", - [ - { - name: "wf.yaml", - content: `on: push` - } - ], + { + name: "wf.yaml", + content: `on: push` + }, nullTrace ); diff --git a/actions-workflow-parser/src/workflows/file-provider.ts b/actions-workflow-parser/src/workflows/file-provider.ts new file mode 100644 index 0000000..8416915 --- /dev/null +++ b/actions-workflow-parser/src/workflows/file-provider.ts @@ -0,0 +1,3 @@ +export interface FileProvider { + getFileContent(path: string): Promise; +} diff --git a/actions-workflow-parser/src/workflows/workflow-parser.test.ts b/actions-workflow-parser/src/workflows/workflow-parser.test.ts index 021ac2b..a4d587a 100644 --- a/actions-workflow-parser/src/workflows/workflow-parser.test.ts +++ b/actions-workflow-parser/src/workflows/workflow-parser.test.ts @@ -11,7 +11,7 @@ jobs: - name: 'Hello \${{ fromJSON('test') == inputs.name }}' run: echo Hello, world!`; - const result = parseWorkflow("main.yaml", [{name: "main.yaml", content: content}], nullTrace); + const result = parseWorkflow({name: "main.yaml", content: content}, nullTrace); expect(result.context.errors.count).toBe(1); expect(result.value).toBeUndefined(); diff --git a/actions-workflow-parser/src/workflows/workflow-parser.ts b/actions-workflow-parser/src/workflows/workflow-parser.ts index a49c903..f291717 100644 --- a/actions-workflow-parser/src/workflows/workflow-parser.ts +++ b/actions-workflow-parser/src/workflows/workflow-parser.ts @@ -3,6 +3,7 @@ import * as templateReader from "../templates/template-reader"; import {TemplateToken} from "../templates/tokens/template-token"; import {TraceWriter} from "../templates/trace-writer"; import {File} from "./file"; +import {FileProvider} from "./file-provider"; import {WORKFLOW_ROOT} from "./workflow-constants"; import {getWorkflowSchema} from "./workflow-schema"; import {YamlObjectReader} from "./yaml-object-reader"; @@ -11,15 +12,11 @@ export interface ParseWorkflowResult { value: TemplateToken | undefined; } -export function parseWorkflow(entryFileName: string, files: File[], trace: TraceWriter): ParseWorkflowResult { +export function parseWorkflow(entryFile: File, trace: TraceWriter, fileProvider?: FileProvider): ParseWorkflowResult { const context = new TemplateContext(new TemplateValidationErrors(), getWorkflowSchema(), trace); - // Add file ids - files.forEach(x => context.getFileId(x.name)); - - const fileId = context.getFileId(entryFileName); - const fileContent = files[fileId - 1].content; - const reader = new YamlObjectReader(context, fileId, fileContent); + const fileId = context.getFileId(entryFile.name); + const reader = new YamlObjectReader(context, fileId, entryFile.content); if (context.errors.count > 0) { // The file is not valid YAML, template errors could be misleading return { diff --git a/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts b/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts index 4ff3b13..d4d4aac 100644 --- a/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts +++ b/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts @@ -81,13 +81,10 @@ it("YAML errors include range information", () => { function parseAsWorkflow(content: string): TemplateToken | undefined { const result = parseWorkflow( - "test.yaml", - [ - { - name: "test.yaml", - content: content - } - ], + { + name: "test.yaml", + content: content + }, nullTrace ); diff --git a/actions-workflow-parser/src/xlang.test.ts b/actions-workflow-parser/src/xlang.test.ts index 906ed63..a52fcf6 100644 --- a/actions-workflow-parser/src/xlang.test.ts +++ b/actions-workflow-parser/src/xlang.test.ts @@ -53,13 +53,10 @@ describe("x-lang tests", () => { } const parseResult = parseWorkflow( - testFileName, - [ - { - name: testFileName, - content: testInput - } - ], + { + name: testFileName, + content: testInput + }, nullTrace ); From b9a01ed5aad01b1a2c7e8748af05d5156e72bf10 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Mon, 6 Feb 2023 12:19:52 -0500 Subject: [PATCH 2/2] Remove undefined return type from `getFileContent` --- actions-workflow-parser/src/workflows/file-provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions-workflow-parser/src/workflows/file-provider.ts b/actions-workflow-parser/src/workflows/file-provider.ts index 8416915..0bdc01d 100644 --- a/actions-workflow-parser/src/workflows/file-provider.ts +++ b/actions-workflow-parser/src/workflows/file-provider.ts @@ -1,3 +1,3 @@ export interface FileProvider { - getFileContent(path: string): Promise; + getFileContent(path: string): Promise; }