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.test.ts b/actions-languageservice/src/hover.test.ts index ab2f5b3..2b00671 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -1,33 +1,38 @@ 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 +describe("hover", () => { + it("on a key", async () => { + const input = `o|n: push jobs: build: - runs-on: [self-hosted, u|]`; - const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, { - line: 0, - character: 0 - }); + runs-on: [self-hosted]`; + 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." ); }); - it("hover on value", async () => { - const input = `on: push + it("on a value", async () => { + const input = `on: pu|sh jobs: build: - runs-on: [self-hosted, u|]`; - const doc = TextDocument.create("test://test/test.yaml", "yaml", 0, input); - const result = await hover(doc, { - line: 0, - character: 5 - }); - expect(result?.contents).toBeUndefined(); + runs-on: [self-hosted]`; + const result = await hover(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); + }); + + it("on a value in a sequence", async () => { + const input = `on: [pull_request, + pu|sh] +jobs: + build: + runs-on: [self-hosted]`; + 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/hover.ts b/actions-languageservice/src/hover.ts index 388236e..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,46 +14,38 @@ 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) { - // 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; diff --git a/actions-languageservice/src/utils/find-token.test.ts b/actions-languageservice/src/utils/find-token.test.ts index 0cfe35e..f79e561 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.String, "on"], ["on-strict", TokenType.Sequence] ], parent: ["on-strict", TokenType.Sequence], key: null, - token: ["non-empty-string", TokenType.String, "push"] + token: ["push-string", TokenType.String, "push"] }); }); @@ -108,7 +108,7 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], + ["on-strict", TokenType.String, "on"], ["on-strict", TokenType.Sequence] ], parent: ["on-strict", TokenType.Sequence], @@ -125,12 +125,12 @@ describe("find-token", () => { ).toEqual({ path: [ ["workflow-root-strict", TokenType.Mapping], - [null, TokenType.String, "on"], + ["on-strict", TokenType.String, "on"], ["on-strict", TokenType.Sequence] ], parent: ["on-strict", TokenType.Sequence], key: null, - token: ["non-empty-string", TokenType.String, "pull_request"] + token: ["pull-request-string", TokenType.String, "pull_request"] }); }); @@ -143,11 +143,11 @@ 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"], ["runs-on", TokenType.Sequence] ], parent: ["runs-on", TokenType.Sequence], @@ -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/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; diff --git a/package-lock.json b/package-lock.json index de24c3c..5924a0a 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"