diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index 061561a..da3b550 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -117,7 +117,7 @@ async function getValues( (parent.definition?.key && defaultValueProviders[parent.definition.key]); if (valueProvider) { - const values = valueProvider(workflowContext); + const values = await valueProvider(workflowContext); return filterAndSortCompletionOptions(values, existingValues); } diff --git a/actions-languageservice/src/validate.test.ts b/actions-languageservice/src/validate.test.ts index 06616e0..ce230e6 100644 --- a/actions-languageservice/src/validate.test.ts +++ b/actions-languageservice/src/validate.test.ts @@ -1,6 +1,7 @@ -import {Diagnostic} from "vscode-languageserver-types"; +import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types"; import {createDocument} from "./test-utils/document"; import {validate} from "./validate"; +import {defaultValueProviders} from "./value-providers/default"; describe("validation", () => { it("valid workflow", async () => { @@ -57,4 +58,68 @@ jobs: } } as Diagnostic); }); + + it("single value not returned by value provider", async () => { + const result = await validate( + createDocument( + "wf.yaml", + `on: push +jobs: + build: + runs-on: does-not-exist + steps: + - run: echo` + ), + defaultValueProviders + ); + + expect(result.length).toBe(1); + expect(result[0]).toEqual({ + message: "Value 'does-not-exist' is not valid", + severity: DiagnosticSeverity.Error, + range: { + end: { + character: 27, + line: 3 + }, + start: { + character: 13, + line: 3 + } + } + } as Diagnostic); + }); + + it("value in sequence not returned by value provider", async () => { + const result = await validate( + createDocument( + "wf.yaml", + `on: push +jobs: + build: + runs-on: + - ubuntu-latest + - does-not-exist + steps: + - run: echo` + ), + defaultValueProviders + ); + + expect(result.length).toBe(1); + expect(result[0]).toEqual({ + message: "Value 'does-not-exist' is not valid", + severity: DiagnosticSeverity.Error, + range: { + end: { + character: 20, + line: 5 + }, + start: { + character: 6, + line: 5 + } + } + } as Diagnostic); + }); }); diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index 36a7afc..89683a1 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -10,6 +10,8 @@ import { WorkflowTemplate } from "@github/actions-workflow-parser"; import {splitAllowedContext} from "@github/actions-workflow-parser/templates/allowed-context"; +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"; import {TokenRange} from "@github/actions-workflow-parser/templates/tokens/token-range"; import {File} from "@github/actions-workflow-parser/workflows/file"; @@ -50,13 +52,15 @@ export async function validate( // Errors will be updated in the context const template = convertWorkflowTemplate(result.context, result.value); - // Validate with value providers - if (valueProviderConfig) { - await validateValueProviders(diagnostics, textDocument.uri, template, result, valueProviderConfig); - } - - // Validate expressions - validateExpressions(diagnostics, result, contextProviderConfig); + // Validate expressions and value providers + await additionalValidations( + diagnostics, + textDocument.uri, + template, + result.value, + valueProviderConfig, + contextProviderConfig + ); } // For now map parser errors directly to diagnostics @@ -101,86 +105,34 @@ function mapRange(range: TokenRange | undefined): Range { }; } -function validateExpressions( - diagnostics: Diagnostic[], - result: ParseWorkflowResult, - contextProviderConfig: ContextProviderConfig | undefined -) { - if (!result.value) { - return; - } - - // Iterate over the parsed workflow - for (const token of TemplateToken.traverse(result.value)) { - if (isBasicExpression(token)) { - // Validate the expression - for (const expression of token.originalExpressions || [token]) { - const allowedContexts = token.definition?.readerContext || []; - const {namedContexts, functions} = splitAllowedContext(allowedContexts); - - let expr: Expr | undefined; - - try { - const l = new Lexer(expression.expression); - const lr = l.lex(); - - const p = new Parser(lr.tokens, namedContexts, functions); - expr = p.parse(); - } catch { - // Ignore any error here, we should've caught this earlier in the parsing process - continue; - } - - try { - const context = getContext(namedContexts, contextProviderConfig); - - const e = new Evaluator(expr, wrapDictionary(context)); - e.evaluate(); - - // Any invalid context access would've thrown an error via the `ErrorDictionary`, for now we don't have to check the actual - // result of the evaluation. - } catch (e) { - if (e instanceof AccessError) { - diagnostics.push({ - message: `Context access might be invalid: ${e.keyName}`, - severity: DiagnosticSeverity.Warning, - range: mapRange(expression.range) - }); - } else { - // Ignore error - } - } - } - } - } -} - -async function validateValueProviders( +async function additionalValidations( diagnostics: Diagnostic[], documentUri: URI, template: WorkflowTemplate, - result: ParseWorkflowResult, - valueProviderConfig: ValueProviderConfig + root: TemplateToken, + valueProviderConfig: ValueProviderConfig | undefined, + contextProviderConfig: ContextProviderConfig | undefined ) { - if (!result.value) { - return; - } + for (const token of TemplateToken.traverse(root)) { + // If this is an expression, validate it + if (isBasicExpression(token)) { + validateExpression(diagnostics, token, contextProviderConfig); + } - // 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. - for (const token of TemplateToken.traverse(result.value)) { - if (token.range && token.definition?.key) { + // 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; let customValues: Value[] | undefined; const customValueProvider = valueProviderConfig[defKey]; if (customValueProvider) { - customValues = await customValueProvider(getProviderContext(documentUri, template, result.value, token)); + customValues = await customValueProvider(getProviderContext(documentUri, template, root, token)); } else { const defaultValueProvider = defaultValueProviders[defKey]; if (defaultValueProvider) { - customValues = defaultValueProvider(getProviderContext(documentUri, template, result.value, token)); + customValues = await defaultValueProvider(getProviderContext(documentUri, template, root, token)); } } @@ -191,11 +143,7 @@ async function validateValueProviders( if (isString(entry)) { if (!customValues.map(x => x.label).includes(entry.value)) { - diagnostics.push({ - message: `Value '${entry.value}' is not allowed`, - severity: DiagnosticSeverity.Error, - range: mapRange(entry.range) - }); + invalidValue(diagnostics, entry); } } } @@ -203,11 +151,7 @@ async function validateValueProviders( if (isString(token)) { if (!customValues.map(x => x.label).includes(token.value)) { - diagnostics.push({ - message: `Value '${token.value}' is not allowed`, - severity: DiagnosticSeverity.Error, - range: mapRange(token.range) - }); + invalidValue(diagnostics, token); } } } @@ -215,6 +159,14 @@ async function validateValueProviders( } } +function invalidValue(diagnostics: Diagnostic[], token: StringToken) { + diagnostics.push({ + message: `Value '${token.value}' is not valid`, + severity: DiagnosticSeverity.Error, + range: mapRange(token.range) + }); +} + function getProviderContext( documentUri: URI, template: WorkflowTemplate, @@ -230,3 +182,48 @@ function getProviderContext( ); return getWorkflowContext(documentUri, template, path); } + +function validateExpression( + diagnostics: Diagnostic[], + token: BasicExpressionToken, + contextProviderConfig: ContextProviderConfig | undefined +) { + // Validate the expression + for (const expression of token.originalExpressions || [token]) { + const allowedContexts = token.definition?.readerContext || []; + const {namedContexts, functions} = splitAllowedContext(allowedContexts); + + let expr: Expr | undefined; + + try { + const l = new Lexer(expression.expression); + const lr = l.lex(); + + const p = new Parser(lr.tokens, namedContexts, functions); + expr = p.parse(); + } catch { + // Ignore any error here, we should've caught this earlier in the parsing process + continue; + } + + try { + const context = getContext(namedContexts, contextProviderConfig); + + const e = new Evaluator(expr, wrapDictionary(context)); + e.evaluate(); + + // Any invalid context access would've thrown an error via the `ErrorDictionary`, for now we don't have to check the actual + // result of the evaluation. + } catch (e) { + if (e instanceof AccessError) { + diagnostics.push({ + message: `Context access might be invalid: ${e.keyName}`, + severity: DiagnosticSeverity.Warning, + range: mapRange(expression.range) + }); + } else { + // Ignore error + } + } + } +} diff --git a/actions-languageservice/src/value-providers/default.ts b/actions-languageservice/src/value-providers/default.ts index 969833d..1b4c877 100644 --- a/actions-languageservice/src/value-providers/default.ts +++ b/actions-languageservice/src/value-providers/default.ts @@ -1,10 +1,10 @@ import {WorkflowContext} from "../context/workflow-context"; -import {Value} from "./config"; +import {Value, ValueProviderConfig} from "./config"; import {needs} from "./needs"; -export const defaultValueProviders: {[key: string]: (workflowContext: WorkflowContext) => Value[]} = { +export const defaultValueProviders: ValueProviderConfig = { needs, - "runs-on": () => + "runs-on": async (_: WorkflowContext) => stringsToValues([ "ubuntu-latest", "ubuntu-18.04", diff --git a/actions-languageservice/src/value-providers/needs.ts b/actions-languageservice/src/value-providers/needs.ts index 5ca3631..8c886b0 100644 --- a/actions-languageservice/src/value-providers/needs.ts +++ b/actions-languageservice/src/value-providers/needs.ts @@ -1,7 +1,7 @@ import {WorkflowContext} from "../context/workflow-context"; import {Value} from "./config"; -export function needs(context: WorkflowContext): Value[] { +export async function needs(context: WorkflowContext): Promise { if (!context.template) { return []; }