diff --git a/actions-languageservice/src/complete.test.ts b/actions-languageservice/src/complete.test.ts index 206f8c1..5a32451 100644 --- a/actions-languageservice/src/complete.test.ts +++ b/actions-languageservice/src/complete.test.ts @@ -217,4 +217,49 @@ jobs: expect(result).not.toBeUndefined(); expect(result.map(x => x.label).sort()).toEqual(["cancel-in-progress", "group"]); }); + + it("job key", async () => { + const input = `on: push +jobs: + build: + runs-|`; + const result = await complete(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result).toHaveLength(20); + }); + + it("job key with comment afterwards", async () => { + const input = `on: push +jobs: + build: + runs-| + #`; + const result = await complete(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result).toHaveLength(20); + }); + + it("job key with other values afterwards", async () => { + const input = `on: push +jobs: + build: + runs-| + + concurrency: 'group-name'`; + const result = await complete(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result).toHaveLength(19); + }); + + it("step key without space after colon", async () => { + const input = `on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - env:| + run: echo`; + const result = await complete(...getPositionFromCursor(input)); + expect(result).toHaveLength(0); + }); }); diff --git a/actions-languageservice/src/complete.ts b/actions-languageservice/src/complete.ts index caaf767..93ab058 100644 --- a/actions-languageservice/src/complete.ts +++ b/actions-languageservice/src/complete.ts @@ -46,6 +46,15 @@ export async function complete( valueProviderConfig?: ValueProviderConfig, contextProviderConfig?: ContextProviderConfig ): Promise { + // Edge case: when completing a key like `foo:|`, do not calculate auto-completions + const charBeforePos = textDocument.getText({ + start: {line: position.line, character: position.character - 1}, + end: {line: position.line, character: position.character} + }); + if (charBeforePos === ":") { + return []; + } + // Fix the input to work around YAML parsing issues const [newDoc, newPos] = transform(textDocument, position); diff --git a/actions-languageservice/src/utils/find-token.test.ts b/actions-languageservice/src/utils/find-token.test.ts new file mode 100644 index 0000000..6ae9757 --- /dev/null +++ b/actions-languageservice/src/utils/find-token.test.ts @@ -0,0 +1,286 @@ +import {isScalar, parseWorkflow} from "@github/actions-workflow-parser/."; +import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; +import {TokenType} from "@github/actions-workflow-parser/templates/tokens/types"; +import {nullTrace} from "../nulltrace"; +import {getPositionFromCursor} from "../test-utils/cursor-position"; +import {findToken} from "./find-token"; + +type testTokenInfo = [definitionKey: string | null, tokenType: TokenType, literalValue?: string]; + +function getTokenInfo(token: TemplateToken | null): testTokenInfo | null { + if (!token) { + return null; + } + + return [ + token.definition?.key ?? null, + token.templateTokenType, + isScalar(token) ? token.toDisplayString() : undefined + ].filter(x => x !== undefined) as testTokenInfo; +} + +function testFindToken(input: string): { + parent: testTokenInfo | null; + key: testTokenInfo | null; + token: testTokenInfo | null; + parentKey: testTokenInfo | null; +} { + const [textDocument, pos] = getPositionFromCursor(input); + const result = parseWorkflow( + "wf.yaml", + [ + { + content: textDocument.getText(), + name: "wf.yaml" + } + ], + nullTrace + ); + + const r = findToken(pos, result.value); + + return { + parent: getTokenInfo(r.parent), + parentKey: getTokenInfo(r.parentKey), + key: getTokenInfo(r.keyToken), + token: getTokenInfo(r.token) + }; +} + +describe("find-token", () => { + it("on string key", () => { + expect(testFindToken(`o|n: push`)).toEqual({ + parent: ["workflow-root-strict", TokenType.Mapping], + parentKey: null, + key: null, + token: [null, TokenType.String, "on"] + }); + }); + + it("on string value", () => { + expect(testFindToken(`on: pu|sh`)).toEqual({ + parent: ["workflow-root-strict", TokenType.Mapping], + parentKey: null, + key: [null, TokenType.String, "on"], + token: ["on-strict", TokenType.String, "push"] + }); + }); + + it("on mapping", () => { + expect( + testFindToken(`on: + pu|sh:`) + ).toEqual({ + parent: ["on-mapping-strict", TokenType.Mapping], + parentKey: [null, TokenType.String, "on"], + key: null, + token: [null, TokenType.String, "push"] + }); + }); + + it("on sequence", () => { + expect( + testFindToken(`on: + - pu|sh`) + ).toEqual({ + parent: ["on-strict", TokenType.Sequence], + parentKey: null, + key: null, + token: ["non-empty-string", TokenType.String, "push"] + }); + }); + + it("on sequence with cursor outside of sequence values", () => { + expect( + testFindToken(`on: + -| push`) + ).toEqual({ + parent: ["on-strict", TokenType.Sequence], + parentKey: null, + key: null, + token: null + }); + }); + + it("on sequence with multiple values", () => { + expect( + testFindToken(`on: + - push + - pull_request|`) + ).toEqual({ + parent: ["on-strict", TokenType.Sequence], + parentKey: null, + key: null, + token: ["non-empty-string", TokenType.String, "pull_request"] + }); + }); + + it("single-line sequence with multiple values", () => { + expect( + testFindToken(`on: push +jobs: + build: + runs-on: [ubuntu-latest, self|`) + ).toEqual({ + parent: ["runs-on", TokenType.Sequence], + parentKey: null, + key: null, + token: ["non-empty-string", TokenType.String, "self"] + }); + }); + + it("jobs key", () => { + expect( + testFindToken(`on: push +jo|bs: + build:`) + ).toEqual({ + parent: ["workflow-root-strict", TokenType.Mapping], + parentKey: null, + key: null, + token: [null, TokenType.String, "jobs"] + }); + }); + + it("value in job", () => { + expect( + testFindToken(`on: push +jobs: + build: + runs-on: ubu|`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: [null, TokenType.String, "runs-on"], + token: ["runs-on", TokenType.String, "ubu"] + }); + }); + + it("key in job", () => { + expect( + testFindToken(`on: push +jobs: + build: + run|s-on: ubu`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: null, + token: [null, TokenType.String, "runs-on"] + }); + }); + + it("pos after colon in empty null mapping ", () => { + expect( + testFindToken(`on: push +jobs: + build: + continue-on-error:|`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: [null, TokenType.String, "continue-on-error"], + token: ["boolean-strategy-context", TokenType.Null, ""] + }); + }); + + it("pos after colon in empty string mapping", () => { + expect( + testFindToken(`on: push +jobs: + build: + container:|`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: [null, TokenType.String, "container"], + token: ["container", TokenType.String, ""] + }); + }); + + it("pos after colon in mapping", () => { + expect( + testFindToken(`on: push +jobs: + build: + continue-on-error:|foo`) + ).toEqual({ + parent: ["jobs", TokenType.Mapping], + parentKey: [null, TokenType.String, "jobs"], + key: ["job-id", TokenType.String, "build"], + token: ["job", TokenType.String, "continue-on-error:foo"] + }); + }); + + it("pos after mapping key", () => { + expect( + testFindToken(`on: push +jobs: + build: + continue-on-error:| foo`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: null, + key: null, + token: null + }); + }); + + it("pos at end of completed mapping key", () => { + expect( + testFindToken(`on: push +jobs: + build: + continue-on-error|: foo`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: null, + token: [null, TokenType.String, "continue-on-error"] + }); + }); + + it("pos in mapping key without comment", () => { + expect( + testFindToken(`on: push +jobs: + build: + runs-|`) + ).toEqual({ + parent: ["jobs", TokenType.Mapping], + parentKey: [null, TokenType.String, "jobs"], + key: ["job-id", TokenType.String, "build"], + token: ["job", TokenType.String, "runs-"] + }); + }); + + it("pos in mapping key before comment", () => { + expect( + testFindToken(`on: push +jobs: + build: + runs-| + #`) + ).toEqual({ + parent: ["jobs", TokenType.Mapping], + parentKey: [null, TokenType.String, "jobs"], + key: ["job-id", TokenType.String, "build"], + token: ["job", TokenType.String, "runs-"] + }); + }); + + it("empty node", () => { + expect( + testFindToken(`on: push +jobs: + build: + concurrency: + runs-on: ubu|`) + ).toEqual({ + parent: ["job-factory", TokenType.Mapping], + parentKey: ["job-id", TokenType.String, "build"], + key: [null, TokenType.String, "runs-on"], + token: ["runs-on", TokenType.String, "ubu"] + }); + }); +}); diff --git a/actions-languageservice/src/utils/find-token.ts b/actions-languageservice/src/utils/find-token.ts index 752a8c2..6a8502a 100644 --- a/actions-languageservice/src/utils/find-token.ts +++ b/actions-languageservice/src/utils/find-token.ts @@ -17,6 +17,18 @@ export type TokenResult = { parentKey: TemplateToken | null; }; +/** + * Find a token at the given position in the document. + * + * If the position is within + * - the key of a mapping, parent will be the mapping, keyToken will be null, and token will be the key. + * - the value of a mapping, parent will be the mapping, keyToken will be the key for the value, and token will be the value + * - a sequence item, parent will be the sequence, keyToken will be null, and token will be the item + * + * @param pos Position within the document for which to find a token + * @param root Root node + * @returns Token result + */ export function findToken(pos: Position, root?: TemplateToken): TokenResult { if (!root) { return { @@ -58,41 +70,31 @@ export function findToken(pos: Position, root?: TemplateToken): TokenResult { for (let i = 0; i < mappingToken.count; i++) { const {key, value} = mappingToken.get(i); - if (onSameLine(pos, key, value)) { - if (posInToken(pos, key)) { - if (key.range!.end[1] + 1 === value.range!.start[1]) { - // There's no space between the key and value, this is not valid - return { - token: null, - keyToken: null, - parent: null, - parentKey: null - }; - } + // If the position is within the key, immediately return it as the token. + if (posInToken(pos, key)) { + return { + parent: mappingToken, + keyToken: null, + token: key, + parentKey: keyToken + }; + } - return { - token: key, - keyToken: null, - parent: mappingToken, - parentKey: keyToken - }; - } - - // Empty nodes positions won't always match the cursor, so check if we're on the same line - if (emptyNode(value)) { - return { - token: value, - keyToken: null, - parent: key, - parentKey: keyToken - }; - } + // If pos, key, and value are on the same line, and value is an empty node (null, empty string) return early + // we cannot reliably check the position in that empty node + if (onSameLine(pos, key, value) && emptyNode(value)) { + return { + parent: mappingToken, + keyToken: key, + token: value, + parentKey: keyToken + }; } s.push({ - token: value, - keyToken: key, parent: mappingToken, + keyToken: key, + token: value, parentKey: keyToken }); } @@ -102,9 +104,9 @@ export function findToken(pos: Position, root?: TemplateToken): TokenResult { const sequenceToken = token as SequenceToken; for (let i = 0; i < sequenceToken.count; i++) { s.push({ - token: sequenceToken.get(i), - keyToken: null, parent: sequenceToken, + keyToken: null, + token: sequenceToken.get(i), parentKey: null }); } diff --git a/actions-languageservice/src/utils/transform.test.ts b/actions-languageservice/src/utils/transform.test.ts index e9b2339..ba4d85d 100644 --- a/actions-languageservice/src/utils/transform.test.ts +++ b/actions-languageservice/src/utils/transform.test.ts @@ -13,6 +13,18 @@ jobs: expect(newPos.character).toEqual(11); }); + it("adds : at end of line with trailing comment", () => { + const [doc, pos] = getPositionFromCursor("on: push\njobs:\n build:\n runs-on|\n#"); + const [newDoc, newPos] = transform(doc, pos); + + expect(newDoc.getText()).toEqual(`on: push +jobs: + build: + runs-on: +#`); + expect(newPos.character).toEqual(11); + }); + it("adds placeholder node in empty sequence", () => { const [doc, pos] = getPositionFromCursor(`on: push jobs: diff --git a/actions-languageservice/src/utils/transform.ts b/actions-languageservice/src/utils/transform.ts index 4f8591e..015f709 100644 --- a/actions-languageservice/src/utils/transform.ts +++ b/actions-languageservice/src/utils/transform.ts @@ -1,4 +1,5 @@ import {Position, TextDocument} from "vscode-languageserver-textdocument"; +import {Range} from "vscode-languageserver-types"; const DUMMY_KEY = "dummy"; @@ -6,10 +7,21 @@ const DUMMY_KEY = "dummy"; // Based on `_transform` in https://github.com/cschleiden/github-actions-parser/blob/main/src/lib/parser/complete.ts#L311 export function transform(doc: TextDocument, pos: Position): [TextDocument, Position] { let offset = doc.offsetAt(pos); - let line = doc.getText({ + + const lineRange: Range = { start: {line: pos.line, character: 0}, end: {line: pos.line, character: Number.MAX_SAFE_INTEGER} - }); + }; + + let line = doc.getText(lineRange); + + // If the line includes a new-line char, strip that out + const newLinePos = line.indexOf("\n"); + if (newLinePos >= 0) { + line = line.substring(0, newLinePos); + } + lineRange.end.character = line.length; + const linePos = pos.character; // Special case for Actions, if this line contains an expression marker, do _not_ transform. This is @@ -47,7 +59,7 @@ export function transform(doc: TextDocument, pos: Position): [TextDocument, Posi newDoc, [ { - range: {start: {line: pos.line, character: 0}, end: {line: pos.line, character: Number.MAX_SAFE_INTEGER}}, + range: lineRange, text: line } ],