diff --git a/actions-languageservice/src/context-providers/github.ts b/actions-languageservice/src/context-providers/github.ts index af43d20..df3d72e 100644 --- a/actions-languageservice/src/context-providers/github.ts +++ b/actions-languageservice/src/context-providers/github.ts @@ -99,7 +99,6 @@ function getEventContext(workflowContext: WorkflowContext): ExpressionData { function merge(d: data.Dictionary, toAdd: Object): data.Dictionary { for (const [key, value] of Object.entries(toAdd)) { if (value && typeof value === "object" && !d.get(key)) { - if (!Array.isArray(value) && Object.entries(value).length === 0) { // Allow an empty object to be any value d.add(key, new data.Null()); diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index a1c172c..ac25ff8 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -96,6 +96,39 @@ jobs: expect(result?.contents).toEqual("Runs your workflow when you push a commit or tag."); }); + it("on a cron schedule", async () => { + const input = `on: + schedule: + - cron: '0,30 0|,12 * * *' +`; + const result = await hover(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual( + "Runs at 0 and 30 minutes past the hour, at 00:00 and 12:00\n\n" + + "Actions schedules run at most every 5 minutes. " + + "[Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)" + ); + }); + + it("on a cron mapping key", async () => { + const input = `on: + schedule: + - c|ron: '0 0 * * *' +`; + const result = await hover(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual(""); + }); + + it("on an invalid cron schedule", async () => { + const input = `on: + schedule: + - cron: '0 0 |* * * * *' +`; + const result = await hover(...getPositionFromCursor(input)); + expect(result).not.toBeUndefined(); + expect(result?.contents).toEqual(""); + }); it("shows context inherited from parent nodes", async () => { const input = ` diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index e7bac56..b24536a 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -1,7 +1,10 @@ import {convertWorkflowTemplate, parseWorkflow, ParseWorkflowResult} from "@github/actions-workflow-parser"; import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert"; import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token"; +import {TokenResult} from "./utils/find-token"; import {File} from "@github/actions-workflow-parser/workflows/file"; +import {isString} from "@github/actions-workflow-parser/templates/tokens/type-guards"; +import {StringToken} from "@github/actions-workflow-parser/templates/tokens/string-token"; import {Position, TextDocument} from "vscode-languageserver-textdocument"; import {Hover} from "vscode-languageserver-types"; import {getWorkflowContext, WorkflowContext} from "./context/workflow-context"; @@ -9,6 +12,7 @@ import {info} from "./log"; import {nullTrace} from "./nulltrace"; import {findToken} from "./utils/find-token"; import {mapRange} from "./utils/range"; +import {getCronDescription} from "@github/actions-workflow-parser/model/converter/cron"; export type DescriptionProvider = { getDescription(context: WorkflowContext, token: TemplateToken, path: TemplateToken[]): Promise; @@ -26,14 +30,26 @@ export async function hover(document: TextDocument, position: Position, config?: }; const result = parseWorkflow(file.name, [file], nullTrace); - const {token, path} = findToken(position, result.value); + const tokenResult = findToken(position, result.value); + const token = tokenResult.token; if (!token?.definition) { return null; } info(`Calculating hover for token with definition ${token.definition.key}`); - let description = await getDescription(document, config, result, token, path); + if (tokenResult.parent && isCronMappingValue(tokenResult)) { + const tokenValue = (token as StringToken).value + let description = getCronDescription(tokenValue); + if (description) { + return { + contents: description, + range: mapRange(token.range) + } as Hover; + } + } + + let description = await getDescription(document, config, result, token, tokenResult.path); const allowedContext = token.definitionInfo?.allowedContext; if (allowedContext && allowedContext?.length > 0) { @@ -64,3 +80,9 @@ async function getDescription( const description = await config.descriptionProvider.getDescription(workflowContext, token, path); return description || defaultDescription; } + +function isCronMappingValue(tokenResult: TokenResult): boolean { + return tokenResult.parent?.definition?.key === "cron-mapping" && + isString(tokenResult.token!) && + tokenResult.token.value !== "cron" +} diff --git a/actions-languageservice/src/validate.expressions.test.ts b/actions-languageservice/src/validate.expressions.test.ts index 9d05070..57a0eaa 100644 --- a/actions-languageservice/src/validate.expressions.test.ts +++ b/actions-languageservice/src/validate.expressions.test.ts @@ -1248,7 +1248,7 @@ jobs: runs-on: ubuntu-latest steps: - run: echo \${{ github.event.client_payload.anything }} - - run: echo \${{ github.event.client_payload.branch }}` + - run: echo \${{ github.event.client_payload.branch }}`; const result = await validate(createDocument("wf.yaml", input)); diff --git a/actions-workflow-parser/src/model/converter/cron-constants.ts b/actions-workflow-parser/src/model/converter/cron-constants.ts new file mode 100644 index 0000000..f24a52f --- /dev/null +++ b/actions-workflow-parser/src/model/converter/cron-constants.ts @@ -0,0 +1,32 @@ +// Constants for parsing and validating cron expressions + +const MONTHS = { + jan: 1, + feb: 2, + mar: 3, + apr: 4, + may: 5, + jun: 6, + jul: 7, + aug: 8, + sep: 9, + oct: 10, + nov: 11, + dec: 12 +}; + +const DAYS = { + sun: 0, + mon: 1, + tue: 2, + wed: 3, + thu: 4, + fri: 5, + sat: 6 +}; + +export const MINUTE_RANGE = {min: 0, max: 59}; +export const HOUR_RANGE = {min: 0, max: 23}; +export const DOM_RANGE = {min: 1, max: 31}; +export const MONTH_RANGE = {min: 1, max: 12, names: MONTHS}; +export const DOW_RANGE = {min: 0, max: 6, names: DAYS}; diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts index d407558..18075a9 100644 --- a/actions-workflow-parser/src/model/converter/cron.test.ts +++ b/actions-workflow-parser/src/model/converter/cron.test.ts @@ -1,63 +1,79 @@ -import { isValidCron } from "./cron" +import {isValidCron, getCronDescription} from "./cron"; -describe("isValidCron", () => { - const valid = [ - ["0 0 * * *", "every day at midnight"], - ["0 000 001 * *", "accepts leading zeros"], - ["15 * * * *", "accepts numbers in range"], - ["2,10 4,5 * * *", "accepts comma separated values"], - ["30 4-6 * * *", "accepts range"], - ["0 4-4 * * *", "accepts range with two equal values"], - ["20/15 * * * *", "accepts step with numerical values"], - ["30 5,17 * * *", "accepts numbers and ranges"], - ["28 */4 * * *", "accepts step with * and numerical value"], - ["28 5,*/4 * * *", "accepts comma separated value with step"], - ["28 5,*/4,6-8 * * *", "accepts comma separated value with step and range"], - ["0 0 * * SUN", "accepts day of week short name"], - ["0 0 * * SUN-TUE", "accepts day of week short name range"], - ["0 0 * * SUN-2", "accepts day of week range combined with number"], - ["0 2-4/5 * * *", "accepts range with step"], - ["0 0 * * *", "accepts multiple spaces"], - ] +describe("cron", () => { + describe("valid cron", () => { + const valid = [ + ["0 0 * * *", "every day at midnight"], + ["0 000 001 * *", "accepts leading zeros"], + ["15 * * * *", "accepts numbers in range"], + ["2,10 4,5 * * *", "accepts comma separated values"], + ["30 4-6 * * *", "accepts range"], + ["0 4-4 * * *", "accepts range with two equal values"], + ["20/15 * * * *", "accepts step with numerical values"], + ["30 5,17 * * *", "accepts numbers and ranges"], + ["28 */4 * * *", "accepts step with * and numerical value"], + ["28 5,*/4 * * *", "accepts comma separated value with step"], + ["28 5,*/4,6-8 * * *", "accepts comma separated value with step and range"], + ["0 0 * * SUN", "accepts day of week short name"], + ["0 0 * * SUN-TUE", "accepts day of week short name range"], + ["0 0 * * SUN-2", "accepts day of week range combined with number"], + ["0 2-4/5 * * *", "accepts range with step"], + ["0 0 * * *", "accepts multiple spaces"] + ]; - for (const [cron, reason] of valid) { - it(`${cron} should be valid: ${reason}`, () => { - expect(isValidCron(cron)).toBe(true) - }) - } + for (const [cron, reason] of valid) { + it(`${cron} should be valid: ${reason}`, () => { + expect(isValidCron(cron)).toBe(true); + }); + } + }); - const invalid = [ - ["0 0 * *", "too few parts"], - ["0 0 * * * * *", "too many parts"], - ["0 -1 * * *", "should not accept negative numbers"], - ["0 1- * * *", "should not accept trailing -"], - ["0 /1 * * *", "should not accept leading / (empty value)"], - ["0 1/ * * *", "should not accept trailing / (empty value)"], - ["0 ,1 * * *", "should not accept leading , (empty value)"], - ["0 1, * * *", "should not accept trailing , (empty value)"], - ["0 5--5 * * *", "should not accept multiple -"], - ["0 *//5 * * *", "should not accept multiple /"], - ["0 */* * * *", "step start and size may not both be *"], - ["0 5/* * * *", "steps size may not be *"], - ["0 *-4 5-* *-* *", "range may not contain *"], - ["0 *//5 * * *", "should not accept multiple /"], - ["0 ,, * * *", "should not accept multiple ,"], - [", , , , ,", "comma is not a valid part"], - ["0 ** * * *", "should not accept multiple *"], - ["0 0 * * BUN", "invalid short name"], - ["0 0 * SUN JAN", "short name in incorrect position"], - ["0 0 * * FRI-TUE", "should not accept short name range with start > end"], - ["0 12-4 * * *", "should not accept nuerical range with start > end"], - ["0 */0 * * *", "step size may not be 0"], - ["0 2/4-5 * * *", "step size may not be a range"], - ["0 2-4-6 * * *", "range may not contain multiple -"], - ["0 2/4/6 * * *", "step may not contain multiple /"], - ["0 * * */FEB */TUE", "step size may not be a short name"], - ] + describe("invalid cron", () => { + const invalid = [ + ["0 0 * *", "too few parts"], + ["0 0 * * * * *", "too many parts"], + ["0 -1 * * *", "should not accept negative numbers"], + ["0 1- * * *", "should not accept trailing -"], + ["0 /1 * * *", "should not accept leading / (empty value)"], + ["0 1/ * * *", "should not accept trailing / (empty value)"], + ["0 ,1 * * *", "should not accept leading , (empty value)"], + ["0 1, * * *", "should not accept trailing , (empty value)"], + ["0 5--5 * * *", "should not accept multiple -"], + ["0 *//5 * * *", "should not accept multiple /"], + ["0 */* * * *", "step start and size may not both be *"], + ["0 5/* * * *", "steps size may not be *"], + ["0 *-4 5-* *-* *", "range may not contain *"], + ["0 ,, * * *", "should not accept multiple ,"], + [", , , , ,", "comma is not a valid part"], + ["0 ** * * *", "should not accept multiple *"], + ["0 0 * * BUN", "invalid short name"], + ["0 0 * SUN JAN", "short name in incorrect position"], + ["0 0 * * FRI-TUE", "should not accept short name range with start > end"], + ["0 12-4 * * *", "should not accept nuerical range with start > end"], + ["0 */0 * * *", "step size may not be 0"], + ["0 2/4-5 * * *", "step size may not be a range"], + ["0 2-4-6 * * *", "range may not contain multiple -"], + ["0 2/4/6 * * *", "step may not contain multiple /"], + ["0 * * */FEB */TUE", "step size may not be a short name"] + ]; - for (const [cron, reason] of invalid) { - it(`${cron} should be invalid: ${reason}`, () => { - expect(isValidCron(cron)).toBe(false) - }) - } -}) \ No newline at end of file + for (const [cron, reason] of invalid) { + it(`${cron} should be invalid: ${reason}`, () => { + expect(isValidCron(cron)).toBe(false); + }); + } + }); + + describe("getCronDescription", () => { + it(`Produces a sentence for valid cron`, () => { + expect(getCronDescription("0 * * * *")).toEqual( + "Runs every hour\n\n" + + "Actions schedules run at most every 5 minutes. [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)" + ); + }); + + it(`Returns nothing for invalid cron`, () => { + expect(getCronDescription("* * * * * *")).toBeUndefined(); + }); + }); +}); diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index aa46ca5..b104eed 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -1,118 +1,115 @@ -const MONTHS = { - jan: 1, - feb: 2, - mar: 3, - apr: 4, - may: 5, - jun: 6, - jul: 7, - aug: 8, - sep: 9, - oct: 10, - nov: 11, - dec: 12, -} +import cronstrue from "cronstrue"; -const DAYS = { - sun: 0, - mon: 1, - tue: 2, - wed: 3, - thu: 4, - fri: 5, - sat: 6, -} +import {MONTH_RANGE, HOUR_RANGE, MINUTE_RANGE, DOM_RANGE, DOW_RANGE} from "./cron-constants"; + +type Range = { + min: number; + max: number; + names?: Record; +}; -// TODO: make this parseCron export function isValidCron(cron: string): boolean { // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule - const parts = cron.split(/ +/) + const parts = cron.split(/ +/); if (parts.length != 5) { - return false + return false; } - - const [minutes, hours, dom, months, dow] = parts + const [minutes, hours, dom, months, dow] = parts; return ( - validateRange(minutes, { min: 0, max: 59 }) && - validateRange(hours, { min: 0, max: 23 }) && - validateRange(dom, { min: 1, max: 31 }) && - validateRange(months, { min: 1, max: 12, names: MONTHS }) && - validateRange(dow, { min: 0, max: 6, names: DAYS }) - ) + validateCronPart(minutes, MINUTE_RANGE) && + validateCronPart(hours, HOUR_RANGE) && + validateCronPart(dom, DOM_RANGE) && + validateCronPart(months, MONTH_RANGE) && + validateCronPart(dow, DOW_RANGE) + ); } -type Range = { - min: number - max: number - names?: Record +export function getCronDescription(cronspec: string): string | undefined { + if (!isValidCron(cronspec)) { + return; + } + + let desc = ""; + try { + desc = cronstrue.toString(cronspec, { + dayOfWeekStartIndexZero: true, + monthStartIndexZero: false, + use24HourTimeFormat: true, + // cronstrue sets the description as the error if throwExceptionOnParseError is false + // so we need to distinguish between an error and a valid description + throwExceptionOnParseError: true + }); + } catch (err) { + return; + } + + // Make first character lowercase + let result = "Runs " + desc.charAt(0).toLowerCase() + desc.slice(1); + result += + "\n\nActions schedules run at most every 5 minutes." + + " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"; + return result; } -function validateRange( - value: string, - range: Range, - allowSeparators = true -): boolean { +function validateCronPart(value: string, range: Range, allowSeparators = true): boolean { if (range.names && range.names[value.toLowerCase()] !== undefined) { - return true + return true; } if (value === "*") { - return true + return true; } // Operator precedence: , > / > - if (value.includes(",")) { if (!allowSeparators) { - return false + return false; } - return value.split(",").every((v) => v && validateRange(v, range)) + return value.split(",").every(v => v && validateCronPart(v, range)); } if (value.includes("/")) { if (!allowSeparators) { - return false + return false; } - const [start, step, ...rest] = value.split("/") - const stepNumber = +step + const [start, step, ...rest] = value.split("/"); + const stepNumber = +step; if (rest.length > 0 || isNaN(stepNumber) || stepNumber <= 0 || !start || !step) { - return false + return false; } // Separators are only allowed in the part before the `/`, e.g. `1-5/2` - return ( - validateRange(start, range) && validateRange(step, range, false) - ) + return validateCronPart(start, range) && validateCronPart(step, range, false); } if (value.includes("-")) { if (!allowSeparators) { - return false + return false; } - const [start, end, ...rest] = value.split("-") + + const [start, end, ...rest] = value.split("-"); if (rest.length > 0 || !start || !end) { - return false + return false; } // Convert name to integers so we can make sure end >= start - const startNumber = convertToNumber(range, start) - const endNumber = convertToNumber(range, end) - return ( - validateRange(start, range, false) && validateRange(end, range, false) && endNumber >= startNumber - ) + const startNumber = convertToNumber(start, range.names); + const endNumber = convertToNumber(end, range.names); + return validateCronPart(start, range, false) && validateCronPart(end, range, false) && endNumber >= startNumber; } - const number = +value - return !isNaN(number) && number >= range.min && number <= range.max + const number = +value; + return !isNaN(number) && number >= range.min && number <= range.max; } -function convertToNumber(range: Range, value: string): number { - if (range.names && range.names[value.toLowerCase()] !== undefined) { - return +range.names[value.toLowerCase()] +// Converts a string integer or a short name to a number +function convertToNumber(value: string, names?: Record): number { + if (names && names[value.toLowerCase()] !== undefined) { + return +names[value.toLowerCase()]; + } else { + return +value; } - else { - return +value - } -} \ No newline at end of file +} diff --git a/actions-workflow-parser/src/model/converter/events.ts b/actions-workflow-parser/src/model/converter/events.ts index bb3e956..1db676f 100644 --- a/actions-workflow-parser/src/model/converter/events.ts +++ b/actions-workflow-parser/src/model/converter/events.ts @@ -13,7 +13,7 @@ import { TypesFilterConfig, WorkflowFilterConfig } from "../workflow-template"; -import {isValidCron} from "./cron" +import {isValidCron} from "./cron"; import {convertStringList} from "./string-list"; import {convertEventWorkflowDispatchInputs} from "./workflow-dispatch"; @@ -146,7 +146,7 @@ function convertSchedule(context: TemplateContext, token: SequenceToken): Schedu const cron = schedule.value.assertString(`schedule cron`); // Validate the cron string if (!isValidCron(cron.value)) { - context.error(cron, "Invalid cron string") + context.error(cron, "Invalid cron string"); } result.push({cron: cron.value}); } else { diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index 171ae72..f7290a2 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -226,7 +226,6 @@ class TemplateReader { } const nextValue = this.readValue(nextDefinition); - mapping.add(nextKey, nextValue); continue; } diff --git a/package-lock.json b/package-lock.json index e245549..537d268 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,9 @@ "./actions-languageserver", "./browser-playground" ], + "dependencies": { + "cronstrue": "^2.21.0" + }, "devDependencies": { "lerna": "^6.0.3" } @@ -5296,6 +5299,14 @@ "node": ">= 6" } }, + "node_modules/cronstrue": { + "version": "2.21.0", + "resolved": "https://registry.npmjs.org/cronstrue/-/cronstrue-2.21.0.tgz", + "integrity": "sha512-YxabE1ZSHA1zJZMPCTSEbc0u4cRRenjqqTgCwJT7OvkspPSvfYFITuPFtsT+VkBuavJtFv2kJXT+mKSnlUJxfg==", + "bin": { + "cronstrue": "bin/cli.js" + } + }, "node_modules/cross-spawn": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", @@ -18165,6 +18176,11 @@ } } }, + "cronstrue": { + "version": "2.21.0", + "resolved": "https://registry.npmjs.org/cronstrue/-/cronstrue-2.21.0.tgz", + "integrity": "sha512-YxabE1ZSHA1zJZMPCTSEbc0u4cRRenjqqTgCwJT7OvkspPSvfYFITuPFtsT+VkBuavJtFv2kJXT+mKSnlUJxfg==" + }, "cross-spawn": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", diff --git a/package.json b/package.json index d28e7ac..6032727 100644 --- a/package.json +++ b/package.json @@ -10,5 +10,8 @@ "devDependencies": { "lerna": "^6.0.3" }, - "name": "actions-languageservices" + "name": "actions-languageservices", + "dependencies": { + "cronstrue": "^2.21.0" + } }