From 009f4e83bf50b3b25686a09827d5b6d08b7df208 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Mon, 5 Dec 2022 16:50:39 -0500 Subject: [PATCH 1/2] Support `steps` context completion and validation --- .../src/complete.expressions.test.ts | 74 ++++++++++++ .../src/context-providers/default.ts | 4 + .../src/context-providers/needs.ts | 1 - .../src/context-providers/steps.ts | 46 ++++++++ .../src/context/workflow-context.test.ts | 43 ++++++- .../src/context/workflow-context.ts | 60 ++++++++-- .../src/validate.expressions.test.ts | 106 ++++++++++++++++++ 7 files changed, 320 insertions(+), 14 deletions(-) create mode 100644 actions-languageservice/src/context-providers/steps.ts diff --git a/actions-languageservice/src/complete.expressions.test.ts b/actions-languageservice/src/complete.expressions.test.ts index c0b4167..67ac358 100644 --- a/actions-languageservice/src/complete.expressions.test.ts +++ b/actions-languageservice/src/complete.expressions.test.ts @@ -240,4 +240,78 @@ jobs: expect(result.map(x => x.label)).toEqual(["another-name", "name"]); }); + + describe("steps context", () => { + it("includes defined step IDs", async () => { + const input = ` + on: push + jobs: + one: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo hello b + - id: c + run: echo "hello \${{ steps.| + `; + const result = await complete(...getPositionFromCursor(input), undefined, contextProviderConfig); + + expect(result.map(x => x.label)).toEqual(["a", "b"]); + }); + + it("step.", async () => { + const input = ` + on: push + jobs: + one: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - run: echo "hello \${{ steps.a.| + `; + const result = await complete(...getPositionFromCursor(input), undefined, contextProviderConfig); + + expect(result.map(x => x.label)).toEqual(["conclusion", "outcome", "outputs"]); + }); + + it("ignores IDs from later steps", async () => { + const input = ` + on: push + jobs: + one: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo "hello \${{ steps.| + - id: c + run: echo hello c + `; + const result = await complete(...getPositionFromCursor(input), undefined, contextProviderConfig); + + expect(result.map(x => x.label)).toEqual(["a"]); + }); + }); + + // The workflow parser doesn't currently generate IDs, so we can't test this yet + it.failing("Ignores generated IDs", async () => { + const input = ` +on: push +jobs: + one: + runs-on: ubuntu-latest + steps: + - run: echo hello a + - id: b + run: echo hello b + - run: echo "hello \${{ steps.| +`; + const result = await complete(...getPositionFromCursor(input), undefined, contextProviderConfig); + + expect(result.map(x => x.label)).toEqual(["b"]); + }); }); diff --git a/actions-languageservice/src/context-providers/default.ts b/actions-languageservice/src/context-providers/default.ts index bb0b28d..d325592 100644 --- a/actions-languageservice/src/context-providers/default.ts +++ b/actions-languageservice/src/context-providers/default.ts @@ -3,6 +3,7 @@ import {WorkflowContext} from "../context/workflow-context"; import {ContextProviderConfig} from "./config"; import {getInputsContext} from "./inputs"; import {getNeedsContext} from "./needs"; +import {getStepsContext} from "./steps"; export async function getContext( names: string[], @@ -46,6 +47,9 @@ async function getDefaultContext(name: string, workflowContext: WorkflowContext) case "inputs": return getInputsContext(workflowContext); + + case "steps": + return getStepsContext(workflowContext); } return undefined; diff --git a/actions-languageservice/src/context-providers/needs.ts b/actions-languageservice/src/context-providers/needs.ts index 5ee2a2e..559aa94 100644 --- a/actions-languageservice/src/context-providers/needs.ts +++ b/actions-languageservice/src/context-providers/needs.ts @@ -21,7 +21,6 @@ function needsJobContext(job?: Job): data.Dictionary { // https://docs.github.com/en/actions/learn-github-actions/contexts#needs-context const d = new data.Dictionary(); - // Once job config contains outputs, this can be populated d.add("outputs", jobOutputs(job)); // Can be "success", "failure", "cancelled", or "skipped" diff --git a/actions-languageservice/src/context-providers/steps.ts b/actions-languageservice/src/context-providers/steps.ts new file mode 100644 index 0000000..beb7217 --- /dev/null +++ b/actions-languageservice/src/context-providers/steps.ts @@ -0,0 +1,46 @@ +import {data} from "@github/actions-expressions"; +import {Step} from "@github/actions-workflow-parser/model/workflow-template"; +import {WorkflowContext} from "../context/workflow-context"; + +export function getStepsContext(workflowContext: WorkflowContext): data.Dictionary { + const d = new data.Dictionary(); + if (!workflowContext.job?.steps) { + return d; + } + + const currentStep = workflowContext.step?.id; + + for (const step of workflowContext.job.steps) { + // We can't reference context from the current step or later steps + if (currentStep && step.id === currentStep) { + break; + } + + if (isGenerated(step)) { + continue; + } + + d.add(step.id, stepContext()); + } + + return d; +} + +function stepContext(): data.Dictionary { + // https://docs.github.com/en/actions/learn-github-actions/contexts#steps-context + const d = new data.Dictionary(); + + d.add("outputs", new data.Null()); + + // Can be "success", "failure", "cancelled", or "skipped" + d.add("conclusion", new data.Null()); + d.add("outcome", new data.Null()); + + return d; +} + +function isGenerated(step: Step): boolean { + // Steps need to explicitly set an ID to be referenced in the context + // Generated IDs always start with "__", which is not allowed by user-defined IDs + return step.id.startsWith("__"); +} diff --git a/actions-languageservice/src/context/workflow-context.test.ts b/actions-languageservice/src/context/workflow-context.test.ts index ab480ff..4a4d162 100644 --- a/actions-languageservice/src/context/workflow-context.test.ts +++ b/actions-languageservice/src/context/workflow-context.test.ts @@ -1,10 +1,11 @@ import {convertWorkflowTemplate, parseWorkflow, WorkflowTemplate} from "@github/actions-workflow-parser"; +import {ActionStep, RunStep} from "@github/actions-workflow-parser/model/workflow-template"; import {nullTrace} from "../nulltrace"; import {getPositionFromCursor} from "../test-utils/cursor-position"; import {findToken} from "../utils/find-token"; import {getWorkflowContext, WorkflowContext} from "./workflow-context"; -function testGetWorkflowContext(input: string): [context: WorkflowContext, template?: WorkflowTemplate] { +function testGetWorkflowContext(input: string): WorkflowContext { const [textDocument, pos] = getPositionFromCursor(input); const result = parseWorkflow( "wf.yaml", @@ -25,12 +26,12 @@ function testGetWorkflowContext(input: string): [context: WorkflowContext, templ const {path} = findToken(pos, result.value); - return [getWorkflowContext(textDocument.uri, template, path), template]; + return getWorkflowContext(textDocument.uri, template, path); } describe("getWorkflowContext", () => { it("context for workflow", () => { - const [context, template] = testGetWorkflowContext(`on: push + const context = testGetWorkflowContext(`on: push name: te|st jobs: build: @@ -44,7 +45,7 @@ jobs: }); it("context for workflow job", () => { - const [context, template] = testGetWorkflowContext(`on: push + const context = testGetWorkflowContext(`on: push jobs: build: runs-on: ubuntu-lat|est @@ -55,4 +56,38 @@ jobs: expect(context.job).not.toBeUndefined(); expect(context.step).toBeUndefined(); }); + + it("context for workflow run step", () => { + const context = testGetWorkflowContext(`on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo |Hello + - uses: actions/checkout@v2`); + expect(context.uri).not.toBe(""); + expect(context.template).not.toBeUndefined(); + expect(context.job).not.toBeUndefined(); + + const step = context.step as RunStep; + expect(step).not.toBeUndefined(); + expect(step.run.toDisplayString()).toBe("echo Hello"); + }); + + it("context for workflow uses step", () => { + const context = testGetWorkflowContext(`on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo Hello + - uses: actions/checkout@v2|`); + expect(context.uri).not.toBe(""); + expect(context.template).not.toBeUndefined(); + expect(context.job).not.toBeUndefined(); + + const step = context.step as ActionStep; + expect(step).not.toBeUndefined(); + expect(step.uses.value).toBe("actions/checkout@v2"); + }); }); diff --git a/actions-languageservice/src/context/workflow-context.ts b/actions-languageservice/src/context/workflow-context.ts index e7a7894..898aa2d 100644 --- a/actions-languageservice/src/context/workflow-context.ts +++ b/actions-languageservice/src/context/workflow-context.ts @@ -1,5 +1,7 @@ -import {WorkflowTemplate} from "@github/actions-workflow-parser"; +import {isMapping, isSequence, WorkflowTemplate} from "@github/actions-workflow-parser"; import {Job, Step} from "@github/actions-workflow-parser/model/workflow-template"; +import {MappingToken} from "@github/actions-workflow-parser/templates/tokens/mapping-token"; +import {SequenceToken} from "@github/actions-workflow-parser/templates/tokens/sequence-token"; import {StringToken} from "@github/actions-workflow-parser/templates/tokens/string-token"; import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; @@ -21,20 +23,60 @@ export function getWorkflowContext( tokenPath: TemplateToken[] ): WorkflowContext { const context: WorkflowContext = {uri: uri, template}; + if (!template) { + return context; + } - if (template) { - // Iterate through the token path to find the job and step - for (let i = 0; i < tokenPath.length; ++i) { - const token = tokenPath[i]; + let stepsSequence: SequenceToken | undefined = undefined; + let stepToken: MappingToken | undefined = undefined; - switch (token.definition?.key) { - case "job-id": { - const jobID = (token as StringToken).value; - context.job = template.jobs.find(job => job.id.value === jobID); + // Iterate through the token path to find the job and step + for (let i = 0; i < tokenPath.length; ++i) { + const token = tokenPath[i]; + + switch (token.definition?.key) { + case "job-id": { + const jobID = (token as StringToken).value; + context.job = template.jobs.find(job => job.id.value === jobID); + break; + } + case "steps": { + if (isSequence(token)) { + stepsSequence = token; } + break; + } + case "regular-step": + case "run-step": { + if (isMapping(token)) { + stepToken = token; + } + break; } } } + context.step = findStep(context.job?.steps, stepsSequence, stepToken); return context; } + +function findStep(steps?: Step[], stepSequence?: SequenceToken, stepToken?: MappingToken): Step | undefined { + if (!steps || !stepSequence || !stepToken) { + return undefined; + } + + // Steps may not define an ID, so find the step by index + let stepIndex = -1; + for (let i = 0; i < stepSequence.count; i++) { + if (stepSequence.get(i) === stepToken) { + stepIndex = i; + break; + } + } + + if (stepIndex === -1 || stepIndex >= steps.length) { + return undefined; + } + + return steps[stepIndex]; +} diff --git a/actions-languageservice/src/validate.expressions.test.ts b/actions-languageservice/src/validate.expressions.test.ts index 9a78a59..faaba00 100644 --- a/actions-languageservice/src/validate.expressions.test.ts +++ b/actions-languageservice/src/validate.expressions.test.ts @@ -113,4 +113,110 @@ jobs: ]); }); }); + + describe("steps context", () => { + it("steps.", async () => { + const input = ` + on: push + jobs: + a: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo \${{ steps.a }} + `; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("steps..outputs.", async () => { + const input = ` + on: push + jobs: + a: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo \${{ steps.a.outputs.anything }} + `; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([]); + }); + + it("invalid reference of later step", async () => { + const input = ` + on: push + jobs: + a: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo \${{ steps.c }} + - id: c + run: echo hello c + `; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: c", + range: { + end: { + character: 36, + line: 9 + }, + start: { + character: 22, + line: 9 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + + it("invalid reference of generated step name", async () => { + const input = ` + on: push + jobs: + a: + runs-on: ubuntu-latest + steps: + - id: a + run: echo hello a + - id: b + run: echo \${{ steps.__run }} + `; + + const result = await validate(createDocument("wf.yaml", input)); + + expect(result).toEqual([ + { + message: "Context access might be invalid: __run", + range: { + end: { + character: 40, + line: 9 + }, + start: { + character: 22, + line: 9 + } + }, + severity: DiagnosticSeverity.Warning + } + ]); + }); + }); }); From 9566fabc4170c753e621409490c2d653d9e788e8 Mon Sep 17 00:00:00 2001 From: github-actions Date: Mon, 5 Dec 2022 22:40:11 +0000 Subject: [PATCH 2/2] v0.1.33 --- actions-languageserver/package.json | 4 ++-- actions-languageservice/package.json | 2 +- lerna.json | 2 +- package-lock.json | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/actions-languageserver/package.json b/actions-languageserver/package.json index db0537c..b23dd44 100644 --- a/actions-languageserver/package.json +++ b/actions-languageserver/package.json @@ -1,6 +1,6 @@ { "name": "@github/actions-languageserver", - "version": "0.1.32", + "version": "0.1.33", "description": "Language server for GitHub Actions", "license": "MIT", "type": "module", @@ -38,7 +38,7 @@ "prettier-fix": "prettier --write ." }, "dependencies": { - "@github/actions-languageservice": "^0.1.32", + "@github/actions-languageservice": "^0.1.33", "@octokit/rest": "^19.0.5", "vscode-languageserver": "^8.0.2", "vscode-languageserver-textdocument": "^1.0.7" diff --git a/actions-languageservice/package.json b/actions-languageservice/package.json index 09aa2af..899d910 100644 --- a/actions-languageservice/package.json +++ b/actions-languageservice/package.json @@ -1,6 +1,6 @@ { "name": "@github/actions-languageservice", - "version": "0.1.32", + "version": "0.1.33", "description": "Language service for GitHub Actions", "license": "MIT", "type": "module", diff --git a/lerna.json b/lerna.json index 46b8da5..ecfc84d 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", "useWorkspaces": true, - "version": "0.1.32" + "version": "0.1.33" } diff --git a/package-lock.json b/package-lock.json index 7b27eb5..b4bb808 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,10 +15,10 @@ }, "actions-languageserver": { "name": "@github/actions-languageserver", - "version": "0.1.32", + "version": "0.1.33", "license": "MIT", "dependencies": { - "@github/actions-languageservice": "^0.1.32", + "@github/actions-languageservice": "^0.1.33", "@octokit/rest": "^19.0.5", "vscode-languageserver": "^8.0.2", "vscode-languageserver-textdocument": "^1.0.7" @@ -37,7 +37,7 @@ }, "actions-languageservice": { "name": "@github/actions-languageservice", - "version": "0.1.32", + "version": "0.1.33", "license": "MIT", "dependencies": { "@github/actions-workflow-parser": "*", @@ -10899,7 +10899,7 @@ "@github/actions-languageserver": { "version": "file:actions-languageserver", "requires": { - "@github/actions-languageservice": "^0.1.32", + "@github/actions-languageservice": "^0.1.33", "@octokit/rest": "^19.0.5", "@types/jest": "^29.0.3", "jest": "^29.0.3",