From 245277c18d24f88ae6f11c20cb0f318a922c3233 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 14:07:32 -0800 Subject: [PATCH 01/12] Re-add parsing and validation for cron, and use cronstrue for sentences --- .../src/model/converter/cron.ts | 193 +++++++++++++----- .../src/templates/template-reader.ts | 15 +- package-lock.json | 16 ++ package.json | 5 +- 4 files changed, 174 insertions(+), 55 deletions(-) diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index aa46ca5..f8305fa 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -1,47 +1,12 @@ -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, -} - -// 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(/ +/) - if (parts.length != 5) { - return false - } - - 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 }) - ) -} +import { + MONTH_RANGE, + HOUR_RANGE, + MINUTE_RANGE, + DOM_RANGE, + DOW_RANGE +} from './cron-constants' type Range = { min: number @@ -49,7 +14,112 @@ type Range = { names?: Record } -function validateRange( +type Schedule = { + minutes?: number[] + hours?: number[] + dom?: number[] + months?: number[] + dow?: number[] +} + +export function isValidCron(cron: string): boolean { + // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule + + const parts = cron.split(/ +/) + if (parts.length != 5) { + return false + } + const [minutes, hours, dom, months, dow] = parts + + return ( + validateCronPart(minutes, MINUTE_RANGE) && + validateCronPart(hours, HOUR_RANGE) && + validateCronPart(dom, DOM_RANGE) && + validateCronPart(months, MONTH_RANGE) && + validateCronPart(dow, DOW_RANGE) + ) +} + +export function getSentence(cronspec: string): string | undefined { + const schedule = getSchedule(cronspec) + if (!schedule) { + 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 + } + + return desc +} + +function parseCronPart(part: string, range: Range): number[] | undefined { + const values: number[] = [] + + if (part === "*") { + return undefined + } + + if (part.includes(",")) { + part.split(",").forEach((v) => { + const value = parseCronPart(v, range) + if (value) { + values.push(...value) + } + }) + } + + if (part.includes("/")) { + const [stepRange, step] = part.split("/") + const stepNumber = +step + let startNumber, endNumber + if (stepRange.includes("-")) { + const [start, end] = stepRange.split("-") + startNumber = convertToNumber(start, range.names) + endNumber = convertToNumber(end, range.names) + } + else { + if (stepRange === "*") { + startNumber = range.min + } + else { + startNumber = convertToNumber(stepRange, range.names) + } + endNumber = range.max + } + for (let i = startNumber; i <= endNumber; i += stepNumber) { + values.push(i) + } + } + + if (part.includes("-")) { + const [start, end] = part.split("-") + const startNumber = convertToNumber(start, range.names) + const endNumber = convertToNumber(end, range.names) + for (let i = startNumber; i <= endNumber; i++) { + values.push(i) + } + } + + const number = convertToNumber(part, range.names) + if (!isNaN(number)) { + values.push(number) + } + + return values.sort((a, b) => a - b) +} + +function validateCronPart( value: string, range: Range, allowSeparators = true @@ -67,7 +137,7 @@ function validateRange( if (!allowSeparators) { return false } - return value.split(",").every((v) => v && validateRange(v, range)) + return value.split(",").every((v) => v && validateCronPart(v, range)) } if (value.includes("/")) { @@ -83,7 +153,7 @@ function validateRange( // Separators are only allowed in the part before the `/`, e.g. `1-5/2` return ( - validateRange(start, range) && validateRange(step, range, false) + validateCronPart(start, range) && validateCronPart(step, range, false) ) } @@ -91,16 +161,17 @@ function validateRange( if (!allowSeparators) { return false } + const [start, end, ...rest] = value.split("-") if (rest.length > 0 || !start || !end) { return false } // Convert name to integers so we can make sure end >= start - const startNumber = convertToNumber(range, start) - const endNumber = convertToNumber(range, end) + const startNumber = convertToNumber(start, range.names) + const endNumber = convertToNumber(end, range.names) return ( - validateRange(start, range, false) && validateRange(end, range, false) && endNumber >= startNumber + validateCronPart(start, range, false) && validateCronPart(end, range, false) && endNumber >= startNumber ) } @@ -108,9 +179,27 @@ function validateRange( 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()] +export function getSchedule(cron: string): Schedule | undefined { + if (!isValidCron(cron)) { + return + } + + const [minutes, hours, dom, months, dow] = cron.split(/ +/) + return { + minutes: parseCronPart(minutes, MINUTE_RANGE), + hours: parseCronPart(hours, HOUR_RANGE), + dom: parseCronPart(dom, DOM_RANGE), + months: parseCronPart(months, MONTH_RANGE), + dow: parseCronPart(dow, DOW_RANGE) + } +} + +// Helpers + +// 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 diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index 171ae72..4919c90 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -1,5 +1,6 @@ // template-reader *just* does schema validation +import {getSentence} from "../model/converter/cron"; import {ObjectReader} from "./object-reader"; import {TemplateSchema} from "./schema"; import {DefinitionInfo} from "./schema/definition-info"; @@ -215,6 +216,7 @@ class TemplateReader { const nextPropertyDef = this._schema.matchPropertyAndFilter(mappingDefinitions, nextKey.value); if (nextPropertyDef) { const nextDefinition = new DefinitionInfo(definition, nextPropertyDef.type); + const nextValue = this.readValue(nextDefinition); // Store the definition on the key, the value may have its own definition nextKey.definitionInfo = nextDefinition; @@ -224,8 +226,17 @@ class TemplateReader { if (nextPropertyDef.description) { nextKey.description = nextPropertyDef.description; } - - const nextValue = this.readValue(nextDefinition); + else if (upperKey === "CRON") { + // Schedules are a special case, we have to calculate the description + if (isString(nextValue)) { + nextValue.description = getSentence((nextValue as StringToken).value) + nextValue.description += "\nActions schedules run at most every 5 minutes." + + " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"; + } + else { + nextValue.description = "Cron expression must be a string" + } + } mapping.add(nextKey, nextValue); continue; diff --git a/package-lock.json b/package-lock.json index 3009b6f..90f53ee 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" } @@ -5273,6 +5276,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", @@ -18052,6 +18063,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" + } } From 08273a85d8027b4b66876a91eb23c4c59e5efab6 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 14:18:05 -0800 Subject: [PATCH 02/12] Add constants and re-add tests: --- .../src/model/converter/cron-constants.ts | 32 +++ .../src/model/converter/cron.test.ts | 209 +++++++++++++----- 2 files changed, 184 insertions(+), 57 deletions(-) create mode 100644 actions-workflow-parser/src/model/converter/cron-constants.ts 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..02df399 --- /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 } \ No newline at end of file diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts index d407558..7c88fa7 100644 --- a/actions-workflow-parser/src/model/converter/cron.test.ts +++ b/actions-workflow-parser/src/model/converter/cron.test.ts @@ -1,63 +1,158 @@ -import { isValidCron } from "./cron" +import { isValidCron, getSchedule, getSentence } 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) + }) + } + }) + + 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 *//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"], + ] + + for (const [cron, reason] of invalid) { + it(`${cron} should be invalid: ${reason}`, () => { + expect(isValidCron(cron)).toBe(false) + }) + } + }) + + describe("getSchedule", () => { + const testCases = [ + { + cron: "0 1 2 3 4", + expected: { + minutes: [0], + hours: [1], + dom: [2], + months: [3], + dow: [4], + } + }, + { + cron: "2,4 * * * *", + expected: { + minutes: [2, 4], + hours: undefined, + dom: undefined, + months: undefined, + dow: undefined, + } + }, + { + cron: "2-4 * * * *", + expected: { + minutes: [2, 3, 4], + hours: undefined, + dom: undefined, + months: undefined, + dow: undefined, + } + }, + { + cron: "*/15 * * * *", + expected: { + minutes: [0, 15, 30, 45], + hours: undefined, + dom: undefined, + months: undefined, + dow: undefined, + } + }, + { + cron: "10/15 * * * *", + expected: { + minutes: [10, 25, 40, 55], + hours: undefined, + dom: undefined, + months: undefined, + dow: undefined, + } + }, + { + cron: "5,*/20 * * * *", + expected: { + minutes: [0, 5, 20, 40], + hours: undefined, + dom: undefined, + months: undefined, + dow: undefined, + } + }, + { + cron: "* * * JAN SUN", + expected: { + minutes: undefined, + hours: undefined, + dom: undefined, + months: [1], + dow: [0], + } + }, + ] + + for (const testCase of testCases) { + it(`getSchedule '${testCase.cron}'`, () => { + expect(getSchedule(testCase.cron)).toEqual(testCase.expected) + }) + } + }) + + describe("getSentence", () => { + it(`Produces a sentence for valid cron`, () => { + expect(getSentence("0 * * * *")).toEqual("Every hour") }) - } - 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"], - ] - - for (const [cron, reason] of invalid) { - it(`${cron} should be invalid: ${reason}`, () => { - expect(isValidCron(cron)).toBe(false) + it(`Returns nothing for invalid cron`, () => { + expect(getSentence("* * * * * *")).toBeUndefined() }) - } + }) }) \ No newline at end of file From 86401f1f549687f1a08c84e672a1821343fc706b Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 15:12:16 -0800 Subject: [PATCH 03/12] Add tests and small bug fixes --- actions-languageservice/src/hover.test.ts | 24 +++++++++++++++++++ .../src/model/converter/cron.test.ts | 1 - .../src/model/converter/cron.ts | 3 ++- .../src/templates/template-reader.ts | 11 ++++++--- 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index 1fce7e9..9299650 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -76,4 +76,28 @@ jobs: expect(result).not.toBeUndefined(); 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 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("Invalid cron expression"); + }); }); diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts index 7c88fa7..6bd3659 100644 --- a/actions-workflow-parser/src/model/converter/cron.test.ts +++ b/actions-workflow-parser/src/model/converter/cron.test.ts @@ -43,7 +43,6 @@ describe("cron", () => { ["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 *"], diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index f8305fa..5246b66 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -60,7 +60,8 @@ export function getSentence(cronspec: string): string | undefined { return } - return desc + // Make first character lowercase + return "Runs " + desc.charAt(0).toLowerCase() + desc.slice(1) } function parseCronPart(part: string, range: Range): number[] | undefined { diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index 4919c90..f0692b2 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -229,9 +229,14 @@ class TemplateReader { else if (upperKey === "CRON") { // Schedules are a special case, we have to calculate the description if (isString(nextValue)) { - nextValue.description = getSentence((nextValue as StringToken).value) - nextValue.description += "\nActions schedules run at most every 5 minutes." + - " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"; + const cronDescription = getSentence((nextValue as StringToken).value) + if (cronDescription) { + nextValue.description = cronDescription + "\n\nActions schedules run at most every 5 minutes." + + " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"; + } + else { + nextValue.description = "Invalid cron expression" + } } else { nextValue.description = "Cron expression must be a string" From 6be2bd1cd7f52145666bba3c7c84b97a28130ef9 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 16:22:18 -0800 Subject: [PATCH 04/12] Move hover logic from parser to language service --- actions-languageservice/src/hover.ts | 35 ++++++++++++++++--- .../src/templates/template-reader.ts | 1 - 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index 7d39495..652ec1e 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -1,12 +1,16 @@ import {parseWorkflow} from "@github/actions-workflow-parser"; 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 {info} from "./log"; import {nullTrace} from "./nulltrace"; import {findToken} from "./utils/find-token"; import {mapRange} from "./utils/range"; +import {getSentence} from "@github/actions-workflow-parser/model/converter/cron"; // Render value description and Context when hovering over a key in a MappingToken export async function hover(document: TextDocument, position: Position): Promise { @@ -16,15 +20,34 @@ export async function hover(document: TextDocument, position: Position): Promise }; const result = parseWorkflow(file.name, [file], nullTrace); - const {token} = findToken(position, result.value); + const tokenResult = findToken(position, result.value); - if (result.value && token) { - return getHover(token); + if (result.value) { + return getHover(tokenResult); } return null; } -function getHover(token: TemplateToken): Hover | null { +function getHover(tokenResult: TokenResult): Hover | null { + const token = tokenResult.token; + if (!token) { + return null; + } + if (tokenResult.parent && isCronMapping(tokenResult.parent) && isString(token)) { + let description = getSentence((token as StringToken).value); + if (description) { + description += "\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 { + contents: description, + range: mapRange(token.range) + } as Hover; + } + return { + contents: "Invalid cron expression", + range: mapRange(token.range) + } as Hover; + } if (token.definition) { info(`Calculating hover for token with definition ${token.definition.key}`); @@ -47,3 +70,7 @@ function getHover(token: TemplateToken): Hover | null { } return null; } + +function isCronMapping(token: TemplateToken): boolean { + return token.definition?.key === "cron-mapping" +} \ No newline at end of file diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index f0692b2..a34a2b5 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -1,6 +1,5 @@ // template-reader *just* does schema validation -import {getSentence} from "../model/converter/cron"; import {ObjectReader} from "./object-reader"; import {TemplateSchema} from "./schema"; import {DefinitionInfo} from "./schema/definition-info"; From e8c5a2174314002c8bacafd8276f5cd3d1f21827 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 16:25:30 -0800 Subject: [PATCH 05/12] Run format --- actions-languageservice/src/context-providers/github.ts | 1 - actions-languageservice/src/hover.test.ts | 4 ++-- actions-languageservice/src/hover.ts | 9 +++++---- actions-languageservice/src/validate.expressions.test.ts | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) 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 9299650..2c589b5 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -86,8 +86,8 @@ jobs: 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)" + "Actions schedules run at most every 5 minutes. " + + "[Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)" ); }); diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index 652ec1e..d00258a 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -36,8 +36,9 @@ function getHover(tokenResult: TokenResult): Hover | null { if (tokenResult.parent && isCronMapping(tokenResult.parent) && isString(token)) { let description = getSentence((token as StringToken).value); if (description) { - description += "\n\nActions schedules run at most every 5 minutes." + - " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)" + description += + "\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 { contents: description, range: mapRange(token.range) @@ -72,5 +73,5 @@ function getHover(tokenResult: TokenResult): Hover | null { } function isCronMapping(token: TemplateToken): boolean { - return token.definition?.key === "cron-mapping" -} \ No newline at end of file + return token.definition?.key === "cron-mapping"; +} 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)); From d3d55ffc0804bff47584e7d0fd138bbce624f769 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 16:27:32 -0800 Subject: [PATCH 06/12] Remove changes in parser --- .../src/templates/template-reader.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index a34a2b5..7523266 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -225,22 +225,6 @@ class TemplateReader { if (nextPropertyDef.description) { nextKey.description = nextPropertyDef.description; } - else if (upperKey === "CRON") { - // Schedules are a special case, we have to calculate the description - if (isString(nextValue)) { - const cronDescription = getSentence((nextValue as StringToken).value) - if (cronDescription) { - nextValue.description = cronDescription + "\n\nActions schedules run at most every 5 minutes." + - " [Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"; - } - else { - nextValue.description = "Invalid cron expression" - } - } - else { - nextValue.description = "Cron expression must be a string" - } - } mapping.add(nextKey, nextValue); continue; From 615ba469604fd510d71eb5c58a839d9713406427 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 16:53:29 -0800 Subject: [PATCH 07/12] Fix hover over 'cron:' token --- actions-languageservice/src/hover.test.ts | 10 ++++++++++ actions-languageservice/src/hover.ts | 11 +++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index 2c589b5..35dc317 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -91,6 +91,16 @@ jobs: ); }); + it("on a cron identifier", 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: diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index d00258a..8ad7107 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -33,8 +33,9 @@ function getHover(tokenResult: TokenResult): Hover | null { if (!token) { return null; } - if (tokenResult.parent && isCronMapping(tokenResult.parent) && isString(token)) { - let description = getSentence((token as StringToken).value); + if (tokenResult.parent && isCronMappingValue(tokenResult)) { + const tokenValue = (token as StringToken).value + let description = getSentence(tokenValue); if (description) { description += "\n\nActions schedules run at most every 5 minutes." + @@ -72,6 +73,8 @@ function getHover(tokenResult: TokenResult): Hover | null { return null; } -function isCronMapping(token: TemplateToken): boolean { - return token.definition?.key === "cron-mapping"; +function isCronMappingValue(tokenResult: TokenResult): boolean { + return tokenResult.parent?.definition?.key === "cron-mapping" && + (tokenResult.token as StringToken).value !== "cron" && + isString(tokenResult.token!); } From af5c15a42252670f59041e33ef1e4ad4e1004efb Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 17:22:47 -0800 Subject: [PATCH 08/12] Small fix --- actions-languageservice/src/hover.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index 354af03..e6d641d 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -90,7 +90,7 @@ async function getDescription( } function isCronMappingValue(tokenResult: TokenResult): boolean { - return tokenResult.parent?.definition?.key === "cron-mapping" && - (tokenResult.token as StringToken).value !== "cron" && + return tokenResult.parent?.definition?.key === "cron-mapping" && + (tokenResult.token as StringToken).value !== "cron" && isString(tokenResult.token!); } From 58d92658b6fe2cbdc00b83a8bb5e5e7045b17bb0 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Mon, 23 Jan 2023 17:50:54 -0800 Subject: [PATCH 09/12] Address feedback --- actions-languageservice/src/hover.test.ts | 4 ++-- actions-languageservice/src/hover.ts | 17 +++++------------ .../src/model/converter/cron.test.ts | 10 ++++++---- .../src/model/converter/cron.ts | 9 +++++---- .../src/templates/template-reader.ts | 2 +- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/actions-languageservice/src/hover.test.ts b/actions-languageservice/src/hover.test.ts index 6a3e35d..a45ba9a 100644 --- a/actions-languageservice/src/hover.test.ts +++ b/actions-languageservice/src/hover.test.ts @@ -110,7 +110,7 @@ jobs: ); }); - it("on a cron identifier", async () => { + it("on a cron mapping key", async () => { const input = `on: schedule: - c|ron: '0 0 * * *' @@ -127,7 +127,7 @@ jobs: `; const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); - expect(result?.contents).toEqual("Invalid cron expression"); + expect(result?.contents).toEqual(""); }); }); diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index e6d641d..2db5dfd 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -12,7 +12,7 @@ import {info} from "./log"; import {nullTrace} from "./nulltrace"; import {findToken} from "./utils/find-token"; import {mapRange} from "./utils/range"; -import {getSentence} from "@github/actions-workflow-parser/model/converter/cron"; +import {getCronDescription} from "@github/actions-workflow-parser/model/converter/cron"; export type DescriptionProvider = { getDescription(context: WorkflowContext, token: TemplateToken, path: TemplateToken[]): Promise; @@ -40,20 +40,13 @@ export async function hover(document: TextDocument, position: Position, config?: if (tokenResult.parent && isCronMappingValue(tokenResult)) { const tokenValue = (token as StringToken).value - let description = getSentence(tokenValue); + let description = getCronDescription(tokenValue); if (description) { - description += - "\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 { contents: description, range: mapRange(token.range) } as Hover; } - return { - contents: "Invalid cron expression", - range: mapRange(token.range) - } as Hover; } let description = await getDescription(document, config, result, token, tokenResult.path); @@ -90,7 +83,7 @@ async function getDescription( } function isCronMappingValue(tokenResult: TokenResult): boolean { - return tokenResult.parent?.definition?.key === "cron-mapping" && - (tokenResult.token as StringToken).value !== "cron" && - isString(tokenResult.token!); + //return tokenResult.parent?.definition?.key === "cron-mapping" && + return isString(tokenResult.token!) && + (tokenResult.token as StringToken).value !== "cron" } diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts index 6bd3659..56a2a3e 100644 --- a/actions-workflow-parser/src/model/converter/cron.test.ts +++ b/actions-workflow-parser/src/model/converter/cron.test.ts @@ -1,4 +1,4 @@ -import { isValidCron, getSchedule, getSentence } from "./cron" +import { isValidCron, getSchedule, getCronDescription } from "./cron" describe("cron", () => { describe("valid cron", () => { @@ -145,13 +145,15 @@ describe("cron", () => { } }) - describe("getSentence", () => { + describe("getCronDescription", () => { it(`Produces a sentence for valid cron`, () => { - expect(getSentence("0 * * * *")).toEqual("Every hour") + 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(getSentence("* * * * * *")).toBeUndefined() + expect(getCronDescription("* * * * * *")).toBeUndefined() }) }) }) \ No newline at end of file diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index 5246b66..1bc2572 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -40,7 +40,7 @@ export function isValidCron(cron: string): boolean { ) } -export function getSentence(cronspec: string): string | undefined { +export function getCronDescription(cronspec: string): string | undefined { const schedule = getSchedule(cronspec) if (!schedule) { return @@ -61,7 +61,10 @@ export function getSentence(cronspec: string): string | undefined { } // Make first character lowercase - return "Runs " + desc.charAt(0).toLowerCase() + desc.slice(1) + 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 parseCronPart(part: string, range: Range): number[] | undefined { @@ -195,8 +198,6 @@ export function getSchedule(cron: string): Schedule | undefined { } } -// Helpers - // Converts a string integer or a short name to a number function convertToNumber(value: string, names?: Record): number { if (names && names[value.toLowerCase()] !== undefined) { diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index 7523266..f7290a2 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -215,7 +215,6 @@ class TemplateReader { const nextPropertyDef = this._schema.matchPropertyAndFilter(mappingDefinitions, nextKey.value); if (nextPropertyDef) { const nextDefinition = new DefinitionInfo(definition, nextPropertyDef.type); - const nextValue = this.readValue(nextDefinition); // Store the definition on the key, the value may have its own definition nextKey.definitionInfo = nextDefinition; @@ -226,6 +225,7 @@ class TemplateReader { nextKey.description = nextPropertyDef.description; } + const nextValue = this.readValue(nextDefinition); mapping.add(nextKey, nextValue); continue; } From e6de0d11b8404ea7703f865908cae000ee82296f Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Tue, 24 Jan 2023 11:14:35 -0800 Subject: [PATCH 10/12] Address feedback --- actions-languageservice/src/hover.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions-languageservice/src/hover.ts b/actions-languageservice/src/hover.ts index 2db5dfd..9d98e6a 100644 --- a/actions-languageservice/src/hover.ts +++ b/actions-languageservice/src/hover.ts @@ -83,7 +83,7 @@ async function getDescription( } function isCronMappingValue(tokenResult: TokenResult): boolean { - //return tokenResult.parent?.definition?.key === "cron-mapping" && - return isString(tokenResult.token!) && - (tokenResult.token as StringToken).value !== "cron" + return tokenResult.parent?.definition?.key === "cron-mapping" && + isString(tokenResult.token!) && + tokenResult.token.value !== "cron" } From 0790776deaac98bededeecbb44185f0dca7b5f6b Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Tue, 24 Jan 2023 12:46:28 -0800 Subject: [PATCH 11/12] Format and remove unused code --- .../src/model/converter/cron-constants.ts | 18 +- .../src/model/converter/cron.test.ts | 120 ++---------- .../src/model/converter/cron.ts | 182 +++++------------- .../src/model/converter/events.ts | 4 +- 4 files changed, 79 insertions(+), 245 deletions(-) diff --git a/actions-workflow-parser/src/model/converter/cron-constants.ts b/actions-workflow-parser/src/model/converter/cron-constants.ts index 02df399..f24a52f 100644 --- a/actions-workflow-parser/src/model/converter/cron-constants.ts +++ b/actions-workflow-parser/src/model/converter/cron-constants.ts @@ -12,8 +12,8 @@ const MONTHS = { sep: 9, oct: 10, nov: 11, - dec: 12, -} + dec: 12 +}; const DAYS = { sun: 0, @@ -22,11 +22,11 @@ const DAYS = { wed: 3, thu: 4, fri: 5, - sat: 6, -} + 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 } \ No newline at end of file +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 56a2a3e..18075a9 100644 --- a/actions-workflow-parser/src/model/converter/cron.test.ts +++ b/actions-workflow-parser/src/model/converter/cron.test.ts @@ -1,4 +1,4 @@ -import { isValidCron, getSchedule, getCronDescription } from "./cron" +import {isValidCron, getCronDescription} from "./cron"; describe("cron", () => { describe("valid cron", () => { @@ -18,15 +18,15 @@ describe("cron", () => { ["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"], - ] + ["0 0 * * *", "accepts multiple spaces"] + ]; for (const [cron, reason] of valid) { it(`${cron} should be valid: ${reason}`, () => { - expect(isValidCron(cron)).toBe(true) - }) + expect(isValidCron(cron)).toBe(true); + }); } - }) + }); describe("invalid cron", () => { const invalid = [ @@ -54,106 +54,26 @@ describe("cron", () => { ["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"], - ] + ["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) - }) + expect(isValidCron(cron)).toBe(false); + }); } - }) - - describe("getSchedule", () => { - const testCases = [ - { - cron: "0 1 2 3 4", - expected: { - minutes: [0], - hours: [1], - dom: [2], - months: [3], - dow: [4], - } - }, - { - cron: "2,4 * * * *", - expected: { - minutes: [2, 4], - hours: undefined, - dom: undefined, - months: undefined, - dow: undefined, - } - }, - { - cron: "2-4 * * * *", - expected: { - minutes: [2, 3, 4], - hours: undefined, - dom: undefined, - months: undefined, - dow: undefined, - } - }, - { - cron: "*/15 * * * *", - expected: { - minutes: [0, 15, 30, 45], - hours: undefined, - dom: undefined, - months: undefined, - dow: undefined, - } - }, - { - cron: "10/15 * * * *", - expected: { - minutes: [10, 25, 40, 55], - hours: undefined, - dom: undefined, - months: undefined, - dow: undefined, - } - }, - { - cron: "5,*/20 * * * *", - expected: { - minutes: [0, 5, 20, 40], - hours: undefined, - dom: undefined, - months: undefined, - dow: undefined, - } - }, - { - cron: "* * * JAN SUN", - expected: { - minutes: undefined, - hours: undefined, - dom: undefined, - months: [1], - dow: [0], - } - }, - ] - - for (const testCase of testCases) { - it(`getSchedule '${testCase.cron}'`, () => { - expect(getSchedule(testCase.cron)).toEqual(testCase.expected) - }) - } - }) + }); 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)" - ) - }) + 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() - }) - }) -}) \ No newline at end of file + expect(getCronDescription("* * * * * *")).toBeUndefined(); + }); + }); +}); diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index 1bc2572..5ff75bc 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -1,35 +1,29 @@ -import cronstrue from 'cronstrue'; +import cronstrue from "cronstrue"; -import { - MONTH_RANGE, - HOUR_RANGE, - MINUTE_RANGE, - DOM_RANGE, - DOW_RANGE -} from './cron-constants' +import {MONTH_RANGE, HOUR_RANGE, MINUTE_RANGE, DOM_RANGE, DOW_RANGE} from "./cron-constants"; type Range = { - min: number - max: number - names?: Record -} + min: number; + max: number; + names?: Record; +}; type Schedule = { - minutes?: number[] - hours?: number[] - dom?: number[] - months?: number[] - dow?: number[] -} + minutes?: number[]; + hours?: number[]; + dom?: number[]; + months?: number[]; + dow?: number[]; +}; 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 ( validateCronPart(minutes, MINUTE_RANGE) && @@ -37,16 +31,15 @@ export function isValidCron(cron: string): boolean { validateCronPart(dom, DOM_RANGE) && validateCronPart(months, MONTH_RANGE) && validateCronPart(dow, DOW_RANGE) - ) + ); } export function getCronDescription(cronspec: string): string | undefined { - const schedule = getSchedule(cronspec) - if (!schedule) { - return + if (!isValidCron(cronspec)) { + return; } - let desc = '' + let desc = ""; try { desc = cronstrue.toString(cronspec, { dayOfWeekStartIndexZero: true, @@ -54,156 +47,77 @@ export function getCronDescription(cronspec: string): string | undefined { 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, - }) + throwExceptionOnParseError: true + }); } catch (err) { - return + 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." + + 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 + return result; } -function parseCronPart(part: string, range: Range): number[] | undefined { - const values: number[] = [] - - if (part === "*") { - return undefined - } - - if (part.includes(",")) { - part.split(",").forEach((v) => { - const value = parseCronPart(v, range) - if (value) { - values.push(...value) - } - }) - } - - if (part.includes("/")) { - const [stepRange, step] = part.split("/") - const stepNumber = +step - let startNumber, endNumber - if (stepRange.includes("-")) { - const [start, end] = stepRange.split("-") - startNumber = convertToNumber(start, range.names) - endNumber = convertToNumber(end, range.names) - } - else { - if (stepRange === "*") { - startNumber = range.min - } - else { - startNumber = convertToNumber(stepRange, range.names) - } - endNumber = range.max - } - for (let i = startNumber; i <= endNumber; i += stepNumber) { - values.push(i) - } - } - - if (part.includes("-")) { - const [start, end] = part.split("-") - const startNumber = convertToNumber(start, range.names) - const endNumber = convertToNumber(end, range.names) - for (let i = startNumber; i <= endNumber; i++) { - values.push(i) - } - } - - const number = convertToNumber(part, range.names) - if (!isNaN(number)) { - values.push(number) - } - - return values.sort((a, b) => a - b) -} - -function validateCronPart( - 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 && validateCronPart(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 ( - validateCronPart(start, range) && validateCronPart(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(start, range.names) - const endNumber = convertToNumber(end, range.names) - return ( - validateCronPart(start, range, false) && validateCronPart(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 -} - -export function getSchedule(cron: string): Schedule | undefined { - if (!isValidCron(cron)) { - return - } - - const [minutes, hours, dom, months, dow] = cron.split(/ +/) - return { - minutes: parseCronPart(minutes, MINUTE_RANGE), - hours: parseCronPart(hours, HOUR_RANGE), - dom: parseCronPart(dom, DOM_RANGE), - months: parseCronPart(months, MONTH_RANGE), - dow: parseCronPart(dow, DOW_RANGE) - } + const number = +value; + return !isNaN(number) && number >= range.min && number <= range.max; } // 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()] + 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 { From ccac2556decf10035675bb15b3d21de920e33e29 Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Tue, 24 Jan 2023 15:00:49 -0800 Subject: [PATCH 12/12] Remove unused schedule type --- actions-workflow-parser/src/model/converter/cron.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts index 5ff75bc..b104eed 100644 --- a/actions-workflow-parser/src/model/converter/cron.ts +++ b/actions-workflow-parser/src/model/converter/cron.ts @@ -8,14 +8,6 @@ type Range = { names?: Record; }; -type Schedule = { - minutes?: number[]; - hours?: number[]; - dom?: number[]; - months?: number[]; - dow?: number[]; -}; - export function isValidCron(cron: string): boolean { // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule