From 09e354a30e46b4befb3175ea99938d094646a3d6 Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Tue, 29 Nov 2022 17:01:43 -0500 Subject: [PATCH 1/6] Read definition from key for hover and complete --- actions-languageservice/src/complete.ts | 11 ++++---- actions-languageservice/src/hover.ts | 34 ++++++++++--------------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index 1ba93ad..3b0d218 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -89,12 +89,13 @@ export async function complete( } } - const values = await getValues(token, parent, valueProviderConfig, workflowContext); + const values = await getValues(token, keyToken, parent, valueProviderConfig, workflowContext); return values.map(value => CompletionItem.create(value.label)); } async function getValues( token: TemplateToken | null, + keyToken: TemplateToken | null, parent: TemplateToken | null, valueProviderConfig: ValueProviderConfig | undefined, workflowContext: WorkflowContext @@ -105,8 +106,8 @@ async function getValues( const existingValues = getExistingValues(token, parent); - if (token?.definition?.key) { - const customValues = await valueProviderConfig?.[token.definition.key]?.get(workflowContext); + if (keyToken?.definition?.key) { + const customValues = await valueProviderConfig?.[keyToken.definition.key]?.get(workflowContext); if (customValues) { return filterAndSortCompletionOptions(customValues, existingValues); @@ -115,7 +116,7 @@ async function getValues( // Use the value provider from the parent if we don't have a value provider for the current key const valueProvider = - (token?.definition?.key && defaultValueProviders[token.definition.key]) || + (keyToken?.definition?.key && defaultValueProviders[keyToken.definition.key]) || (parent.definition?.key && defaultValueProviders[parent.definition.key]); if (valueProvider) { @@ -124,7 +125,7 @@ async function getValues( } // Use the definition if there are no value providers - const def = token?.definition || parent.definition; + const def = keyToken?.definition || parent.definition; if (!def) { return []; } diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index 388236e..ba272dc 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -17,43 +17,35 @@ export async function hover(document: TextDocument, position: Position): Promise const {token, keyToken, parent} = findToken(position, result.value); if (result.value && token) { - // If the parent is a MappingToken and no keyToken was returned, our token is the key - if (parent && isMapping(parent) && !keyToken) { - const value = parent.find(token.toString()); - if (value) { - return getHover(token, value); - } - } + return getHover(token); } return null; } -// PositionToken is the token that the cursor is on -// DescriptionToken may differ if the description is stored on an associated token, such as when hovering over a key in a mapping -function getHover(positionToken: TemplateToken, descriptionToken: TemplateToken): Hover | null { - if (descriptionToken.definition) { +function getHover(token: TemplateToken): Hover | null { + if (token.definition) { let description = ""; - if (descriptionToken.description) { - description = descriptionToken.description; + if (token.description) { + description = token.description; } - if (descriptionToken.definition.evaluatorContext.length > 0) { + if (token.definition.evaluatorContext.length > 0) { // Only add padding if there is a description - description += `${ - description.length > 0 ? `\n\n` : "" - }**Context:** ${descriptionToken.definition.evaluatorContext.join(", ")}`; + description += `${description.length > 0 ? `\n\n` : ""}**Context:** ${token.definition.evaluatorContext.join( + ", " + )}`; } return { contents: description, range: { start: { - line: positionToken.range!.start[0] - 1, - character: positionToken.range!.start[1] - 1 + line: token.range!.start[0] - 1, + character: token.range!.start[1] - 1 }, end: { - line: positionToken.range!.end[0] - 1, - character: positionToken.range!.end[1] - 1 + line: token.range!.end[0] - 1, + character: token.range!.end[1] - 1 } } } as Hover; From 9225d347e8369a7b5c112343f77c4595c330c184 Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Wed, 30 Nov 2022 14:39:21 -0500 Subject: [PATCH 2/6] Handle sequences, update tests --- actions-languageservice/src/hover.test.ts | 34 ++++--- .../src/utils/find-token.test.ts | 94 +++++++++---------- .../src/value-providers/definition.ts | 5 + 3 files changed, 73 insertions(+), 60 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index ab2f5b3..e90041c 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -1,17 +1,15 @@ import {TextDocument} from "vscode-languageserver-textdocument"; import {hover} from "./hover"; +import {getPositionFromCursor} from "./test-utils/cursor-position"; describe("validation", () => { it("valid workflow", async () => { - const input = `on: push + const input = `o|n: push jobs: build: - runs-on: [self-hosted, u|]`; + runs-on: [self-hosted]`; const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, { - line: 0, - character: 0 - }); + const result = await hover(doc, getPositionFromCursor(input)[1]); expect(result).not.toBeUndefined(); expect(result?.contents).toEqual( "The name of the GitHub event that triggers the workflow. You can provide a single event string, array of events, array of event types, or an event configuration map that schedules a workflow or restricts the execution of a workflow to specific files, tags, or branch changes. For a list of available events, see https://help.github.com/en/github/automating-your-workflow-with-github-actions/events-that-trigger-workflows." @@ -19,15 +17,25 @@ jobs: }); it("hover on value", async () => { - const input = `on: push + const input = `on: pu|sh jobs: build: - runs-on: [self-hosted, u|]`; + runs-on: [self-hosted]`; const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, { - line: 0, - character: 5 - }); - expect(result?.contents).toBeUndefined(); + const result = await hover(doc, getPositionFromCursor(input)[1]); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); + }); + + it("hover on sequence value", async () => { + const input = `on: [pull_request, + pu|sh] +jobs: + build: + runs-on: [self-hosted]`; + const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); + const result = await hover(doc, getPositionFromCursor(input)[1]); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); }); }); diff --git a/actions-languageservice/src/utils/find-token.test.ts b/actions-languageservice/src/utils/find-token.test.ts index 0cfe35e..788095d 100644 --- a/actions-languageservice/src/utils/find-token.test.ts +++ b/actions-languageservice/src/utils/find-token.test.ts @@ -53,7 +53,7 @@ describe("find-token", () => { path: [["workflow-root-strict", TokenType.Mapping]], parent: ["workflow-root-strict", TokenType.Mapping], key: null, - token: [null, TokenType.String, "on"] + token: ["on-strict", TokenType.String, "on"] }); }); @@ -61,11 +61,11 @@ describe("find-token", () => { expect(testFindToken(`on: pu|sh`)).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"] + ["on-strict", TokenType.String, "on"] ], parent: ["workflow-root-strict", TokenType.Mapping], - key: [null, TokenType.String, "on"], - token: ["on-strict", TokenType.String, "push"] + key: ["on-strict", TokenType.String, "on"], + token: ["push-string", TokenType.String, "push"] }); }); @@ -76,12 +76,12 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], + ["on-strict", TokenType.String, "on"], ["on-mapping-strict", TokenType.Mapping] ], parent: ["on-mapping-strict", TokenType.Mapping], key: null, - token: [null, TokenType.String, "push"] + token: ["push", TokenType.String, "push"] }); }); @@ -92,12 +92,12 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], - ["on-strict", TokenType.Sequence] + ["on-strict", TokenType.String, "on"], + ["on-sequence-strict", TokenType.Sequence] ], - parent: ["on-strict", TokenType.Sequence], + parent: ["on-sequence-strict", TokenType.Sequence], key: null, - token: ["non-empty-string", TokenType.String, "push"] + token: ["push-string", TokenType.String, "push"] }); }); @@ -108,10 +108,10 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], - ["on-strict", TokenType.Sequence] + ["on-strict", TokenType.String, "on"], + ["on-sequence-strict", TokenType.Sequence] ], - parent: ["on-strict", TokenType.Sequence], + parent: ["on-sequence-strict", TokenType.Sequence], key: null, token: null }); @@ -125,12 +125,12 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], - ["on-strict", TokenType.Sequence] + ["on-strict", TokenType.String, "on"], + ["on-sequence-strict", TokenType.Sequence] ], - parent: ["on-strict", TokenType.Sequence], + parent: ["on-sequence-strict", TokenType.Sequence], key: null, - token: ["non-empty-string", TokenType.String, "pull_request"] + token: ["pull-request-string", TokenType.String, "pull_request"] }); }); @@ -143,14 +143,14 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping], - [null, TokenType.String, "runs-on"], - ["runs-on", TokenType.Sequence] + ["runs-on", TokenType.String, "runs-on"], + ["sequence-of-non-empty-string", TokenType.Sequence] ], - parent: ["runs-on", TokenType.Sequence], + parent: ["sequence-of-non-empty-string", TokenType.Sequence], key: null, token: ["non-empty-string", TokenType.String, "self"] }); @@ -165,7 +165,7 @@ jo|bs: path: [["workflow-root-strict", TokenType.Mapping]], parent: ["workflow-root-strict", TokenType.Mapping], key: null, - token: [null, TokenType.String, "jobs"] + token: ["jobs", TokenType.String, "jobs"] }); }); @@ -178,15 +178,15 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping], - [null, TokenType.String, "runs-on"] + ["runs-on", TokenType.String, "runs-on"] ], parent: ["job-factory", TokenType.Mapping], - key: [null, TokenType.String, "runs-on"], - token: ["runs-on", TokenType.String, "ubu"] + key: ["runs-on", TokenType.String, "runs-on"], + token: ["non-empty-string", TokenType.String, "ubu"] }); }); @@ -199,14 +199,14 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping] ], parent: ["job-factory", TokenType.Mapping], key: null, - token: [null, TokenType.String, "runs-on"] + token: ["runs-on", TokenType.String, "runs-on"] }); }); @@ -219,14 +219,14 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping] ], parent: ["job-factory", TokenType.Mapping], - key: [null, TokenType.String, "continue-on-error"], - token: ["boolean-strategy-context", TokenType.Null, ""] + key: ["boolean-strategy-context", TokenType.String, "continue-on-error"], + token: [null, TokenType.Null, ""] }); }); @@ -239,14 +239,14 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping] ], parent: ["job-factory", TokenType.Mapping], - key: [null, TokenType.String, "container"], - token: ["container", TokenType.String, ""] + key: ["container", TokenType.String, "container"], + token: ["string", TokenType.String, ""] }); }); @@ -259,13 +259,13 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"] ], parent: ["jobs", TokenType.Mapping], key: ["job-id", TokenType.String, "build"], - token: ["job", TokenType.String, "continue-on-error:foo"] + token: [null, TokenType.String, "continue-on-error:foo"] }); }); @@ -278,7 +278,7 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping] @@ -298,14 +298,14 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping] ], parent: ["job-factory", TokenType.Mapping], key: null, - token: [null, TokenType.String, "continue-on-error"] + token: ["boolean-strategy-context", TokenType.String, "continue-on-error"] }); }); @@ -318,13 +318,13 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"] ], parent: ["jobs", TokenType.Mapping], key: ["job-id", TokenType.String, "build"], - token: ["job", TokenType.String, "runs-"] + token: [null, TokenType.String, "runs-"] }); }); @@ -338,13 +338,13 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"] ], parent: ["jobs", TokenType.Mapping], key: ["job-id", TokenType.String, "build"], - token: ["job", TokenType.String, "runs-"] + token: [null, TokenType.String, "runs-"] }); }); @@ -358,15 +358,15 @@ jobs: ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "jobs"], + ["jobs", TokenType.String, "jobs"], ["jobs", TokenType.Mapping], ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping], - [null, TokenType.String, "runs-on"] + ["runs-on", TokenType.String, "runs-on"] ], parent: ["job-factory", TokenType.Mapping], - key: [null, TokenType.String, "runs-on"], - token: ["runs-on", TokenType.String, "ubu"] + key: ["runs-on", TokenType.String, "runs-on"], + token: ["non-empty-string", TokenType.String, "ubu"] }); }); }); diff --git a/actions-languageservice/src/value-providers/definition.ts b/actions-languageservice/src/value-providers/definition.ts index ad3e5d2..4b51d60 100644 --- a/actions-languageservice/src/value-providers/definition.ts +++ b/actions-languageservice/src/value-providers/definition.ts @@ -2,6 +2,7 @@ import {BooleanDefinition} from "@github/actions-workflow-parser/templates/schem import {Definition} from "@github/actions-workflow-parser/templates/schema/definition"; import {MappingDefinition} from "@github/actions-workflow-parser/templates/schema/mapping-definition"; import {OneOfDefinition} from "@github/actions-workflow-parser/templates/schema/one-of-definition"; +import {SequenceDefinition} from "@github/actions-workflow-parser/templates/schema/sequence-definition"; import {StringDefinition} from "@github/actions-workflow-parser/templates/schema/string-definition"; import {getWorkflowSchema} from "@github/actions-workflow-parser/workflows/workflow-schema"; import {Value} from "./config"; @@ -26,6 +27,10 @@ export function definitionValues(def: Definition): Value[] { return stringsToValues([def.constant]); } + if (def instanceof SequenceDefinition && def.itemType && schema.definitions[def.itemType]) { + return definitionValues(schema.definitions[def.itemType]); + } + return []; } From 272e29a775680bac7ec0766a7bdf9e96ab93b02e Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Wed, 30 Nov 2022 15:21:16 -0500 Subject: [PATCH 3/6] Remove sequence definition support, fix test --- actions-languageservice/src/hover.test.ts | 11 ++++------- .../src/utils/find-token.test.ts | 16 ++++++++-------- .../src/value-providers/definition.ts | 5 ----- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index e90041c..bc55809 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -2,14 +2,13 @@ import {TextDocument} from "vscode-languageserver-textdocument"; import {hover} from "./hover"; import {getPositionFromCursor} from "./test-utils/cursor-position"; -describe("validation", () => { +describe("hover", () => { it("valid workflow", async () => { const input = `o|n: push jobs: build: runs-on: [self-hosted]`; - const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, getPositionFromCursor(input)[1]); + const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); expect(result?.contents).toEqual( "The name of the GitHub event that triggers the workflow. You can provide a single event string, array of events, array of event types, or an event configuration map that schedules a workflow or restricts the execution of a workflow to specific files, tags, or branch changes. For a list of available events, see https://help.github.com/en/github/automating-your-workflow-with-github-actions/events-that-trigger-workflows." @@ -21,8 +20,7 @@ jobs: jobs: build: runs-on: [self-hosted]`; - const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, getPositionFromCursor(input)[1]); + const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); }); @@ -33,8 +31,7 @@ jobs: jobs: build: runs-on: [self-hosted]`; - const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, getPositionFromCursor(input)[1]); + const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); }); diff --git a/actions-languageservice/src/utils/find-token.test.ts b/actions-languageservice/src/utils/find-token.test.ts index 788095d..f79e561 100644 --- a/actions-languageservice/src/utils/find-token.test.ts +++ b/actions-languageservice/src/utils/find-token.test.ts @@ -93,9 +93,9 @@ describe("find-token", () => { path: [ ["workflow-root-strict", TokenType.Mapping], ["on-strict", TokenType.String, "on"], - ["on-sequence-strict", TokenType.Sequence] + ["on-strict", TokenType.Sequence] ], - parent: ["on-sequence-strict", TokenType.Sequence], + parent: ["on-strict", TokenType.Sequence], key: null, token: ["push-string", TokenType.String, "push"] }); @@ -109,9 +109,9 @@ describe("find-token", () => { path: [ ["workflow-root-strict", TokenType.Mapping], ["on-strict", TokenType.String, "on"], - ["on-sequence-strict", TokenType.Sequence] + ["on-strict", TokenType.Sequence] ], - parent: ["on-sequence-strict", TokenType.Sequence], + parent: ["on-strict", TokenType.Sequence], key: null, token: null }); @@ -126,9 +126,9 @@ describe("find-token", () => { path: [ ["workflow-root-strict", TokenType.Mapping], ["on-strict", TokenType.String, "on"], - ["on-sequence-strict", TokenType.Sequence] + ["on-strict", TokenType.Sequence] ], - parent: ["on-sequence-strict", TokenType.Sequence], + parent: ["on-strict", TokenType.Sequence], key: null, token: ["pull-request-string", TokenType.String, "pull_request"] }); @@ -148,9 +148,9 @@ jobs: ["job-id", TokenType.String, "build"], ["job-factory", TokenType.Mapping], ["runs-on", TokenType.String, "runs-on"], - ["sequence-of-non-empty-string", TokenType.Sequence] + ["runs-on", TokenType.Sequence] ], - parent: ["sequence-of-non-empty-string", TokenType.Sequence], + parent: ["runs-on", TokenType.Sequence], key: null, token: ["non-empty-string", TokenType.String, "self"] }); diff --git a/actions-languageservice/src/value-providers/definition.ts b/actions-languageservice/src/value-providers/definition.ts index 4b51d60..ad3e5d2 100644 --- a/actions-languageservice/src/value-providers/definition.ts +++ b/actions-languageservice/src/value-providers/definition.ts @@ -2,7 +2,6 @@ import {BooleanDefinition} from "@github/actions-workflow-parser/templates/schem import {Definition} from "@github/actions-workflow-parser/templates/schema/definition"; import {MappingDefinition} from "@github/actions-workflow-parser/templates/schema/mapping-definition"; import {OneOfDefinition} from "@github/actions-workflow-parser/templates/schema/one-of-definition"; -import {SequenceDefinition} from "@github/actions-workflow-parser/templates/schema/sequence-definition"; import {StringDefinition} from "@github/actions-workflow-parser/templates/schema/string-definition"; import {getWorkflowSchema} from "@github/actions-workflow-parser/workflows/workflow-schema"; import {Value} from "./config"; @@ -27,10 +26,6 @@ export function definitionValues(def: Definition): Value[] { return stringsToValues([def.constant]); } - if (def instanceof SequenceDefinition && def.itemType && schema.definitions[def.itemType]) { - return definitionValues(schema.definitions[def.itemType]); - } - return []; } From cd0c8367695ca8b1ee942301bb6e21ca4a9410ac Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Thu, 1 Dec 2022 11:37:50 -0500 Subject: [PATCH 4/6] Fix validation with moved definition --- actions-languageservice/src/hover.ts | 4 +-- actions-languageservice/src/validate.test.ts | 29 ++++++++++++++++++++ actions-languageservice/src/validate.ts | 29 ++++++++------------ 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index ba272dc..5154db8 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -1,4 +1,4 @@ -import {isMapping, parseWorkflow} from "@github/actions-workflow-parser"; +import {parseWorkflow} from "@github/actions-workflow-parser"; import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; import {File} from "@github/actions-workflow-parser/workflows/file"; import {Position, TextDocument} from "vscode-languageserver-textdocument"; @@ -14,7 +14,7 @@ export async function hover(document: TextDocument, position: Position): Promise }; const result = parseWorkflow(file.name, [file], nullTrace); - const {token, keyToken, parent} = findToken(position, result.value); + const {token} = findToken(position, result.value); if (result.value && token) { return getHover(token); diff --git a/actions-languageservice/src/validate.test.ts b/actions-languageservice/src/validate.test.ts index 53ebffc..d10116e 100644 --- a/actions-languageservice/src/validate.test.ts +++ b/actions-languageservice/src/validate.test.ts @@ -158,4 +158,33 @@ jobs: } } as Diagnostic); }); + + it("unknown event type", async () => { + const result = await validate( + createDocument( + "wf.yaml", + `on: [push, check_run, pr] +jobs: + build: + runs-on: + - ubuntu-latest` + ), + defaultValueProviders + ); + + expect(result.length).toBe(1); + expect(result[0]).toEqual({ + message: "Unexpected value 'pr'", + range: { + end: { + character: 24, + line: 0 + }, + start: { + character: 22, + line: 0 + } + } + } as Diagnostic); + }); }); diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index 81023e4..4624dd8 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -3,7 +3,6 @@ import {Expr} from "@github/actions-expressions/ast"; import { convertWorkflowTemplate, isBasicExpression, - isSequence, isString, parseWorkflow, ParseWorkflowResult, @@ -11,6 +10,7 @@ import { } from "@github/actions-workflow-parser"; import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert"; import {splitAllowedContext} from "@github/actions-workflow-parser/templates/allowed-context"; +import {Definition} from "@github/actions-workflow-parser/templates/schema/definition"; import {BasicExpressionToken} from "@github/actions-workflow-parser/templates/tokens/basic-expression-token"; import {StringToken} from "@github/actions-workflow-parser/templates/tokens/string-token"; import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; @@ -114,12 +114,18 @@ async function additionalValidations( valueProviderConfig: ValueProviderConfig | undefined, contextProviderConfig: ContextProviderConfig | undefined ) { - for (const token of TemplateToken.traverse(root)) { + for (const [parent, token, key] of TemplateToken.traverse(root)) { + // If the token is a value in a pair, use the key definition for validation + // If the token has a parent (map, sequence, etc), use this definition for validation + const validationToken = key || parent || token; + const validationDefinition = validationToken.definition; + // If this is an expression, validate it if (isBasicExpression(token)) { await validateExpression( diagnostics, token, + validationDefinition, contextProviderConfig, getProviderContext(documentUri, template, root, token) ); @@ -127,8 +133,8 @@ async function additionalValidations( // Allowed values coming from the schema have already been validated. Only check if // a value provider is defined for a token and if it is, validate the values match. - if (valueProviderConfig && token.range && token.definition?.key) { - const defKey = token.definition.key; + if (valueProviderConfig && token.range && validationDefinition) { + const defKey = validationDefinition.key; // Try a custom value provider first let valueProvider = valueProviderConfig[defKey]; @@ -141,18 +147,6 @@ async function additionalValidations( const customValues = await valueProvider.get(getProviderContext(documentUri, template, root, token)); const customValuesMap = new Set(customValues.map(x => x.label)); - if (isSequence(token)) { - for (let i = 0; i < token.count; ++i) { - const entry = token.get(i); - - if (isString(entry)) { - if (!customValuesMap.has(entry.value)) { - invalidValue(diagnostics, entry, valueProvider.kind); - } - } - } - } - if (isString(token)) { if (!customValuesMap.has(token.value)) { invalidValue(diagnostics, token, valueProvider.kind); @@ -202,12 +196,13 @@ function getProviderContext( async function validateExpression( diagnostics: Diagnostic[], token: BasicExpressionToken, + definition: Definition | undefined, contextProviderConfig: ContextProviderConfig | undefined, workflowContext: WorkflowContext ) { // Validate the expression for (const expression of token.originalExpressions || [token]) { - const allowedContexts = token.definition?.readerContext || []; + const allowedContexts = definition?.readerContext || []; const {namedContexts, functions} = splitAllowedContext(allowedContexts); let expr: Expr | undefined; From 231757d3ffeaf69b47d4fe2cb3d2fdcf6598818a Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Fri, 2 Dec 2022 14:33:52 -0500 Subject: [PATCH 5/6] Update parser version --- package-lock.json | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9056587..8090418 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,9 @@ "actions-languageservice", "actions-languageserver" ], + "dependencies": { + "@github/actions-workflow-parser": "^0.0.24" + }, "devDependencies": { "lerna": "^6.0.3" } @@ -665,9 +668,9 @@ "link": true }, "node_modules/@github/actions-workflow-parser": { - "version": "0.0.23", - "resolved": "https://npm.pkg.github.com/download/@github/actions-workflow-parser/0.0.23/4fe24783761ab3ae620f6a4199c27f6d1bd2da03", - "integrity": "sha512-oYV5pKB9jryDM8BKzR0amuL96Xhq/aCswffv/ugQntogmKPQloYFJQv1Rpe9WMFQvM4Ws2KpYvpBsVwS3rwZyw==", + "version": "0.0.24", + "resolved": "https://npm.pkg.github.com/download/@github/actions-workflow-parser/0.0.24/7bee081e2d0caa2152a4f2264905f4cf96500226", + "integrity": "sha512-GnYIs8wnJCV0a5RBL1KYvdtSkUJbLW8Vic+dIiKyRCbIt2e0WuDlmVxh6Mllj+d7bAFiNVWDzUux23co1JTUzQ==", "license": "MIT", "dependencies": { "@github/actions-expressions": "*", @@ -10927,9 +10930,9 @@ } }, "@github/actions-workflow-parser": { - "version": "0.0.23", - "resolved": "https://npm.pkg.github.com/download/@github/actions-workflow-parser/0.0.23/4fe24783761ab3ae620f6a4199c27f6d1bd2da03", - "integrity": "sha512-oYV5pKB9jryDM8BKzR0amuL96Xhq/aCswffv/ugQntogmKPQloYFJQv1Rpe9WMFQvM4Ws2KpYvpBsVwS3rwZyw==", + "version": "0.0.24", + "resolved": "https://npm.pkg.github.com/download/@github/actions-workflow-parser/0.0.24/7bee081e2d0caa2152a4f2264905f4cf96500226", + "integrity": "sha512-GnYIs8wnJCV0a5RBL1KYvdtSkUJbLW8Vic+dIiKyRCbIt2e0WuDlmVxh6Mllj+d7bAFiNVWDzUux23co1JTUzQ==", "requires": { "@github/actions-expressions": "*", "yaml": "^2.0.0-8" From 2a106063b7573b7783dfa92ce70a9ae5d822027c Mon Sep 17 00:00:00 2001 From: Beth Brennan Date: Fri, 2 Dec 2022 15:23:15 -0500 Subject: [PATCH 6/6] Improve test names --- actions-languageservice/src/hover.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index bc55809..2b00671 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -3,7 +3,7 @@ import {hover} from "./hover"; import {getPositionFromCursor} from "./test-utils/cursor-position"; describe("hover", () => { - it("valid workflow", async () => { + it("on a key", async () => { const input = `o|n: push jobs: build: @@ -15,7 +15,7 @@ jobs: ); }); - it("hover on value", async () => { + it("on a value", async () => { const input = `on: pu|sh jobs: build: @@ -25,7 +25,7 @@ jobs: expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); }); - it("hover on sequence value", async () => { + it("on a value in a sequence", async () => { const input = `on: [pull_request, pu|sh] jobs: