diff --git a/actions-languageserver/src/index.ts b/actions-languageserver/src/index.ts index 8523f5e..19839e2 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,14 @@ documents.onDidChangeContent((change) => { }); async function validateTextDocument(textDocument: TextDocument): Promise { - const result = await validate(textDocument); + const result = await validate( + textDocument, + valueProviders( + sessionToken, + repos.find((repo) => textDocument.uri.startsWith(repo.workspaceUri)), + cache + ) + ); connection.sendDiagnostics({ uri: textDocument.uri, diagnostics: result }); } @@ -111,7 +119,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..9e3fc8e --- /dev/null +++ b/actions-languageserver/src/value-providers.ts @@ -0,0 +1,35 @@ +import { ValueProviderConfig } from "@github/actions-languageservice"; +import { WorkflowContext } from "@github/actions-languageservice/context/workflow-context"; +import { ValueProviderKind } 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, + repo: RepositoryContext | undefined, + cache: TTLCache +): ValueProviderConfig { + if (!repo || !sessionToken) { + return {}; + } + + const octokit = new Octokit({ + auth: sessionToken, + }); + + return { + "job-environment": { + kind: ValueProviderKind.AllowedValues, + get: (_: WorkflowContext) => + getEnvironments(octokit, cache, repo.owner, repo.name), + }, + "runs-on": { + kind: ValueProviderKind.SuggestedValues, + get: (_: WorkflowContext) => + getRunnerLabels(octokit, cache, repo.owner, repo.name), + }, + }; +} diff --git a/actions-languageserver/src/value-providers/runs-on.ts b/actions-languageserver/src/value-providers/runs-on.ts index 4fda670..ed869fb 100644 --- a/actions-languageserver/src/value-providers/runs-on.ts +++ b/actions-languageserver/src/value-providers/runs-on.ts @@ -9,8 +9,8 @@ export async function getRunnerLabels( name: string ): Promise { const defaultLabels = [ - "ubuntu-22.04", "ubuntu-latest", + "ubuntu-22.04", "ubuntu-20.04", "ubuntu-18.04", "windows-latest", @@ -24,10 +24,16 @@ export async function getRunnerLabels( "self-hosted", ]; - const repoLabels = await cache.get(`${owner}/${name}/runner-labels`, undefined, () => fetchRunnerLabels(client, owner, name)); + const repoLabels = await cache.get( + `${owner}/${name}/runner-labels`, + undefined, + () => fetchRunnerLabels(client, owner, name) + ); + for (const label of defaultLabels) { repoLabels.add(label); } + return Array.from(repoLabels).map((label) => ({ label })); } diff --git a/actions-languageservice/src/complete.test.ts b/actions-languageservice/src/complete.test.ts index d0e55db..9d8124b 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, ValueProviderKind} from "./value-providers/config"; describe("completion", () => { it("runs-on", async () => { @@ -179,14 +179,13 @@ 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") { - return [{label: "my-custom-label"}]; - } - return []; - }; const config: ValueProviderConfig = { - getCustomValues: getCustomValues + "runs-on": { + kind: ValueProviderKind.SuggestedValues, + get: async (_: WorkflowContext) => { + return [{label: "my-custom-label"}]; + } + } }; const result = await complete(...getPositionFromCursor(input), config); diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index b205c07..7471262 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]?.get(workflowContext); if (customValues) { return filterAndSortCompletionOptions(customValues, existingValues); @@ -117,7 +117,7 @@ async function getValues( (parent.definition?.key && defaultValueProviders[parent.definition.key]); if (valueProvider) { - const values = valueProvider(workflowContext); + const values = await valueProvider.get(workflowContext); return filterAndSortCompletionOptions(values, 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/index.ts b/actions-languageservice/src/index.ts index db8e8e6..d019592 100644 --- a/actions-languageservice/src/index.ts +++ b/actions-languageservice/src/index.ts @@ -2,4 +2,4 @@ export {complete} from "./complete"; export {ContextProviderConfig} from "./context-providers/config"; export {hover} from "./hover"; export {validate} from "./validate"; -export {ValueProviderConfig} from "./value-providers/config"; +export {ValueProviderConfig, ValueProviderKind} from "./value-providers/config"; diff --git a/actions-languageservice/src/validate.test.ts b/actions-languageservice/src/validate.test.ts index 06616e0..53ebffc 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,104 @@ jobs: } } as Diagnostic); }); + + it("single value not returned by suggested 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' might not be valid", + severity: DiagnosticSeverity.Warning, + 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' might not be valid", + severity: DiagnosticSeverity.Warning, + range: { + end: { + character: 20, + line: 5 + }, + start: { + character: 6, + line: 5 + } + } + } as Diagnostic); + }); + + it("single value not returned by allowed value provider", async () => { + const result = await validate( + createDocument( + "wf.yaml", + `on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo + build: + runs-on: ubuntu-latest + needs: test2 + steps: + - run: echo` + ), + defaultValueProviders + ); + + expect(result.length).toBe(1); + expect(result[0]).toEqual({ + message: "Value 'test2' is not valid", + severity: DiagnosticSeverity.Error, + range: { + end: { + character: 16, + line: 8 + }, + start: { + character: 11, + line: 8 + } + } + } as Diagnostic); + }); }); diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index cf8ea00..88b8bac 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -3,21 +3,29 @@ 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 {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"; 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 {ValueProviderConfig, ValueProviderKind} from "./value-providers/config"; +import {defaultValueProviders} from "./value-providers/default"; /** * Validates a workflow file @@ -42,11 +50,18 @@ 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 expressions and value providers + await additionalValidations( + diagnostics, + textDocument.uri, + template, + result.value, + valueProviderConfig, + contextProviderConfig + ); + } // For now map parser errors directly to diagnostics for (const error of result.context.errors.getErrors()) { @@ -90,56 +105,135 @@ function mapRange(range: TokenRange | undefined): Range { }; } -function validateExpressions( - diagnotics: Diagnostic[], - result: ParseWorkflowResult, +async function additionalValidations( + diagnostics: Diagnostic[], + documentUri: URI, + template: WorkflowTemplate, + root: TemplateToken, + valueProviderConfig: ValueProviderConfig | undefined, contextProviderConfig: ContextProviderConfig | undefined ) { - if (!result.value) { - return; - } - - // Iterate over the parsed workflow - for (const token of TemplateToken.traverse(result.value)) { + for (const token of TemplateToken.traverse(root)) { + // If this is an expression, validate it if (isBasicExpression(token)) { - // Validate the expression - for (const expression of token.originalExpressions || [token]) { - const allowedContexts = token.definition?.readerContext || []; - const {namedContexts, functions} = splitAllowedContext(allowedContexts); + validateExpression(diagnostics, token, contextProviderConfig); + } - let expr: Expr | undefined; + // 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; - try { - const l = new Lexer(expression.expression); - const lr = l.lex(); + // Try a custom value provider first + let valueProvider = valueProviderConfig[defKey]; + if (!valueProvider) { + // fall back to default + valueProvider = defaultValueProviders[defKey]; + } - 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; + if (valueProvider) { + 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); + } + } + } } - 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) { - diagnotics.push({ - message: `Context access might be invalid: ${e.keyName}`, - severity: DiagnosticSeverity.Warning, - range: mapRange(expression.range) - }); - } else { - // Ignore error + if (isString(token)) { + if (!customValuesMap.has(token.value)) { + invalidValue(diagnostics, token, valueProvider.kind); } } } } } } + +function invalidValue(diagnostics: Diagnostic[], token: StringToken, kind: ValueProviderKind) { + switch (kind) { + case ValueProviderKind.AllowedValues: + diagnostics.push({ + message: `Value '${token.value}' is not valid`, + severity: DiagnosticSeverity.Error, + range: mapRange(token.range) + }); + break; + + case ValueProviderKind.SuggestedValues: + diagnostics.push({ + message: `Value '${token.value}' might not be valid`, + severity: DiagnosticSeverity.Warning, + range: mapRange(token.range) + }); + break; + } +} + +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); +} + +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/config.ts b/actions-languageservice/src/value-providers/config.ts index ad87506..34b7337 100644 --- a/actions-languageservice/src/value-providers/config.ts +++ b/actions-languageservice/src/value-providers/config.ts @@ -5,15 +5,16 @@ export interface Value { description?: string; } -export type ValueProvider = () => Value[]; +export enum ValueProviderKind { + AllowedValues, + SuggestedValues +} + +export type ValueProvider = { + kind: ValueProviderKind; + get: (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; } diff --git a/actions-languageservice/src/value-providers/default.ts b/actions-languageservice/src/value-providers/default.ts index 969833d..be59611 100644 --- a/actions-languageservice/src/value-providers/default.ts +++ b/actions-languageservice/src/value-providers/default.ts @@ -1,25 +1,28 @@ import {WorkflowContext} from "../context/workflow-context"; -import {Value} from "./config"; +import {ValueProviderConfig, ValueProviderKind} from "./config"; import {needs} from "./needs"; +import {stringsToValues} from "./strings-to-values"; -export const defaultValueProviders: {[key: string]: (workflowContext: WorkflowContext) => Value[]} = { - needs, - "runs-on": () => - stringsToValues([ - "ubuntu-latest", - "ubuntu-18.04", - "ubuntu-16.04", - "windows-latest", - "windows-2019", - "windows-2016", - "macos-latest", - "macos-10.15", - "macos-10.14", - "macos-10.13", - "self-hosted" - ]) +export const defaultValueProviders: ValueProviderConfig = { + needs: { + kind: ValueProviderKind.AllowedValues, + get: needs + }, + "runs-on": { + kind: ValueProviderKind.SuggestedValues, + get: async (_: WorkflowContext) => + stringsToValues([ + "ubuntu-latest", + "ubuntu-18.04", + "ubuntu-16.04", + "windows-latest", + "windows-2019", + "windows-2016", + "macos-latest", + "macos-10.15", + "macos-10.14", + "macos-10.13", + "self-hosted" + ]) + } }; - -export function stringsToValues(labels: string[]): Value[] { - return labels.map(x => ({label: x})); -} diff --git a/actions-languageservice/src/value-providers/definition.ts b/actions-languageservice/src/value-providers/definition.ts index 71aeefb..ad3e5d2 100644 --- a/actions-languageservice/src/value-providers/definition.ts +++ b/actions-languageservice/src/value-providers/definition.ts @@ -5,7 +5,7 @@ import {OneOfDefinition} from "@github/actions-workflow-parser/templates/schema/ 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"; -import {stringsToValues} from "./default"; +import {stringsToValues} from "./strings-to-values"; export function definitionValues(def: Definition): Value[] { const schema = getWorkflowSchema(); 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 []; } diff --git a/actions-languageservice/src/value-providers/strings-to-values.ts b/actions-languageservice/src/value-providers/strings-to-values.ts new file mode 100644 index 0000000..ff95f87 --- /dev/null +++ b/actions-languageservice/src/value-providers/strings-to-values.ts @@ -0,0 +1,5 @@ +import {Value} from "./config"; + +export function stringsToValues(labels: string[]): Value[] { + return labels.map(x => ({label: x})); +}