Merge pull request #30 from github/elbrenn/key

Use new definition location
This commit is contained in:
Beth Brennan
2022-12-02 16:50:54 -05:00
committed by GitHub
7 changed files with 133 additions and 108 deletions
+6 -5
View File
@@ -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 [];
}
+23 -18
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
});
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.");
});
});
+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;
+9 -6
View File
@@ -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"