Context-aware autocomplete and validation for action.yml runs section (#289)
- Set main as required in node-runs-strict schema definition - Add validation for invalid key combinations based on using value - Add validation for missing required keys (main for node, steps for composite, image for docker) - Filter autocomplete suggestions based on using value - Prioritize 'using' in completions when not set yet Fixes context-aware autocomplete for action.yml files where different action types (node, composite, docker) have different valid keys under runs:
This commit is contained in:
eric sciple
@@ -184,6 +184,84 @@ runs:
|
||||
|
||||
expect(labels).toContain("using");
|
||||
});
|
||||
|
||||
it("filters runs keys for node20 actions", async () => {
|
||||
const [doc, position] = createActionDocument(`name: Test
|
||||
description: Test
|
||||
runs:
|
||||
using: node20
|
||||
|`);
|
||||
const completions = await complete(doc, position);
|
||||
const labels = completions.map(c => c.label);
|
||||
|
||||
// Should show Node.js action keys
|
||||
expect(labels).toContain("main");
|
||||
expect(labels).toContain("pre");
|
||||
expect(labels).toContain("post");
|
||||
expect(labels).toContain("pre-if");
|
||||
expect(labels).toContain("post-if");
|
||||
|
||||
// Should NOT show composite or docker keys
|
||||
expect(labels).not.toContain("steps");
|
||||
expect(labels).not.toContain("image");
|
||||
expect(labels).not.toContain("entrypoint");
|
||||
});
|
||||
|
||||
it("filters runs keys for composite actions", async () => {
|
||||
const [doc, position] = createActionDocument(`name: Test
|
||||
description: Test
|
||||
runs:
|
||||
using: composite
|
||||
|`);
|
||||
const completions = await complete(doc, position);
|
||||
const labels = completions.map(c => c.label);
|
||||
|
||||
// Should show composite action keys
|
||||
expect(labels).toContain("steps");
|
||||
|
||||
// Should NOT show Node.js or docker keys
|
||||
expect(labels).not.toContain("main");
|
||||
expect(labels).not.toContain("pre");
|
||||
expect(labels).not.toContain("post");
|
||||
expect(labels).not.toContain("image");
|
||||
});
|
||||
|
||||
it("filters runs keys for docker actions", async () => {
|
||||
const [doc, position] = createActionDocument(`name: Test
|
||||
description: Test
|
||||
runs:
|
||||
using: docker
|
||||
|`);
|
||||
const completions = await complete(doc, position);
|
||||
const labels = completions.map(c => c.label);
|
||||
|
||||
// Should show Docker action keys
|
||||
expect(labels).toContain("image");
|
||||
expect(labels).toContain("args");
|
||||
expect(labels).toContain("env");
|
||||
expect(labels).toContain("entrypoint");
|
||||
expect(labels).toContain("pre-entrypoint");
|
||||
expect(labels).toContain("post-entrypoint");
|
||||
|
||||
// Should NOT show Node.js or composite keys
|
||||
expect(labels).not.toContain("main");
|
||||
expect(labels).not.toContain("steps");
|
||||
});
|
||||
|
||||
it("prioritizes using when not set", async () => {
|
||||
const [doc, position] = createActionDocument(`name: Test
|
||||
description: Test
|
||||
runs:
|
||||
|`);
|
||||
const completions = await complete(doc, position);
|
||||
|
||||
// Find the using completion
|
||||
const usingCompletion = completions.find(c => c.label === "using");
|
||||
expect(usingCompletion).toBeDefined();
|
||||
|
||||
// It should have a sortText that makes it sort first
|
||||
expect(usingCompletion?.sortText).toBe("0_using");
|
||||
});
|
||||
});
|
||||
|
||||
describe("branding completions", () => {
|
||||
|
||||
@@ -38,6 +38,24 @@ import {Value, ValueProviderConfig} from "./value-providers/config.js";
|
||||
import {defaultValueProviders} from "./value-providers/default.js";
|
||||
import {DefinitionValueMode, definitionValues, TokenStructure} from "./value-providers/definition.js";
|
||||
|
||||
/**
|
||||
* Valid keys for each action type under the `runs:` section.
|
||||
* Source: https://github.com/actions/runner/blob/main/src/Runner.Worker/ActionManifestManager.cs
|
||||
*/
|
||||
const ACTION_NODE_KEYS = new Set(["using", "main", "pre", "post", "pre-if", "post-if"]);
|
||||
const ACTION_COMPOSITE_KEYS = new Set(["using", "steps"]);
|
||||
const ACTION_DOCKER_KEYS = new Set([
|
||||
"using",
|
||||
"image",
|
||||
"args",
|
||||
"env",
|
||||
"entrypoint",
|
||||
"pre-entrypoint",
|
||||
"pre-if",
|
||||
"post-entrypoint",
|
||||
"post-if"
|
||||
]);
|
||||
|
||||
export function getExpressionInput(input: string, pos: number): string {
|
||||
// Find start marker around the cursor position
|
||||
let startPos = input.lastIndexOf(OPEN_EXPRESSION, pos);
|
||||
@@ -137,7 +155,7 @@ export async function complete(
|
||||
const indentString = " ".repeat(indentation.tabSize);
|
||||
|
||||
// YAML key/value completions
|
||||
const values = await getValues(
|
||||
let values = await getValues(
|
||||
token,
|
||||
keyToken,
|
||||
parent,
|
||||
@@ -147,6 +165,11 @@ export async function complete(
|
||||
schema
|
||||
);
|
||||
|
||||
// Filter action.yml `runs:` completions based on `using:` value
|
||||
if (isAction && parsedTemplate.value) {
|
||||
values = filterActionRunsCompletions(values, path, parsedTemplate.value);
|
||||
}
|
||||
|
||||
// Offer "(switch to list)" / "(switch to mapping)" when the schema allows alternative forms
|
||||
const escapeHatches = getEscapeHatchCompletions(token, keyToken, indentString, newPos, schema);
|
||||
values.push(...escapeHatches);
|
||||
@@ -603,3 +626,90 @@ function getOffsetInContent(tokenRange: TokenRange, currentInput: string, pos: P
|
||||
// = 32 + 11 = 43
|
||||
return lengthOfContentBeforeCurrentLine + pos.character;
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters action.yml `runs:` completions based on the `using:` value.
|
||||
*
|
||||
* When the user is completing keys under `runs:`:
|
||||
* - If `using: node20` is set, only show Node.js action keys
|
||||
* - If `using: composite` is set, only show composite action keys
|
||||
* - If `using: docker` is set, only show Docker action keys
|
||||
* - If `using:` is not set, show all keys but prioritize `using` first
|
||||
*/
|
||||
function filterActionRunsCompletions(values: Value[], path: TemplateToken[], root: TemplateToken): Value[] {
|
||||
// Find the runs mapping from the root
|
||||
let runsMapping: MappingToken | undefined;
|
||||
if (root instanceof MappingToken) {
|
||||
for (let i = 0; i < root.count; i++) {
|
||||
const {key, value} = root.get(i);
|
||||
if (key.toString().toLowerCase() === "runs" && value instanceof MappingToken) {
|
||||
runsMapping = value;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!runsMapping) {
|
||||
return values;
|
||||
}
|
||||
|
||||
// Check if the runs mapping is in our path (meaning we're completing inside it)
|
||||
const isInsideRuns = path.some(token => token === runsMapping);
|
||||
if (!isInsideRuns) {
|
||||
return values;
|
||||
}
|
||||
|
||||
// Also verify we're completing at the runs level, not deeper (like inside steps)
|
||||
// The runs mapping should be the last mapping in the path before the completion position
|
||||
// or the path should only have root -> runs
|
||||
let lastMappingIndex = -1;
|
||||
for (let i = path.length - 1; i >= 0; i--) {
|
||||
if (path[i] instanceof MappingToken) {
|
||||
lastMappingIndex = i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (lastMappingIndex === -1) {
|
||||
return values;
|
||||
}
|
||||
|
||||
// If the last mapping in path is not the runs mapping, we're nested deeper (e.g., inside steps)
|
||||
const lastMapping = path[lastMappingIndex];
|
||||
if (lastMapping !== runsMapping) {
|
||||
return values;
|
||||
}
|
||||
|
||||
// Get the using value from the runs mapping
|
||||
let usingValue: string | undefined;
|
||||
for (let i = 0; i < runsMapping.count; i++) {
|
||||
const {key, value} = runsMapping.get(i);
|
||||
if (key.toString().toLowerCase() === "using") {
|
||||
usingValue = value.toString();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Determine which keys to allow
|
||||
let allowedKeys: Set<string>;
|
||||
|
||||
if (!usingValue) {
|
||||
// No using value set - show all keys but prioritize "using"
|
||||
return values.map(v => {
|
||||
if (v.label.toLowerCase() === "using") {
|
||||
return {...v, sortText: "0_using"}; // Sort first
|
||||
}
|
||||
return v;
|
||||
});
|
||||
} else if (usingValue.match(/^node\d+$/i)) {
|
||||
allowedKeys = ACTION_NODE_KEYS;
|
||||
} else if (usingValue.toLowerCase() === "composite") {
|
||||
allowedKeys = ACTION_COMPOSITE_KEYS;
|
||||
} else if (usingValue.toLowerCase() === "docker") {
|
||||
allowedKeys = ACTION_DOCKER_KEYS;
|
||||
} else {
|
||||
// Unknown using value - show all
|
||||
return values;
|
||||
}
|
||||
|
||||
// Filter to only allowed keys
|
||||
return values.filter(v => allowedKeys.has(v.label.toLowerCase()));
|
||||
}
|
||||
|
||||
@@ -347,4 +347,85 @@ runs:
|
||||
expect(diagnostics).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid key combinations based on using type", () => {
|
||||
it("reports error for node20 action with steps", async () => {
|
||||
const doc = createActionDocument(`
|
||||
name: My Action
|
||||
description: Invalid - node20 with steps
|
||||
runs:
|
||||
using: node20
|
||||
main: index.js
|
||||
steps:
|
||||
- run: echo "hello"
|
||||
shell: bash
|
||||
`);
|
||||
const diagnostics = await validate(doc);
|
||||
expect(diagnostics.length).toBeGreaterThan(0);
|
||||
// Schema reports "Unexpected value 'steps'" for invalid keys
|
||||
expect(diagnostics.some(d => d.message.includes("steps"))).toBe(true);
|
||||
});
|
||||
|
||||
it("reports error for composite action with main", async () => {
|
||||
const doc = createActionDocument(`
|
||||
name: My Action
|
||||
description: Invalid - composite with main
|
||||
runs:
|
||||
using: composite
|
||||
steps:
|
||||
- run: echo "hello"
|
||||
shell: bash
|
||||
main: index.js
|
||||
`);
|
||||
const diagnostics = await validate(doc);
|
||||
expect(diagnostics.length).toBeGreaterThan(0);
|
||||
// Schema reports "Unexpected value 'main'" for invalid keys
|
||||
expect(diagnostics.some(d => d.message.includes("main"))).toBe(true);
|
||||
});
|
||||
|
||||
it("reports error for docker action with steps", async () => {
|
||||
const doc = createActionDocument(`
|
||||
name: My Action
|
||||
description: Invalid - docker with steps
|
||||
runs:
|
||||
using: docker
|
||||
image: Dockerfile
|
||||
steps:
|
||||
- run: echo "hello"
|
||||
shell: bash
|
||||
`);
|
||||
const diagnostics = await validate(doc);
|
||||
expect(diagnostics.length).toBeGreaterThan(0);
|
||||
// Schema reports "Unexpected value 'steps'" for invalid keys
|
||||
expect(diagnostics.some(d => d.message.includes("steps"))).toBe(true);
|
||||
});
|
||||
|
||||
it("reports error for docker action with main", async () => {
|
||||
const doc = createActionDocument(`
|
||||
name: My Action
|
||||
description: Invalid - docker with main
|
||||
runs:
|
||||
using: docker
|
||||
image: Dockerfile
|
||||
main: index.js
|
||||
`);
|
||||
const diagnostics = await validate(doc);
|
||||
expect(diagnostics.length).toBeGreaterThan(0);
|
||||
// Schema reports "Unexpected value 'main'" for invalid keys
|
||||
expect(diagnostics.some(d => d.message.includes("main"))).toBe(true);
|
||||
});
|
||||
|
||||
it("reports error for node20 action missing main", async () => {
|
||||
const doc = createActionDocument(`
|
||||
name: My Action
|
||||
description: Invalid - node20 without main
|
||||
runs:
|
||||
using: node20
|
||||
pre: setup.js
|
||||
`);
|
||||
const diagnostics = await validate(doc);
|
||||
expect(diagnostics.length).toBeGreaterThan(0);
|
||||
expect(diagnostics.some(d => d.message.includes("main"))).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
import {isMapping} from "@actions/workflow-parser";
|
||||
import {isActionStep} from "@actions/workflow-parser/model/type-guards";
|
||||
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
|
||||
import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token";
|
||||
import {SequenceToken} from "@actions/workflow-parser/templates/tokens/sequence-token";
|
||||
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
|
||||
import {File} from "@actions/workflow-parser/workflows/file";
|
||||
@@ -16,6 +17,31 @@ import {getOrConvertActionTemplate, getOrParseAction} from "./utils/workflow-cac
|
||||
import {validateActionReference} from "./validate-action-reference.js";
|
||||
import {ValidationConfig} from "./validate.js";
|
||||
|
||||
/**
|
||||
* Valid keys for each action type under the `runs:` section.
|
||||
* Source: https://github.com/actions/runner/blob/main/src/Runner.Worker/ActionManifestManager.cs
|
||||
*/
|
||||
const NODE_KEYS = new Set(["using", "main", "pre", "post", "pre-if", "post-if"]);
|
||||
const COMPOSITE_KEYS = new Set(["using", "steps"]);
|
||||
const DOCKER_KEYS = new Set([
|
||||
"using",
|
||||
"image",
|
||||
"args",
|
||||
"env",
|
||||
"entrypoint",
|
||||
"pre-entrypoint",
|
||||
"pre-if",
|
||||
"post-entrypoint",
|
||||
"post-if"
|
||||
]);
|
||||
|
||||
/**
|
||||
* Required keys for each action type (besides 'using').
|
||||
*/
|
||||
const NODE_REQUIRED_KEYS = ["main"];
|
||||
const COMPOSITE_REQUIRED_KEYS = ["steps"];
|
||||
const DOCKER_REQUIRED_KEYS = ["image"];
|
||||
|
||||
/**
|
||||
* Validates an action.yml file
|
||||
*
|
||||
@@ -57,6 +83,12 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
|
||||
});
|
||||
}
|
||||
|
||||
// Validate runs key combinations based on using type
|
||||
if (result.value) {
|
||||
const runsKeyDiagnostics = validateRunsKeys(result.value);
|
||||
diagnostics.push(...runsKeyDiagnostics);
|
||||
}
|
||||
|
||||
// Validate composite action steps if we have a parsed result
|
||||
if (result.value) {
|
||||
const template = getOrConvertActionTemplate(result.context, result.value, textDocument.uri, {
|
||||
@@ -102,3 +134,103 @@ function findStepsSequence(root: TemplateToken): SequenceToken | undefined {
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that the keys under `runs:` are valid for the specified `using:` type.
|
||||
*/
|
||||
function validateRunsKeys(root: TemplateToken): Diagnostic[] {
|
||||
const diagnostics: Diagnostic[] = [];
|
||||
|
||||
// Find the runs mapping from the root
|
||||
let runsMapping: MappingToken | undefined;
|
||||
if (root instanceof MappingToken) {
|
||||
for (let i = 0; i < root.count; i++) {
|
||||
const {key, value} = root.get(i);
|
||||
if (key.toString().toLowerCase() === "runs" && value instanceof MappingToken) {
|
||||
runsMapping = value;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!runsMapping) {
|
||||
return diagnostics;
|
||||
}
|
||||
|
||||
// Get the using value from the runs mapping
|
||||
let usingValue: string | undefined;
|
||||
for (let i = 0; i < runsMapping.count; i++) {
|
||||
const {key, value} = runsMapping.get(i);
|
||||
if (key.toString().toLowerCase() === "using") {
|
||||
usingValue = value.toString();
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!usingValue) {
|
||||
return diagnostics; // No using value, let schema validation handle it
|
||||
}
|
||||
|
||||
// Determine allowed keys, required keys, and action type name
|
||||
let allowedKeys: Set<string>;
|
||||
let requiredKeys: string[];
|
||||
let actionType: string;
|
||||
|
||||
if (usingValue.match(/^node\d+$/i)) {
|
||||
allowedKeys = NODE_KEYS;
|
||||
requiredKeys = NODE_REQUIRED_KEYS;
|
||||
actionType = "Node.js";
|
||||
} else if (usingValue.toLowerCase() === "composite") {
|
||||
allowedKeys = COMPOSITE_KEYS;
|
||||
requiredKeys = COMPOSITE_REQUIRED_KEYS;
|
||||
actionType = "composite";
|
||||
} else if (usingValue.toLowerCase() === "docker") {
|
||||
allowedKeys = DOCKER_KEYS;
|
||||
requiredKeys = DOCKER_REQUIRED_KEYS;
|
||||
actionType = "Docker";
|
||||
} else {
|
||||
return diagnostics; // Unknown type, let schema validation handle it
|
||||
}
|
||||
|
||||
// Get all present keys
|
||||
const presentKeys = new Set<string>();
|
||||
for (let i = 0; i < runsMapping.count; i++) {
|
||||
const {key} = runsMapping.get(i);
|
||||
presentKeys.add(key.toString().toLowerCase());
|
||||
}
|
||||
|
||||
// Check for invalid keys
|
||||
for (let i = 0; i < runsMapping.count; i++) {
|
||||
const {key} = runsMapping.get(i);
|
||||
const keyStr = key.toString().toLowerCase();
|
||||
|
||||
if (!allowedKeys.has(keyStr)) {
|
||||
diagnostics.push({
|
||||
severity: DiagnosticSeverity.Error,
|
||||
range: mapRange(key.range),
|
||||
message: `'${key.toString()}' is not valid for ${actionType} actions (using: ${usingValue})`
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Check for missing required keys
|
||||
for (const requiredKey of requiredKeys) {
|
||||
if (!presentKeys.has(requiredKey)) {
|
||||
// Find the 'using' key to report the error location
|
||||
let usingKeyRange = runsMapping.range;
|
||||
for (let i = 0; i < runsMapping.count; i++) {
|
||||
const {key} = runsMapping.get(i);
|
||||
if (key.toString().toLowerCase() === "using") {
|
||||
usingKeyRange = key.range;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
diagnostics.push({
|
||||
severity: DiagnosticSeverity.Error,
|
||||
range: mapRange(usingKeyRange),
|
||||
message: `'${requiredKey}' is required for ${actionType} actions (using: ${usingValue})`
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return diagnostics;
|
||||
}
|
||||
|
||||
@@ -267,6 +267,7 @@
|
||||
},
|
||||
"main": {
|
||||
"type": "non-empty-string",
|
||||
"required": true,
|
||||
"description": "The file that contains your action code. The runtime specified in using executes this file.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runsmain)"
|
||||
},
|
||||
"pre": {
|
||||
|
||||
Reference in New Issue
Block a user