From a32295a09a478c1517b1d7f8e95d774351610ad8 Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Mon, 28 Nov 2022 15:47:05 -0800 Subject: [PATCH] Use value providers for validation --- actions-languageserver/src/index.ts | 8 +- actions-languageserver/src/on-completion.ts | 45 ++------ actions-languageserver/src/value-providers.ts | 48 ++++++++ actions-languageservice/src/complete.test.ts | 10 +- actions-languageservice/src/complete.ts | 4 +- actions-languageservice/src/validate.ts | 105 ++++++++++++++++-- .../src/value-providers/config.ts | 11 +- 7 files changed, 164 insertions(+), 67 deletions(-) create mode 100644 actions-languageserver/src/value-providers.ts diff --git a/actions-languageserver/src/index.ts b/actions-languageserver/src/index.ts index 8523f5e..2c84793 100644 --- a/actions-languageserver/src/index.ts +++ b/actions-languageserver/src/index.ts @@ -19,6 +19,7 @@ import { } from "./initializationOptions"; import { onCompletion } from "./on-completion"; import { TTLCache } from "./utils/cache"; +import { valueProviders } from "./value-providers"; // Create a connection for the server, using Node's IPC as a transport. // Also include all preview / proposed LSP features. @@ -90,7 +91,10 @@ documents.onDidChangeContent((change) => { }); async function validateTextDocument(textDocument: TextDocument): Promise { - const result = await validate(textDocument); + const result = await validate( + textDocument, + valueProviders(sessionToken, repoWorkspaceMap) + ); connection.sendDiagnostics({ uri: textDocument.uri, diagnostics: result }); } @@ -111,7 +115,7 @@ connection.onCompletion( documents.get(textDocument.uri)!, sessionToken, repos.find((repo) => textDocument.uri.startsWith(repo.workspaceUri)), - cache, + cache ); } ); diff --git a/actions-languageserver/src/on-completion.ts b/actions-languageserver/src/on-completion.ts index 7d5ff07..28f8fb7 100644 --- a/actions-languageserver/src/on-completion.ts +++ b/actions-languageserver/src/on-completion.ts @@ -1,51 +1,20 @@ import { complete } from "@github/actions-languageservice/complete"; -import { WorkflowContext } from "@github/actions-languageservice/context/workflow-context"; -import { Value, ValueProviderConfig } from "@github/actions-languageservice/value-providers/config"; -import { Octokit } from "@octokit/rest"; import { CompletionItem, Position } from "vscode-languageserver"; import { TextDocument } from "vscode-languageserver-textdocument"; import { RepositoryContext } from "./initializationOptions"; import { TTLCache } from "./utils/cache"; -import { getEnvironments } from "./value-providers/job-environment"; -import { getRunnerLabels } from "./value-providers/runs-on"; +import { valueProviders } from "./value-providers"; export async function onCompletion( position: Position, document: TextDocument, sessionToken: string | undefined, repoContext: RepositoryContext | undefined, - cache: TTLCache, + cache: TTLCache ): Promise { - const config: ValueProviderConfig = { - getCustomValues: async (key: string, context: WorkflowContext) => - getCustomValues(key, context, sessionToken, repoContext, cache), - }; - return await complete(document, position, config); -} - - -async function getCustomValues( - key: string, - _: WorkflowContext, - sessionToken: string | undefined, - repo: RepositoryContext | undefined, - cache: TTLCache, -): Promise { - if (!sessionToken || !repo) { - return; - } - - const octokit = new Octokit({ - auth: sessionToken, - }); - - switch (key) { - case "job-environment": { - return await getEnvironments(octokit, cache, repo.owner, repo.name); - } - - case "runs-on": { - return await getRunnerLabels(octokit, cache, repo.owner, repo.name); - } - } + return await complete( + document, + position, + repoContext && valueProviders(sessionToken, repoContext, cache) + ); } diff --git a/actions-languageserver/src/value-providers.ts b/actions-languageserver/src/value-providers.ts new file mode 100644 index 0000000..dbbb60b --- /dev/null +++ b/actions-languageserver/src/value-providers.ts @@ -0,0 +1,48 @@ +import { ValueProviderConfig } from "@github/actions-languageservice"; +import { WorkflowContext } from "@github/actions-languageservice/context/workflow-context"; +import { Value } from "@github/actions-languageservice/value-providers/config"; +import { Octokit } from "@octokit/rest"; +import { RepositoryContext } from "./initializationOptions"; +import { TTLCache } from "./utils/cache"; +import { getEnvironments } from "./value-providers/job-environment"; +import { getRunnerLabels } from "./value-providers/runs-on"; + +export function valueProviders( + sessionToken: string | undefined, + repoContext: RepositoryContext, + cache: TTLCache +): ValueProviderConfig { + return { + "job-environment": passOctoKit( + sessionToken, + repoContext, + cache, + getEnvironments + ), + "runs-on": passOctoKit(sessionToken, repoContext, cache, getRunnerLabels), + }; +} + +function passOctoKit( + sessionToken: string | undefined, + repo: RepositoryContext, + cache: TTLCache, + f: ( + octokit: Octokit, + cache: TTLCache, + owner: string, + name: string + ) => Promise +) { + return async function (context: WorkflowContext): Promise { + if (!sessionToken) { + return []; + } + + const octokit = new Octokit({ + auth: sessionToken, + }); + + return f(octokit, cache, repo.owner, repo.name); + }; +} diff --git a/actions-languageservice/src/complete.test.ts b/actions-languageservice/src/complete.test.ts index d0e55db..1d9e2e2 100644 --- a/actions-languageservice/src/complete.test.ts +++ b/actions-languageservice/src/complete.test.ts @@ -1,7 +1,7 @@ import {complete} from "./complete"; import {WorkflowContext} from "./context/workflow-context"; import {getPositionFromCursor} from "./test-utils/cursor-position"; -import {Value, ValueProviderConfig} from "./value-providers/config"; +import {ValueProviderConfig} from "./value-providers/config"; describe("completion", () => { it("runs-on", async () => { @@ -179,14 +179,10 @@ jobs: it("custom value providers override defaults", async () => { const input = "on: push\njobs:\n build:\n runs-on: |"; - const getCustomValues = async (key: string, _: WorkflowContext): Promise => { - if (key === "runs-on") { + const config: ValueProviderConfig = { + "runs-on": async (_: WorkflowContext) => { return [{label: "my-custom-label"}]; } - return []; - }; - const config: ValueProviderConfig = { - getCustomValues: getCustomValues }; const result = await complete(...getPositionFromCursor(input), config); diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index b205c07..061561a 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -104,7 +104,7 @@ async function getValues( const existingValues = getExistingValues(token, parent); if (token?.definition?.key) { - const customValues = await valueProviderConfig?.getCustomValues(token.definition.key, workflowContext); + const customValues = await valueProviderConfig?.[token.definition.key]?.(workflowContext); if (customValues) { return filterAndSortCompletionOptions(customValues, existingValues); @@ -131,7 +131,7 @@ async function getValues( return filterAndSortCompletionOptions(values, existingValues); } -function getExistingValues(token: TemplateToken | null, parent: TemplateToken) { +export function getExistingValues(token: TemplateToken | null, parent: TemplateToken) { // For incomplete YAML, we may only have a parent token if (token) { if (!isString(token)) { diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index cf8ea00..36a7afc 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -3,21 +3,27 @@ import {Expr} from "@github/actions-expressions/ast"; import { convertWorkflowTemplate, isBasicExpression, + isSequence, + isString, parseWorkflow, - ParseWorkflowResult + ParseWorkflowResult, + WorkflowTemplate } from "@github/actions-workflow-parser"; import {splitAllowedContext} from "@github/actions-workflow-parser/templates/allowed-context"; 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"; import {TextDocument} from "vscode-languageserver-textdocument"; -import {Diagnostic, DiagnosticSeverity, Range} from "vscode-languageserver-types"; +import {Diagnostic, DiagnosticSeverity, Range, URI} from "vscode-languageserver-types"; import {ContextProviderConfig} from "./context-providers/config"; import {getContext} from "./context-providers/default"; +import {getWorkflowContext, WorkflowContext} from "./context/workflow-context"; import {AccessError, wrapDictionary} from "./expression-validation/error-dictionary"; import {nullTrace} from "./nulltrace"; -import {ValueProviderConfig} from "./value-providers/config"; +import {findToken} from "./utils/find-token"; +import {Value, ValueProviderConfig} from "./value-providers/config"; +import {defaultValueProviders} from "./value-providers/default"; /** * Validates a workflow file @@ -42,11 +48,16 @@ export async function validate( const result: ParseWorkflowResult = parseWorkflow(file.name, [file], nullTrace); if (result.value) { // Errors will be updated in the context - convertWorkflowTemplate(result.context, result.value); - } + const template = convertWorkflowTemplate(result.context, result.value); - // Validate expressions - validateExpressions(diagnostics, result, contextProviderConfig); + // Validate with value providers + if (valueProviderConfig) { + await validateValueProviders(diagnostics, textDocument.uri, template, result, valueProviderConfig); + } + + // Validate expressions + validateExpressions(diagnostics, result, contextProviderConfig); + } // For now map parser errors directly to diagnostics for (const error of result.context.errors.getErrors()) { @@ -91,7 +102,7 @@ function mapRange(range: TokenRange | undefined): Range { } function validateExpressions( - diagnotics: Diagnostic[], + diagnostics: Diagnostic[], result: ParseWorkflowResult, contextProviderConfig: ContextProviderConfig | undefined ) { @@ -130,7 +141,7 @@ function validateExpressions( // result of the evaluation. } catch (e) { if (e instanceof AccessError) { - diagnotics.push({ + diagnostics.push({ message: `Context access might be invalid: ${e.keyName}`, severity: DiagnosticSeverity.Warning, range: mapRange(expression.range) @@ -143,3 +154,79 @@ function validateExpressions( } } } + +async function validateValueProviders( + diagnostics: Diagnostic[], + documentUri: URI, + template: WorkflowTemplate, + result: ParseWorkflowResult, + valueProviderConfig: ValueProviderConfig +) { + if (!result.value) { + return; + } + + // 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) { + const defKey = token.definition.key; + + let customValues: Value[] | undefined; + + const customValueProvider = valueProviderConfig[defKey]; + if (customValueProvider) { + customValues = await customValueProvider(getProviderContext(documentUri, template, result.value, token)); + } else { + const defaultValueProvider = defaultValueProviders[defKey]; + if (defaultValueProvider) { + customValues = defaultValueProvider(getProviderContext(documentUri, template, result.value, token)); + } + } + + if (customValues) { + if (isSequence(token)) { + for (let i = 0; i < token.count; ++i) { + const entry = token.get(i); + + 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) + }); + } + } + } + } + + 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) + }); + } + } + } + } + } +} + +function getProviderContext( + documentUri: URI, + template: WorkflowTemplate, + root: TemplateToken, + token: TemplateToken +): WorkflowContext { + const {parent, path} = findToken( + { + line: token.range!.start[0], + character: token.range!.start[1] + }, + root + ); + return getWorkflowContext(documentUri, template, path); +} diff --git a/actions-languageservice/src/value-providers/config.ts b/actions-languageservice/src/value-providers/config.ts index ad87506..20fb54c 100644 --- a/actions-languageservice/src/value-providers/config.ts +++ b/actions-languageservice/src/value-providers/config.ts @@ -5,15 +5,8 @@ export interface Value { description?: string; } -export type ValueProvider = () => Value[]; +export type ValueProvider = (context: WorkflowContext) => Promise; export interface ValueProviderConfig { - getCustomValues: (key: string, context: WorkflowContext) => Promise; - getActionInputs?: (owner: string, name: string, ref: string, path?: string) => Promise; -} - -export interface ActionInput { - name: string; - description?: string; - required?: boolean; + [definitionKey: string]: ValueProvider; }