diff --git a/languageservice/src/complete.test.ts b/languageservice/src/complete.test.ts index 756c138..7213dad 100644 --- a/languageservice/src/complete.test.ts +++ b/languageservice/src/complete.test.ts @@ -494,12 +494,15 @@ jobs: expect(result.filter(x => x.label === "run-name").map(x => x.textEdit?.newText)).toEqual(["run-name: "]); }); - it("adds new line for nested mapping", async () => { + it("does not show mapping keys when user has started typing a scalar value", async () => { + // User typed `workflow_dispatch: in` - they've committed to a scalar value + // Should not show mapping keys like `inputs` const input = "on:\n workflow_dispatch: in|"; const result = await complete(...getPositionFromCursor(input)); - expect(result.filter(x => x.label === "inputs").map(x => x.textEdit?.newText)).toEqual(["\n inputs:\n "]); + // No mapping keys should be shown since user started typing a scalar + expect(result.filter(x => x.label === "inputs")).toEqual([]); }); it("adds : for one-of", async () => { @@ -510,40 +513,59 @@ jobs: expect(result.filter(x => x.label === "types").map(x => x.textEdit?.newText)).toEqual(["types: "]); }); - it("adds newline and indentation for one-of in key mode", async () => { + it("does not show mapping keys for one-of when user has typed a scalar value", async () => { + // User typed `check_run: ty` - they've committed to scalar form + // The only valid value for check_run scalar is null, so no completions const input = "on:\n check_run: ty|"; const result = await complete(...getPositionFromCursor(input)); - // When completing a one-of property in key mode (after colon on same line), - // insert newline + indentation + key + colon to create valid YAML structure - expect(result.filter(x => x.label === "types").map(x => x.textEdit?.newText)).toEqual(["\n types: "]); + // check_run's scalar form only accepts null, so typing anything should show no completions + // (we don't show mapping keys like `types` anymore - user should use `check_run (full syntax)` instead) + expect(result.filter(x => x.label === "types")).toEqual([]); }); - it("handles mixed string and mapping completions for one-of in key mode", async () => { + it("shows all options for one-of when user hasn't committed to a type yet", async () => { + // At `permissions: |` user hasn't typed anything yet - show all options const input = "on: push\npermissions: |"; const result = await complete(...getPositionFromCursor(input)); - // String values (read-all, write-all) should insert directly without newline + // String values (read-all, write-all) should be available expect(result.filter(x => x.label === "read-all").map(x => x.textEdit?.newText)).toEqual(["read-all"]); expect(result.filter(x => x.label === "write-all").map(x => x.textEdit?.newText)).toEqual(["write-all"]); - // Mapping keys with one-of types should insert with newline and indentation + // Mapping keys should also be available (user hasn't committed yet) expect(result.filter(x => x.label === "actions").map(x => x.textEdit?.newText)).toEqual(["\n actions: "]); expect(result.filter(x => x.label === "contents").map(x => x.textEdit?.newText)).toEqual(["\n contents: "]); }); - it("shows both simple and full syntax for null+mapping one-of", async () => { - // check_run is a one-of: [null, mapping]. Show both: - // - check_run (simple, just the key with colon) - // - check_run (full syntax) (ready to add mapping keys) + it("filters to scalar options when user has started typing a scalar", async () => { + // User typed `permissions: r` - they've committed to scalar form + const input = "on: push\npermissions: r|"; + + const result = await complete(...getPositionFromCursor(input)); + + // Only scalar values should be shown (filtering on 'r') + expect(result.some(x => x.label === "read-all")).toBe(true); + // Mapping keys should NOT be shown + expect(result.filter(x => x.label === "actions")).toEqual([]); + expect(result.filter(x => x.label === "contents")).toEqual([]); + }); + + it("shows full syntax for null+mapping one-of (skips null-only scalar)", async () => { + // check_run is a one-of: [null, mapping]. + // Since the scalar form is only null (no string constants), we skip it + // to avoid clobbering string constants from elsewhere in the schema. + // User should see check_run (full syntax) for the mapping form. const input = "on:\n |"; const result = await complete(...getPositionFromCursor(input)); - // Should have both check_run and check_run (full syntax) + // Should NOT have plain check_run (null-only scalar is skipped) + // Instead, string constant check_run from on-string-strict is available expect(result.some(x => x.label === "check_run")).toBe(true); + // Full syntax variant should be available expect(result.some(x => x.label === "check_run (full syntax)")).toBe(true); }); @@ -610,4 +632,44 @@ jobs: expect(result.find(x => x.label === "runs-on (list)")?.filterText).toEqual("runs-on"); expect(result.find(x => x.label === "runs-on (full syntax)")?.filterText).toEqual("runs-on"); }); + + it("scalar event completion inserts inline without newline", async () => { + // At `on: |` user is completing the value for 'on' key + // Scalar events like `push`, `check_run` should insert inline + const input = "on: |"; + + const result = await complete(...getPositionFromCursor(input)); + + // Scalar forms should NOT have newline - they insert inline + const push = result.find(x => x.label === "push"); + expect(push?.textEdit?.newText).toEqual("push"); + + const checkRun = result.find(x => x.label === "check_run"); + expect(checkRun?.textEdit?.newText).toEqual("check_run"); + + // Full syntax form inserts as a mapping key (with newline in Key mode) + // This is expected behavior - it starts the mapping form + const checkRunFull = result.find(x => x.label === "check_run (full syntax)"); + // In Key mode: \n + indent + key + : + \n + indent + indent (for nested content) + expect(checkRunFull?.textEdit?.newText).toEqual("\n check_run:\n "); + }); + + it("filters to sequence options when user has started a sequence", async () => { + // User started a sequence with `- ` syntax - they've committed to sequence form + const input = `on: push +jobs: + build: + runs-on: + - |`; + + const result = await complete(...getPositionFromCursor(input)); + + // Should show runner labels (sequence item values) + expect(result.some(x => x.label === "ubuntu-latest")).toBe(true); + expect(result.some(x => x.label === "macos-latest")).toBe(true); + + // Should NOT show mapping keys like `group` or `labels` (those are for full syntax) + expect(result.filter(x => x.label === "group")).toEqual([]); + expect(result.filter(x => x.label === "labels")).toEqual([]); + }); }); diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index 96ea3b0..7c93963 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -24,7 +24,7 @@ import {isPlaceholder, transform} from "./utils/transform.js"; import {fetchOrConvertWorkflowTemplate, fetchOrParseWorkflow} from "./utils/workflow-cache.js"; import {Value, ValueProviderConfig} from "./value-providers/config.js"; import {defaultValueProviders} from "./value-providers/default.js"; -import {DefinitionValueMode, definitionValues} from "./value-providers/definition.js"; +import {DefinitionValueMode, definitionValues, TokenStructure} from "./value-providers/definition.js"; export function getExpressionInput(input: string, pos: number): string { // Find start marker around the cursor position @@ -143,6 +143,17 @@ export async function complete( }); } +/** + * Retrieves completion values for a token based on value providers and definitions. + * + * This function determines which values to suggest for auto-completion by: + * 1. First checking for custom value providers configured for the token's definition key + * 2. Then checking for default value providers for the token's definition key + * 3. Finally falling back to values derived from the token's schema definition + * + * The results are filtered to exclude duplicates (e.g., keys already defined in a mapping + * or values already present in a sequence) and sorted alphabetically. + */ async function getValues( token: TemplateToken | null, keyToken: TemplateToken | null, @@ -182,10 +193,75 @@ async function getValues( return []; } - const values = definitionValues(def, indentation, keyToken ? DefinitionValueMode.Key : DefinitionValueMode.Parent); + // When a schema allows multiple formats (e.g., `runs-on` can be a string OR a mapping), + // only suggest completions that match what the user has already started typing. + // For example, if they've started a mapping, don't suggest string values. + const tokenStructure = getTokenStructure(token); + const values = definitionValues( + def, + indentation, + keyToken ? DefinitionValueMode.Key : DefinitionValueMode.Parent, + tokenStructure + ); return filterAndSortCompletionOptions(values, existingValues); } +/** + * Determines what YAML structure the user has committed to, if any. + * + * Returns: + * - "mapping" if the user has started a key-value structure (e.g., `runs-on:\n group: |`) + * - "sequence" if the user has started a list (e.g., `runs-on:\n - |`) + * - "scalar" if the user has started typing a plain value (e.g., `runs-on: ubuntu-|`) + * - undefined if the user hasn't committed yet (e.g., `runs-on: |` with nothing typed) + */ +function getTokenStructure(token: TemplateToken | null): TokenStructure { + if (!token) { + return undefined; + } + + switch (token.templateTokenType) { + case TokenType.Mapping: + return "mapping"; + case TokenType.Sequence: + return "sequence"; + case TokenType.Null: + // Null means `key: ` with nothing - user hasn't committed to a type yet + return undefined; + case TokenType.String: { + // Empty string means `key: |` - user hasn't committed yet + // Non-empty string means user has started typing a scalar value + const stringToken = token.assertString("getTokenStructure expected string token"); + if (stringToken.value === "") { + return undefined; + } + return "scalar"; + } + case TokenType.Boolean: + case TokenType.Number: + return "scalar"; + default: + return undefined; + } +} + +/** + * Collects values that are already present in the current context, so they can be + * excluded from completion suggestions. + * + * For sequences (lists), returns all existing items. For example, if the user has: + * labels: + * - bug + * - | + * This returns {"bug"} so we don't suggest "bug" again. + * + * For mappings, returns all existing keys. For example, if the user has: + * jobs: + * build: + * runs-on: ubuntu-latest + * | + * This returns {"runs-on"} so we don't suggest "runs-on" again. + */ export function getExistingValues(token: TemplateToken | null, parent: TemplateToken) { // For incomplete YAML, we may only have a parent token if (token) { diff --git a/languageservice/src/value-providers/definition.ts b/languageservice/src/value-providers/definition.ts index a37ff3e..1765a9b 100644 --- a/languageservice/src/value-providers/definition.ts +++ b/languageservice/src/value-providers/definition.ts @@ -1,3 +1,4 @@ +import {NullDefinition} from "@actions/workflow-parser/templates/schema/null-definition"; import {BooleanDefinition} from "@actions/workflow-parser/templates/schema/boolean-definition"; import {Definition} from "@actions/workflow-parser/templates/schema/definition"; import {DefinitionType} from "@actions/workflow-parser/templates/schema/definition-type"; @@ -24,7 +25,35 @@ export enum DefinitionValueMode { Key } -export function definitionValues(def: Definition, indentation: string, mode: DefinitionValueMode): Value[] { +/** + * What YAML structure the user has started typing. + * Used to filter completions - e.g., if user started a mapping, don't show string completions. + */ +export type TokenStructure = "scalar" | "sequence" | "mapping" | undefined; + +/** + * Generates completion values from a workflow schema definition. + * + * This is the fallback when no custom or default value provider exists for a token. + * It reads the schema definition to determine what values are valid. + * + * Examples: + * - For a job definition this returns keys like "runs-on", "steps", "env", "timeout-minutes", etc. + * - For `shell: |`, the schema says it's a string with no constants, + * so this returns no completions + * - For `continue-on-error: |` on a step, the schema has a boolean definition, + * so this returns ["true", "false"] + * + * @param tokenStructure - If provided, filters completions to only those matching + * the YAML structure the user has already started (e.g., only mapping keys if + * they've started a mapping) + */ +export function definitionValues( + def: Definition, + indentation: string, + mode: DefinitionValueMode, + tokenStructure?: TokenStructure +): Value[] { const schema = getWorkflowSchema(); if (def instanceof MappingDefinition) { @@ -32,7 +61,7 @@ export function definitionValues(def: Definition, indentation: string, mode: Def } if (def instanceof OneOfDefinition) { - return oneOfValues(def, schema.definitions, indentation, mode); + return oneOfValues(def, schema.definitions, indentation, mode, tokenStructure); } if (def instanceof BooleanDefinition) { @@ -58,6 +87,16 @@ export function definitionValues(def: Definition, indentation: string, mode: Def return []; } +/** + * Returns completion items for keys in a mapping (object). + * + * For example, given the job definition, this returns completions for + * "runs-on", "steps", "env", etc. Each completion includes appropriate + * insert text based on the expected value type: + * - Sequence properties insert `key:\n - ` to start a list + * - Mapping properties insert `key:\n ` to start nested keys + * - Scalar properties insert `key: ` for inline values + */ function mappingValues( mappingDefinition: MappingDefinition, definitions: {[key: string]: Definition}, @@ -123,15 +162,43 @@ function mappingValues( return properties; } +/** + * Returns completions for values that can be one of several types. + * + * For example, `on:` can be a string ("push"), a list (["push", "pull_request"]), + * or a mapping with event configuration. This function collects completions from + * all valid variants. + * + * If the user has already started typing a specific structure (e.g., started a list), + * only completions for that structure are returned. + */ function oneOfValues( oneOfDefinition: OneOfDefinition, definitions: {[key: string]: Definition}, indentation: string, - mode: DefinitionValueMode + mode: DefinitionValueMode, + tokenStructure?: TokenStructure ): Value[] { const values: Value[] = []; for (const key of oneOfDefinition.oneOf) { - values.push(...definitionValues(definitions[key], indentation, mode)); + const variantDef = definitions[key]; + + // Should never happen - the schema should always have valid references + if (!variantDef) { + continue; + } + + // Skip variants that don't match what the user has already started typing. + // For example, if user is at `runs-on:\n |` (inside a mapping), skip the string + // variant - only include the mapping variant that suggests keys like "group" or "labels". + if (tokenStructure) { + const variantBucket = getStructuralBucket(variantDef.definitionType); + if (variantBucket !== tokenStructure) { + continue; + } + } + + values.push(...definitionValues(variantDef, indentation, mode, tokenStructure)); } return distinctValues(values); } @@ -167,8 +234,15 @@ function getStructuralBucket(defType: DefinitionType): StructuralBucket { } /** - * Expand a one-of definition into multiple completion items based on structural types. - * Returns one completion per unique structural type (scalar, sequence, mapping). + * Creates completion items for a key whose value can be multiple formats. + * + * For example, `runs-on` can be a string, list, or mapping. This function creates + * separate completions for each format: + * - "runs-on" for the string form (`runs-on: ubuntu-latest`) + * - "runs-on (list)" for the list form (`runs-on:\n - ubuntu-latest`) + * - "runs-on (full syntax)" for the mapping form (`runs-on:\n group: my-group`) + * + * The qualifier (list/full syntax) is only added when multiple formats exist. */ function expandOneOfToCompletions( oneOfDef: OneOfDefinition, @@ -185,11 +259,19 @@ function expandOneOfToCompletions( mapping: false }; + // Track if scalar bucket only contains null (no actual string/boolean/number values) + let scalarIsOnlyNull = true; + for (const variantKey of oneOfDef.oneOf) { const variantDef = definitions[variantKey]; if (variantDef) { const bucket = getStructuralBucket(variantDef.definitionType); buckets[bucket] = true; + + // Check if this scalar is NOT null + if (bucket === "scalar" && !(variantDef instanceof NullDefinition)) { + scalarIsOnlyNull = false; + } } } @@ -201,8 +283,15 @@ function expandOneOfToCompletions( // Emit completions in order: scalar, sequence, mapping // Use sortText to preserve this order (scalar sorts first, then 1=sequence, 2=mapping) - if (buckets.scalar) { - // In Key mode, insert newline and indentation to produce valid YAML structure + // + // In Key mode (after colon on same line), skip the key completion if scalar only allows null. + // Example: at `on: |`, we want `check_run` to insert inline, not start a new mapping. + // + // In Parent mode (typing a new key), we DO show it since `check_run:` with no value + // is valid (triggers on all check_run events). + const skipNullOnlyScalar = mode === DefinitionValueMode.Key && scalarIsOnlyNull; + if (buckets.scalar && !skipNullOnlyScalar) { + // If cursor is after colon (`on: |`), insert newline first so result is `on:\n check_run: ` const insertText = mode === DefinitionValueMode.Key ? `\n${indentation}${key}: ` : `${key}: `; results.push({ label: key,