Merge branch 'main' into cschleiden/validate-if-expressions

This commit is contained in:
Christopher Schleiden
2022-12-02 13:53:22 -08:00
committed by GitHub
13 changed files with 220 additions and 114 deletions
+2 -2
View File
@@ -1,6 +1,6 @@
{
"name": "@github/actions-languageserver",
"version": "0.1.25",
"version": "0.1.27",
"description": "Language server for GitHub Actions",
"license": "MIT",
"type": "module",
@@ -38,7 +38,7 @@
"prettier-fix": "prettier --write ."
},
"dependencies": {
"@github/actions-languageservice": "^0.1.25",
"@github/actions-languageservice": "^0.1.27",
"@octokit/rest": "^19.0.5",
"vscode-languageserver": "^8.0.2",
"vscode-languageserver-textdocument": "^1.0.7"
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "@github/actions-languageservice",
"version": "0.1.25",
"version": "0.1.27",
"description": "Language service for GitHub Actions",
"license": "MIT",
"type": "module",
@@ -212,4 +212,32 @@ jobs:
expect(result.map(x => x.label)).toEqual(["build_id"]);
});
it("inputs", async () => {
const input = `
on:
workflow_dispatch:
inputs:
name:
type: string
default: some value
another-name:
type: string
jobs:
a:
outputs:
build_id: my-build-id
runs-on: ubuntu-latest
steps:
- run: echo hello a
b:
needs: [a]
runs-on: ubuntu-latest
steps:
- run: echo "hello \${{ inputs.|
`;
const result = await complete(...getPositionFromCursor(input), undefined, contextProviderConfig);
expect(result.map(x => x.label)).toEqual(["another-name", "name"]);
});
});
+6 -5
View File
@@ -92,12 +92,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
@@ -108,8 +109,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);
@@ -118,7 +119,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) {
@@ -127,7 +128,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 [];
}
@@ -1,6 +1,7 @@
import {data} from "@github/actions-expressions";
import {WorkflowContext} from "../context/workflow-context";
import {ContextProviderConfig} from "./config";
import {getInputsContext} from "./inputs";
import {getNeedsContext} from "./needs";
export async function getContext(
@@ -13,7 +14,7 @@ export async function getContext(
for (const contextName of names) {
let value: data.Dictionary | undefined;
value = getDefaultContext(contextName, workflowContext);
value = await getDefaultContext(contextName, workflowContext);
if (!value) {
value = await config?.getContext(contextName);
@@ -29,7 +30,7 @@ export async function getContext(
return context;
}
function getDefaultContext(name: string, workflowContext: WorkflowContext): data.Dictionary | undefined {
async function getDefaultContext(name: string, workflowContext: WorkflowContext): Promise<data.Dictionary | undefined> {
switch (name) {
case "runner":
return objectToDictionary({
@@ -39,8 +40,12 @@ function getDefaultContext(name: string, workflowContext: WorkflowContext): data
tool_cache: "/opt/hostedtoolcache",
temp: "/home/runner/work/_temp"
});
case "needs":
return getNeedsContext(workflowContext);
case "inputs":
return getInputsContext(workflowContext);
}
return undefined;
@@ -0,0 +1,51 @@
import {data} from "@github/actions-expressions";
import {WorkflowContext} from "../context/workflow-context";
export function getInputsContext(workflowContext: WorkflowContext): data.Dictionary {
const d = new data.Dictionary();
if (!workflowContext?.template?.events) {
return d;
}
const event = workflowContext.template.events["workflow_dispatch"];
if (!event) {
return d;
}
const inputs = event.inputs;
for (const inputName of Object.keys(inputs)) {
const input = inputs[inputName];
switch (input.type) {
case "choice":
if (input.default) {
d.add(inputName, new data.StringData(input.default as string));
} else {
// Default to the first input or an empty string
d.add(inputName, new data.StringData((input.options || [""])[0]));
}
break;
case "environment":
if (input.default) {
d.add(inputName, new data.StringData(input.default as string));
} else {
// For now default to an empty value if there is no default value. This will always be an environment, so
// we could also dynamically look up environments and default to the first one, but leaving this as a
// future enhancement for now.
d.add(inputName, new data.StringData(""));
}
break;
case "boolean":
d.add(inputName, new data.BooleanData((input.default as boolean) || false));
break;
case "string":
default:
d.add(inputName, new data.StringData((input.default as string) || inputName));
break;
}
}
return d;
}
+22 -17
View File
@@ -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
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.");
});
expect(result?.contents).toBeUndefined();
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.");
});
});
+15 -23
View File
@@ -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;
@@ -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"]
});
});
});
@@ -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);
});
});
+12 -17
View File
@@ -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;
+1 -1
View File
@@ -1,5 +1,5 @@
{
"$schema": "node_modules/lerna/schemas/lerna-schema.json",
"useWorkspaces": true,
"version": "0.1.25"
"version": "0.1.27"
}
+4 -4
View File
@@ -15,10 +15,10 @@
},
"actions-languageserver": {
"name": "@github/actions-languageserver",
"version": "0.1.25",
"version": "0.1.27",
"license": "MIT",
"dependencies": {
"@github/actions-languageservice": "^0.1.25",
"@github/actions-languageservice": "^0.1.27",
"@octokit/rest": "^19.0.5",
"vscode-languageserver": "^8.0.2",
"vscode-languageserver-textdocument": "^1.0.7"
@@ -37,7 +37,7 @@
},
"actions-languageservice": {
"name": "@github/actions-languageservice",
"version": "0.1.25",
"version": "0.1.27",
"license": "MIT",
"dependencies": {
"@github/actions-workflow-parser": "*",
@@ -10899,7 +10899,7 @@
"@github/actions-languageserver": {
"version": "file:actions-languageserver",
"requires": {
"@github/actions-languageservice": "^0.1.25",
"@github/actions-languageservice": "^0.1.27",
"@octokit/rest": "^19.0.5",
"@types/jest": "^29.0.3",
"jest": "^29.0.3",